From 630615ea2ed0fe28aa93a8a554f4f9fa3a0a89b6 Mon Sep 17 00:00:00 2001
From: Leynos <leynos@troubledskies.net>
Date: Thu, 31 Jul 2025 11:34:02 +0100
Subject: [PATCH 1/5] Add Ninja file generator

---
 docs/netsuke-design.md       |   3 +
 docs/roadmap.md              |   8 +-
 src/lib.rs                   |   1 +
 src/ninja_gen.rs             | 164 +++++++++++++++++++++++++++++++++++
 tests/cucumber.rs            |   1 +
 tests/features/ninja.feature |  12 +++
 tests/ninja_gen_tests.rs     |  41 +++++++++
 tests/steps/mod.rs           |   1 +
 tests/steps/ninja_steps.rs   |  21 +++++
 9 files changed, 248 insertions(+), 4 deletions(-)
 create mode 100644 src/ninja_gen.rs
 create mode 100644 tests/features/ninja.feature
 create mode 100644 tests/ninja_gen_tests.rs
 create mode 100644 tests/steps/ninja_steps.rs

diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md
index 9820e8e0..bfe304a2 100644
--- a/docs/netsuke-design.md
+++ b/docs/netsuke-design.md
@@ -1071,6 +1071,9 @@ representation portable.
   generator reports `IrGenError::MultipleRules` when encountered.
 - Duplicate output files are rejected. Attempting to define the same output
   path twice results in `IrGenError::DuplicateOutput`.
+- The Ninja generator sorts actions and edges before output and
+  deduplicates edges by their first explicit output to ensure deterministic
+  `build.ninja` files.
 
 ## Section 6: Process Management and Secure Execution
 
diff --git a/docs/roadmap.md b/docs/roadmap.md
index 8e1dc7b2..dd08fcde 100644
--- a/docs/roadmap.md
+++ b/docs/roadmap.md
@@ -55,11 +55,11 @@ compilation pipeline from parsing to execution.
 
 - [ ] **Code Generation and Execution:**
 
-  - [ ] Implement the Ninja file synthesizer in
-    [src/ninja_gen.rs](src/ninja_gen.rs) to traverse the BuildGraph IR.
+  - [x] Implement the Ninja file synthesizer in
+    [src/ninja_gen.rs](src/ninja_gen.rs) to traverse the BuildGraph IR. *(done)*
 
-  - [ ] Write logic to generate Ninja rule statements from ir::Action structs
-    and build statements from ir::BuildEdge structs.
+  - [x] Write logic to generate Ninja rule statements from ir::Action structs
+    and build statements from ir::BuildEdge structs. *(done)*
 
   - [ ] Implement the process management logic in `main.rs` to invoke the ninja
     executable as a subprocess using `std::process::Command`.
diff --git a/src/lib.rs b/src/lib.rs
index 1999d5e0..adbc03fd 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -8,4 +8,5 @@ pub mod cli;
 pub mod hasher;
 pub mod ir;
 pub mod manifest;
+pub mod ninja_gen;
 pub mod runner;
