From 70c1023115671d4b6be368515a32fb96d9d7a0c8 Mon Sep 17 00:00:00 2001 From: leynos Date: Sat, 23 May 2026 03:14:30 +0200 Subject: [PATCH 01/12] Add ExecPlan for 3.14.3 deps lowering Draft an execution plan for lowering target and action `deps` into a new implicit-dependency class on the IR `BuildEdge` and emitting them as Ninja `|` implicit dependencies between explicit inputs and `||` order-only deps. The plan records the cycle-participation decision (explicit inputs and implicit deps participate; order-only deps do not), defers rule-level `Rule.deps` rejection to roadmap item 3.14.6, and keeps action hashing unchanged because `implicit_deps` lives on `BuildEdge` rather than `Action`. Validation runs through `make check-fmt`, `make lint`, `make test`, `make markdownlint`, `make nixie`, and `coderabbit review --agent`. The plan is DRAFT and must be approved before implementation begins. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../3-14-3-lower-target-and-action-deps.md | 726 ++++++++++++++++++ 1 file changed, 726 insertions(+) create mode 100644 docs/execplans/3-14-3-lower-target-and-action-deps.md diff --git a/docs/execplans/3-14-3-lower-target-and-action-deps.md b/docs/execplans/3-14-3-lower-target-and-action-deps.md new file mode 100644 index 00000000..dcd79527 --- /dev/null +++ b/docs/execplans/3-14-3-lower-target-and-action-deps.md @@ -0,0 +1,726 @@ +# 3.14.3. Lower target and action `deps` into implicit IR and Ninja edges + +This ExecPlan (execution plan) is a living document. The sections +`Constraints`, `Tolerances`, `Risks`, `Progress`, `Surprises & Discoveries`, +`Decision Log`, and `Outcomes & Retrospective` must be kept up to date as work +proceeds. + +Status: DRAFT + +## Purpose / big picture + +Roadmap item `3.14.3` closes a long-standing gap in Netsuke's manifest contract. +Today the AST `Target` struct accepts a `deps` field, documentation already +describes it as Ninja's implicit-dependency class, and the user guide shows +authors writing `deps: my_app` and `deps: [build/utils.h]`. None of that flows +through. `Target.deps` is parsed, rendered through Jinja, and then silently +discarded during Intermediate Representation (IR) generation. The generated +`build.ninja` never reflects the author's stated prerequisites, cycle detection +never observes them, and incremental rebuilds skip files that the manifest +clearly asks Netsuke to track. + +After this work is complete, target-level and action-level `deps:` entries are +lowered into a dedicated implicit-dependency edge class on the IR `BuildEdge`, +participate in cycle detection alongside explicit `sources` inputs, and appear +in the generated `build.ninja` between the explicit input list and the +order-only marker (`|` between explicit inputs and `||`). Targets and actions +keep `sources` in the explicit recipe-input class, so `$in` and the Jinja `ins` +placeholder continue to expand only to material inputs. + +Observable success means: + +1. A manifest with `deps: hello` produces a Ninja edge of the form + `build out: rule | hello`, where `| hello` appears before any + `|| order_only_deps` block. +2. The IR `BuildEdge` exposes an `implicit_deps: Vec` field + populated from `target.deps`, while `inputs` continues to mirror + `target.sources` only. +3. A manifest whose only cycle runs through `deps` (for example, `a` declares + `deps: b` and `b` declares `deps: a`) fails IR generation with + `IrGenError::CircularDependency`, the same way explicit-input cycles fail + today. +4. `docs/users-guide.md` documents which dependency classes participate in + cycle detection, and + `docs/formal-verification-methods-in-netsuke.md` records the chosen + cycle-participation contract. +5. `docs/roadmap.md` marks `3.14.3` done only after the implementation and + validation gates pass and the draft pull request is ready. + +This plan was drafted on 2026-05-23 against the +`3-14-3-lower-target-and-action-deps` branch. Implementation must not begin +until the user explicitly approves the plan. + +## Constraints + +- Keep `target.sources` as the sole contributor to the explicit recipe-input + class. `$in` and Jinja `ins` must continue to expand to source paths only. +- Add a separate implicit-dependency class for `deps` at the IR layer. The + field must affect Ninja's rebuild and ordering decisions without leaking + into recipe interpolation. +- Cycle detection must traverse `target.sources` and `target.deps` paths + uniformly. `order_only_deps` remain outside cycle traversal. +- Preserve the existing implicit `phony: true` treatment of top-level + `actions` after they pass through `deserialize_actions` and the manifest + expansion pass. `target.deps` lowering applies identically to entries + loaded via `targets` and via `actions`. +- Do not change action hashing semantics. Implicit deps live on `BuildEdge`, + not on `Action`; identical recipes with different implicit-dep sets must + continue to share an action identifier. +- Do not introduce rule-level `deps` lowering. The design doc explicitly + forbids accepting rule-level `deps` as an alias for the planned `deps_from` + contract. The existing `Rule.deps` AST field is out of scope here and is + tracked by roadmap item `3.14.6`. +- Do not introduce runtime-condition semantics or change manifest-time + `foreach`/`when` evaluation. `deps` are static manifest data and must reach + the IR through the same selected entries already produced by the manifest + loader. +- Keep manifest-order stability. `implicit_deps` must appear in the order the + author wrote them, matching the existing behaviour of `inputs` and + `order_only_deps`. Do not sort implicit deps during Ninja emission. +- Keep domain and policy logic at the manifest/IR boundary. Hexagonal + layering: `src/manifest/` owns parsing and template policy, `src/ir/` + owns the static graph contract, and `src/ninja_gen.rs` is a pure rendering + adapter. The Ninja generator must not infer or reinterpret dependency + semantics; it must only render whatever IR class it is handed. +- Use existing `ortho_config` integration for any new command-line or + configuration surface discovered during implementation. Do not add a + parallel configuration loader or untranslated help path. +- Obtain explicit approval before adding any new external dependency. +- Do not introduce `unsafe` code. +- Use en-GB Oxford spelling in documentation, except for external API names + and established computing terms such as `serialization` and + `deserialization`. +- Use `rstest` for unit and integration tests, with shared fixtures and + parameterized cases where they remove duplication. +- Use `rstest-bdd` for behavioural coverage that is externally observable + through generated `build.ninja` output. Reuse the manifest-then-Ninja + pipeline already exercised by `tests/features/ir.feature` and + `tests/features/ninja.feature` rather than inventing a new harness. +- If implementation introduces a new invariant over a range of inputs + (for example, "any cycle that traverses implicit deps is detected"), + add `proptest` coverage. If the invariant is a contractual business + axiom, stop and propose a substantive `kani` or `verus` approach before + proceeding. Do not add a thin proof that merely restates the property. +- Keep every Rust source file below the 400-line cap from `AGENTS.md`. +- Mark roadmap item `3.14.3` done only after the implementation, tests, + documentation, `coderabbit review --agent` follow-ups, and quality gates + all pass. + +## Tolerances (exception triggers) + +- Scope: if implementation requires touching more than 16 files or roughly + 700 net new lines, stop and request approval of a revised scope. +- Interface: if a public Rust API signature other than the documented + `BuildEdge.implicit_deps` field addition must change, stop and explain the + options. Adding the field is expected to ripple through every + `BuildEdge { ... }` literal in tests and doctests; that ripple is in + scope. A signature change to `BuildGraph::from_manifest` or to the public + `Action` shape is out of scope. +- Manifest schema: if implementation needs to reject or rename any AST field + outside `Target.deps` lowering (for example, removing the unused + `Rule.deps`), stop and ask whether to fold that into this task or defer to + roadmap item `3.14.6`. +- Cycle semantics: if any proposed implementation would change cycle + participation for `order_only_deps`, stop and update the + `Decision Log` before editing the cycle module. +- Dependencies: if a new crate, Cargo feature, external tool, Kani harness, + or Verus setup is required, stop and ask for approval. +- Determinism: if any proposed change reorders or sorts implicit-dep paths + in the generated Ninja text, stop and require approval. Manifest-order + stability is a user-visible contract. +- Testing: if a host-dependent fixture, sandboxed temporary path, or + shell-tool probe makes any new test flaky after two focused fixes, stop + and redesign the test boundary around dependency injection or explicit + configuration overrides. +- Validation: if `make check-fmt`, `make lint`, or `make test` still fails + after two focused fix attempts, stop and record the failing commands and + log paths. +- Review: if `coderabbit review --agent` reports unresolved concerns after + a major milestone, address them before proceeding. If a concern conflicts + with this plan, stop and ask for direction. + +## Risks + +- Risk: a `BuildEdge { ... }` field addition ripples through many literals + in production tests and doctests. Severity: medium. Likelihood: high. + Mitigation: enumerate every literal during Stage B and add the new + `implicit_deps: Vec::new()` initialiser as a mechanical sweep before any + cycle or generator changes go in. + +- Risk: the cycle detector currently iterates only `edge.inputs`. Extending + it to traverse implicit deps could regress missing-dependency reporting if + the chained iteration is structured carelessly. Severity: medium. + Likelihood: medium. Mitigation: keep `record_missing_dependency` + class-agnostic; introduce a single chained iterator and add a regression + test where a manifest declares a missing implicit dep alongside a present + explicit input. + +- Risk: emitting `| ` between inputs and `||` is mostly + mechanical, but Ninja accepts an empty explicit input list with a + non-empty implicit dep list (`build out: rule | dep`). A naive + implementation that prefixes the implicit-dep block with a leading space + could emit `build out: rule | dep` or worse. Severity: low. + Likelihood: medium. Mitigation: keep the conditional blocks symmetric with + the existing `||` block and snapshot two cases: with and without explicit + inputs. + +- Risk: snapshot drift. Existing snapshots in `tests/snapshots/ninja/` + cover manifests without `deps:` fields, so they should remain stable. + An accidental change to whitespace, ordering, or default emission could + silently invalidate them. Severity: medium. Likelihood: low. Mitigation: + run the snapshot suite first, after only the field addition, before + changing emission code. + +- Risk: documentation drift between + `docs/users-guide.md`, `docs/netsuke-design.md`, + `docs/developers-guide.md`, and the new cycle-participation note in + `docs/formal-verification-methods-in-netsuke.md`. Severity: medium. + Likelihood: medium. Mitigation: update all four documents in the same + commit as the cycle-detection change and cross-reference the user-guide + paragraph from the formal-verification doc. + +- Risk: an author who currently treats `deps` as documentation only might + rely on Netsuke silently ignoring the field. Severity: low. Likelihood: + low. Mitigation: confirm the user-guide already documents the intended + semantics (it does, lines 199–252) and rely on the existing public + contract rather than introducing a deprecation pathway. + +- Risk: scope creep into roadmap item `3.14.6` if the change accidentally + also touches `Rule.deps`. Severity: medium. Likelihood: medium. + Mitigation: explicit `Constraints` entry above; add a comment on + `src/ast.rs` near `Rule.deps` only if needed for reviewer clarity, and + defer any actual cleanup to `3.14.6`. + +## Relevant context + +The manifest load pipeline is unchanged by this task. `src/manifest/mod.rs` +still parses YAML into `ManifestValue`, registers helpers and macros, runs +`expand_foreach`, deserializes into `NetsukeManifest`, and renders string +fields. `src/manifest/expand.rs` still owns the `foreach`/`when` policy and +emits filtering observability through `tracing::debug!`. None of that needs +to change for `3.14.3`. + +`src/ast.rs` defines `Target.deps: StringOrList` at line 222 and +`Rule.deps: StringOrList` at line 130. Top-level `actions` deserialize as +`Target` values with `phony = true` via `deserialize_actions` at lines 40–49. +This is the deserialization boundary that gives both manifest sections the +same downstream treatment. + +`src/manifest/render.rs` already renders `target.deps` and +`target.order_only_deps` through `render_string_or_list`. No change is +required there; template expansion in `deps` already works. + +`src/ir/mod.rs` re-exports the public IR types. `src/ir/graph.rs` defines +`BuildEdge` with `inputs`, `explicit_outputs`, `implicit_outputs`, and +`order_only_deps`. `BuildEdge` is missing an `implicit_deps` field; the +design doc `docs/netsuke-design.md` §5.3 and the class diagram at lines +1792–1801 already prescribe the field. + +`src/ir/from_manifest.rs` constructs each `BuildEdge` in `process_targets` +(lines 53–112). The current code calls `to_paths(&target.sources)` for the +explicit input list and `to_paths(&target.order_only_deps)` for the +order-only list, but never reads `target.deps`. The new field must be +populated next to `inputs` so the construction site stays compact. + +`src/ir/cycle.rs` walks `edge.inputs` in `CycleDetector::visit` (line 87). +The current `record_missing_dependency` and `canonicalize_cycle` helpers are +class-agnostic; only the iterator needs to expand to chain implicit deps. + +`src/ninja_gen.rs::DisplayEdge::fmt` (lines 281–298) emits the build line +shape `build [ | imp_outs]: [ ][ || order_only]`. A new +conditional block must be inserted between the explicit-input block and the +order-only block to emit `| ` when the implicit-dep list is +non-empty. The macro helpers (`write_kv!`, `write_flag!`) are unrelated and +must not change. + +`src/hasher.rs::ActionHasher::hash` hashes only the `Action` struct, never +the edge. Adding `implicit_deps` to `BuildEdge` leaves action identities +stable. Implicit-dep paths must not appear in any `Recipe::Command` text; +that is already enforced by the design contract that recipe interpolation +sees only `sources` and `outputs`. + +The user-facing dependency-field documentation lives in +`docs/users-guide.md` lines 192–252. The cycle-participation contract is +discussed in `docs/formal-verification-methods-in-netsuke.md` lines 237–248, +which explicitly asks for the chosen scope to be recorded before proofs +become gating. The design doc `docs/netsuke-design.md` §§2.4 and 5.3 already +describes the intended Ninja mapping and implementation narrative. + +Relevant skills and documents to keep open while implementing: + +- `leta`: navigate Rust symbols and references before editing code. Use + `leta show`, `leta refs`, and `leta grep` rather than reading entire + files. +- `rust-router`: route Rust-specific questions to the smallest useful + follow-on skill. Likely follow-ons are `rust-types-and-apis` for the IR + field addition, `arch-crate-design` if the boundary between manifest, IR, + and Ninja modules comes up, `rust-errors` if `IrGenError` needs new + variants (it should not), and `rust-performance-and-layout` only if the + cycle detector grows allocation hotspots (it should not). +- `hexagonal-architecture`: keep the dependency-class policy on the + manifest/IR boundary. The Ninja adapter must not infer dep semantics. +- `execplans`: keep this plan current. +- `commit-message`: commit with a file-based message after gates pass. +- `pr-creation` and `en-gb-oxendict`: use them when opening or revising the + draft pull request. +- `kani`: load only if the Kani option in `Decision Log` below is chosen. + Roadmap item `4.2.1` is the proper home for bounded IR cycle proofs; + `3.14.3` should default to `proptest` plus targeted `rstest` cases + unless escalation is approved. + +Use these repository documents: + +- `docs/roadmap.md`: source of roadmap item `3.14.3`. +- `docs/netsuke-design.md`: design source for the dependency-field lowering + table (§2.4) and the AST-to-IR transformation narrative (§5.3). +- `docs/formal-verification-methods-in-netsuke.md`: cycle-participation + contract entry that this task must populate. +- `docs/users-guide.md`: user-facing manifest and CLI behaviour for `deps`, + `sources`, and `order_only_deps`. +- `docs/developers-guide.md`: internal implementation and testing practices, + including the IR/Ninja module boundary. +- `docs/ortho-config-users-guide.md`: configuration layering and localized + help support if any config or CLI surface changes are required. +- `docs/rstest-bdd-users-guide.md`: BDD feature and step guidance. +- `docs/rust-testing-with-rstest-fixtures.md`: fixture and parameterization + patterns for unit tests. +- `docs/rust-doctest-dry-guide.md`: doctest guidance because the + `BuildEdge` field addition ripples into public Rustdoc examples. +- `docs/reliable-testing-in-rust-via-dependency-injection.md`: dependency + injection guidance for any environment, clock, process, or filesystem + effects introduced by tests. + +## Prior art + +External lookup tools (firecrawl MCP) are not connected in this session, so +this section relies on the design doc's own cross-references plus the +behaviour visible in the Ninja documentation already cited there. Add +external references during implementation only if a design ambiguity +appears. + +The relevant prior-art points already captured in the repository: + +- Ninja's manual (`docs/netsuke-design.md` §2.4 footnote `[^7]`) defines + the explicit-input, implicit-dep, and order-only-dep classes precisely + by their punctuation: `build outs: rule ins | imp_deps || order_only`. + Netsuke must mirror that vocabulary without re-implementing Ninja's + parser semantics. +- Bazel separates `srcs` (inputs that propagate into actions) from `deps` + (transitive dependency edges). The lesson worth borrowing is the strict + separation of "what the recipe consumes" from "what the build depends + on for freshness". Netsuke already has that separation in name; this + task makes it true. +- Buck2 distinguishes `srcs`, `deps`, and `exec_deps` similarly. The + lesson worth borrowing is to keep cycle detection over all freshness- + affecting dependencies. Order-only edges are an explicit escape hatch + and remain outside the cycle. + +Planning implication: keep the wording "implicit dependency" everywhere +(user guide, design doc, developer guide), in line with Ninja's manual, +and do not invent a Netsuke-specific term that diverges from the backend. + +## Progress + +- [x] (2026-05-23T00:00Z) Loaded `leta`, `rust-router`, + `hexagonal-architecture`, `execplans`, and `firecrawl` skills; added + the worktree to the leta workspace. +- [x] (2026-05-23T00:00Z) Reviewed `AGENTS.md`, `docs/roadmap.md`, + `docs/netsuke-design.md` §§2.3, 2.4, 5.3, 5.4, the past `3.14.1` and + `3.14.2` execplans, and the current IR/Ninja/cycle code paths. +- [x] (2026-05-23T00:00Z) Used a Wyvern agent team to validate the + architectural choices and inventory every test, fixture, and + snapshot touched by `Target.deps`, `BuildEdge`, or the generated + Ninja edge shape. +- [x] (2026-05-23T00:00Z) Drafted this pre-implementation ExecPlan. +- [ ] User approval of this ExecPlan. +- [ ] Stage A: confirm the AST-to-IR gap and snapshot baselines before + changing behaviour. +- [ ] Stage B: add `BuildEdge.implicit_deps` and update every literal + construction site, doctest, and fixture initialiser. +- [ ] Stage C: populate `implicit_deps` from `target.deps` in + `from_manifest::process_targets` and add `rstest` coverage that + asserts the IR field is populated for targets and actions alike. +- [ ] Stage D: extend `ir::cycle::CycleDetector` to traverse implicit deps + and add a cycle-through-implicit-deps regression test, including a + `proptest`-driven check that any cycle introduced through implicit + deps is detected. +- [ ] Stage E: update `ninja_gen::DisplayEdge::fmt` to emit + `| ` between the explicit-input block and the + order-only block, with `rstest` coverage for the empty-input case. +- [ ] Stage F: add a manifest-then-Ninja `rstest-bdd` scenario asserting + the generated `build.ninja` contains the implicit-dep separator and + that recipe interpolation sees only `sources`. +- [ ] Stage G: update `docs/users-guide.md`, `docs/developers-guide.md`, + `docs/netsuke-design.md` cross-references, and + `docs/formal-verification-methods-in-netsuke.md` to record the + chosen cycle-participation contract. +- [ ] Stage H: run `coderabbit review --agent` and resolve concerns; run + final gates, mark roadmap item `3.14.3` done, commit, push, and + open the draft pull request. + +## Surprises & Discoveries + +(populate during implementation) + +## Decision Log + +- Decision: lower `target.deps` into a new `BuildEdge.implicit_deps` + field rather than overloading `BuildEdge.inputs`. + Rationale: the design doc §2.4 table makes the class distinction + explicit; recipe interpolation must see only `sources`. Adding a + separate field keeps the contract one-to-one with Ninja's `|` syntax + and preserves the current `$in` behaviour. + Date/Author: 2026-05-23 / planning agent. + +- Decision: include `implicit_deps` in cycle detection alongside + `inputs`, and continue to exclude `order_only_deps`. + Rationale: the cycle-participation contract in + `docs/formal-verification-methods-in-netsuke.md` enumerates three + options. `deps` affect Ninja's freshness and ordering decisions; a + cycle through `deps` is just as much a build cycle as a cycle through + `sources`, and excluding it would defer the failure to Ninja with a + worse diagnostic. `order_only_deps` impose ordering without affecting + rebuild and are correctly outside cycle traversal today. + Date/Author: 2026-05-23 / planning agent. + +- Decision: leave action hashing unchanged. + Rationale: `ActionHasher::hash` hashes the `Action` struct only, and + implicit deps live on `BuildEdge`. Two edges with identical recipes + and identical outputs but different implicit-dep sets should still + share an action identifier, because the recipe text is identical and + the action's behaviour is identical. Existing snapshot hashes remain + stable. + Date/Author: 2026-05-23 / planning agent. + +- Decision: do not sort `implicit_deps` during Ninja emission. + Rationale: `inputs` and `order_only_deps` already preserve manifest + order. Manifest order is itself deterministic. Sorting `implicit_deps` + alone would create an inconsistent public contract; sorting all three + would break the existing snapshot baseline. Authors may rely on the + order they wrote. + Date/Author: 2026-05-23 / planning agent. + +- Decision: defer rule-level `Rule.deps` rejection to roadmap item + `3.14.6`. + Rationale: `3.14.3` is additive at the target/action layer. + `Rule.deps` is currently parsed and rendered but never lowered, so the + field is effectively a documentation lie. Removing or rejecting it now + would conflate an additive target/action change with a breaking AST + change, and `3.14.6` already names the `deps_from` rework that + includes "without accepting rule-level `deps` as an alias". The PR + description must call out this deferral so the gap is tracked. + Date/Author: 2026-05-23 / planning agent. + +- Decision: prefer `proptest` over `kani` for the cycle-detection + invariant in this task. + Rationale: bounded model-checking of cycle detection is the explicit + scope of roadmap item `4.2.1`, which is still open. `3.14.3` only + extends the traversal set; a property test over generated manifests + with mixed `sources`/`deps`/`order_only_deps` lists exercises the + same invariant without prematurely owning a Kani harness that + `4.2.1` will redesign. If escalation is requested, add a bounded + Kani harness under `tools/kani/` per the existing repository + integration plan. + Date/Author: 2026-05-23 / planning agent. + +- Decision: emit the new `| ` block between explicit + inputs and the `||` block in `ninja_gen.rs`, conditional on a + non-empty list and structured symmetrically with the existing + conditional blocks. + Rationale: Ninja accepts `build out: rule | dep` with no explicit + inputs; the conditional-block style keeps that case clean without + whitespace gymnastics. + Date/Author: 2026-05-23 / planning agent. + +## Implementation plan + +Stage A audits the gap. Confirm by reading the live code that the AST +deserializes `Target.deps` and the renderer renders it, but +`process_targets` never reads the field. Run the existing snapshot suite +and capture the baseline: + +```sh +cargo test --workspace ninja_snapshot_tests +``` + +Expected: the existing two Ninja snapshots pass unchanged. Record any +unexpected diffs in `Surprises & Discoveries` and stop before changing +behaviour. + +Stage B adds the IR field. In `src/ir/graph.rs`, insert +`pub implicit_deps: Vec` on `BuildEdge` immediately after +`inputs`, mirroring the design doc's class diagram. Update every literal +construction site in production tests and doctests with +`implicit_deps: Vec::new()`. The construction sites known today are: + +- `src/ir/cycle.rs::tests::build_edge` (lines 165–175). +- `src/ir/mod.rs` module doctest (lines 9–25). +- `src/ir/graph.rs` doctest fixtures embedded in `IrGenError` + examples (lines 160–229 across the variant docs). +- `src/ninja_gen.rs::generate` and `generate_into` doctests + (lines 66–88 and 100–127). +- `tests/ninja_gen_tests.rs` at every literal `BuildEdge { ... }` block + (the Wyvern agent counted ten such blocks; recount before editing). +- `tests/ir_tests.rs` at two literal blocks (lines 28–41 and 85–103). + +Run the workspace test suite after the field addition with +`implicit_deps: Vec::new()` everywhere. Nothing else should change yet. +Expected: snapshots stay byte-identical and the suite passes. + +Stage C populates the field. In `src/ir/from_manifest.rs::process_targets`, +add `let implicit_deps = to_paths(&target.deps);` alongside the existing +`let inputs = ...` and pass it into the `BuildEdge` literal. Keep `inputs` +sourced from `target.sources` only. Add `rstest` coverage in +`tests/ir_from_manifest_tests.rs` that: + +1. asserts `BuildEdge.implicit_deps` is populated from `target.deps` + for a manifest declared in `targets:`; +2. asserts the same for a manifest declared in `actions:` (which + exercises the `phony: true` default through the + `manifest.actions.iter().chain(&manifest.targets)` chain); +3. asserts `BuildEdge.inputs` remains empty for a target whose only + prerequisite is in `deps:`; +4. asserts that a recipe command does not see implicit-dep paths in + `$in` or in the interpolated `ins` placeholder. + +Prefer literal YAML manifests in the test cases for readability, in the +style of the existing `skipped_manifest_conditions_do_not_contribute_to_ir` +parameterized cases. + +Stage D extends cycle detection. In `src/ir/cycle.rs::CycleDetector::visit`, +extend the chained iterator on line 87 to walk both `edge.inputs` and +`edge.implicit_deps`. Keep `record_missing_dependency` unchanged; it is +class-agnostic. Add the following coverage: + +- A unit test in `src/ir/cycle.rs::tests` that constructs a two-node cycle + through `implicit_deps` only (`a -> b -> a` where neither edge appears in + `inputs`) and asserts `CycleDetector::find_cycle` returns the canonical + cycle. +- A unit test that mixes `inputs` and `implicit_deps` in a single cycle + and confirms cycle detection still terminates with the canonical + ordering. +- A unit test that adds a missing implicit dep alongside a present + explicit input and asserts `missing_dependencies` records the + implicit-dep gap without crashing the traversal. +- A `proptest` case that generates small bounded manifests (at most ten + targets, three deps per slot) and asserts the cycle detector reports a + cycle whenever the union graph of explicit inputs and implicit deps + contains one. If `proptest` is not already wired in this crate, gate the + case behind a `cfg(feature = "proptest")` until escalation is approved; + do not add `proptest` to the default dependency surface without explicit + approval. + +Stage E updates Ninja emission. In `src/ninja_gen.rs::DisplayEdge::fmt`, +add a conditional block between the explicit-input write and the +order-only write: + +```rust +if !self.edge.implicit_deps.is_empty() { + write!(f, " | {}", join(&self.edge.implicit_deps))?; +} +``` + +Add `rstest` coverage in `tests/ninja_gen_tests.rs` for: + +1. Explicit inputs only (existing baseline; should be unchanged). +2. Explicit inputs plus implicit deps: + `build out: rule in | dep`. +3. Implicit deps only (no explicit inputs): + `build out: rule | dep`. +4. Explicit inputs, implicit deps, and order-only deps together: + `build out: rule in | dep || stamp`. +5. Phony edge with implicit deps: + `build phony_action: phony | dep`. + +Stage F adds behavioural coverage. Extend the existing IR or Ninja +feature in `tests/features/ir.feature` or `tests/features/ninja.feature` +with a scenario that: + +1. compiles a manifest declaring `targets:` and `actions:` entries with + `deps:` set; +2. asserts the IR `BuildEdge` for each output exposes the implicit-dep + class; +3. invokes the Ninja generator and asserts the generated text contains + the `| ` separator on the expected build line. + +Reuse the existing step modules under `tests/bdd/steps/`. If a new +assertion step is required, add it to `tests/bdd/steps/ir.rs` or +`tests/bdd/steps/ninja.rs` and keep each file under 400 lines. + +Stage G updates documentation. The four documents to touch are: + +- `docs/users-guide.md`: in the dependency-fields section (lines 192–252), + add a paragraph after the `order_only_deps` description recording which + dependency classes participate in cycle detection. State explicitly that + `sources` and `deps` participate and that `order_only_deps` does not. + This is the user-visible cycle-participation contract. +- `docs/formal-verification-methods-in-netsuke.md`: in the + "Cycle-participation contract" section (lines 237–248), record the + decision that explicit inputs and implicit deps participate and that + order-only deps do not. Cross-reference the new user-guide paragraph. +- `docs/netsuke-design.md`: confirm §§2.4 and 5.3 already describe the + intended lowering and class diagram. If wording drifted during + implementation, align the prose with the implementation. Do not + rewrite the design narrative. +- `docs/developers-guide.md`: add a short subsection in the IR section + (around the "Test suite map" or "Manifest processing helpers" area) + explaining the cycle-detection class set and pointing implementers at + `src/ir/cycle.rs::CycleDetector::visit`. + +Stage H validates and closes. Run `coderabbit review --agent` after Stage +F is green; address every concern. Run the final gates sequentially with +`tee` logs, mark roadmap item `3.14.3` done in `docs/roadmap.md`, commit, +push, and open the draft pull request whose title includes `(3.14.3)` and +whose summary links this ExecPlan. + +## Validation plan + +Before editing implementation code, run a narrow baseline to capture +existing snapshot behaviour: + +```sh +cargo test --workspace ninja_snapshot_tests \ + 2>&1 \ + | tee /tmp/baseline-netsuke-3-14-3-lower-target-and-action-deps.out +``` + +Expected: both Ninja snapshots pass unchanged. + +After Stage B (field addition only): + +```sh +cargo test --workspace \ + 2>&1 \ + | tee /tmp/stage-b-netsuke-3-14-3-lower-target-and-action-deps.out +``` + +Expected: the whole suite passes with the new field initialised to +`Vec::new()` everywhere; no snapshot drift. + +After Stage C (population from `target.deps`): + +```sh +cargo test --workspace ir_from_manifest_tests \ + 2>&1 \ + | tee /tmp/stage-c-netsuke-3-14-3-lower-target-and-action-deps.out +``` + +Expected: the new parameterized cases pass; existing IR cases continue +to pass. + +After Stage D (cycle detection): + +```sh +cargo test -p netsuke ir::cycle \ + 2>&1 \ + | tee /tmp/stage-d-netsuke-3-14-3-lower-target-and-action-deps.out +``` + +Expected: the new cycle tests pass; the existing +`circular.yml` regression continues to pass; the `proptest` case (if +added) terminates within its default budget. + +After Stage E (Ninja emission): + +```sh +cargo test --workspace ninja_gen_tests \ + 2>&1 \ + | tee /tmp/stage-e-netsuke-3-14-3-lower-target-and-action-deps.out +``` + +Expected: every new edge-shape case passes; existing baselines and +snapshots remain stable except where a new snapshot intentionally +captures the new shape. + +After Stage F (behavioural coverage): + +```sh +cargo test --test bdd_tests implicit_deps \ + 2>&1 \ + | tee /tmp/stage-f-netsuke-3-14-3-lower-target-and-action-deps.out +``` + +Expected: the new BDD scenario passes; the existing IR and Ninja +scenarios continue to pass. + +Final validation must run these commands sequentially with `pipefail` +and capture logs. Sub-agents must not run tests; this list runs in the +implementation session only: + +```sh +set -o pipefail +make fmt 2>&1 | tee /tmp/fmt-netsuke-3-14-3-lower-target-and-action-deps.out +make check-fmt 2>&1 | tee /tmp/check-fmt-netsuke-3-14-3-lower-target-and-action-deps.out +make lint 2>&1 | tee /tmp/lint-netsuke-3-14-3-lower-target-and-action-deps.out +make test 2>&1 | tee /tmp/test-netsuke-3-14-3-lower-target-and-action-deps.out +make markdownlint 2>&1 | tee /tmp/markdownlint-netsuke-3-14-3-lower-target-and-action-deps.out +make nixie 2>&1 | tee /tmp/nixie-netsuke-3-14-3-lower-target-and-action-deps.out +``` + +Expected successful final output is that each command exits with status +`0`. If `make fmt` reports the pre-existing repository-wide Markdown +backlog logged by earlier execplans, restore any unrelated formatter +churn before continuing. + +After Stage F is green, run the CodeRabbit review pass and address every +concern. Re-run the final gates after each batch of fixes: + +```sh +coderabbit review --agent \ + 2>&1 \ + | tee /tmp/coderabbit-netsuke-3-14-3-lower-target-and-action-deps.out +``` + +## Idempotence and recovery + +Every stage is additive and re-runnable. Stages B through E commit small +diffs; if a stage fails validation, revert that commit with +`git reset --hard HEAD~1` (after confirming with the user) and retry. +Snapshot regeneration is intentionally a separate, reviewable commit +that uses `cargo insta accept` only after a maintainer has eyeballed the +diff. + +If the user later changes the cycle-participation contract decision, the +single behaviour change required is in `src/ir/cycle.rs::visit`. The +field addition, IR population, and Ninja emission do not depend on which +classes participate in cycle detection. + +## Interfaces and dependencies + +This task introduces one new field on a public IR struct and does not +introduce any new Rust crate dependencies. The exact surface change is: + +```rust +// src/ir/graph.rs +pub struct BuildEdge { + pub action_id: String, + pub inputs: Vec, + pub implicit_deps: Vec, + pub explicit_outputs: Vec, + pub implicit_outputs: Vec, + pub order_only_deps: Vec, + pub phony: bool, + pub always: bool, +} +``` + +The new field is documented with a Rustdoc comment mirroring the design +doc's class-diagram comment. No new error variants are introduced on +`IrGenError` or `NinjaGenError`. + +`ir::cycle::CycleDetector::visit` continues to return +`Option>`; only its internal iterator changes. + +`ninja_gen::DisplayEdge::fmt` continues to return `fmt::Result`; only its +emission shape changes. + +No new CLI flags, configuration keys, manifest schema fields, or +environment variables are introduced. + +## Outcomes & Retrospective + +(populate at completion) + +## Revision note + +(populate when revising) From 3dc71f598f1d75f62853ed2b34373e02ae20936a Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 24 May 2026 14:54:39 +0200 Subject: [PATCH 02/12] Add implicit dependency field to IR edges Introduce the `BuildEdge.implicit_deps` field and initialise it as empty at every existing construction site. Keep behaviour unchanged for this step so the later manifest lowering, cycle detection, and Ninja rendering changes can be reviewed separately. --- .../3-14-3-lower-target-and-action-deps.md | 43 ++++++++++++++++--- src/ir/cycle.rs | 1 + src/ir/from_manifest.rs | 1 + src/ir/graph.rs | 2 + src/ninja_gen.rs | 4 ++ tests/ir_tests.rs | 3 ++ tests/ninja_gen_tests.rs | 10 +++++ 7 files changed, 58 insertions(+), 6 deletions(-) diff --git a/docs/execplans/3-14-3-lower-target-and-action-deps.md b/docs/execplans/3-14-3-lower-target-and-action-deps.md index dcd79527..66533487 100644 --- a/docs/execplans/3-14-3-lower-target-and-action-deps.md +++ b/docs/execplans/3-14-3-lower-target-and-action-deps.md @@ -332,11 +332,22 @@ and do not invent a Netsuke-specific term that diverges from the backend. snapshot touched by `Target.deps`, `BuildEdge`, or the generated Ninja edge shape. - [x] (2026-05-23T00:00Z) Drafted this pre-implementation ExecPlan. -- [ ] User approval of this ExecPlan. -- [ ] Stage A: confirm the AST-to-IR gap and snapshot baselines before - changing behaviour. -- [ ] Stage B: add `BuildEdge.implicit_deps` and update every literal - construction site, doctest, and fixture initialiser. +- [x] (2026-05-24T00:00Z) User approved implementation of this ExecPlan + and explicitly requested the `leta`, `rust-router`, and + `hexagonal-architecture` skills plus a leta workspace. +- [x] (2026-05-24T00:00Z) Stage A confirmed the AST-to-IR gap and ran + the planned baseline command. The command exited successfully and + logged to + `/tmp/baseline-netsuke-3-14-3-lower-target-and-action-deps.out`. +- [x] (2026-05-24T00:00Z) Stage A follow-up identified the meaningful + snapshot tests: `touch_manifest_ninja_validation`, + `conditional_manifest_ninja_snapshot`, and + `generate_multiline_script_snapshot`. +- [x] (2026-05-24T00:00Z) Stage B added + `BuildEdge.implicit_deps` with empty initialisers at all current + construction sites. `cargo test --workspace` passed and logged to + `/tmp/stage-b-netsuke-3-14-3-lower-target-and-action-deps.out`. +- [ ] Stage B follow-up: commit the behaviour-neutral IR field addition. - [ ] Stage C: populate `implicit_deps` from `target.deps` in `from_manifest::process_targets` and add `rstest` coverage that asserts the IR field is populated for targets and actions alike. @@ -360,7 +371,27 @@ and do not invent a Netsuke-specific term that diverges from the backend. ## Surprises & Discoveries -(populate during implementation) +- (2026-05-24T00:00Z) `leta workspace add` reported that the worktree was + already registered. No workspace repair was required. + +- (2026-05-24T00:00Z) Live code still matches the planned gap: + `BuildGraph::process_targets` constructs `inputs` from `target.sources` + and `order_only_deps` from `target.order_only_deps`, but never reads + `target.deps`; `CycleDetector::visit` traverses only `edge.inputs`; and + `DisplayEdge::fmt` renders explicit inputs followed directly by + `order_only_deps`. + +- (2026-05-24T00:00Z) The planned baseline command + `cargo test --workspace ninja_snapshot_tests` is green but selects zero + tests in this repository state. Treat it as a compile/filter smoke test + only, and find the actual snapshot test names before relying on snapshot + coverage. + +- (2026-05-24T00:00Z) `make fmt` still triggers the pre-existing + repository-wide Markdown backlog noted in the validation plan. The + formatter touched many unrelated Markdown files before failing; those + unrelated changes were restored, and Rust formatting was applied with + `cargo fmt --all`. ## Decision Log diff --git a/src/ir/cycle.rs b/src/ir/cycle.rs index afad3cec..20570b7f 100644 --- a/src/ir/cycle.rs +++ b/src/ir/cycle.rs @@ -166,6 +166,7 @@ mod tests { BuildEdge { action_id: "id".into(), inputs: inputs.iter().map(|name| path(name)).collect(), + implicit_deps: Vec::new(), explicit_outputs: vec![path(output)], implicit_outputs: Vec::new(), order_only_deps: Vec::new(), diff --git a/src/ir/from_manifest.rs b/src/ir/from_manifest.rs index 820ff1ec..aee99d54 100644 --- a/src/ir/from_manifest.rs +++ b/src/ir/from_manifest.rs @@ -90,6 +90,7 @@ impl BuildGraph { let edge = BuildEdge { action_id, inputs: inputs.clone(), + implicit_deps: Vec::new(), explicit_outputs: outputs.clone(), implicit_outputs: Vec::new(), order_only_deps: to_paths(&target.order_only_deps), diff --git a/src/ir/graph.rs b/src/ir/graph.rs index a297ac0a..70b9f24f 100644 --- a/src/ir/graph.rs +++ b/src/ir/graph.rs @@ -48,6 +48,8 @@ pub struct BuildEdge { pub action_id: String, /// Explicit inputs that trigger a rebuild when changed. pub inputs: Vec, + /// Implicit dependencies that trigger a rebuild without entering recipes. + pub implicit_deps: Vec, /// Outputs explicitly generated by the command. pub explicit_outputs: Vec, /// Outputs implicitly generated by the command (Ninja `|`). diff --git a/src/ninja_gen.rs b/src/ninja_gen.rs index b44fb2d9..28f1f727 100644 --- a/src/ninja_gen.rs +++ b/src/ninja_gen.rs @@ -75,6 +75,7 @@ macro_rules! write_flag { /// }); /// graph.targets.insert(Utf8PathBuf::from("out"), BuildEdge { /// action_id: "a".into(), inputs: Vec::new(), +/// implicit_deps: Vec::new(), /// explicit_outputs: vec![Utf8PathBuf::from("out")], /// implicit_outputs: Vec::new(), order_only_deps: Vec::new(), /// phony: false, always: false @@ -112,6 +113,7 @@ pub fn generate(graph: &BuildGraph) -> Result { /// }); /// graph.targets.insert(Utf8PathBuf::from("out"), BuildEdge { /// action_id: "a".into(), inputs: Vec::new(), +/// implicit_deps: Vec::new(), /// explicit_outputs: vec![Utf8PathBuf::from("out")], /// implicit_outputs: Vec::new(), order_only_deps: Vec::new(), /// phony: false, always: false @@ -319,6 +321,7 @@ mod tests { let edge = BuildEdge { action_id: "a".into(), inputs: vec![Utf8PathBuf::from("in")], + implicit_deps: Vec::new(), explicit_outputs: vec![Utf8PathBuf::from("out")], implicit_outputs: Vec::new(), order_only_deps: Vec::new(), @@ -360,6 +363,7 @@ mod tests { let edge = BuildEdge { action_id: "a".into(), inputs: Vec::new(), + implicit_deps: Vec::new(), explicit_outputs: vec![Utf8PathBuf::from("out")], implicit_outputs: Vec::new(), order_only_deps: Vec::new(), diff --git a/tests/ir_tests.rs b/tests/ir_tests.rs index 0c70ba08..c8067ddc 100644 --- a/tests/ir_tests.rs +++ b/tests/ir_tests.rs @@ -28,6 +28,7 @@ fn create_action_and_edge() { let edge = BuildEdge { action_id: "id".into(), inputs: vec![Utf8PathBuf::from("in")], + implicit_deps: Vec::new(), explicit_outputs: vec![Utf8PathBuf::from("out")], implicit_outputs: Vec::new(), order_only_deps: Vec::new(), @@ -85,6 +86,7 @@ fn build_graph_duplicate_targets() { let edge1 = BuildEdge { action_id: "a".into(), inputs: vec![Utf8PathBuf::from("in")], + implicit_deps: Vec::new(), explicit_outputs: vec![Utf8PathBuf::from("out")], implicit_outputs: Vec::new(), order_only_deps: Vec::new(), @@ -94,6 +96,7 @@ fn build_graph_duplicate_targets() { let edge2 = BuildEdge { action_id: "a".into(), inputs: vec![Utf8PathBuf::from("in")], + implicit_deps: Vec::new(), explicit_outputs: vec![Utf8PathBuf::from("out")], implicit_outputs: Vec::new(), order_only_deps: Vec::new(), diff --git a/tests/ninja_gen_tests.rs b/tests/ninja_gen_tests.rs index 68f4fd17..c67a937a 100644 --- a/tests/ninja_gen_tests.rs +++ b/tests/ninja_gen_tests.rs @@ -58,6 +58,7 @@ fn ninja_integration_setup() -> Option { BuildEdge { action_id: "a".into(), inputs: vec![Utf8PathBuf::from("in")], + implicit_deps: Vec::new(), explicit_outputs: vec![Utf8PathBuf::from("out")], implicit_outputs: Vec::new(), order_only_deps: Vec::new(), @@ -83,6 +84,7 @@ fn ninja_integration_setup() -> Option { BuildEdge { action_id: "compile".into(), inputs: vec![Utf8PathBuf::from("a.c"), Utf8PathBuf::from("b.c")], + implicit_deps: Vec::new(), explicit_outputs: vec![Utf8PathBuf::from("ab.o")], implicit_outputs: Vec::new(), order_only_deps: Vec::new(), @@ -108,6 +110,7 @@ fn ninja_integration_setup() -> Option { BuildEdge { action_id: "b".into(), inputs: vec![Utf8PathBuf::from("in")], + implicit_deps: Vec::new(), explicit_outputs: vec![Utf8PathBuf::from("out"), Utf8PathBuf::from("log")], implicit_outputs: vec![Utf8PathBuf::from("out.d")], order_only_deps: vec![Utf8PathBuf::from("stamp")], @@ -168,6 +171,7 @@ fn generate_multiline_script_snapshot() -> Result<()> { BuildEdge { action_id: "script".into(), inputs: Vec::new(), + implicit_deps: Vec::new(), explicit_outputs: vec![Utf8PathBuf::from("out")], implicit_outputs: Vec::new(), order_only_deps: Vec::new(), @@ -207,6 +211,7 @@ fn generate_multiline_script_snapshot() -> Result<()> { edge: BuildEdge { action_id: "script".into(), inputs: Vec::new(), + implicit_deps: Vec::new(), explicit_outputs: vec![Utf8PathBuf::from("out")], implicit_outputs: Vec::new(), order_only_deps: Vec::new(), @@ -229,6 +234,7 @@ fn generate_multiline_script_snapshot() -> Result<()> { edge: BuildEdge { action_id: "percent".into(), inputs: Vec::new(), + implicit_deps: Vec::new(), explicit_outputs: vec![Utf8PathBuf::from("out")], implicit_outputs: Vec::new(), order_only_deps: Vec::new(), @@ -251,6 +257,7 @@ fn generate_multiline_script_snapshot() -> Result<()> { edge: BuildEdge { action_id: "tick".into(), inputs: Vec::new(), + implicit_deps: Vec::new(), explicit_outputs: vec![Utf8PathBuf::from("out")], implicit_outputs: Vec::new(), order_only_deps: Vec::new(), @@ -273,6 +280,7 @@ fn generate_multiline_script_snapshot() -> Result<()> { edge: BuildEdge { action_id: "hello".into(), inputs: Vec::new(), + implicit_deps: Vec::new(), explicit_outputs: vec![Utf8PathBuf::from("say-hello")], implicit_outputs: Vec::new(), order_only_deps: Vec::new(), @@ -367,6 +375,7 @@ fn errors_when_action_missing() -> Result<()> { let edge = BuildEdge { action_id: "missing".into(), inputs: Vec::new(), + implicit_deps: Vec::new(), explicit_outputs: vec![Utf8PathBuf::from("out")], implicit_outputs: Vec::new(), order_only_deps: Vec::new(), @@ -408,6 +417,7 @@ fn generate_format_error() -> Result<()> { let edge = BuildEdge { action_id: "a".into(), inputs: Vec::new(), + implicit_deps: Vec::new(), explicit_outputs: vec![Utf8PathBuf::from("out")], implicit_outputs: Vec::new(), order_only_deps: Vec::new(), From 9b8d030b9a8424d9ddccdfdf6a810f358b2320f9 Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 24 May 2026 14:59:53 +0200 Subject: [PATCH 03/12] Lower target deps into IR edges Populate `BuildEdge.implicit_deps` from manifest `deps` while keeping `sources` as the only explicit recipe input class. Preserve documented `{{ ins }}` and `{{ outs }}` recipe placeholders through manifest rendering so IR interpolation can substitute them from explicit inputs and outputs before action hashing. --- .../3-14-3-lower-target-and-action-deps.md | 27 ++++- src/ir/cmd_interpolate.rs | 7 +- src/ir/from_manifest.rs | 3 +- src/manifest/render.rs | 20 +++- tests/ir_from_manifest_tests.rs | 113 ++++++++++++++++++ 5 files changed, 161 insertions(+), 9 deletions(-) diff --git a/docs/execplans/3-14-3-lower-target-and-action-deps.md b/docs/execplans/3-14-3-lower-target-and-action-deps.md index 66533487..1f508df1 100644 --- a/docs/execplans/3-14-3-lower-target-and-action-deps.md +++ b/docs/execplans/3-14-3-lower-target-and-action-deps.md @@ -347,10 +347,16 @@ and do not invent a Netsuke-specific term that diverges from the backend. `BuildEdge.implicit_deps` with empty initialisers at all current construction sites. `cargo test --workspace` passed and logged to `/tmp/stage-b-netsuke-3-14-3-lower-target-and-action-deps.out`. -- [ ] Stage B follow-up: commit the behaviour-neutral IR field addition. -- [ ] Stage C: populate `implicit_deps` from `target.deps` in - `from_manifest::process_targets` and add `rstest` coverage that - asserts the IR field is populated for targets and actions alike. +- [x] (2026-05-24T00:00Z) Stage B follow-up committed the + behaviour-neutral IR field addition as `3dc71f5`. +- [x] (2026-05-24T00:00Z) Stage C populated `implicit_deps` from + `target.deps`, added target/action IR coverage, and preserved + documented `{{ ins }}` / `{{ outs }}` placeholders through manifest + rendering so IR interpolation can substitute them from explicit + sources and outputs. `make check-fmt`, `make lint`, and `make test` + passed with logs under `/tmp/*stage-c-netsuke-3-14-3-*`. +- [ ] Stage C follow-up: commit manifest lowering and placeholder + preservation. - [ ] Stage D: extend `ir::cycle::CycleDetector` to traverse implicit deps and add a cycle-through-implicit-deps regression test, including a `proptest`-driven check that any cycle introduced through implicit @@ -393,6 +399,19 @@ and do not invent a Netsuke-specific term that diverges from the backend. unrelated changes were restored, and Rust formatting was applied with `cargo fmt --all`. +- (2026-05-24T00:00Z) The planned Stage C command + `cargo test --workspace ir_from_manifest_tests` has the same filter + problem as the Stage A baseline: it compiles but selects zero tests. + The meaningful command is `cargo test --test ir_from_manifest_tests`, + which executed all 15 tests after the Stage C fixes. + +- (2026-05-24T00:00Z) The user-guide-documented `{{ ins }}` and + `{{ outs }}` placeholders were not surviving manifest rendering in + recipe command strings when no user variable named `ins` or `outs` + existed. Stage C now preserves those placeholders through manifest + rendering and lets IR interpolation substitute them alongside `$in` and + `$out`. + ## Decision Log - Decision: lower `target.deps` into a new `BuildEdge.implicit_deps` diff --git a/src/ir/cmd_interpolate.rs b/src/ir/cmd_interpolate.rs index 08322584..115eed2f 100644 --- a/src/ir/cmd_interpolate.rs +++ b/src/ir/cmd_interpolate.rs @@ -137,10 +137,13 @@ fn find_substitution<'a>( } fn substitute(template: &str, ins: &[String], outs: &[String]) -> String { - let chars: Vec = template.chars().collect(); let ins_joined = ins.join(" "); let outs_joined = outs.join(" "); - let mut out = String::with_capacity(template.len()); + let placeholder_template = template + .replace("{{ ins }}", &ins_joined) + .replace("{{ outs }}", &outs_joined); + let chars: Vec = placeholder_template.chars().collect(); + let mut out = String::with_capacity(placeholder_template.len()); let mut in_backticks = false; let mut i = 0; while let Some(&ch) = chars.get(i) { diff --git a/src/ir/from_manifest.rs b/src/ir/from_manifest.rs index aee99d54..85e5bff1 100644 --- a/src/ir/from_manifest.rs +++ b/src/ir/from_manifest.rs @@ -59,6 +59,7 @@ impl BuildGraph { for target in manifest.actions.iter().chain(&manifest.targets) { let outputs = to_paths(&target.name); let inputs = to_paths(&target.sources); + let implicit_deps = to_paths(&target.deps); let target_name = get_target_display_name(&outputs); let action_id = match &target.recipe { Recipe::Rule { rule } => { @@ -90,7 +91,7 @@ impl BuildGraph { let edge = BuildEdge { action_id, inputs: inputs.clone(), - implicit_deps: Vec::new(), + implicit_deps, explicit_outputs: outputs.clone(), implicit_outputs: Vec::new(), order_only_deps: to_paths(&target.order_only_deps), diff --git a/src/manifest/render.rs b/src/manifest/render.rs index b118ab11..d3b43fc3 100644 --- a/src/manifest/render.rs +++ b/src/manifest/render.rs @@ -34,7 +34,7 @@ fn render_rule(rule: &mut crate::ast::Rule, env: &Environment, vars: &Vars) -> R render_string_or_list(&mut rule.deps, env, vars)?; match &mut rule.recipe { Recipe::Command { command } => { - *command = render_str_with(env, command, vars, || "render rule command".into())?; + *command = render_recipe_str_with(env, command, vars, || "render rule command".into())?; } Recipe::Script { script } => { *script = render_str_with(env, script, vars, || "render rule script".into())?; @@ -52,7 +52,7 @@ fn render_target(target: &mut Target, env: &Environment) -> Result<()> { render_string_or_list(&mut target.order_only_deps, env, &target.vars)?; match &mut target.recipe { Recipe::Command { command } => { - *command = render_str_with(env, command, &target.vars, || { + *command = render_recipe_str_with(env, command, &target.vars, || { "render target command".into() })?; } @@ -98,6 +98,22 @@ fn render_str_with( env.render_str(tpl, ctx).with_context(what) } +fn render_recipe_str_with( + env: &Environment, + tpl: &str, + ctx: &Vars, + what: impl FnOnce() -> String, +) -> Result { + let mut recipe_ctx = ctx.clone(); + recipe_ctx + .entry("ins".into()) + .or_insert_with(|| ManifestValue::String("{{ ins }}".into())); + recipe_ctx + .entry("outs".into()) + .or_insert_with(|| ManifestValue::String("{{ outs }}".into())); + render_str_with(env, tpl, &recipe_ctx, what) +} + #[cfg(test)] mod tests { use super::*; diff --git a/tests/ir_from_manifest_tests.rs b/tests/ir_from_manifest_tests.rs index 21fb89cc..b0229f8f 100644 --- a/tests/ir_from_manifest_tests.rs +++ b/tests/ir_from_manifest_tests.rs @@ -3,6 +3,7 @@ use anyhow::{Context, Result, bail, ensure}; use camino::Utf8PathBuf; use netsuke::{ + ast::Recipe, ir::{BuildGraph, IrGenError}, manifest, }; @@ -118,6 +119,118 @@ fn skipped_manifest_conditions_do_not_contribute_to_ir( Ok(()) } +#[rstest] +#[case::target_deps( + concat!( + "netsuke_version: '1.0.0'\n", + "targets:\n", + " - name: out/app\n", + " deps: [include/config.h, generated/stamp]\n", + " command: echo $out\n", + ), + "out/app", + false, +)] +#[case::action_deps( + concat!( + "netsuke_version: '1.0.0'\n", + "actions:\n", + " - name: regenerate\n", + " deps: [schemas/user.yml, tools/generator]\n", + " command: echo $out\n", + "targets: []\n", + ), + "regenerate", + true, +)] +fn manifest_deps_populate_implicit_deps( + #[case] yaml: &str, + #[case] output: &str, + #[case] expected_phony: bool, +) -> Result<()> { + let manifest = manifest::from_str(yaml)?; + let graph = BuildGraph::from_manifest(&manifest).context("expected graph generation")?; + let edge = graph + .targets + .get(&Utf8PathBuf::from(output)) + .with_context(|| format!("expected edge for {output}"))?; + + ensure!( + edge.implicit_deps + == vec![ + Utf8PathBuf::from(if expected_phony { + "schemas/user.yml" + } else { + "include/config.h" + }), + Utf8PathBuf::from(if expected_phony { + "tools/generator" + } else { + "generated/stamp" + }), + ], + "unexpected implicit deps for {output}: {:?}", + edge.implicit_deps + ); + ensure!( + edge.inputs.is_empty(), + "deps must not be explicit recipe inputs: {:?}", + edge.inputs + ); + ensure!( + edge.phony == expected_phony, + "unexpected phony flag for {output}: {}", + edge.phony + ); + Ok(()) +} + +#[rstest] +fn manifest_deps_do_not_contribute_to_recipe_inputs() -> Result<()> { + let yaml = concat!( + "netsuke_version: '1.0.0'\n", + "rules:\n", + " - name: compile\n", + " command: echo $in {{ ins }} > $out\n", + "targets:\n", + " - name: out/app\n", + " sources: src/main.c\n", + " deps: [include/config.h, generated/stamp]\n", + " rule: compile\n", + ); + let manifest = manifest::from_str(yaml)?; + let graph = BuildGraph::from_manifest(&manifest).context("expected graph generation")?; + let edge = graph + .targets + .get(&Utf8PathBuf::from("out/app")) + .context("expected edge for out/app")?; + let action = graph + .actions + .get(&edge.action_id) + .context("expected action for out/app")?; + let Recipe::Command { command } = &action.recipe else { + bail!("expected command recipe"); + }; + + ensure!( + command == "echo src/main.c src/main.c > out/app", + "deps should not appear in recipe interpolation: {command}" + ); + ensure!( + edge.inputs == vec![Utf8PathBuf::from("src/main.c")], + "sources should remain the explicit inputs" + ); + ensure!( + edge.implicit_deps + == vec![ + Utf8PathBuf::from("include/config.h"), + Utf8PathBuf::from("generated/stamp"), + ], + "deps should populate only implicit deps" + ); + Ok(()) +} + #[derive(Debug)] enum ExpectedError { DuplicateOutput(Vec), From 06a3c6f3fad746957d5719bac5f28378f0111624 Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 24 May 2026 15:03:15 +0200 Subject: [PATCH 04/12] Detect cycles through implicit deps Traverse `BuildEdge.implicit_deps` alongside explicit inputs during IR cycle analysis while continuing to ignore order-only dependencies. Add regression coverage for implicit-only cycles, mixed cycles, missing implicit dependencies, and bounded small cycles without adding a new test dependency. --- .../3-14-3-lower-target-and-action-deps.md | 27 ++++-- src/ir/cycle.rs | 96 +++++++++++++++++-- 2 files changed, 108 insertions(+), 15 deletions(-) diff --git a/docs/execplans/3-14-3-lower-target-and-action-deps.md b/docs/execplans/3-14-3-lower-target-and-action-deps.md index 1f508df1..1ac76e2c 100644 --- a/docs/execplans/3-14-3-lower-target-and-action-deps.md +++ b/docs/execplans/3-14-3-lower-target-and-action-deps.md @@ -355,12 +355,14 @@ and do not invent a Netsuke-specific term that diverges from the backend. rendering so IR interpolation can substitute them from explicit sources and outputs. `make check-fmt`, `make lint`, and `make test` passed with logs under `/tmp/*stage-c-netsuke-3-14-3-*`. -- [ ] Stage C follow-up: commit manifest lowering and placeholder - preservation. -- [ ] Stage D: extend `ir::cycle::CycleDetector` to traverse implicit deps - and add a cycle-through-implicit-deps regression test, including a - `proptest`-driven check that any cycle introduced through implicit - deps is detected. +- [x] (2026-05-24T00:00Z) Stage C follow-up committed manifest lowering + and placeholder preservation as `9b8d030`. +- [x] (2026-05-24T00:00Z) Stage D extended + `ir::cycle::CycleDetector` to traverse implicit deps, added + implicit-only, mixed, missing-implicit, and bounded small-cycle + regression tests, and passed `make check-fmt`, `make lint`, and + `make test` with logs under `/tmp/*stage-d-netsuke-3-14-3-*`. +- [ ] Stage D follow-up: commit cycle detection changes. - [ ] Stage E: update `ninja_gen::DisplayEdge::fmt` to emit `| ` between the explicit-input block and the order-only block, with `rstest` coverage for the empty-input case. @@ -412,6 +414,11 @@ and do not invent a Netsuke-specific term that diverges from the backend. rendering and lets IR interpolation substitute them alongside `$in` and `$out`. +- (2026-05-24T00:00Z) `proptest` is not present in `Cargo.toml` or + `Cargo.lock`. Because the plan prohibits adding a dependency without + explicit approval, Stage D used deterministic bounded coverage over + small generated cycles instead of adding `proptest`. + ## Decision Log - Decision: lower `target.deps` into a new `BuildEdge.implicit_deps` @@ -473,6 +480,14 @@ and do not invent a Netsuke-specific term that diverges from the backend. integration plan. Date/Author: 2026-05-23 / planning agent. +- Decision: do not add `proptest` for `3.14.3`; use deterministic + bounded cycle coverage instead. + Rationale: `proptest` is not already wired into this crate, and adding a + new dependency requires explicit approval under this plan. The bounded + test still exercises mixed explicit/implicit cycles without widening + the dependency surface. + Date/Author: 2026-05-24 / implementation agent. + - Decision: emit the new `| ` block between explicit inputs and the `||` block in `ninja_gen.rs`, conditional on a non-empty list and structured symmetrically with the existing diff --git a/src/ir/cycle.rs b/src/ir/cycle.rs index 20570b7f..375babab 100644 --- a/src/ir/cycle.rs +++ b/src/ir/cycle.rs @@ -84,7 +84,7 @@ impl CycleDetector<'_> { .targets .get(&node) .into_iter() - .flat_map(|edge| edge.inputs.iter()) + .flat_map(|edge| edge.inputs.iter().chain(&edge.implicit_deps)) .find_map(|dep| self.visit_dependency(&node, dep)) { return Some(cycle); @@ -162,11 +162,11 @@ mod tests { Utf8PathBuf::from(name) } - fn build_edge(inputs: &[&str], output: &str) -> BuildEdge { + fn build_edge(inputs: &[&str], implicit_deps: &[&str], output: &str) -> BuildEdge { BuildEdge { action_id: "id".into(), inputs: inputs.iter().map(|name| path(name)).collect(), - implicit_deps: Vec::new(), + implicit_deps: implicit_deps.iter().map(|name| path(name)).collect(), explicit_outputs: vec![path(output)], implicit_outputs: Vec::new(), order_only_deps: Vec::new(), @@ -175,10 +175,42 @@ mod tests { } } + fn next_cycle_index(index: usize, cycle_len: usize) -> usize { + if index + 1 == cycle_len { 0 } else { index + 1 } + } + + fn insert_cycle_edge( + targets: &mut HashMap, + index: usize, + cycle_len: usize, + implicit_index: usize, + ) { + let output = format!("n{index}"); + let dep = format!("n{}", next_cycle_index(index, cycle_len)); + let edge = if index == implicit_index { + build_edge(&[], &[&dep], &output) + } else { + build_edge(&[&dep], &[], &output) + }; + targets.insert(output.into(), edge); + } + + fn assert_bounded_cycle_detected(cycle_len: usize, implicit_index: usize) { + let mut targets = HashMap::new(); + for index in 0..cycle_len { + insert_cycle_edge(&mut targets, index, cycle_len, implicit_index); + } + + assert!( + CycleDetector::find_cycle(&targets).is_some(), + "expected cycle with length {cycle_len} and implicit edge at {implicit_index}", + ); + } + #[test] fn cycle_detector_detects_self_edge_cycle() { let mut targets = HashMap::new(); - targets.insert(path("a"), build_edge(&["a"], "a")); + targets.insert(path("a"), build_edge(&["a"], &[], "a")); let cycle = CycleDetector::find_cycle(&targets).expect("cycle"); assert_eq!(cycle, vec![path("a"), path("a")]); @@ -189,8 +221,8 @@ mod tests { let mut targets = HashMap::new(); let a = path("a"); let b = path("b"); - targets.insert(a.clone(), build_edge(&["b"], "a")); - targets.insert(b.clone(), build_edge(&[], "b")); + targets.insert(a.clone(), build_edge(&["b"], &[], "a")); + targets.insert(b.clone(), build_edge(&[], &[], "b")); let mut detector = CycleDetector::new(&targets); assert!(detector.visit(a.clone()).is_none()); @@ -205,7 +237,7 @@ mod tests { #[test] fn cycle_detector_records_missing_dependencies() { let mut targets = HashMap::new(); - targets.insert(path("a"), build_edge(&["b"], "a")); + targets.insert(path("a"), build_edge(&["b"], &[], "a")); let mut detector = CycleDetector::new(&targets); assert!(detector.visit(path("a")).is_none()); @@ -216,13 +248,59 @@ mod tests { #[test] fn find_cycle_identifies_cycle() { let mut targets = HashMap::new(); - targets.insert(path("a"), build_edge(&["b"], "a")); - targets.insert(path("b"), build_edge(&["a"], "b")); + targets.insert(path("a"), build_edge(&["b"], &[], "a")); + targets.insert(path("b"), build_edge(&["a"], &[], "b")); let cycle = CycleDetector::find_cycle(&targets).expect("cycle"); assert_eq!(cycle, vec![path("a"), path("b"), path("a")]); } + #[test] + fn find_cycle_identifies_implicit_dependency_cycle() { + let mut targets = HashMap::new(); + targets.insert(path("a"), build_edge(&[], &["b"], "a")); + targets.insert(path("b"), build_edge(&[], &["a"], "b")); + + let cycle = CycleDetector::find_cycle(&targets).expect("cycle"); + assert_eq!(cycle, vec![path("a"), path("b"), path("a")]); + } + + #[test] + fn find_cycle_identifies_mixed_input_and_implicit_dependency_cycle() { + let mut targets = HashMap::new(); + targets.insert(path("a"), build_edge(&["b"], &[], "a")); + targets.insert(path("b"), build_edge(&[], &["c"], "b")); + targets.insert(path("c"), build_edge(&["a"], &[], "c")); + + let cycle = CycleDetector::find_cycle(&targets).expect("cycle"); + assert_eq!(cycle, vec![path("a"), path("b"), path("c"), path("a")]); + } + + #[test] + fn cycle_detector_records_missing_implicit_dependencies() { + let mut targets = HashMap::new(); + targets.insert(path("a"), build_edge(&["b"], &["missing"], "a")); + targets.insert(path("b"), build_edge(&[], &[], "b")); + + let mut detector = CycleDetector::new(&targets); + assert!(detector.visit(path("a")).is_none()); + + assert_eq!( + detector.missing_dependencies(), + &[(path("a"), path("missing"))], + ); + } + + #[test] + fn bounded_cycles_through_inputs_or_implicit_deps_are_detected() { + let cases = (2..=5).flat_map(|cycle_len| { + (0..cycle_len).map(move |implicit_index| (cycle_len, implicit_index)) + }); + for (cycle_len, implicit_index) in cases { + assert_bounded_cycle_detected(cycle_len, implicit_index); + } + } + #[test] fn canonicalize_cycle_rotates_smallest_node() { let cycle = vec![path("c"), path("a"), path("b"), path("c")]; From f6faba3f58949d032b66b5648a2a161e3e9abe6b Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 24 May 2026 15:05:40 +0200 Subject: [PATCH 05/12] Render implicit deps in Ninja edges Emit `BuildEdge.implicit_deps` with Ninja's single-pipe separator between explicit inputs and order-only dependencies. Cover explicit, implicit-only, order-only, and phony action edge shapes so the separator ordering remains stable. --- .../3-14-3-lower-target-and-action-deps.md | 21 +++- src/ninja_gen.rs | 3 + tests/ninja_gen_tests.rs | 104 ++++++++++++++++++ 3 files changed, 124 insertions(+), 4 deletions(-) diff --git a/docs/execplans/3-14-3-lower-target-and-action-deps.md b/docs/execplans/3-14-3-lower-target-and-action-deps.md index 1ac76e2c..818b54a6 100644 --- a/docs/execplans/3-14-3-lower-target-and-action-deps.md +++ b/docs/execplans/3-14-3-lower-target-and-action-deps.md @@ -362,10 +362,16 @@ and do not invent a Netsuke-specific term that diverges from the backend. implicit-only, mixed, missing-implicit, and bounded small-cycle regression tests, and passed `make check-fmt`, `make lint`, and `make test` with logs under `/tmp/*stage-d-netsuke-3-14-3-*`. -- [ ] Stage D follow-up: commit cycle detection changes. -- [ ] Stage E: update `ninja_gen::DisplayEdge::fmt` to emit - `| ` between the explicit-input block and the - order-only block, with `rstest` coverage for the empty-input case. +- [x] (2026-05-24T00:00Z) Stage D follow-up committed cycle detection + changes as `06a3c6f`. +- [x] (2026-05-24T00:00Z) Stage E updated + `ninja_gen::DisplayEdge::fmt` to emit `| ` between + explicit inputs and order-only deps. Added `rstest` edge-shape + coverage for explicit-plus-implicit, implicit-only, + explicit-plus-implicit-plus-order-only, and phony action cases. + `cargo test --test ninja_gen_tests`, `make lint`, and `make test` + passed with logs under `/tmp/*stage-e-netsuke-3-14-3-*`. +- [ ] Stage E follow-up: commit Ninja emission changes. - [ ] Stage F: add a manifest-then-Ninja `rstest-bdd` scenario asserting the generated `build.ninja` contains the implicit-dep separator and that recipe interpolation sees only `sources`. @@ -419,6 +425,13 @@ and do not invent a Netsuke-specific term that diverges from the backend. explicit approval, Stage D used deterministic bounded coverage over small generated cycles instead of adding `proptest`. +- (2026-05-24T00:00Z) `BuildEdge.phony` does not make + `ninja_gen::DisplayEdge` emit Ninja's built-in `phony` rule by itself; + the generator still renders the edge's `action_id`. The Stage E phony + test therefore registers an action named `phony` and verifies + `build phony_action: phony | dep` in the build line while preserving + the existing rule-emission behaviour. + ## Decision Log - Decision: lower `target.deps` into a new `BuildEdge.implicit_deps` diff --git a/src/ninja_gen.rs b/src/ninja_gen.rs index 28f1f727..bbb9898a 100644 --- a/src/ninja_gen.rs +++ b/src/ninja_gen.rs @@ -290,6 +290,9 @@ impl Display for DisplayEdge<'_> { if !self.edge.inputs.is_empty() { write!(f, " {}", join(&self.edge.inputs))?; } + if !self.edge.implicit_deps.is_empty() { + write!(f, " | {}", join(&self.edge.implicit_deps))?; + } if !self.edge.order_only_deps.is_empty() { write!(f, " || {}", join(&self.edge.order_only_deps))?; } diff --git a/tests/ninja_gen_tests.rs b/tests/ninja_gen_tests.rs index c67a937a..c69b7726 100644 --- a/tests/ninja_gen_tests.rs +++ b/tests/ninja_gen_tests.rs @@ -124,6 +124,110 @@ fn ninja_integration_setup() -> Option { "build out log | out.d: b in || stamp\n\n", ), )] +#[case::explicit_inputs_plus_implicit_deps( + Action { + recipe: Recipe::Command { command: "true".into() }, + description: None, + depfile: None, + deps_format: None, + pool: None, + restat: false, + }, + BuildEdge { + action_id: "b".into(), + inputs: vec![Utf8PathBuf::from("in")], + implicit_deps: vec![Utf8PathBuf::from("dep")], + explicit_outputs: vec![Utf8PathBuf::from("out")], + implicit_outputs: Vec::new(), + order_only_deps: Vec::new(), + phony: false, + always: false, + }, + Utf8PathBuf::from("out"), + concat!( + "rule b\n", + " command = true\n\n", + "build out: b in | dep\n\n", + ), +)] +#[case::implicit_deps_without_explicit_inputs( + Action { + recipe: Recipe::Command { command: "true".into() }, + description: None, + depfile: None, + deps_format: None, + pool: None, + restat: false, + }, + BuildEdge { + action_id: "b".into(), + inputs: Vec::new(), + implicit_deps: vec![Utf8PathBuf::from("dep")], + explicit_outputs: vec![Utf8PathBuf::from("out")], + implicit_outputs: Vec::new(), + order_only_deps: Vec::new(), + phony: false, + always: false, + }, + Utf8PathBuf::from("out"), + concat!( + "rule b\n", + " command = true\n\n", + "build out: b | dep\n\n", + ), +)] +#[case::all_dependency_classes( + Action { + recipe: Recipe::Command { command: "true".into() }, + description: None, + depfile: None, + deps_format: None, + pool: None, + restat: false, + }, + BuildEdge { + action_id: "b".into(), + inputs: vec![Utf8PathBuf::from("in")], + implicit_deps: vec![Utf8PathBuf::from("dep")], + explicit_outputs: vec![Utf8PathBuf::from("out")], + implicit_outputs: Vec::new(), + order_only_deps: vec![Utf8PathBuf::from("stamp")], + phony: false, + always: false, + }, + Utf8PathBuf::from("out"), + concat!( + "rule b\n", + " command = true\n\n", + "build out: b in | dep || stamp\n\n", + ), +)] +#[case::phony_action_with_implicit_deps( + Action { + recipe: Recipe::Command { command: "true".into() }, + description: None, + depfile: None, + deps_format: None, + pool: None, + restat: false, + }, + BuildEdge { + action_id: "phony".into(), + inputs: Vec::new(), + implicit_deps: vec![Utf8PathBuf::from("dep")], + explicit_outputs: vec![Utf8PathBuf::from("phony_action")], + implicit_outputs: Vec::new(), + order_only_deps: Vec::new(), + phony: true, + always: false, + }, + Utf8PathBuf::from("phony_action"), + concat!( + "rule phony\n", + " command = true\n\n", + "build phony_action: phony | dep\n\n", + ), +)] fn generate_ninja_scenarios( #[case] action: Action, #[case] edge: BuildEdge, From 1a2c686eb9f206a11b611418394da7a0ca6d7517 Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 24 May 2026 15:09:06 +0200 Subject: [PATCH 06/12] Cover implicit deps in BDD scenarios Add an end-to-end manifest fixture that lowers target and action `deps` into IR implicit deps and renders them as Ninja single-pipe dependencies. Assert that recipe interpolation still receives only explicit `sources`, including the documented `{{ ins }}` placeholder path. --- .../3-14-3-lower-target-and-action-deps.md | 18 +++-- tests/bdd/steps/ir.rs | 69 +++++++++++++++++++ tests/bdd_tests.rs | 2 +- tests/data/implicit_deps.yml | 17 +++++ tests/features/ninja.feature | 12 ++++ 5 files changed, 113 insertions(+), 5 deletions(-) create mode 100644 tests/data/implicit_deps.yml diff --git a/docs/execplans/3-14-3-lower-target-and-action-deps.md b/docs/execplans/3-14-3-lower-target-and-action-deps.md index 818b54a6..c3c3173d 100644 --- a/docs/execplans/3-14-3-lower-target-and-action-deps.md +++ b/docs/execplans/3-14-3-lower-target-and-action-deps.md @@ -371,10 +371,15 @@ and do not invent a Netsuke-specific term that diverges from the backend. explicit-plus-implicit-plus-order-only, and phony action cases. `cargo test --test ninja_gen_tests`, `make lint`, and `make test` passed with logs under `/tmp/*stage-e-netsuke-3-14-3-*`. -- [ ] Stage E follow-up: commit Ninja emission changes. -- [ ] Stage F: add a manifest-then-Ninja `rstest-bdd` scenario asserting - the generated `build.ninja` contains the implicit-dep separator and - that recipe interpolation sees only `sources`. +- [x] (2026-05-24T00:00Z) Stage E follow-up committed Ninja emission + changes as `f6faba3`. +- [x] (2026-05-24T00:00Z) Stage F added a manifest-then-Ninja + `rstest-bdd` scenario and fixture covering target/action implicit + deps, explicit recipe inputs, generated Ninja separators, and + `{{ ins }}` interpolation excluding implicit deps. `cargo test + --test bdd_tests implicit`, `make lint`, and `make test` passed + with logs under `/tmp/*stage-f-netsuke-3-14-3-*`. +- [ ] Stage F follow-up: commit behavioural coverage. - [ ] Stage G: update `docs/users-guide.md`, `docs/developers-guide.md`, `docs/netsuke-design.md` cross-references, and `docs/formal-verification-methods-in-netsuke.md` to record the @@ -432,6 +437,11 @@ and do not invent a Netsuke-specific term that diverges from the backend. `build phony_action: phony | dep` in the build line while preserving the existing rule-emission behaviour. +- (2026-05-24T00:00Z) Editing only a `.feature` file did not cause Cargo + to rebuild the `rstest-bdd` generated test binary. Stage F made a + comment-only touch to `tests/bdd_tests.rs` so the macro expansion picks + up the new scenario. + ## Decision Log - Decision: lower `target.deps` into a new `BuildEdge.implicit_deps` diff --git a/tests/bdd/steps/ir.rs b/tests/bdd/steps/ir.rs index e94337c8..66b5c515 100644 --- a/tests/bdd/steps/ir.rs +++ b/tests/bdd/steps/ir.rs @@ -3,6 +3,7 @@ use crate::bdd::fixtures::{RefCellOptionExt, TestWorld}; use crate::bdd::helpers::parse_store::store_parse_outcome; use anyhow::{Context, Result, anyhow, ensure}; +use camino::Utf8PathBuf; use netsuke::ir::BuildGraph; use rstest_bdd_macros::{given, then, when}; @@ -43,6 +44,64 @@ where Ok(()) } +fn parse_path_list(paths: &str) -> Vec { + paths + .split(',') + .map(str::trim) + .filter(|path| !path.is_empty()) + .map(Utf8PathBuf::from) + .collect() +} + +#[derive(Clone, Copy)] +enum TargetPathField { + Inputs, + ImplicitDeps, +} + +impl TargetPathField { + fn paths(self, edge: &netsuke::ir::BuildEdge) -> &[Utf8PathBuf] { + match self { + Self::Inputs => &edge.inputs, + Self::ImplicitDeps => &edge.implicit_deps, + } + } + + const fn label(self) -> &'static str { + match self { + Self::Inputs => "inputs", + Self::ImplicitDeps => "implicit deps", + } + } +} + +fn assert_target_paths( + world: &TestWorld, + target: &str, + expected: &str, + field: TargetPathField, +) -> Result<()> { + let expected_paths = parse_path_list(expected); + let actual_paths = world + .build_graph + .with_ref(|graph| { + graph + .targets + .get(&Utf8PathBuf::from(target)) + .map(|edge| field.paths(edge).to_vec()) + }) + .context("build graph should be available")? + .with_context(|| format!("target {target} should be present"))?; + ensure!( + actual_paths == expected_paths, + "expected {} {:?} for {target}, got {:?}", + field.label(), + expected_paths, + actual_paths + ); + Ok(()) +} + // --------------------------------------------------------------------------- // Given/When steps // --------------------------------------------------------------------------- @@ -131,6 +190,16 @@ fn ir_generation_fails(world: &TestWorld) -> Result<()> { Ok(()) } +#[then("the graph target {target:string} has inputs {paths:string}")] +fn graph_target_inputs(world: &TestWorld, target: &str, paths: &str) -> Result<()> { + assert_target_paths(world, target, paths, TargetPathField::Inputs) +} + +#[then("the graph target {target:string} has implicit deps {paths:string}")] +fn graph_target_implicit_deps(world: &TestWorld, target: &str, paths: &str) -> Result<()> { + assert_target_paths(world, target, paths, TargetPathField::ImplicitDeps) +} + // --------------------------------------------------------------------------- // Implementation helpers // --------------------------------------------------------------------------- diff --git a/tests/bdd_tests.rs b/tests/bdd_tests.rs index 0fdf74ed..bb546402 100644 --- a/tests/bdd_tests.rs +++ b/tests/bdd_tests.rs @@ -14,7 +14,7 @@ pub use bdd::fixtures::*; use rstest_bdd_macros::scenarios; -// Autodiscover all cross-platform scenarios from feature files +// Autodiscover all cross-platform scenarios from feature files. // The fixtures parameter ensures TestWorld is injected into each generated test scenarios!("tests/features", fixtures = [world: TestWorld]); diff --git a/tests/data/implicit_deps.yml b/tests/data/implicit_deps.yml new file mode 100644 index 00000000..650d72e5 --- /dev/null +++ b/tests/data/implicit_deps.yml @@ -0,0 +1,17 @@ +netsuke_version: "1.0.0" +rules: + - name: compile + command: "echo $in {{ ins }} > $out" +targets: + - name: out/app + sources: src/main.c + deps: + - include/config.h + - generated/stamp + rule: compile +actions: + - name: regenerate + deps: + - schemas/user.yml + - tools/generator + command: "echo $out" diff --git a/tests/features/ninja.feature b/tests/features/ninja.feature index d714fd90..085abe45 100644 --- a/tests/features/ninja.feature +++ b/tests/features/ninja.feature @@ -28,3 +28,15 @@ Feature: Ninja file generation And the ninja file is generated Then ninja generation fails mentioning the removed action id + Scenario: Target and action deps become implicit Ninja dependencies + When the manifest file "tests/data/implicit_deps.yml" is compiled to IR + Then the graph target "out/app" has inputs "src/main.c" + And the graph target "out/app" has implicit deps "include/config.h, generated/stamp" + And the graph target "regenerate" has inputs "" + And the graph target "regenerate" has implicit deps "schemas/user.yml, tools/generator" + When the ninja file is generated + Then the ninja file contains "build out/app: " + And the ninja file contains " src/main.c | include/config.h generated/stamp" + And the ninja file contains "build regenerate: " + And the ninja file contains " | schemas/user.yml tools/generator" + And the ninja file contains "command = echo src/main.c src/main.c > out/app" From f875e692faeaa2ad69fca009e4dd6d911ac5c69a Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 24 May 2026 15:10:46 +0200 Subject: [PATCH 07/12] Document implicit dependency semantics Record that manifest `sources` become explicit recipe inputs, `deps` become implicit dependencies, and order-only dependencies remain outside cycle detection. Align the user, developer, design, and formal-verification notes with the implemented IR and Ninja dependency classes. --- docs/developers-guide.md | 14 ++++++++++++++ .../3-14-3-lower-target-and-action-deps.md | 15 ++++++++++----- docs/formal-verification-methods-in-netsuke.md | 16 ++++++---------- docs/netsuke-design.md | 4 ++-- docs/users-guide.md | 14 ++++++++++---- 5 files changed, 42 insertions(+), 21 deletions(-) diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 59f7bd28..2e516c7e 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -96,6 +96,20 @@ Netsuke uses a mixed strategy: - Behavioural step definitions and fixtures live in `tests/bdd/`. - Behavioural test discovery is defined in `tests/bdd_tests.rs`. +## IR dependency classes + +`src/ir/from_manifest.rs` lowers manifest `sources` into +`BuildEdge.inputs`, manifest `deps` into `BuildEdge.implicit_deps`, and +manifest `order_only_deps` into `BuildEdge.order_only_deps`. Keep those classes +separate: recipe interpolation (`$in` and `{{ ins }}`) receives only +`BuildEdge.inputs`, while `src/ninja_gen.rs` renders implicit deps with +Ninja's single-pipe separator. + +`src/ir/cycle.rs::CycleDetector::visit` traverses `inputs` and +`implicit_deps` when detecting cycles. It intentionally does not traverse +`order_only_deps`, because order-only dependencies express scheduling order +rather than rebuild freshness. + ## Behavioural testing strategy Behavioural tests run through `cargo test` using `rstest-bdd`, not a bespoke diff --git a/docs/execplans/3-14-3-lower-target-and-action-deps.md b/docs/execplans/3-14-3-lower-target-and-action-deps.md index c3c3173d..f1e780b4 100644 --- a/docs/execplans/3-14-3-lower-target-and-action-deps.md +++ b/docs/execplans/3-14-3-lower-target-and-action-deps.md @@ -379,11 +379,16 @@ and do not invent a Netsuke-specific term that diverges from the backend. `{{ ins }}` interpolation excluding implicit deps. `cargo test --test bdd_tests implicit`, `make lint`, and `make test` passed with logs under `/tmp/*stage-f-netsuke-3-14-3-*`. -- [ ] Stage F follow-up: commit behavioural coverage. -- [ ] Stage G: update `docs/users-guide.md`, `docs/developers-guide.md`, - `docs/netsuke-design.md` cross-references, and - `docs/formal-verification-methods-in-netsuke.md` to record the - chosen cycle-participation contract. +- [x] (2026-05-24T00:00Z) Stage F follow-up committed behavioural + coverage as `1a2c686`. +- [x] (2026-05-24T00:00Z) Stage G updated + `docs/users-guide.md`, `docs/developers-guide.md`, + `docs/netsuke-design.md`, and + `docs/formal-verification-methods-in-netsuke.md` with the + dependency-class and cycle-participation contract. `git diff + --check`, touched-file markdownlint, `make markdownlint`, and + `make nixie` passed; logs are under `/tmp/*stage-g-netsuke-3-14-3-*`. +- [ ] Stage G follow-up: commit documentation updates. - [ ] Stage H: run `coderabbit review --agent` and resolve concerns; run final gates, mark roadmap item `3.14.3` done, commit, push, and open the draft pull request. diff --git a/docs/formal-verification-methods-in-netsuke.md b/docs/formal-verification-methods-in-netsuke.md index 100a975a..0616ee72 100644 --- a/docs/formal-verification-methods-in-netsuke.md +++ b/docs/formal-verification-methods-in-netsuke.md @@ -236,16 +236,12 @@ that affects manifest authoring. The project documentation should state whether: ### Cycle-participation contract -`BuildEdge` records explicit inputs, explicit outputs, implicit outputs, and -order-only dependencies, but the current cycle detector walks `edge.inputs` -only.[^5][^7] This contract should be documented in the user guide under the -dependency and build-graph semantics chapter, as it defines the behaviour users -observe when circular dependencies are detected. Before proofs are written, the -intended scope of cycle detection should be recorded explicitly: - -- explicit inputs only, -- explicit inputs plus order-only dependencies, or -- explicit, implicit, and order-only dependencies together. +`BuildEdge` records explicit inputs, implicit dependencies, explicit outputs, +implicit outputs, and order-only dependencies.[^5][^7] Cycle detection walks +explicit inputs and implicit dependencies: in manifest terms, `sources` and +`deps` participate. Order-only dependencies do not participate, because they +only enforce build ordering and do not affect rebuild freshness. The user guide +documents the same contract in its target dependency-field section. ### Determinism contract diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index baf5d797..d7ce59e7 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -1747,11 +1747,11 @@ pub struct BuildEdge { /// The unique identifier of the Action used for this edge. pub action_id: String, - /// Explicit inputs that, when changed, trigger a rebuild. + /// Explicit recipe inputs that, when changed, trigger a rebuild. pub inputs: Vec, /// Implicit dependencies that must be built first and trigger rebuilds. - /// Maps to Ninja's '|' syntax and remains separate from `$in`. + /// Maps to Ninja's '|' syntax and remains separate from `$in` / `{{ ins }}`. pub implicit_deps: Vec, /// Outputs explicitly generated by the command. diff --git a/docs/users-guide.md b/docs/users-guide.md index 5b60be79..43513d40 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -231,15 +231,21 @@ targets: - `recipe`: How to build the target. Defined by one of `rule`, `command`, or `script` (mutually exclusive). -- `sources`: Input file path(s) (`StringOrList`). If a source matches another - target's `name`, an implicit dependency is created. +- `sources`: Input file path(s) (`StringOrList`). Sources are explicit recipe + inputs: they are passed to `$in` and `{{ ins }}` and trigger rebuilds when + changed. -- `deps` (Optional): Explicit target dependencies (`StringOrList`). Changes - trigger rebuilds. +- `deps` (Optional): Implicit target dependencies (`StringOrList`). Changes + trigger rebuilds, but these paths are not passed to `$in` or `{{ ins }}`. + Maps to Ninja `|`. - `order_only_deps` (Optional): Dependencies that must run first but whose changes don't trigger rebuilds (`StringOrList`). Maps to Ninja `||`. +Cycle detection traverses `sources` and `deps`, because both classes affect +the build graph and rebuild freshness. `order_only_deps` only enforce build +ordering and do not participate in Netsuke's cycle detection. + - `vars` (Optional): Target-specific variables that override global `vars`. - `phony` (Optional, default `false`): Treat target as logical, not a file. From 90bcb3a75ea889ed98a233b069b6ce5351ee3dac Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 24 May 2026 15:13:22 +0200 Subject: [PATCH 08/12] Update implicit deps execplan progress Record the final deterministic validation state before CodeRabbit review and note the existing Markdown formatting exception that prevents using `make fmt` as an all-or-nothing gate for this branch. --- .../3-14-3-lower-target-and-action-deps.md | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/docs/execplans/3-14-3-lower-target-and-action-deps.md b/docs/execplans/3-14-3-lower-target-and-action-deps.md index f1e780b4..0da5b3c5 100644 --- a/docs/execplans/3-14-3-lower-target-and-action-deps.md +++ b/docs/execplans/3-14-3-lower-target-and-action-deps.md @@ -5,7 +5,7 @@ This ExecPlan (execution plan) is a living document. The sections `Decision Log`, and `Outcomes & Retrospective` must be kept up to date as work proceeds. -Status: DRAFT +Status: ACTIVE ## Purpose / big picture @@ -388,7 +388,12 @@ and do not invent a Netsuke-specific term that diverges from the backend. dependency-class and cycle-participation contract. `git diff --check`, touched-file markdownlint, `make markdownlint`, and `make nixie` passed; logs are under `/tmp/*stage-g-netsuke-3-14-3-*`. -- [ ] Stage G follow-up: commit documentation updates. +- [x] (2026-05-24T00:00Z) Stage G follow-up committed documentation + updates as `f875e69`. +- [x] (2026-05-24T00:00Z) Final deterministic gates before CodeRabbit + passed for `make check-fmt`, `make lint`, `make test`, + `make markdownlint`, and `make nixie` with logs under + `/tmp/*netsuke-3-14-3-lower-target-and-action-deps.out`. - [ ] Stage H: run `coderabbit review --agent` and resolve concerns; run final gates, mark roadmap item `3.14.3` done, commit, push, and open the draft pull request. @@ -447,6 +452,12 @@ and do not invent a Netsuke-specific term that diverges from the backend. comment-only touch to `tests/bdd_tests.rs` so the macro expansion picks up the new scenario. +- (2026-05-24T00:00Z) The final `make fmt` attempt repeated the + pre-existing Markdown formatting backlog and rewrote unrelated + documentation before failing. The unrelated churn was restored, and the + deterministic validation gates that do not rewrite unrelated files all + passed before CodeRabbit review. + ## Decision Log - Decision: lower `target.deps` into a new `BuildEdge.implicit_deps` From 03f1aecc8c9dff6c96f90144b963fceaa0e648f0 Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 24 May 2026 15:15:27 +0200 Subject: [PATCH 09/12] Close roadmap item 3.14.3 Mark the implicit dependency lowering roadmap item complete after the implementation, deterministic gates, and CodeRabbit review all passed. Update the ExecPlan with the final review and validation state. --- .../3-14-3-lower-target-and-action-deps.md | 15 ++++++++++++--- docs/roadmap.md | 8 ++++---- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/docs/execplans/3-14-3-lower-target-and-action-deps.md b/docs/execplans/3-14-3-lower-target-and-action-deps.md index 0da5b3c5..02657e58 100644 --- a/docs/execplans/3-14-3-lower-target-and-action-deps.md +++ b/docs/execplans/3-14-3-lower-target-and-action-deps.md @@ -394,9 +394,18 @@ and do not invent a Netsuke-specific term that diverges from the backend. passed for `make check-fmt`, `make lint`, `make test`, `make markdownlint`, and `make nixie` with logs under `/tmp/*netsuke-3-14-3-lower-target-and-action-deps.out`. -- [ ] Stage H: run `coderabbit review --agent` and resolve concerns; run - final gates, mark roadmap item `3.14.3` done, commit, push, and - open the draft pull request. +- [x] (2026-05-24T00:00Z) `coderabbit review --agent` completed with + zero findings. Review output is logged at + `/tmp/coderabbit-netsuke-3-14-3-lower-target-and-action-deps.out`. +- [x] (2026-05-24T00:00Z) Marked roadmap item `3.14.3` and its + subitems complete after implementation, deterministic gates, and + CodeRabbit review all passed. +- [x] (2026-05-24T00:00Z) Closing gates passed after the roadmap and + execplan completion updates: `make check-fmt`, `make lint`, + `make test`, `make markdownlint`, and `make nixie`. Logs are under + `/tmp/*final-netsuke-3-14-3-lower-target-and-action-deps.out`. +- [ ] Stage H: commit closure updates, push, and open the draft pull + request. ## Surprises & Discoveries diff --git a/docs/roadmap.md b/docs/roadmap.md index 989519f8..47931ccd 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -159,14 +159,14 @@ and agents. expansion. - [x] Support complementary branches such as `when: command_available(...)` and `when: not command_available(...)`. -- [ ] 3.14.3. Lower target and action `deps` into implicit IR and Ninja +- [x] 3.14.3. Lower target and action `deps` into implicit IR and Ninja dependency edges. Requires 1.2.2 and 1.3.2. See [netsuke-design.md §§2.4 and 5.3](netsuke-design.md). - - [ ] Keep `sources` in the explicit recipe-input class used for `ins` and + - [x] Keep `sources` in the explicit recipe-input class used for `ins` and `$in`. - - [ ] Add a separate implicit dependency class for `deps` so they affect + - [x] Add a separate implicit dependency class for `deps` so they affect ordering and rebuild decisions without appearing in recipe arguments. - - [ ] Align cycle detection, generated Ninja output, and user-facing + - [x] Align cycle detection, generated Ninja output, and user-facing dependency documentation. - [ ] 3.14.4. Add `command_available(name, **kwargs)` as a non-throwing executable probe. Requires 3.5.1. See From b7b55898d182ad334f0e1ff2b9ed557fcae08f54 Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 24 May 2026 15:16:58 +0200 Subject: [PATCH 10/12] Record implicit deps PR handoff Mark the ExecPlan complete after pushing the branch and refreshing the draft pull request with the final validation and reviewer context. --- .../3-14-3-lower-target-and-action-deps.md | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/docs/execplans/3-14-3-lower-target-and-action-deps.md b/docs/execplans/3-14-3-lower-target-and-action-deps.md index 02657e58..1ebe239d 100644 --- a/docs/execplans/3-14-3-lower-target-and-action-deps.md +++ b/docs/execplans/3-14-3-lower-target-and-action-deps.md @@ -5,7 +5,7 @@ This ExecPlan (execution plan) is a living document. The sections `Decision Log`, and `Outcomes & Retrospective` must be kept up to date as work proceeds. -Status: ACTIVE +Status: COMPLETE ## Purpose / big picture @@ -404,8 +404,9 @@ and do not invent a Netsuke-specific term that diverges from the backend. execplan completion updates: `make check-fmt`, `make lint`, `make test`, `make markdownlint`, and `make nixie`. Logs are under `/tmp/*final-netsuke-3-14-3-lower-target-and-action-deps.out`. -- [ ] Stage H: commit closure updates, push, and open the draft pull - request. +- [x] (2026-05-24T00:00Z) Pushed the branch and refreshed draft pull + request [#315](https://github.com/leynos/netsuke/pull/315) with + full branch context, validation evidence, and review entrypoints. ## Surprises & Discoveries @@ -832,7 +833,21 @@ environment variables are introduced. ## Outcomes & Retrospective -(populate at completion) +Roadmap item `3.14.3` is implemented. Target-level and action-level `deps` +now lower into `BuildEdge.implicit_deps`, participate in cycle detection, +and render as Ninja implicit dependencies between explicit inputs and +order-only dependencies. Recipe interpolation continues to use explicit +`sources` only for `$in` and `{{ ins }}`. + +The work also fixed a related placeholder-preservation bug: documented +`{{ ins }}` and `{{ outs }}` recipe placeholders now survive manifest +rendering so IR command interpolation can substitute them. + +Validation passed for the Rust formatting check, lint/doc checks, full +test suite, Markdown linting, Mermaid validation, and CodeRabbit review. +`make fmt` remains blocked by unrelated repository-wide Markdown formatting +backlog; the branch restored that unrelated churn and relied on +non-mutating gates for completion. ## Revision note From 87de1ae0b2d3d5411ec287ded87ee0128c906136 Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 25 May 2026 21:00:30 +0200 Subject: [PATCH 11/12] Extract missing dependency test helper Remove duplicated `CycleDetector` setup from the missing dependency tests by sharing the assertion skeleton inside the cycle test module. --- src/ir/cycle.rs | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/src/ir/cycle.rs b/src/ir/cycle.rs index 375babab..18f5c7a6 100644 --- a/src/ir/cycle.rs +++ b/src/ir/cycle.rs @@ -175,6 +175,15 @@ mod tests { } } + fn assert_missing_deps( + targets: HashMap, + expected: &[(Utf8PathBuf, Utf8PathBuf)], + ) { + let mut detector = CycleDetector::new(&targets); + assert!(detector.visit(path("a")).is_none()); + assert_eq!(detector.missing_dependencies(), expected); + } + fn next_cycle_index(index: usize, cycle_len: usize) -> usize { if index + 1 == cycle_len { 0 } else { index + 1 } } @@ -238,11 +247,7 @@ mod tests { fn cycle_detector_records_missing_dependencies() { let mut targets = HashMap::new(); targets.insert(path("a"), build_edge(&["b"], &[], "a")); - - let mut detector = CycleDetector::new(&targets); - assert!(detector.visit(path("a")).is_none()); - - assert_eq!(detector.missing_dependencies(), &[(path("a"), path("b"))],); + assert_missing_deps(targets, &[(path("a"), path("b"))]); } #[test] @@ -281,14 +286,7 @@ mod tests { let mut targets = HashMap::new(); targets.insert(path("a"), build_edge(&["b"], &["missing"], "a")); targets.insert(path("b"), build_edge(&[], &[], "b")); - - let mut detector = CycleDetector::new(&targets); - assert!(detector.visit(path("a")).is_none()); - - assert_eq!( - detector.missing_dependencies(), - &[(path("a"), path("missing"))], - ); + assert_missing_deps(targets, &[(path("a"), path("missing"))]); } #[test] From abb5dea42894a8f8a014734062a9d9fe2e43c1f9 Mon Sep 17 00:00:00 2001 From: leynos Date: Tue, 26 May 2026 23:05:30 +0200 Subject: [PATCH 12/12] Borrow targets in missing dependency test helper Pass the test target map by reference so Clippy does not flag the helper for taking ownership of data it only inspects. --- src/ir/cycle.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ir/cycle.rs b/src/ir/cycle.rs index 18f5c7a6..8cf1bd8e 100644 --- a/src/ir/cycle.rs +++ b/src/ir/cycle.rs @@ -176,10 +176,10 @@ mod tests { } fn assert_missing_deps( - targets: HashMap, + targets: &HashMap, expected: &[(Utf8PathBuf, Utf8PathBuf)], ) { - let mut detector = CycleDetector::new(&targets); + let mut detector = CycleDetector::new(targets); assert!(detector.visit(path("a")).is_none()); assert_eq!(detector.missing_dependencies(), expected); } @@ -247,7 +247,7 @@ mod tests { fn cycle_detector_records_missing_dependencies() { let mut targets = HashMap::new(); targets.insert(path("a"), build_edge(&["b"], &[], "a")); - assert_missing_deps(targets, &[(path("a"), path("b"))]); + assert_missing_deps(&targets, &[(path("a"), path("b"))]); } #[test] @@ -286,7 +286,7 @@ mod tests { let mut targets = HashMap::new(); targets.insert(path("a"), build_edge(&["b"], &["missing"], "a")); targets.insert(path("b"), build_edge(&[], &[], "b")); - assert_missing_deps(targets, &[(path("a"), path("missing"))]); + assert_missing_deps(&targets, &[(path("a"), path("missing"))]); } #[test]