From 983dbe1bd856bf089a1b3ed5a33597ef678d57d6 Mon Sep 17 00:00:00 2001 From: Leynos Date: Mon, 15 Sep 2025 21:29:34 +0100 Subject: [PATCH 01/11] Refactor cycle detection to own traversal state --- docs/netsuke-design.md | 25 +++++++++++++++++++++---- src/ir/cycle.rs | 33 ++++++++++++++++++--------------- 2 files changed, 39 insertions(+), 19 deletions(-) diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index bb21c4d8..3b56264a 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -400,6 +400,22 @@ The cleaner model is: - `always`: When set to `true`, the target runs on every invocation regardless of timestamps or dependencies. The default value is `false`. + +### 2.7 Table: Netsuke Manifest vs. Makefile + +To illustrate the ergonomic advantages of the Netsuke schema, the following +table compares a simple C compilation project defined in both a traditional +`Makefile` and a `Netsukefile` file. The comparison highlights Netsuke's +explicit, structured, and self-documenting nature. + +| Feature | Makefile Example | Netsukefile Example | +| --------------- | ---------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------- | +| Variables | CC=gcc | { vars: { cc: gcc } } | +| Macros | define greet\\t@echo Hello $$1endef | { macros: { signature: "greet(name)", body: "Hello {{ name }}" } } | +| Rule Definition | %.o: %.c\\n\\t$(CC) -c $< -o $@ | { rules: { name: compile, command: "{{ cc }} -c {{ ins }} -o {{ outs }}", description: "Compiling {{ outs }}" } } | +| Target Build | my_program: main.o utils.o\\t$(CC) $^ -o $@ | { targets: { name: my_program, rule: link, sources: [main.o, utils.o] } | +| Readability | Relies on cryptic automatic variables ($@, $\<, $^) and implicit pattern matching. | Uses explicit, descriptive keys (name, rule, sources) and standard YAML list/map syntax. | + ### 2.5 Generated Targets and Actions with `foreach` Large sets of similar outputs or setup actions can clutter a manifest when @@ -1882,10 +1898,11 @@ This transformation involves several steps: deterministic error messages. Traversal state is implemented in the dedicated `ir::cycle` module. Its - `CycleDetector` helper owns the recursion stack and visitation map. Keys are - cloned from the `targets` map so traversal leaves the input graph untouched. - Missing dependencies encountered during traversal are logged, collected, and - returned alongside any cycle to aid diagnostics. + `CycleDetector` helper exposes a `detect` API and owns the recursion stack + and visitation map. Keys are cloned from the `targets` map so traversal + leaves the input graph untouched. Missing dependencies encountered during + traversal are logged, collected, and returned alongside any cycle to aid + diagnostics. ### 5.4 Ninja File Synthesis (`ninja_gen.rs`) diff --git a/src/ir/cycle.rs b/src/ir/cycle.rs index afad3cec..da7cee6a 100644 --- a/src/ir/cycle.rs +++ b/src/ir/cycle.rs @@ -20,16 +20,7 @@ pub(crate) struct CycleDetectionReport { pub(crate) fn analyse(targets: &HashMap) -> CycleDetectionReport { let mut detector = CycleDetector::new(targets); - let mut cycle = None; - for node in targets.keys() { - if detector.is_visited(node) { - continue; - } - if let Some(found) = detector.visit(node.clone()) { - cycle = Some(found); - break; - } - } + let cycle = detector.detect(); CycleDetectionReport { cycle, missing_dependencies: detector.missing_dependencies, @@ -53,6 +44,18 @@ impl CycleDetector<'_> { } } + fn detect(&mut self) -> Option> { + for node in self.targets.keys() { + if self.is_visited(node) { + continue; + } + if let Some(cycle) = self.visit(node.clone()) { + return Some(cycle); + } + } + None + } + fn is_visited(&self, node: &Utf8PathBuf) -> bool { matches!(self.states.get(node), Some(VisitState::Visited)) } @@ -146,10 +149,10 @@ fn canonicalize_cycle(mut cycle: Vec) -> Vec { .enumerate() .min_by(|(_, a), (_, b)| a.cmp(b)) .map_or(0, |(idx, _)| idx); - let (prefix, suffix) = cycle.split_at_mut(len); - prefix.rotate_left(start); - if let (Some(first), Some(last)) = (prefix.first().cloned(), suffix.first_mut()) { - *last = first; + cycle.pop(); + cycle.rotate_left(start); + if let Some(first) = cycle.first().cloned() { + cycle.push(first); } cycle } @@ -192,7 +195,7 @@ mod tests { targets.insert(b.clone(), build_edge(&[], "b")); let mut detector = CycleDetector::new(&targets); - assert!(detector.visit(a.clone()).is_none()); + assert!(detector.detect().is_none()); assert!(detector.is_visited(&a)); assert!(detector.is_visited(&b)); assert!( From a8531efbe1035237962ef435b31d85498fa1aab6 Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 27 May 2026 11:36:58 +0200 Subject: [PATCH 02/11] Extract shared helper for canonicalize_cycle tests (#74) Deduplicate the two table-driven canonicalize_cycle tests by introducing `check_canonicalize_cycle` in the tests module. Co-authored-by: Cursor --- src/ir/cycle.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/ir/cycle.rs b/src/ir/cycle.rs index da7cee6a..1b5849ed 100644 --- a/src/ir/cycle.rs +++ b/src/ir/cycle.rs @@ -225,19 +225,20 @@ mod tests { assert_eq!(cycle, vec![path("a"), path("b"), path("a")]); } + fn check_canonicalize_cycle(input: &[&str], expected: &[&str]) { + let cycle: Vec = input.iter().map(|&s| path(s)).collect(); + let canonical = canonicalize_cycle(cycle); + let want: Vec = expected.iter().map(|&s| path(s)).collect(); + assert_eq!(canonical, want); + } + #[test] fn canonicalize_cycle_rotates_smallest_node() { - let cycle = vec![path("c"), path("a"), path("b"), path("c")]; - let canonical = canonicalize_cycle(cycle); - let expected = vec![path("a"), path("b"), path("c"), path("a")]; - assert_eq!(canonical, expected); + check_canonicalize_cycle(&["c", "a", "b", "c"], &["a", "b", "c", "a"]); } #[test] fn canonicalize_cycle_handles_reverse_direction() { - let cycle = vec![path("c"), path("b"), path("a"), path("c")]; - let canonical = canonicalize_cycle(cycle); - let expected = vec![path("a"), path("c"), path("b"), path("a")]; - assert_eq!(canonical, expected); + check_canonicalize_cycle(&["c", "b", "a", "c"], &["a", "c", "b", "a"]); } } From 641c6715286f3b29757c9914c22e3a9f38d7a698 Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 27 May 2026 11:43:48 +0200 Subject: [PATCH 03/11] Always pop stack when visit returns from dependency walk (#74) Pair every `stack.push` with `stack.pop` even when a cycle is found, and only mark the node `Visited` when no cycle was detected. Co-authored-by: Cursor --- src/ir/cycle.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/ir/cycle.rs b/src/ir/cycle.rs index 1b5849ed..49e958b9 100644 --- a/src/ir/cycle.rs +++ b/src/ir/cycle.rs @@ -83,19 +83,20 @@ impl CycleDetector<'_> { self.stack.push(node.clone()); - if let Some(cycle) = self + let cycle = self .targets .get(&node) .into_iter() .flat_map(|edge| edge.inputs.iter()) - .find_map(|dep| self.visit_dependency(&node, dep)) - { - return Some(cycle); - } + .find_map(|dep| self.visit_dependency(&node, dep)); self.stack.pop(); - self.states.insert(node, VisitState::Visited); - None + + if cycle.is_none() { + self.states.insert(node, VisitState::Visited); + } + + cycle } #[cfg(test)] From 4bcd16e3fe738718b9bfb3f6b5c7cdc3afdd24d8 Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 27 May 2026 11:46:16 +0200 Subject: [PATCH 04/11] Assert stack is empty after cycle detection (#74) Add a regression test that `CycleDetector::detect` leaves the traversal stack empty once a cycle has been found. Co-authored-by: Cursor --- src/ir/cycle.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/ir/cycle.rs b/src/ir/cycle.rs index 49e958b9..9ee9dc54 100644 --- a/src/ir/cycle.rs +++ b/src/ir/cycle.rs @@ -226,6 +226,20 @@ mod tests { assert_eq!(cycle, vec![path("a"), path("b"), path("a")]); } + #[test] + fn cycle_detector_stack_is_empty_after_cycle_detected() { + let mut targets = HashMap::new(); + targets.insert(path("a"), build_edge(&["b"], "a")); + targets.insert(path("b"), build_edge(&["a"], "b")); + + let mut detector = CycleDetector::new(&targets); + assert!(detector.detect().is_some(), "expected a cycle"); + assert!( + detector.stack.is_empty(), + "stack must be empty after cycle detection", + ); + } + fn check_canonicalize_cycle(input: &[&str], expected: &[&str]) { let cycle: Vec = input.iter().map(|&s| path(s)).collect(); let canonical = canonicalize_cycle(cycle); From e75e16d260665ea80bdf7af4ccf57fe4d53697f4 Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 27 May 2026 11:47:13 +0200 Subject: [PATCH 05/11] Expand cycle detection module documentation (#74) Document `analyse`, `CycleDetectionReport`, and the `CycleDetector` traversal flow in the module-level rustdoc. Co-authored-by: Cursor --- src/ir/cycle.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/ir/cycle.rs b/src/ir/cycle.rs index 9ee9dc54..781799f4 100644 --- a/src/ir/cycle.rs +++ b/src/ir/cycle.rs @@ -1,4 +1,18 @@ -//! Cycle detection utilities for the IR target graph. +//! Cycle detection for the IR target graph. +//! +//! The public entry point is [`analyse`], which accepts the target map +//! (`HashMap`) produced by IR lowering and +//! returns a [`CycleDetectionReport`]. The report carries an optional +//! detected cycle — an ordered, canonicalised list of paths — together +//! with any dependencies referenced by a target but absent from the map. +//! +//! Traversal state is managed by the private [`CycleDetector`] struct, +//! which owns the DFS recursion stack and per-node visitation map. +//! Callers drive detection through [`CycleDetector::detect`], which +//! iterates over every node in the target map and delegates depth-first +//! visiting to `visit` and `visit_dependency`. Detected cycles are +//! normalised by [`canonicalize_cycle`] to produce deterministic error +//! messages regardless of traversal order. use std::collections::HashMap; From b7a4253ac3aa881e2c141b1f4cda8e95cf7e1f3d Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 27 May 2026 11:48:11 +0200 Subject: [PATCH 06/11] Document cycle detection types and methods (#74) Add rustdoc to `CycleDetectionReport`, `analyse`, `CycleDetector`, and related helpers in `src/ir/cycle.rs`. Co-authored-by: Cursor --- src/ir/cycle.rs | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/ir/cycle.rs b/src/ir/cycle.rs index 781799f4..6723cc0d 100644 --- a/src/ir/cycle.rs +++ b/src/ir/cycle.rs @@ -27,11 +27,22 @@ enum VisitState { Visited, } +/// The result of a cycle-detection pass over the target graph. +/// +/// `cycle` is `Some` when a dependency cycle was found; the vec holds the +/// cycle's nodes in canonical order, with the first node repeated as the +/// last element. `missing_dependencies` lists every `(dependent, dep)` +/// pair where `dep` is referenced but absent from the target map. pub(crate) struct CycleDetectionReport { pub(crate) cycle: Option>, pub(crate) missing_dependencies: Vec<(Utf8PathBuf, Utf8PathBuf)>, } +/// Analyse `targets` for dependency cycles and missing dependencies. +/// +/// Returns a [`CycleDetectionReport`] containing the first detected cycle +/// (if any) and the full list of missing-dependency pairs encountered +/// during traversal. pub(crate) fn analyse(targets: &HashMap) -> CycleDetectionReport { let mut detector = CycleDetector::new(targets); let cycle = detector.detect(); @@ -41,6 +52,10 @@ pub(crate) fn analyse(targets: &HashMap) -> CycleDetecti } } +/// Depth-first cycle detector that owns its traversal state. +/// +/// Create with [`CycleDetector::new`] and drive detection with +/// [`CycleDetector::detect`]. struct CycleDetector<'targets> { targets: &'targets HashMap, stack: Vec, @@ -49,6 +64,8 @@ struct CycleDetector<'targets> { } impl CycleDetector<'_> { + /// Create a new detector borrowing `targets` for the duration of the + /// traversal. fn new(targets: &HashMap) -> CycleDetector<'_> { CycleDetector { targets, @@ -58,6 +75,8 @@ impl CycleDetector<'_> { } } + /// Walk every node in the target map and return the first cycle found, + /// or `None` if the graph is acyclic. fn detect(&mut self) -> Option> { for node in self.targets.keys() { if self.is_visited(node) { @@ -70,10 +89,15 @@ impl CycleDetector<'_> { None } + /// Return `true` if `node` has been fully visited. fn is_visited(&self, node: &Utf8PathBuf) -> bool { matches!(self.states.get(node), Some(VisitState::Visited)) } + /// Visit `node` depth-first. + /// + /// Returns `Some(cycle)` if a back-edge to an in-progress node is + /// discovered, `None` otherwise. fn visit(&mut self, node: Utf8PathBuf) -> Option> { match self.states.get(&node) { Some(VisitState::Visited) => return None, @@ -125,6 +149,8 @@ impl CycleDetector<'_> { } impl CycleDetector<'_> { + /// Record `dep` as missing and return `true` if `dep` is absent from the + /// target map; return `false` if it is present. fn record_missing_dependency(&mut self, node: &Utf8PathBuf, dep: &Utf8PathBuf) -> bool { if self.targets.contains_key(dep) { return false; @@ -139,6 +165,10 @@ impl CycleDetector<'_> { true } + /// Optionally record `dep` as missing, then visit it. + /// + /// Returns early with `None` when the dependency is absent from the target + /// map. fn visit_dependency( &mut self, node: &Utf8PathBuf, @@ -152,6 +182,11 @@ impl CycleDetector<'_> { } } +/// Rotate `cycle` so that the lexicographically smallest node appears +/// first, then re-close it by appending the first node. +/// +/// The input must contain at least two nodes; the first and last node are +/// expected to be identical (the standard DFS cycle representation). fn canonicalize_cycle(mut cycle: Vec) -> Vec { debug_assert!( cycle.len() >= 2, From 733dbedd17e239cd0d25006f11af10b692f8f77e Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 27 May 2026 11:49:17 +0200 Subject: [PATCH 07/11] Add proptest invariants for canonicalize_cycle (#74) Verify idempotence, rotation invariance, smallest-first ordering, and cycle closure with property-based tests. Co-authored-by: Cursor --- src/ir/cycle.rs | 69 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/src/ir/cycle.rs b/src/ir/cycle.rs index 6723cc0d..3149aff5 100644 --- a/src/ir/cycle.rs +++ b/src/ir/cycle.rs @@ -305,4 +305,73 @@ mod tests { fn canonicalize_cycle_handles_reverse_direction() { check_canonicalize_cycle(&["c", "b", "a", "c"], &["a", "c", "b", "a"]); } + + mod property_tests { + use proptest::prelude::*; + + use super::super::canonicalize_cycle; + use super::path; + + /// Generate a non-empty list of distinct single-character node names. + fn node_names(min: usize, max: usize) -> impl Strategy> { + proptest::collection::vec("[a-z]", min..=max).prop_filter( + "nodes must be unique", + |v| { + let set: std::collections::HashSet<_> = v.iter().collect(); + set.len() == v.len() + }, + ) + } + + /// Build a closed cycle from `nodes`: [...nodes, nodes[0]]. + fn make_cycle(nodes: &[String]) -> Vec { + let mut cycle: Vec<_> = nodes.iter().map(|s| path(s)).collect(); + cycle.push(path(&nodes[0])); + cycle + } + + proptest! { + /// Canonicalisation is idempotent: applying it twice yields the + /// same result as applying it once. + #[test] + fn canonicalize_is_idempotent(nodes in node_names(2, 10)) { + let cycle = make_cycle(&nodes); + let once = canonicalize_cycle(cycle.clone()); + let twice = canonicalize_cycle(once.clone()); + prop_assert_eq!(once, twice); + } + + /// All rotations of a cycle canonicalise to the same sequence. + #[test] + fn all_rotations_canonicalise_identically(nodes in node_names(2, 8)) { + let base = canonicalize_cycle(make_cycle(&nodes)); + for i in 1..nodes.len() { + let mut rotated = nodes.clone(); + rotated.rotate_left(i); + let result = canonicalize_cycle(make_cycle(&rotated)); + prop_assert_eq!(&base, &result); + } + } + + /// The first node in the canonical form is lexicographically <= + /// every other non-terminal node. + #[test] + fn canonical_first_node_is_smallest(nodes in node_names(2, 10)) { + let canonical = canonicalize_cycle(make_cycle(&nodes)); + let interior = &canonical[..canonical.len() - 1]; + let first = &canonical[0]; + for node in interior { + prop_assert!(first <= node); + } + } + + /// The canonical form is closed: first and last elements are + /// equal. + #[test] + fn canonical_cycle_is_closed(nodes in node_names(2, 10)) { + let canonical = canonicalize_cycle(make_cycle(&nodes)); + prop_assert_eq!(canonical.first(), canonical.last()); + } + } + } } From 9414c90f1d12d9bd9e9ec9b0bc9b01486180c57f Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 27 May 2026 11:51:40 +0200 Subject: [PATCH 08/11] Sort target keys before cycle detection (#74) Iterate nodes in deterministic order so the first reported cycle is stable across runs. Co-authored-by: Cursor --- src/ir/cycle.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/ir/cycle.rs b/src/ir/cycle.rs index 3149aff5..4fb3c4b2 100644 --- a/src/ir/cycle.rs +++ b/src/ir/cycle.rs @@ -78,11 +78,13 @@ impl CycleDetector<'_> { /// Walk every node in the target map and return the first cycle found, /// or `None` if the graph is acyclic. fn detect(&mut self) -> Option> { - for node in self.targets.keys() { - if self.is_visited(node) { + let mut nodes: Vec = self.targets.keys().cloned().collect(); + nodes.sort(); + for node in nodes { + if self.is_visited(&node) { continue; } - if let Some(cycle) = self.visit(node.clone()) { + if let Some(cycle) = self.visit(node) { return Some(cycle); } } From 194c44e453c6aa33e9a7c653be5dca907df10b4b Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 27 May 2026 11:54:55 +0200 Subject: [PATCH 09/11] Add cycle determinism and multi-cycle tests (#74) Add `find_cycle_is_deterministic` (100-run stability check) and `find_cycle_detects_one_of_multiple_disjoint_cycles` for two disjoint 2-node cycles. Accept `&Utf8Path` in visit helpers to avoid redundant clones. Co-authored-by: Cursor --- src/ir/cycle.rs | 76 +++++++++++++++++++++++++++++++++++-------------- 1 file changed, 55 insertions(+), 21 deletions(-) diff --git a/src/ir/cycle.rs b/src/ir/cycle.rs index 4fb3c4b2..3c367e3e 100644 --- a/src/ir/cycle.rs +++ b/src/ir/cycle.rs @@ -16,7 +16,7 @@ use std::collections::HashMap; -use camino::Utf8PathBuf; +use camino::{Utf8Path, Utf8PathBuf}; use super::BuildEdge; @@ -81,10 +81,10 @@ impl CycleDetector<'_> { let mut nodes: Vec = self.targets.keys().cloned().collect(); nodes.sort(); for node in nodes { - if self.is_visited(&node) { + if self.is_visited(node.as_path()) { continue; } - if let Some(cycle) = self.visit(node) { + if let Some(cycle) = self.visit(node.as_path()) { return Some(cycle); } } @@ -92,7 +92,7 @@ impl CycleDetector<'_> { } /// Return `true` if `node` has been fully visited. - fn is_visited(&self, node: &Utf8PathBuf) -> bool { + fn is_visited(&self, node: &Utf8Path) -> bool { matches!(self.states.get(node), Some(VisitState::Visited)) } @@ -100,40 +100,40 @@ impl CycleDetector<'_> { /// /// Returns `Some(cycle)` if a back-edge to an in-progress node is /// discovered, `None` otherwise. - fn visit(&mut self, node: Utf8PathBuf) -> Option> { - match self.states.get(&node) { + fn visit(&mut self, node: &Utf8Path) -> Option> { + match self.states.get(node) { Some(VisitState::Visited) => return None, Some(VisitState::Visiting) => { let idx = self .stack .iter() - .position(|n| n == &node) + .position(|n| n.as_path() == node) .unwrap_or_else(|| { debug_assert!(false, "visiting node must be on the stack"); 0 }); let mut cycle: Vec = self.stack.iter().skip(idx).cloned().collect(); - cycle.push(node); + cycle.push(node.to_path_buf()); return Some(canonicalize_cycle(cycle)); } None => { - self.states.insert(node.clone(), VisitState::Visiting); + self.states.insert(node.to_path_buf(), VisitState::Visiting); } } - self.stack.push(node.clone()); + self.stack.push(node.to_path_buf()); let cycle = self .targets - .get(&node) + .get(node) .into_iter() .flat_map(|edge| edge.inputs.iter()) - .find_map(|dep| self.visit_dependency(&node, dep)); + .find_map(|dep| self.visit_dependency(node, dep)); self.stack.pop(); if cycle.is_none() { - self.states.insert(node, VisitState::Visited); + self.states.insert(node.to_path_buf(), VisitState::Visited); } cycle @@ -153,7 +153,7 @@ impl CycleDetector<'_> { impl CycleDetector<'_> { /// Record `dep` as missing and return `true` if `dep` is absent from the /// target map; return `false` if it is present. - fn record_missing_dependency(&mut self, node: &Utf8PathBuf, dep: &Utf8PathBuf) -> bool { + fn record_missing_dependency(&mut self, node: &Utf8Path, dep: &Utf8Path) -> bool { if self.targets.contains_key(dep) { return false; } @@ -163,7 +163,8 @@ impl CycleDetector<'_> { dependent = %node, "skipping dependency missing from targets during cycle detection", ); - self.missing_dependencies.push((node.clone(), dep.clone())); + self.missing_dependencies + .push((node.to_path_buf(), dep.to_path_buf())); true } @@ -173,14 +174,14 @@ impl CycleDetector<'_> { /// map. fn visit_dependency( &mut self, - node: &Utf8PathBuf, - dep: &Utf8PathBuf, + node: &Utf8Path, + dep: &Utf8Path, ) -> Option> { if self.record_missing_dependency(node, dep) { return None; } - self.visit(dep.clone()) + self.visit(dep) } } @@ -248,8 +249,8 @@ mod tests { let mut detector = CycleDetector::new(&targets); assert!(detector.detect().is_none()); - assert!(detector.is_visited(&a)); - assert!(detector.is_visited(&b)); + assert!(detector.is_visited(a.as_path())); + assert!(detector.is_visited(b.as_path())); assert!( detector.stack.is_empty(), "stack should be empty after complete traversal", @@ -262,7 +263,7 @@ mod tests { targets.insert(path("a"), build_edge(&["b"], "a")); let mut detector = CycleDetector::new(&targets); - assert!(detector.visit(path("a")).is_none()); + assert!(detector.visit(path("a").as_path()).is_none()); assert_eq!(detector.missing_dependencies(), &[(path("a"), path("b"))],); } @@ -277,6 +278,39 @@ mod tests { assert_eq!(cycle, vec![path("a"), path("b"), path("a")]); } + #[test] + fn find_cycle_is_deterministic() { + let mut targets = HashMap::new(); + targets.insert(path("p"), build_edge(&["q"], "p")); + targets.insert(path("q"), build_edge(&["p"], "q")); + targets.insert(path("x"), build_edge(&["y"], "x")); + targets.insert(path("y"), build_edge(&["x"], "y")); + + let first = CycleDetector::find_cycle(&targets).expect("cycle"); + for _ in 1..100 { + let cycle = CycleDetector::find_cycle(&targets).expect("cycle"); + if cycle != first { + panic!( + "find_cycle returned inconsistent results across runs: \ + first={first:?}, got={cycle:?}", + ); + } + } + // Probabilistic guard: 100 runs; `detect` sorts keys for stable traversal. + tracing::info!("find_cycle returned the same cycle across 100 runs"); + } + + #[test] + fn find_cycle_detects_one_of_multiple_disjoint_cycles() { + let mut targets = HashMap::new(); + targets.insert(path("p"), build_edge(&["q"], "p")); + targets.insert(path("q"), build_edge(&["p"], "q")); + targets.insert(path("x"), build_edge(&["y"], "x")); + targets.insert(path("y"), build_edge(&["x"], "y")); + + assert!(CycleDetector::find_cycle(&targets).is_some()); + } + #[test] fn cycle_detector_stack_is_empty_after_cycle_detected() { let mut targets = HashMap::new(); From 35481cdae17aa701f10c92bdf367f90260722f7b Mon Sep 17 00:00:00 2001 From: leynos Date: Thu, 28 May 2026 02:00:36 +0200 Subject: [PATCH 10/11] Fix formatting and lint issues in cycle detection tests (#74) Adjust `visit_dependency` signature and `node_names` `prop_filter` chain to satisfy `rustfmt`. Replace `if ... { panic!() }` with `assert!` in the determinism test to satisfy `clippy::manual_assert`, and use `.get()` / `.first()` with `.expect()` in proptest helpers to satisfy `clippy::indexing_slicing`. --- src/ir/cycle.rs | 42 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/src/ir/cycle.rs b/src/ir/cycle.rs index 3c367e3e..f6d42913 100644 --- a/src/ir/cycle.rs +++ b/src/ir/cycle.rs @@ -172,11 +172,7 @@ impl CycleDetector<'_> { /// /// Returns early with `None` when the dependency is absent from the target /// map. - fn visit_dependency( - &mut self, - node: &Utf8Path, - dep: &Utf8Path, - ) -> Option> { + fn visit_dependency(&mut self, node: &Utf8Path, dep: &Utf8Path) -> Option> { if self.record_missing_dependency(node, dep) { return None; } @@ -289,12 +285,11 @@ mod tests { let first = CycleDetector::find_cycle(&targets).expect("cycle"); for _ in 1..100 { let cycle = CycleDetector::find_cycle(&targets).expect("cycle"); - if cycle != first { - panic!( - "find_cycle returned inconsistent results across runs: \ - first={first:?}, got={cycle:?}", - ); - } + assert!( + cycle == first, + "find_cycle returned inconsistent results across runs: \ + first={first:?}, got={cycle:?}", + ); } // Probabilistic guard: 100 runs; `detect` sorts keys for stable traversal. tracing::info!("find_cycle returned the same cycle across 100 runs"); @@ -350,19 +345,20 @@ mod tests { /// Generate a non-empty list of distinct single-character node names. fn node_names(min: usize, max: usize) -> impl Strategy> { - proptest::collection::vec("[a-z]", min..=max).prop_filter( - "nodes must be unique", - |v| { - let set: std::collections::HashSet<_> = v.iter().collect(); - set.len() == v.len() - }, - ) + proptest::collection::vec("[a-z]", min..=max).prop_filter("nodes must be unique", |v| { + let set: std::collections::HashSet<_> = v.iter().collect(); + set.len() == v.len() + }) } /// Build a closed cycle from `nodes`: [...nodes, nodes[0]]. fn make_cycle(nodes: &[String]) -> Vec { let mut cycle: Vec<_> = nodes.iter().map(|s| path(s)).collect(); - cycle.push(path(&nodes[0])); + cycle.push(path( + nodes + .first() + .expect("node_names generates at least two nodes"), + )); cycle } @@ -394,8 +390,12 @@ mod tests { #[test] fn canonical_first_node_is_smallest(nodes in node_names(2, 10)) { let canonical = canonicalize_cycle(make_cycle(&nodes)); - let interior = &canonical[..canonical.len() - 1]; - let first = &canonical[0]; + let interior = canonical + .get(..canonical.len().saturating_sub(1)) + .expect("canonicalize_cycle produces at least two nodes"); + let first = canonical + .first() + .expect("canonicalize_cycle produces at least one node"); for node in interior { prop_assert!(first <= node); } From 0f51928b9fb91057ba48d98ad05780d6d9f59eb3 Mon Sep 17 00:00:00 2001 From: leynos Date: Thu, 28 May 2026 02:03:24 +0200 Subject: [PATCH 11/11] Fix markdownlint violations in design doc Remove an extra blank line before a heading and delete a duplicate Section 2.7 table block that was repeated later in the document. --- docs/netsuke-design.md | 30 +++++++----------------------- 1 file changed, 7 insertions(+), 23 deletions(-) diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index 3b56264a..ae0ec4d8 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -400,7 +400,6 @@ The cleaner model is: - `always`: When set to `true`, the target runs on every invocation regardless of timestamps or dependencies. The default value is `false`. - ### 2.7 Table: Netsuke Manifest vs. Makefile To illustrate the ergonomic advantages of the Netsuke schema, the following @@ -616,21 +615,6 @@ output: manifest keys for recipe selection. Warnings should be reserved for degraded behaviour that Netsuke can classify itself. -### 2.7 Table: Netsuke Manifest vs. Makefile - -To illustrate the ergonomic advantages of the Netsuke schema, the following -table compares a simple C compilation project defined in both a traditional -`Makefile` and a `Netsukefile` file. The comparison highlights Netsuke's -explicit, structured, and self-documenting nature. - -| Feature | Makefile Example | Netsukefile Example | -| --------------- | ---------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------- | -| Variables | CC=gcc | { vars: { cc: gcc } } | -| Macros | define greet\\t@echo Hello $$1endef | { macros: { signature: "greet(name)", body: "Hello {{ name }}" } } | -| Rule Definition | %.o: %.c\\n\\t$(CC) -c $< -o $@ | { rules: { name: compile, command: "{{ cc }} -c {{ ins }} -o {{ outs }}", description: "Compiling {{ outs }}" } } | -| Target Build | my_program: main.o utils.o\\t$(CC) $^ -o $@ | { targets: { name: my_program, rule: link, sources: [main.o, utils.o] } | -| Readability | Relies on cryptic automatic variables ($@, $\<, $^) and implicit pattern matching. | Uses explicit, descriptive keys (name, rule, sources) and standard YAML list/map syntax. | - ## Section 3: Parsing and Deserialization Strategy Once the Jinja evaluation stage has produced a pure YAML string, the next @@ -640,7 +624,7 @@ data structures are crucial for the robustness and maintainability of Netsuke. ### 3.1 Crate Selection: `serde_saphyr` -Netsuke now relies on `serde_saphyr` for YAML parsing and serialisation. The +Netsuke now relies on `serde_saphyr` for YAML parsing and serialization. The crate wraps the actively maintained `saphyr` parser while preserving the familiar `serde_yaml`-style API: helpers such as `from_str`, `from_reader`, and `to_string` integrate cleanly with `serde` derives, and the error type exposes @@ -1249,11 +1233,11 @@ Semantics honour platform conventions while enforcing predictable behaviour: bit. Empty `PATH` segments (leading, trailing, or `::`) map to the working directory when `cwd_mode` is `"auto"` or `"always"`. - On Windows, the lookup respects `PATHEXT` when the command lacks an - extension. Comparisons are case-insensitive, results normalise both slash + extension. Comparisons are case-insensitive, results normalize both slash styles, and `cwd_mode` defaults to skipping the working directory to avoid the platform’s surprise "search CWD first" rule. Opting in via `"always"` restores that behaviour. -- Canonicalisation happens after discovery and only when requested so that +- Canonicalization happens after discovery and only when requested so that manifests can balance reproducibility against host-specific absolute paths. The resolver keeps a small LRU cache keyed by the command, a fingerprint of @@ -1722,7 +1706,7 @@ use camino::Utf8PathBuf; /// The complete, static build graph. pub struct BuildGraph { /// A map of all unique actions (rules) in the build. - /// The key is a hash of a canonical JSON serialisation of the action's + /// The key is a hash of a canonical JSON serialization of the action's /// properties to enable deduplication. pub actions: HashMap, @@ -1984,7 +1968,7 @@ manifest. No Ninja specific placeholders are stored in the IR to keep the representation portable. - Actions are deduplicated using a SHA-256 hash of a canonical JSON - serialisation of their recipe, inputs, and outputs. Because commands embed + serialization of their recipe, inputs, and outputs. Because commands embed shell-quoted file paths, two targets share an identifier only when both the command text and file sets match exactly. - Multiple rule references in a single target are not yet supported. The IR @@ -2254,7 +2238,7 @@ enrichment: `.with_context(|| "Failed to build the internal build graph from the manifest")?` . -4. This process of propagation and contextualisation repeats as the error +4. This process of propagation and contextualization repeats as the error bubbles up towards `main`. Use `anyhow::Context` to add detail, but never convert a `miette::Diagnostic` into a plain `anyhow::Error`--doing so would discard spans and help text. @@ -2354,7 +2338,7 @@ given). /// This is the default subcommand. Build(BuildArgs), /// Remove build artefacts and intermediate files. Clean, - /// Display the build dependency graph in DOT format for visualisation. + /// Display the build dependency graph in DOT format for visualization. Graph, /// Write the Ninja manifest to `FILE` without invoking Ninja.