diff --git a/src/ninja_gen.rs b/src/ninja_gen.rs
new file mode 100644
index 00000000..7c4403a0
--- /dev/null
+++ b/src/ninja_gen.rs
@@ -0,0 +1,164 @@
+//! Ninja file generator.
+//!
+//! This module converts a [`crate::ir::BuildGraph`] into the textual
+//! representation expected by the Ninja build system.
+//! The output is deterministic: actions and edges are sorted to ensure
+//! stable snapshots for testing.
+
+use crate::ast::Recipe;
+use crate::ir::{BuildEdge, BuildGraph};
+use std::collections::{HashMap, HashSet};
+use std::fmt::Write;
+use std::path::PathBuf;
+
+/// Generate a Ninja build file as a string.
+#[must_use]
+pub fn generate(graph: &BuildGraph) -> String {
+    let mut out = String::new();
+    write_rules(&mut out, &graph.actions);
+    write_edges(&mut out, &graph.targets);
+    write_defaults(&mut out, &graph.default_targets);
+    out
+}
+
+fn write_rules(out: &mut String, actions: &HashMap<String, crate::ir::Action>) {
+    let mut ids: Vec<_> = actions.keys().collect();
+    ids.sort();
+    for id in ids {
+        let action = &actions[id];
+        let _ = writeln!(out, "rule {id}");
+        match &action.recipe {
+            Recipe::Command { command } => {
+                let _ = writeln!(out, "  command = {command}");
+            }
+            Recipe::Script { script } => {
+                let _ = writeln!(out, "  command = /bin/sh -e -c \"");
+                for line in script.lines() {
+                    let _ = writeln!(out, "    {line}");
+                }
+                let _ = writeln!(out, "  \"");
+            }
+            Recipe::Rule { .. } => unreachable!("rules do not reference other rules"),
+        }
+        if let Some(desc) = &action.description {
+            let _ = writeln!(out, "  description = {desc}");
+        }
+        if let Some(depfile) = &action.depfile {
+            let _ = writeln!(out, "  depfile = {depfile}");
+        }
+        if let Some(deps_format) = &action.deps_format {
+            let _ = writeln!(out, "  deps = {deps_format}");
+        }
+        if let Some(pool) = &action.pool {
+            let _ = writeln!(out, "  pool = {pool}");
+        }
+        if action.restat {
+            let _ = writeln!(out, "  restat = 1");
+        }
+        let _ = writeln!(out);
+    }
+}
+
+fn write_edges(out: &mut String, targets: &HashMap<PathBuf, BuildEdge>) {
+    let mut edges: Vec<&BuildEdge> = targets.values().collect();
+    edges.sort_by(|a, b| a.explicit_outputs.cmp(&b.explicit_outputs));
+    let mut seen = HashSet::new();
+    for edge in edges {
+        if edge
+            .explicit_outputs
+            .first()
+            .is_some_and(|f| !seen.insert(f))
+        {
+            continue;
+        }
+        write_edge(out, edge);
+    }
+}
+
+fn write_edge(out: &mut String, edge: &BuildEdge) {
+    let outputs = join(&edge.explicit_outputs);
+    let _ = write!(out, "build {outputs}");
+    if !edge.implicit_outputs.is_empty() {
+        let _ = write!(out, " | {}", join(&edge.implicit_outputs));
+    }
+    let rule = if edge.phony { "phony" } else { &edge.action_id };
+    let _ = write!(out, ": {rule}");
+    if !edge.inputs.is_empty() {
+        let _ = write!(out, " {}", join(&edge.inputs));
+    }
+    if !edge.order_only_deps.is_empty() {
+        let _ = write!(out, " || {}", join(&edge.order_only_deps));
+    }
+    let _ = writeln!(out);
+    if edge.always {
+        let _ = writeln!(out, "  restat = 1");
+    }
+    let _ = writeln!(out);
+}
+
+fn write_defaults(out: &mut String, defaults: &[PathBuf]) {
+    if defaults.is_empty() {
+        return;
+    }
+    let mut defs: Vec<_> = defaults.iter().collect();
+    defs.sort();
+    let _ = writeln!(
+        out,
+        "default {}",
+        defs.iter()
+            .map(|p| p.display().to_string())
+            .collect::<Vec<_>>()
+            .join(" ")
+    );
+}
+
+fn join(paths: &[PathBuf]) -> String {
+    paths
+        .iter()
+        .map(|p| p.display().to_string())
+        .collect::<Vec<_>>()
+        .join(" ")
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::ir::{Action, BuildEdge, BuildGraph};
+    use rstest::rstest;
+
+    #[rstest]
+    fn generate_simple_ninja() {
+        let action = Action {
+            recipe: Recipe::Command {
+                command: "echo hi".into(),
+            },
+            description: None,
+            depfile: None,
+            deps_format: None,
+            pool: None,
+            restat: false,
+        };
+        let edge = BuildEdge {
+            action_id: "a".into(),
+            inputs: vec![PathBuf::from("in")],
+            explicit_outputs: vec![PathBuf::from("out")],
+            implicit_outputs: Vec::new(),
+            order_only_deps: Vec::new(),
+            phony: false,
+            always: false,
+        };
+        let mut graph = BuildGraph::default();
+        graph.actions.insert("a".into(), action);
+        graph.targets.insert(PathBuf::from("out"), edge);
+        graph.default_targets.push(PathBuf::from("out"));
+
+        let ninja = generate(&graph);
+        let expected = concat!(
+            "rule a\n",
+            "  command = echo hi\n\n",
+            "build out: a in\n\n",
+            "default out\n"
+        );
+        assert_eq!(ninja, expected);
+    }
+}
diff --git a/tests/cucumber.rs b/tests/cucumber.rs
index 90f57129..56c422d8 100644
--- a/tests/cucumber.rs
+++ b/tests/cucumber.rs
@@ -7,6 +7,7 @@ pub struct CliWorld {
     pub manifest: Option<netsuke::ast::NetsukeManifest>,
     pub manifest_error: Option<String>,
     pub build_graph: Option<netsuke::ir::BuildGraph>,
+    pub ninja: Option<String>,
 }
 
 mod steps;
diff --git a/tests/features/ninja.feature b/tests/features/ninja.feature
new file mode 100644
index 00000000..1f98fd0b
--- /dev/null
+++ b/tests/features/ninja.feature
@@ -0,0 +1,12 @@
+Feature: Ninja file generation
+
+  Scenario: Generate build statements
+    When the manifest file "tests/data/rules.yml" is compiled to IR
+    And the ninja file is generated
+    Then the ninja file contains "rule"
+    And the ninja file contains "build hello.o:"
+
+  Scenario: Phony target rule
+    When the manifest file "tests/data/phony.yml" is compiled to IR
+    And the ninja file is generated
+    Then the ninja file contains "build clean: phony"
diff --git a/tests/ninja_gen_tests.rs b/tests/ninja_gen_tests.rs
new file mode 100644
index 00000000..fb896975
--- /dev/null
+++ b/tests/ninja_gen_tests.rs
@@ -0,0 +1,41 @@
+//! Unit tests for Ninja file generation.
+
+use netsuke::ast::Recipe;
+use netsuke::ir::{Action, BuildEdge, BuildGraph};
+use netsuke::ninja_gen::generate;
+use rstest::rstest;
+use std::path::PathBuf;
+
+#[rstest]
+fn generate_phony() {
+    let action = Action {
+        recipe: Recipe::Command {
+            command: "true".into(),
+        },
+        description: None,
+        depfile: None,
+        deps_format: None,
+        pool: None,
+        restat: false,
+    };
+    let edge = BuildEdge {
+        action_id: "a".into(),
+        inputs: vec![PathBuf::from("in")],
+        explicit_outputs: vec![PathBuf::from("out")],
+        implicit_outputs: Vec::new(),
+        order_only_deps: Vec::new(),
+        phony: true,
+        always: false,
+    };
+    let mut graph = BuildGraph::default();
+    graph.actions.insert("a".into(), action);
+    graph.targets.insert(PathBuf::from("out"), edge);
+
+    let ninja = generate(&graph);
+    let expected = concat!(
+        "rule a\n",
+        "  command = true\n\n",
+        "build out: phony in\n\n",
+    );
+    assert_eq!(ninja, expected);
+}
diff --git a/tests/steps/mod.rs b/tests/steps/mod.rs
index 8524b752..cb30bcdf 100644
--- a/tests/steps/mod.rs
+++ b/tests/steps/mod.rs
@@ -1,3 +1,4 @@
 mod cli_steps;
 mod ir_steps;
 mod manifest_steps;
