Fix hotpatch cascade replaying workspace crates not built for the active target - #5746
Fix hotpatch cascade replaying workspace crates not built for the active target#5746humdrum00001010 wants to merge 2 commits into
Conversation
|
Reviewed the diff against the surrounding cascade/replay code. The fix looks correct for the described bug. One gap: the seeds aren't filtered. In while let Some(c) = to_visit.pop() {
if !visited.insert(c.clone()) {
continue;
}
if c != tip_crate_name && !artifacts.workspace_rustc.contains_crate(&c) {
continue; // edited crate isn't part of this build's target graph
}
self.modified_crates.insert(c.clone());
...(keeping the Minor/non-blocking:
Unit tests are reasonable and CI is green. With the seed filter added this looks good to merge. |
|
Ack. Didn't mean to send you automated review feedback. Never-the-less, I'd be interested in whether the review comments make sense to you. |
|
Scope note: the indexing is ~2ms of a ~250ms patch here, under 1% — it only shows past ~400 members. The skip saves a reload on an uncommon edit. Only the replay fix changes whether --hot-patch works at all, and just for workspaces with a crate outside the served target. |
|
Seed case is patched. The crates you edit and the dependents found while walking now pass one exclusion check, so neither route reaches replay uncaptured. Covered by tests against a real cargo workspace with a cfg-gated native-only dep. |
15ceadf to
b9f0498
Compare
…play Workspace dependency graphs are target-agnostic, so the cascade that decides which crates a patch carries could pick up one the active build never compiled - a native-only sibling, or a dependency behind `[target.'cfg(...)'.dependencies]` while serving wasm. Nothing was captured to replay for it, so the patch failed with `Missing rustc args for replay: '<crate>'` until `dx serve` was restarted. Exclude such crates on both routes into the set and prune the walk through them; a dependent with a compiled path back to the edited crate is still reached along that path. Reverse-dependency lookup moves to `Workspace`, which owns the graph, and indexes members once at load rather than rescanning and re-normalising every member per query - the cascade asks once per visited crate, so a patch made ~2xV of those passes. On this workspace that walk drops from ~2.1ms to ~30us.
Editing an excluded crate still ran a full patch cycle - rebuild, relink, jump table, client reload - with an empty replay list, since the tip is always rebuilt. The edit cannot reach the running binary, so return before building. Files that map to no workspace crate say nothing about relevance and keep going through the normal patch path.
b9f0498 to
b31d74b
Compare
|
@nicoburns The seed case wasn't that performance critical from my reproduction, but I patched. could you review this? |
patch_rebuild's cascade decides which workspace crates a hot patch must carry. It walksworkspace_dependents_of, which reads the whole-workspace Cargo metadata graph — and that graph is target-agnostic. So the cascade could pick up a crate the active build never compiled: a native-only sibling, or a dependency behind[target.'cfg(not(target_arch = "wasm32"))'.dependencies]while serving wasm. No rustc invocation was captured for such a crate, socompile_workspace_hotpatchhard-errored withMissing rustc args for replay: '<crate>', and hot-patching stayed broken untildx servewas restarted.First commit excludes those crates on both routes into the replay set — the crates the user edited and the dependents found by walking — and prunes the walk through them, since anything reachable only via a crate this target never compiled doesn't consume the change either. A dependent that also has a compiled path back to the edited crate is still reached along that path, so excluding one route never drops a crate that genuinely needs the patch. The walk is extracted as
crates_to_replayand covered by eight tests over a fake dependents graph, including the "cascade is unchanged when every crate is built" baseline.Second commit stops the wasted work this exposes: editing an excluded crate still ran a full patch cycle — thin rebuild, relink, jump table, client reload — with an empty replay list, because the tip is always rebuilt. It now returns before starting the build, while leaving behaviour untouched when a changed file maps to no workspace crate at all.
Verified on a workspace with a cfg-gated native-only dependency (
wasm_app→shared_lib, plusnative_ffidepending onshared_liband reachable fromwasm_apponly for non-wasm targets):native_ffi(never built for wasm32)Missing rustc args for replay: 'native_ffi', builder stuckshared_lib(shared, in the graph)cargo test --workspace --testspasses (208 suites), as docargo fmtandcargo clippy --all-targets -- -D warnings.Same root cause as #5596, which was closed as a duplicate of #5540 — see the repro and analysis posted there.
Closes #5540