Fix #183: interpolated deps trigger interpolated producers + cycle detection - #187
Merged
guybedford merged 1 commit intoApr 27, 2026
Merged
Conversation
…183) Three related fixes for interpolated task resolution: 1. **Producer-chain through interpolated dep** — `expand_interpolate_match` used to fall straight to `add_file` when a consumer's interpolated dep wasn't already in `file_nodes`. If another task's interpolated *target* pattern would have produced that file, that producer was never driven and the build failed with "File not found". Now the dep lookup also scans `interpolate_nodes` for a matching target pattern (`match_interpolate_target`) and recursively expands the producer instead. 2. **Name-only interpolation tasks reachable as plain dep refs** — a task like `name = 'test:build:#'` with no `#` in deps wasn't registered in `interpolate_nodes` (`is_interpolate_target` only inspected deps), so `dep = 'test:build:two'` couldn't resolve to it. `is_interpolate_target` now also considers `name`/`targets` containing `#`, and `add_job` dedupes by interpolated name (returning the existing instance) — without that, the for-target expansion loop in EIM creates a fresh node every visit and recurses without bound when the consumer has no targets to register in `file_nodes`. 3. **Cycle detection** — `drive_all` recursively walks `job.deps` with only a self-edge guard, so an indirect cycle (e.g. two tasks claiming the same file as their target) blew the stack at execution time. Added `check_acyclic`, an iterative tri-colour DFS run after expansion in `run()`. Surfaces a clear "Circular dependency detected" error with the cycle path instead of a stack overflow. Refactors `lookup_target` and `lookup_task_name` around two shared helpers (`match_interpolate_target`, `interpolate_dep_input`) so the matching logic lives in one place. `expand_interpolate_match` now takes `dep_path: Option<&str>` to handle interpolation tasks with no `#` in deps. Adds `test8` covering both regressions: a chain where the consumer's interpolated dep is the producer's interpolated target, reached via plain dep refs (no `:` prefix). Subtasks are namespaced as `t8:*` to stay out of `test:run`'s `:test[0-9]*` glob.
guybedford
added a commit
that referenced
this pull request
Apr 27, 2026
* fix: race in CmdPool::new_exec causing intermittent "no entry found for key" panics (#185) new_exec inserted into cmd_execs before awaiting fs::create_dir_all and only later inserted into execs in the engine dispatch. While the await yielded, another exec_future polled by select_all could observe cmd_execs populated but execs still empty and panic indexing execs at mod.rs:201. Hoist all create_dir_all awaits into a pre-pass so cmd_execs.insert and the engine's execs.insert run synchronously without yielding between them. * deps: update across the dependency tree In-range bumps (cargo update): tokio 1.37→1.52, regex 1.10→1.12, serde 1.0.197→1.0.228, serde_json 1.0.116→1.0.149, uuid 1.8→1.23, and many transitive crates. Major bumps with no source impact: base64 0.21→0.22, convert_case 0.6→0.11, dirs 4→6, directories 4→6, sha2 0.10→0.11, toml 0.5→0.8. notify 4 → 8 + notify-debouncer-mini 0.7: replaced the removed top-level watcher() / DebouncedEvent with a debouncer-driven setup. Internal channel events are now PathBuf-only (WatchEvent type alias) since chomp only ever acted on the path. Watcher trait sites use &mut dyn Watcher to match the debouncer's accessor. hyper 0.14 → 1, hyper-tls 0.5 → 0.6, warp 0.3 → 0.4, plus hyper-util, http-body-util, bytes: rewrote http_client::fetch_uri_cached on hyper-util's legacy Client + BodyExt::collect; switched the dev server to Bytes-bodied responses (warp::Body: From<Bytes>) and read files with fs::read instead of FramedRead since hyper 1 has no Body::wrap_stream. Boxed the warp filter chain so the spawned serve future is Send. Removed the unused optional rustls dep. v8 + serde_v8 are intentionally left at their current versions — the jump from 0.89 → 147 spans many breaking API changes (ScriptOrigin, HandleScope, slot API, callback signatures) and warrants a dedicated PR. * test: align test5 expectation with current Rollup quote-style Recent Rollup preserves source quote style instead of normalising to double quotes, so dep.ts's `'dep'` round-trips as `'dep'` in the bundle. Update the assert fixture to match. * fix: interpolated deps trigger interpolated producers; detect cycles (#183) (#187) Three related fixes for interpolated task resolution: 1. **Producer-chain through interpolated dep** — `expand_interpolate_match` used to fall straight to `add_file` when a consumer's interpolated dep wasn't already in `file_nodes`. If another task's interpolated *target* pattern would have produced that file, that producer was never driven and the build failed with "File not found". Now the dep lookup also scans `interpolate_nodes` for a matching target pattern (`match_interpolate_target`) and recursively expands the producer instead. 2. **Name-only interpolation tasks reachable as plain dep refs** — a task like `name = 'test:build:#'` with no `#` in deps wasn't registered in `interpolate_nodes` (`is_interpolate_target` only inspected deps), so `dep = 'test:build:two'` couldn't resolve to it. `is_interpolate_target` now also considers `name`/`targets` containing `#`, and `add_job` dedupes by interpolated name (returning the existing instance) — without that, the for-target expansion loop in EIM creates a fresh node every visit and recurses without bound when the consumer has no targets to register in `file_nodes`. 3. **Cycle detection** — `drive_all` recursively walks `job.deps` with only a self-edge guard, so an indirect cycle (e.g. two tasks claiming the same file as their target) blew the stack at execution time. Added `check_acyclic`, an iterative tri-colour DFS run after expansion in `run()`. Surfaces a clear "Circular dependency detected" error with the cycle path instead of a stack overflow. Refactors `lookup_target` and `lookup_task_name` around two shared helpers (`match_interpolate_target`, `interpolate_dep_input`) so the matching logic lives in one place. `expand_interpolate_match` now takes `dep_path: Option<&str>` to handle interpolation tasks with no `#` in deps. Adds `test8` covering both regressions: a chain where the consumer's interpolated dep is the producer's interpolated target, reached via plain dep refs (no `:` prefix). Subtasks are namespaced as `t8:*` to stay out of `test:run`'s `:test[0-9]*` glob.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #186.
Summary
Three related fixes for interpolated task resolution (#183):
Producer-chain through interpolated dep. When a task's interpolated dep (e.g.
dep = 'dst/#/manifest.yml') expands to a path that's also another task's interpolated target,expand_interpolate_matchused to fall straight toadd_fileand the build failed withFile ... not found. Now the dep resolution scansinterpolate_nodesfor a target-pattern match (match_interpolate_target) and recursively drives the producer instead.Name-only interpolation tasks reachable as plain dep refs. A task with
name = 'build:#'and no#in deps wasn't registered ininterpolate_nodes(is_interpolate_targetonly inspected deps), so a plain dep'build:two'couldn't resolve to it. Broadenedis_interpolate_targetto also considername/targets. This also required dedupingadd_jobby interpolated name — without that, the for-target expansion loop in EIM creates a fresh node every visit and recurses unboundedly when the consumer has no targets to register infile_nodes.Cycle detection.
drive_allrecurses intojob.depswith only a self-edge guard, so indirect cycles (e.g. two tasks claiming the same file as their target) blow the stack at execution time. Addedcheck_acyclic, an iterative tri-colour DFS run after expansion. SurfacesCircular dependency detected: a → b → ainstead of a stack overflow.lookup_targetandlookup_task_nameare refactored around two shared helpers (match_interpolate_target,interpolate_dep_input) so the matching logic lives in one place.expand_interpolate_matchnow takesdep_path: Option<&str>to handle name-only interpolation tasks.Test plan
test8covers both fixes: a chain where the consumer's interpolated dep IS the producer's interpolated target, reached via plain dep refs (no:prefix). Subtasks namespaced ast8:*to stay out oftest:run's:test[0-9]*glob.output/dist/one/config.ymlnow errors with a clear cycle message instead of overflowing.chomp -c test/chompfile.toml testends with√ :test.