+mod ninja_steps;
diff --git a/tests/steps/ninja_steps.rs b/tests/steps/ninja_steps.rs
new file mode 100644
index 00000000..6c2ceda5
--- /dev/null
+++ b/tests/steps/ninja_steps.rs
@@ -0,0 +1,21 @@
+//! Step definitions for Ninja file generation scenarios.
+
+use crate::CliWorld;
+use cucumber::{then, when};
+use netsuke::ninja_gen;
+
+#[when("the ninja file is generated")]
+fn generate_ninja(world: &mut CliWorld) {
+    let graph = world.build_graph.as_ref().expect("graph");
+    world.ninja = Some(ninja_gen::generate(graph));
+}
+
+#[expect(
+    clippy::needless_pass_by_value,
+    reason = "Cucumber requires owned String arguments"
+)]
+#[then(expr = "the ninja file contains {string}")]
+fn ninja_contains(world: &mut CliWorld, text: String) {
+    let ninja = world.ninja.as_ref().expect("ninja");
+    assert!(ninja.contains(&text));
+}

From 8f64cef84cf07ce740daa377625206f178396fd6 Mon Sep 17 00:00:00 2001
From: Leynos <leynos@troubledskies.net>
Date: Thu, 31 Jul 2025 13:04:04 +0100
Subject: [PATCH 2/5] Add Ninja snapshot validation

---
 Cargo.lock                                    | 77 ++++++++++++++++++-
 Cargo.toml                                    |  2 +
 docs/netsuke-design.md                        |  2 +
 tests/ninja_snapshot_tests.rs                 | 76 ++++++++++++++++++
 ..._snapshot_tests__touch_manifest_ninja.snap |  8 ++
 5 files changed, 164 insertions(+), 1 deletion(-)
 create mode 100644 tests/ninja_snapshot_tests.rs
 create mode 100644 tests/snapshots/ninja/ninja_snapshot_tests__touch_manifest_ninja.snap

diff --git a/Cargo.lock b/Cargo.lock
index 6bc9d9c8..419b0a0f 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -370,6 +370,12 @@ dependencies = [
  "windows-sys 0.60.2",
 ]
 
+[[package]]
+name = "fastrand"
+version = "2.3.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "37909eebbb50d72f9059c3b6d82c0463f2ff062c9e95845c43a6c9c0355411be"
+
 [[package]]
 name = "futures"
 version = "0.3.31"
@@ -475,6 +481,18 @@ dependencies = [
  "version_check",
 ]
 
+[[package]]
+name = "getrandom"
+version = "0.3.3"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "26145e563e54f2cadc477553f1ec5ee650b00862f0a58bcd12cbdc5f0ea2d2f4"
+dependencies = [
+ "cfg-if",
+ "libc",
+ "r-efi",
+ "wasi 0.14.2+wasi-0.2.4",
+]
+
 [[package]]
 name = "gherkin"
 version = "0.14.0"
@@ -584,6 +602,18 @@ version = "1.1.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "a257582fdcde896fd96463bf2d40eefea0580021c0712a0e2b028b60b47a837a"
 
+[[package]]
+name = "insta"
+version = "1.43.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "154934ea70c58054b556dd430b99a98c2a7ff5309ac9891597e339b5c28f4371"
+dependencies = [
+ "console",
+ "once_cell",
+ "serde",
+ "similar",
+]
+
 [[package]]
 name = "inventory"
 version = "0.3.20"
@@ -710,7 +740,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "78bed444cc8a2160f01cbcf811ef18cac863ad68ae8ca62092e8db51d51c761c"
 dependencies = [
  "libc",
- "wasi",
+ "wasi 0.11.1+wasi-snapshot-preview1",
  "windows-sys 0.59.0",
 ]
 
@@ -721,12 +751,14 @@ dependencies = [
  "anyhow",
  "clap",
  "cucumber",
+ "insta",
  "itoa",
  "rstest",
  "semver",
  "serde",
  "serde_yml",
  "sha2",
+ "tempfile",
  "thiserror",
  "tokio",
 ]
@@ -850,6 +882,12 @@ dependencies = [
  "proc-macro2",
 ]
 
+[[package]]
+name = "r-efi"
+version = "5.3.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "69cdb34c158ceb288df11e18b4bd39de994f6657d83847bdffdbd7f346754b0f"
+
 [[package]]
 name = "regex"
 version = "1.11.1"
@@ -1048,6 +1086,12 @@ dependencies = [
  "digest",
 ]
 
+[[package]]
+name = "similar"
+version = "2.7.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "bbbb5d9659141646ae647b42fe094daf6c6192d1620870b449d9557f748b2daa"
+
 [[package]]
 name = "slab"
 version = "0.4.10"
@@ -1121,6 +1165,19 @@ dependencies = [
  "syn",
 ]
 
+[[package]]
+name = "tempfile"
+version = "3.20.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e8a64e3985349f2441a1a9ef0b853f869006c3855f2cda6862a94d26ebb9d6a1"
+dependencies = [
+ "fastrand",
+ "getrandom",
+ "once_cell",
+ "rustix",
+ "windows-sys 0.59.0",
+]
+
 [[package]]
 name = "terminal_size"
 version = "0.4.2"
@@ -1260,6 +1317,15 @@ version = "0.11.1+wasi-snapshot-preview1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "ccf3ec651a847eb01de73ccad15eb7d99f80485de043efb2f370cd654f4ea44b"
 
+[[package]]
+name = "wasi"
+version = "0.14.2+wasi-0.2.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "9683f9a5a998d873c0d21fcbe3c083009670149a8fab228644b8bd36b2c48cb3"
+dependencies = [
+ "wit-bindgen-rt",
+]
+
 [[package]]
 name = "winapi-util"
 version = "0.1.9"
@@ -1414,3 +1480,12 @@ name = "windows_x86_64_msvc"
 version = "0.53.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "271414315aff87387382ec3d271b52d7ae78726f5d44ac98b4f4030c91880486"
+
+[[package]]
+name = "wit-bindgen-rt"
+version = "0.39.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "6f42320e61fe2cfd34354ecb597f86f413484a798ba44a8ca1165c58d42da6c1"
+dependencies = [
+ "bitflags 2.9.1",
+]
diff --git a/Cargo.toml b/Cargo.toml
index d8967ba6..02e7933d 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -57,6 +57,8 @@ float_arithmetic = "deny"
 rstest = "0.18.0"
 cucumber = "0.20.0"
 tokio = { version = "1", features = ["macros", "rt-multi-thread"], default-features = false }
+insta = { version = "1", features = ["yaml"] }
+tempfile = "3"
 
 [[test]]
 name = "cucumber"
diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md
index bfe304a2..6b405287 100644
--- a/docs/netsuke-design.md
+++ b/docs/netsuke-design.md
@@ -1074,6 +1074,8 @@ representation portable.
 - The Ninja generator sorts actions and edges before output and
   deduplicates edges by their first explicit output to ensure deterministic
   `build.ninja` files.
+- Integration tests snapshot the generated Ninja file with `insta` and
+  execute the Ninja binary to validate structure and no-op behaviour.
 
 ## Section 6: Process Management and Secure Execution
 
diff --git a/tests/ninja_snapshot_tests.rs b/tests/ninja_snapshot_tests.rs
new file mode 100644
index 00000000..501d5a95
--- /dev/null
+++ b/tests/ninja_snapshot_tests.rs
@@ -0,0 +1,76 @@
+//! End-to-end validation of Ninja file generation.
+//!
+//! These tests generate a Ninja file from a manifest, snapshot the
+//! output using `insta`, and validate it with the real `ninja`
+//! executable. The manifest uses a simple TOUCH rule so the build is
+//! fast and deterministic.
+
+use insta::{Settings, assert_snapshot};
+use netsuke::{ir::BuildGraph, manifest, ninja_gen};
+use std::{fs, process::Command};
+use tempfile::tempdir;
+
+fn run_ok(cmd: &mut Command) -> String {
+    let out = cmd.output().expect("should spawn command");
+    assert!(
+        out.status.success(),
+        "command failed: {}",
+        String::from_utf8_lossy(&out.stderr)
+    );
+    String::from_utf8(out.stdout).expect("stdout utf8")
+}
+
+#[test]
+fn touch_manifest_ninja_validation() {
+    let manifest_yaml = r#"
+        netsuke_version: "1.0.0"
+        rules:
+          - name: touch
+            recipe:
+              kind: command
+              command: "python3 -c 'import os,sys; open(sys.argv[1],\"a\").close()' $out"
+        targets:
+          - name: out/a
+            sources: in/a
+            recipe:
+              kind: rule
+              rule: touch
+    "#;
+
+    let manifest = manifest::from_str(manifest_yaml).expect("parse manifest");
+    let ir = BuildGraph::from_manifest(&manifest).expect("ir generation");
+    let ninja_content = ninja_gen::generate(&ir);
+
+    let mut settings = Settings::new();
+    settings.set_snapshot_path(concat!(
+        env!("CARGO_MANIFEST_DIR"),
+        "/tests/snapshots/ninja"
+    ));
+    settings.bind(|| {
+        assert_snapshot!("touch_manifest_ninja", ninja_content);
+    });
+
+    let dir = tempdir().expect("tempdir");
+    let build_file = dir.path().join("build.ninja");
+    fs::write(&build_file, &ninja_content).expect("write ninja");
+    fs::create_dir_all(dir.path().join("in")).expect("dir");
+    fs::write(dir.path().join("in/a"), "").expect("input");
+
+    let ninja_cmd = |args: &[&str]| {
+        let mut cmd = Command::new("ninja");
+        cmd.arg("-f").arg(&build_file).args(args);
+        cmd.current_dir(&dir);
+        run_ok(&mut cmd)
+    };
+
+    let _ = ninja_cmd(&["-t", "rules"]);
+    let _ = ninja_cmd(&["-t", "targets", "all"]);
+    let _ = ninja_cmd(&["-t", "query", "out/a"]);
+
+    let _ = ninja_cmd(&["-w", "dupbuild=err", "-d", "stats"]);
+    let second = ninja_cmd(&["-n", "-d", "explain", "-v"]);
+    assert!(
+        second.contains("no work to do"),
+        "expected no-op second pass, got:\n{second}"
+    );
+}
diff --git a/tests/snapshots/ninja/ninja_snapshot_tests__touch_manifest_ninja.snap b/tests/snapshots/ninja/ninja_snapshot_tests__touch_manifest_ninja.snap
new file mode 100644
index 00000000..ca715357
--- /dev/null
+++ b/tests/snapshots/ninja/ninja_snapshot_tests__touch_manifest_ninja.snap
@@ -0,0 +1,8 @@
+---
+source: tests/ninja_snapshot_tests.rs
+expression: ninja_content
+---
+rule ca3067639652d0018b982cd2fc8262e3a02f4404f60148b8493de0f656d9b1a2
+  command = python3 -c 'import os,sys; open(sys.argv[1],"a").close()' $out
+
+build out/a: ca3067639652d0018b982cd2fc8262e3a02f4404f60148b8493de0f656d9b1a2 in/a

From 77ca463be41a9b323c278dde835b6bb06714f210 Mon Sep 17 00:00:00 2001
From: Leynos <leynos@troubledskies.net>
Date: Thu, 31 Jul 2025 13:04:14 +0100
Subject: [PATCH 3/5] Show Ninja version in CI

---
 .github/workflows/ci.yml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 77966b3d..dbe43610 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -18,6 +18,8 @@ jobs:
       - uses: actions/checkout@v4
       - name: Setup Rust
         uses: leynos/shared-actions/.github/actions/setup-rust@v1.1.0
+      - name: Show Ninja version
+        run: ninja --version
       - name: Format
         run: make check-fmt
       - name: Lint

From f1f26e4a9d8955863cba81a046fabb027b138a97 Mon Sep 17 00:00:00 2001
From: Leynos <leynos@troubledskies.net>
Date: Thu, 31 Jul 2025 13:15:36 +0100
Subject: [PATCH 4/5] Refine Ninja generator stability

---
 docs/netsuke-design.md |   5 +-
 src/ninja_gen.rs       | 205 +++++++++++++++++++++++------------------
 2 files changed, 120 insertions(+), 90 deletions(-)

diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md
index 6b405287..267becef 100644
--- a/docs/netsuke-design.md
+++ b/docs/netsuke-design.md
@@ -1072,8 +1072,9 @@ representation portable.
 - Duplicate output files are rejected. Attempting to define the same output
   path twice results in `IrGenError::DuplicateOutput`.
 - The Ninja generator sorts actions and edges before output and
-  deduplicates edges by their first explicit output to ensure deterministic
-  `build.ninja` files.
+  deduplicates edges based on their full set of explicit outputs. Sorting uses
+  the joined path strings to keep ordering stable across platforms, ensuring
+  deterministic `build.ninja` files.
 - Integration tests snapshot the generated Ninja file with `insta` and
   execute the Ninja binary to validate structure and no-op behaviour.
 
diff --git a/src/ninja_gen.rs b/src/ninja_gen.rs
index 7c4403a0..32817bc7 100644
--- a/src/ninja_gen.rs
+++ b/src/ninja_gen.rs
@@ -1,123 +1,152 @@
 //! Ninja file generator.
 //!
 //! This module converts a [`crate::ir::BuildGraph`] into the textual
-//! representation expected by the Ninja build system.
-//! The output is deterministic: actions and edges are sorted to ensure
-//! stable snapshots for testing.
+//! representation expected by the Ninja build system. The generator sorts
+//! actions and edges to ensure deterministic output for snapshot tests.
 
 use crate::ast::Recipe;
 use crate::ir::{BuildEdge, BuildGraph};
-use std::collections::{HashMap, HashSet};
-use std::fmt::Write;
+use std::collections::HashSet;
+use std::fmt::{self, Display, Formatter, Write};
 use std::path::PathBuf;
 
 /// Generate a Ninja build file as a string.
+///
+/// # Panics
+///
+/// Panics if a build edge references an unknown action.
 #[must_use]
 pub fn generate(graph: &BuildGraph) -> String {
     let mut out = String::new();
-    write_rules(&mut out, &graph.actions);
-    write_edges(&mut out, &graph.targets);
-    write_defaults(&mut out, &graph.default_targets);
+
+    let mut actions: Vec<_> = graph.actions.iter().collect();
+    actions.sort_by_key(|(id, _)| *id);
+    for (id, action) in actions {
+        write!(out, "{}", NamedAction { id, action }).expect("write Ninja rule");
+    }
+
+    let mut edges: Vec<_> = graph.targets.values().collect();
+    edges.sort_by(|a, b| path_key(&a.explicit_outputs).cmp(&path_key(&b.explicit_outputs)));
+    let mut seen = HashSet::new();
+    for edge in edges {
+        let key = path_key(&edge.explicit_outputs);
+        if !seen.insert(key.clone()) {
+            continue;
+        }
+        let action = graph.actions.get(&edge.action_id).expect("action");
+        write!(
+            out,
+            "{}",
+            DisplayEdge {
+                edge,
+                action_restat: action.restat,
+            }
+        )
+        .expect("write Ninja edge");
+    }
+
+    if !graph.default_targets.is_empty() {
+        let mut defs: Vec<_> = graph.default_targets.iter().collect();
+        defs.sort();
+        writeln!(
+            out,
+            "default {}",
+            defs.iter()
+                .map(|p| p.display().to_string())
+                .collect::<Vec<_>>()
+                .join(" ")
+        )
+        .expect("write defaults");
+    }
+
     out
 }
 
-fn write_rules(out: &mut String, actions: &HashMap<String, crate::ir::Action>) {
-    let mut ids: Vec<_> = actions.keys().collect();
-    ids.sort();
-    for id in ids {
-        let action = &actions[id];
-        let _ = writeln!(out, "rule {id}");
-        match &action.recipe {
-            Recipe::Command { command } => {
-                let _ = writeln!(out, "  command = {command}");
-            }
+/// Convert a slice of paths into a space-separated string.
+fn join(paths: &[PathBuf]) -> String {
+    paths
+        .iter()
+        .map(|p| p.display().to_string())
+        .collect::<Vec<_>>()
+        .join(" ")
+}
+
+/// Generate a stable key for a list of paths.
+fn path_key(paths: &[PathBuf]) -> String {
+    let mut parts: Vec<_> = paths.iter().map(|p| p.display().to_string()).collect();
+    parts.sort();
+    parts.join("\u{0}")
+}
+
+/// Wrapper struct to display a rule with its identifier.
+struct NamedAction<'a> {
+    id: &'a str,
+    action: &'a crate::ir::Action,
+}
+
+impl Display for NamedAction<'_> {
+    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
+        writeln!(f, "rule {}", self.id)?;
+        match &self.action.recipe {
+            Recipe::Command { command } => writeln!(f, "  command = {command}")?,
             Recipe::Script { script } => {
-                let _ = writeln!(out, "  command = /bin/sh -e -c \"");
+                writeln!(f, "  command = /bin/sh -e -c \"")?;
                 for line in script.lines() {
-                    let _ = writeln!(out, "    {line}");
+                    writeln!(f, "    {line}")?;
                 }
-                let _ = writeln!(out, "  \"");
+                writeln!(f, "  \"")?;
             }
             Recipe::Rule { .. } => unreachable!("rules do not reference other rules"),
         }
-        if let Some(desc) = &action.description {
-            let _ = writeln!(out, "  description = {desc}");
+        if let Some(desc) = &self.action.description {
+            writeln!(f, "  description = {desc}")?;
         }
-        if let Some(depfile) = &action.depfile {
-            let _ = writeln!(out, "  depfile = {depfile}");
+        if let Some(depfile) = &self.action.depfile {
+            writeln!(f, "  depfile = {depfile}")?;
         }
-        if let Some(deps_format) = &action.deps_format {
-            let _ = writeln!(out, "  deps = {deps_format}");
+        if let Some(deps_format) = &self.action.deps_format {
+            writeln!(f, "  deps = {deps_format}")?;
         }
-        if let Some(pool) = &action.pool {
-            let _ = writeln!(out, "  pool = {pool}");
+        if let Some(pool) = &self.action.pool {
+            writeln!(f, "  pool = {pool}")?;
         }
-        if action.restat {
-            let _ = writeln!(out, "  restat = 1");
+        if self.action.restat {
+            writeln!(f, "  restat = 1")?;
         }
-        let _ = writeln!(out);
+        writeln!(f)
     }
 }
 
-fn write_edges(out: &mut String, targets: &HashMap<PathBuf, BuildEdge>) {
-    let mut edges: Vec<&BuildEdge> = targets.values().collect();
-    edges.sort_by(|a, b| a.explicit_outputs.cmp(&b.explicit_outputs));
-    let mut seen = HashSet::new();
-    for edge in edges {
-        if edge
-            .explicit_outputs
-            .first()
-            .is_some_and(|f| !seen.insert(f))
-        {
-            continue;
-        }
-        write_edge(out, edge);
-    }
-}
-
-fn write_edge(out: &mut String, edge: &BuildEdge) {
-    let outputs = join(&edge.explicit_outputs);
-    let _ = write!(out, "build {outputs}");
-    if !edge.implicit_outputs.is_empty() {
-        let _ = write!(out, " | {}", join(&edge.implicit_outputs));
-    }
-    let rule = if edge.phony { "phony" } else { &edge.action_id };
-    let _ = write!(out, ": {rule}");
-    if !edge.inputs.is_empty() {
-        let _ = write!(out, " {}", join(&edge.inputs));
-    }
-    if !edge.order_only_deps.is_empty() {
-        let _ = write!(out, " || {}", join(&edge.order_only_deps));
-    }
-    let _ = writeln!(out);
-    if edge.always {
-        let _ = writeln!(out, "  restat = 1");
-    }
-    let _ = writeln!(out);
+/// Wrapper struct to display a build edge.
+struct DisplayEdge<'a> {
+    edge: &'a BuildEdge,
+    action_restat: bool,
 }
 
-fn write_defaults(out: &mut String, defaults: &[PathBuf]) {
-    if defaults.is_empty() {
-        return;
+impl Display for DisplayEdge<'_> {
+    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
+        write!(f, "build {}", join(&self.edge.explicit_outputs))?;
+        if !self.edge.implicit_outputs.is_empty() {
+            write!(f, " | {}", join(&self.edge.implicit_outputs))?;
+        }
+        let rule = if self.edge.phony {
+            "phony"
+        } else {
+            &self.edge.action_id
+        };
+        write!(f, ": {rule}")?;
+        if !self.edge.inputs.is_empty() {
+            write!(f, " {}", join(&self.edge.inputs))?;
+        }
+        if !self.edge.order_only_deps.is_empty() {
+            write!(f, " || {}", join(&self.edge.order_only_deps))?;
+        }
+        writeln!(f)?;
+        if self.edge.always && !self.action_restat {
+            writeln!(f, "  restat = 1")?;
+        }
+        writeln!(f)
     }
-    let mut defs: Vec<_> = defaults.iter().collect();
-    defs.sort();
-    let _ = writeln!(
-        out,
-        "default {}",
-        defs.iter()
-            .map(|p| p.display().to_string())
-            .collect::<Vec<_>>()
-            .join(" ")
-    );
-}
-
-fn join(paths: &[PathBuf]) -> String {
-    paths
-        .iter()
-        .map(|p| p.display().to_string())
-        .collect::<Vec<_>>()
-        .join(" ")
 }
 
 #[cfg(test)]

From 665013959bf6eb27565a0b9423e1c0569ccd42f9 Mon Sep 17 00:00:00 2001
From: Leynos <leynos@troubledskies.net>
Date: Thu, 31 Jul 2025 15:20:45 +0100
Subject: [PATCH 5/5] Extend ninja generator tests

---
 docs/netsuke-design.md                        |  4 +
 tests/ninja_gen_tests.rs                      | 81 +++++++++++++++++++
 tests/ninja_snapshot_tests.rs                 | 71 +++++++++++++++-
 ..._snapshot_tests__touch_manifest_ninja.snap |  7 +-
 4 files changed, 159 insertions(+), 4 deletions(-)

diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md
index 267becef..b1796235 100644
--- a/docs/netsuke-design.md
+++ b/docs/netsuke-design.md
@@ -1075,6 +1075,10 @@ representation portable.
   deduplicates edges based on their full set of explicit outputs. Sorting uses
   the joined path strings to keep ordering stable across platforms, ensuring
   deterministic `build.ninja` files.
+- Script actions are emitted under `/bin/sh -e -c` with each line indented so
+  multi-line recipes execute portably.
+- Optional fields such as `description`, `depfile`, `deps`, `pool`, and
+  `restat` are written only when present, matching Ninja's syntax exactly.
 - Integration tests snapshot the generated Ninja file with `insta` and
   execute the Ninja binary to validate structure and no-op behaviour.
 
diff --git a/tests/ninja_gen_tests.rs b/tests/ninja_gen_tests.rs
index fb896975..f02af7fd 100644
--- a/tests/ninja_gen_tests.rs
+++ b/tests/ninja_gen_tests.rs
@@ -39,3 +39,84 @@ fn generate_phony() {
     );
     assert_eq!(ninja, expected);
 }
+
+#[rstest]
+fn generate_script_rule_with_fields() {
+    let action = Action {
+        recipe: Recipe::Script {
+            script: "echo hi\necho there".into(),
+        },
+        description: Some("desc".into()),
+        depfile: Some("file.d".into()),
+        deps_format: Some("gcc".into()),
+        pool: Some("pool".into()),
+        restat: true,
+    };
+    let edge = BuildEdge {
+        action_id: "a".into(),
+        inputs: vec![PathBuf::from("in")],
+        explicit_outputs: vec![PathBuf::from("out")],
+        implicit_outputs: vec![PathBuf::from("imp")],
+        order_only_deps: vec![PathBuf::from("oo")],
+        phony: false,
+        always: false,
+    };
+    let mut graph = BuildGraph::default();
+    graph.actions.insert("a".into(), action);
+    graph.targets.insert(PathBuf::from("out"), edge);
+    graph.default_targets.push(PathBuf::from("out"));
+
+    let ninja = generate(&graph);
+    let expected = concat!(
+        "rule a\n",
+        "  command = /bin/sh -e -c \"\n",
+        "    echo hi\n",
+        "    echo there\n",
+        "  \"\n",
+        "  description = desc\n",
+        "  depfile = file.d\n",
+        "  deps = gcc\n",
+        "  pool = pool\n",
+        "  restat = 1\n\n",
+        "build out | imp: a in || oo\n\n",
+        "default out\n",
+    );
+    assert_eq!(ninja, expected);
+}
+
+#[rstest(
+    action_restat,
+    always,
+    case(false, true),
+    case(true, true),
+    case(false, false)
+)]
+fn restat_for_always_edges(action_restat: bool, always: bool) {
+    let action = Action {
+        recipe: Recipe::Command {
+            command: "true".into(),
+        },
+        description: None,
+        depfile: None,
+        deps_format: None,
+        pool: None,
+        restat: action_restat,
+    };
+    let edge = BuildEdge {
+        action_id: "a".into(),
+        inputs: vec![PathBuf::from("in")],
+        explicit_outputs: vec![PathBuf::from("out")],
+        implicit_outputs: Vec::new(),
+        order_only_deps: Vec::new(),
+        phony: false,
+        always,
+    };
+    let mut graph = BuildGraph::default();
+    graph.actions.insert("a".into(), action);
+    graph.targets.insert(PathBuf::from("out"), edge);
+
+    let ninja = generate(&graph);
+    let build_restat = concat!("build out: a in\n", "  restat = 1\n");
+    let has_build_restat = ninja.contains(build_restat);
+    assert_eq!(has_build_restat, !action_restat && always);
+}
diff --git a/tests/ninja_snapshot_tests.rs b/tests/ninja_snapshot_tests.rs
index 501d5a95..dbcc2460 100644
--- a/tests/ninja_snapshot_tests.rs
+++ b/tests/ninja_snapshot_tests.rs
@@ -28,7 +28,7 @@ fn touch_manifest_ninja_validation() {
           - name: touch
             recipe:
               kind: command
-              command: "python3 -c 'import os,sys; open(sys.argv[1],\"a\").close()' $out"
+              command: "python3 -c 'import os,sys; [open(a,\"a\").close() for a in sys.argv[1:]]' $out"
         targets:
           - name: out/a
             sources: in/a
@@ -74,3 +74,72 @@ fn touch_manifest_ninja_validation() {
         "expected no-op second pass, got:\n{second}"
     );
 }
+
+#[test]
+fn ordering_and_deduplication() {
+    let manifest_yaml = r#"
+        netsuke_version: "1.0.0"
+        rules:
+          - name: z
+            recipe:
+              kind: command
+              command: "python3 -c 'import os,sys; [open(a,\"a\").close() for a in sys.argv[1:]]' $out"
+          - name: a
+            recipe:
+              kind: command
+              command: "python3 -c 'import os,sys; [open(a,\"a\").close() for a in sys.argv[1:]]' $out"
+        targets:
+          - name: out/b
+            sources: in/b
+            recipe:
+              kind: rule
+              rule: z
+          - name: out/a
+            sources: in/a
+            recipe:
+              kind: rule
+              rule: a
+          - name: [out/c1, out/c2]
+            sources: in/c
+            recipe:
+              kind: rule
+              rule: a
+        defaults: [out/c1, out/a]
+    "#;
+
+    let manifest = manifest::from_str(manifest_yaml).expect("parse manifest");
+    let ir = BuildGraph::from_manifest(&manifest).expect("ir generation");
+    let ninja_first = ninja_gen::generate(&ir);
+    let ninja_second = ninja_gen::generate(&ir);
+    assert_eq!(ninja_first, ninja_second);
+
+    let build_lines: Vec<_> = ninja_first
+        .lines()
+        .filter(|l| l.starts_with("build "))
+        .collect();
+    assert_eq!(build_lines.len(), 3);
+
+    assert!(ninja_first.contains("default out/a out/c1"));
+
+    let dir = tempdir().expect("tempdir");
+    let build_file = dir.path().join("build.ninja");
+    fs::write(&build_file, &ninja_first).expect("write ninja");
+    fs::create_dir_all(dir.path().join("in")).expect("dir");
+    fs::create_dir_all(dir.path().join("out")).expect("dir");
+    fs::write(dir.path().join("in/a"), "").expect("in");
+    fs::write(dir.path().join("in/b"), "").expect("in");
+    fs::write(dir.path().join("in/c"), "").expect("in");
+
+    let ninja_cmd = |args: &[&str]| {
+        let mut cmd = Command::new("ninja");
+        cmd.arg("-f").arg(&build_file).args(args);
+        cmd.current_dir(&dir);
+        run_ok(&mut cmd)
+    };
+
+    let _ = ninja_cmd(&["-t", "targets", "all"]);
+    let _ = ninja_cmd(&["-w", "dupbuild=err", "-d", "stats"]);
+    let second = ninja_cmd(&["-n", "-d", "explain", "-v"]);
+    println!("second output: {second}");
+    assert!(second.contains("no work to do"));
+}
diff --git a/tests/snapshots/ninja/ninja_snapshot_tests__touch_manifest_ninja.snap b/tests/snapshots/ninja/ninja_snapshot_tests__touch_manifest_ninja.snap
index ca715357..dab024ce 100644
--- a/tests/snapshots/ninja/ninja_snapshot_tests__touch_manifest_ninja.snap
+++ b/tests/snapshots/ninja/ninja_snapshot_tests__touch_manifest_ninja.snap
@@ -1,8 +1,9 @@
 ---
 source: tests/ninja_snapshot_tests.rs
+assertion_line: 50
 expression: ninja_content
 ---
-rule ca3067639652d0018b982cd2fc8262e3a02f4404f60148b8493de0f656d9b1a2
-  command = python3 -c 'import os,sys; open(sys.argv[1],"a").close()' $out
+rule edca4a86968d45d5dd7b150cb9f29adaec9345fe416a5cdbecb08dd15259e605
+  command = python3 -c 'import os,sys; [open(a,"a").close() for a in sys.argv[1:]]' $out
 
-build out/a: ca3067639652d0018b982cd2fc8262e3a02f4404f60148b8493de0f656d9b1a2 in/a
+build out/a: edca4a86968d45d5dd7b150cb9f29adaec9345fe416a5cdbecb08dd15259e605 in/a