From 01d145509e38dbe45509a09fef6a3688de096053 Mon Sep 17 00:00:00 2001 From: Leynos Date: Tue, 5 Aug 2025 08:03:28 +0100 Subject: [PATCH 1/4] Generate Ninja build file before invoking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Parse Netsukefile and convert to IR - Write Ninja script to a temp file and pass via -f - Cover manifest→IR→Ninja pipeline with CLI tests --- Cargo.toml | 2 +- src/runner.rs | 23 ++++++++++++--- tests/runner_tests.rs | 67 ++++++++++++++++++++++++++++++++++++++++--- tests/support/mod.rs | 34 ++++++++++++++++++---- 4 files changed, 111 insertions(+), 15 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index b9c2d888..36407b5d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,6 +13,7 @@ thiserror = "1" sha2 = "0.10" itoa = "1" itertools = "0.12" +tempfile = "3" [lints.clippy] pedantic = { level = "warn", priority = -1 } @@ -59,7 +60,6 @@ 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/src/runner.rs b/src/runner.rs index a90b92d7..e308670e 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -4,11 +4,15 @@ //! handles command execution. It now delegates build requests to the Ninja //! subprocess, streaming its output back to the user. -use crate::cli::{Cli, Commands}; +use crate::{ + cli::{Cli, Commands}, + ir, manifest, ninja_gen, +}; use std::io::{self, BufRead, BufReader, Write}; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; use std::thread; +use tempfile::NamedTempFile; /// Execute the parsed [`Cli`] commands. /// @@ -35,8 +39,9 @@ pub fn run(cli: &Cli) -> io::Result<()> { /// Invoke the Ninja executable with the provided CLI settings. /// -/// The function forwards the job count and working directory to Ninja and -/// streams its standard output and error back to the user. +/// This loads the `Netsukefile`, converts it into a Ninja build file, and then +/// forwards the job count and working directory to Ninja. The child's standard +/// output and error are streamed back to the user. /// /// # Errors /// @@ -47,6 +52,15 @@ pub fn run(cli: &Cli) -> io::Result<()> { /// /// Panics if the child's output streams cannot be captured. pub fn run_ninja(program: &Path, cli: &Cli, targets: &[String]) -> io::Result<()> { + let manifest = manifest::from_path(&cli.file).map_err(io::Error::other)?; + let graph = ir::BuildGraph::from_manifest(&manifest).map_err(io::Error::other)?; + let ninja_script = ninja_gen::generate(&graph); + + let mut build_file = NamedTempFile::new().map_err(io::Error::other)?; + build_file.write_all(ninja_script.as_bytes())?; + build_file.flush()?; + let build_path: PathBuf = build_file.path().to_path_buf(); + let mut cmd = Command::new(program); if let Some(dir) = &cli.directory { cmd.current_dir(dir).arg("-C").arg(dir); @@ -54,6 +68,7 @@ pub fn run_ninja(program: &Path, cli: &Cli, targets: &[String]) -> io::Result<() if let Some(jobs) = cli.jobs { cmd.arg("-j").arg(jobs.to_string()); } + cmd.arg("-f").arg(&build_path); cmd.args(targets); cmd.stdout(Stdio::piped()); cmd.stderr(Stdio::piped()); diff --git a/tests/runner_tests.rs b/tests/runner_tests.rs index 79cb9a82..854516a5 100644 --- a/tests/runner_tests.rs +++ b/tests/runner_tests.rs @@ -3,12 +3,15 @@ use netsuke::cli::{Cli, Commands}; use netsuke::runner; use rstest::rstest; +use std::fs; +use std::io::Write; use std::path::{Path, PathBuf}; +use tempfile::NamedTempFile; /// Creates a default CLI configuration for testing Ninja invocation. -fn test_cli() -> Cli { +fn test_cli(file: PathBuf) -> Cli { Cli { - file: PathBuf::from("Netsukefile"), + file, directory: None, jobs: None, command: Some(Commands::Build { @@ -24,15 +27,71 @@ mod support; #[case(1, false)] fn run_ninja_status(#[case] code: i32, #[case] succeeds: bool) { let (_dir, path) = support::fake_ninja(code); - let cli = test_cli(); + let mut manifest = NamedTempFile::new().expect("manifest"); + write_manifest(&mut manifest); + let cli = test_cli(manifest.path().to_path_buf()); let result = runner::run_ninja(&path, &cli, &[]); assert_eq!(result.is_ok(), succeeds); } #[rstest] fn run_ninja_not_found() { - let cli = test_cli(); + let mut manifest = NamedTempFile::new().expect("manifest"); + write_manifest(&mut manifest); + let cli = test_cli(manifest.path().to_path_buf()); let err = runner::run_ninja(Path::new("does-not-exist"), &cli, &[]).expect_err("process should fail"); assert_eq!(err.kind(), std::io::ErrorKind::NotFound); } + +#[rstest] +fn run_pipeline_generates_ninja() { + use netsuke::ast::Recipe; + use netsuke::hasher::ActionHasher; + use netsuke::ir::Action; + + let (_dir, path, capture) = support::fake_ninja_capture(); + let mut manifest = NamedTempFile::new().expect("manifest"); + write_manifest(&mut manifest); + let cli = Cli { + file: manifest.path().to_path_buf(), + directory: None, + jobs: None, + command: Some(Commands::Build { + targets: Vec::new(), + }), + }; + + runner::run_ninja(&path, &cli, &[]).expect("run ninja"); + + let generated = fs::read_to_string(&capture).expect("captured build"); + + let action = Action { + recipe: Recipe::Command { + command: "echo hi".into(), + }, + description: None, + depfile: None, + deps_format: None, + pool: None, + restat: false, + }; + let hash = ActionHasher::hash(&action); + let expected = + format!("rule {hash}\n command = echo hi\n\nbuild out: {hash}\n\ndefault out\n"); + assert_eq!(generated, expected); +} + +fn write_manifest(file: &mut NamedTempFile) { + let manifest = concat!( + "netsuke_version: \"1.0.0\"\n", + "targets:\n", + " - name: out\n", + " recipe:\n", + " kind: command\n", + " command: echo hi\n", + "defaults:\n", + " - out\n", + ); + file.write_all(manifest.as_bytes()).expect("write manifest"); +} diff --git a/tests/support/mod.rs b/tests/support/mod.rs index 722f3cae..28147cb5 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -5,14 +5,10 @@ use std::io::Write; use std::path::PathBuf; use tempfile::TempDir; -/// Create a fake Ninja executable that exits with `exit_code`. -/// -/// Returns the temporary directory and the path to the executable. -pub fn fake_ninja(exit_code: i32) -> (TempDir, PathBuf) { - let dir = TempDir::new().expect("temp dir"); +fn write_script(dir: &TempDir, body: &str) -> PathBuf { let path = dir.path().join("ninja"); let mut file = File::create(&path).expect("script"); - writeln!(file, "#!/bin/sh\nexit {exit_code}").expect("write script"); + file.write_all(body.as_bytes()).expect("write script"); #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; @@ -20,5 +16,31 @@ pub fn fake_ninja(exit_code: i32) -> (TempDir, PathBuf) { perms.set_mode(0o755); fs::set_permissions(&path, perms).expect("perms"); } + path +} + +/// Create a fake Ninja executable that exits with `exit_code`. +/// +/// Returns the temporary directory and the path to the executable. +pub fn fake_ninja(exit_code: i32) -> (TempDir, PathBuf) { + let dir = TempDir::new().expect("temp dir"); + let body = format!("#!/bin/sh\nexit {exit_code}\n"); + let path = write_script(&dir, &body); (dir, path) } + +/// Create a fake Ninja that copies the provided build file to a capture path. +/// +/// Returns the temporary directory, path to the executable and the capture file +/// location. +#[allow(dead_code, reason = "not every test exercises the capture variant")] +pub fn fake_ninja_capture() -> (TempDir, PathBuf, PathBuf) { + let dir = TempDir::new().expect("temp dir"); + let capture = dir.path().join("captured.ninja"); + let body = format!( + "#!/bin/sh\nwhile [ \"$1\" != \"\" ]; do\n if [ \"$1\" = \"-f\" ]; then\n shift\n cat \"$1\" > \"{}\"\n fi\n shift\ndone\n", + capture.display() + ); + let path = write_script(&dir, &body); + (dir, path, capture) +} From fb5fcf5c8b7d0ec0238296ff22dccb7d21a45c43 Mon Sep 17 00:00:00 2001 From: Leynos Date: Tue, 5 Aug 2025 08:57:05 +0100 Subject: [PATCH 2/4] Refactor runner and test helpers --- src/runner.rs | 34 +++++++++++++++++++++------------- tests/runner_tests.rs | 36 +++++++----------------------------- tests/support/mod.rs | 21 ++++++++++++++++++++- 3 files changed, 48 insertions(+), 43 deletions(-) diff --git a/src/runner.rs b/src/runner.rs index e308670e..5484cfc0 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -9,7 +9,7 @@ use crate::{ ir, manifest, ninja_gen, }; use std::io::{self, BufRead, BufReader, Write}; -use std::path::{Path, PathBuf}; +use std::path::Path; use std::process::{Command, Stdio}; use std::thread; use tempfile::NamedTempFile; @@ -39,9 +39,10 @@ pub fn run(cli: &Cli) -> io::Result<()> { /// Invoke the Ninja executable with the provided CLI settings. /// -/// This loads the `Netsukefile`, converts it into a Ninja build file, and then -/// forwards the job count and working directory to Ninja. The child's standard -/// output and error are streamed back to the user. +/// This loads the `Netsukefile`, converts it to an intermediate representation, +/// generates a temporary Ninja build script, and executes Ninja with this +/// script. Job count and working directory options are forwarded to Ninja, and +/// the child's standard output and error are streamed back to the user. /// /// # Errors /// @@ -52,14 +53,9 @@ pub fn run(cli: &Cli) -> io::Result<()> { /// /// Panics if the child's output streams cannot be captured. pub fn run_ninja(program: &Path, cli: &Cli, targets: &[String]) -> io::Result<()> { - let manifest = manifest::from_path(&cli.file).map_err(io::Error::other)?; - let graph = ir::BuildGraph::from_manifest(&manifest).map_err(io::Error::other)?; - let ninja_script = ninja_gen::generate(&graph); - - let mut build_file = NamedTempFile::new().map_err(io::Error::other)?; - build_file.write_all(ninja_script.as_bytes())?; - build_file.flush()?; - let build_path: PathBuf = build_file.path().to_path_buf(); + // Keep the file handle alive so the temporary script outlives the child + // process. + let build_file = manifest_to_build_file(&cli.file)?; let mut cmd = Command::new(program); if let Some(dir) = &cli.directory { @@ -68,7 +64,7 @@ pub fn run_ninja(program: &Path, cli: &Cli, targets: &[String]) -> io::Result<() if let Some(jobs) = cli.jobs { cmd.arg("-j").arg(jobs.to_string()); } - cmd.arg("-f").arg(&build_path); + cmd.arg("-f").arg(build_file.path()); cmd.args(targets); cmd.stdout(Stdio::piped()); cmd.stderr(Stdio::piped()); @@ -109,3 +105,15 @@ pub fn run_ninja(program: &Path, cli: &Cli, targets: &[String]) -> io::Result<() )) } } + +/// Generate a temporary Ninja build file from a manifest. +fn manifest_to_build_file(path: &Path) -> io::Result { + let manifest = manifest::from_path(path).map_err(io::Error::other)?; + let graph = ir::BuildGraph::from_manifest(&manifest).map_err(io::Error::other)?; + let ninja_script = ninja_gen::generate(&graph); + + let mut build_file = NamedTempFile::new().map_err(io::Error::other)?; + build_file.write_all(ninja_script.as_bytes())?; + build_file.flush()?; + Ok(build_file) +} diff --git a/tests/runner_tests.rs b/tests/runner_tests.rs index 854516a5..cc79efd3 100644 --- a/tests/runner_tests.rs +++ b/tests/runner_tests.rs @@ -4,12 +4,11 @@ use netsuke::cli::{Cli, Commands}; use netsuke::runner; use rstest::rstest; use std::fs; -use std::io::Write; use std::path::{Path, PathBuf}; use tempfile::NamedTempFile; /// Creates a default CLI configuration for testing Ninja invocation. -fn test_cli(file: PathBuf) -> Cli { +fn cli_with_manifest(file: PathBuf) -> Cli { Cli { file, directory: None, @@ -28,8 +27,8 @@ mod support; fn run_ninja_status(#[case] code: i32, #[case] succeeds: bool) { let (_dir, path) = support::fake_ninja(code); let mut manifest = NamedTempFile::new().expect("manifest"); - write_manifest(&mut manifest); - let cli = test_cli(manifest.path().to_path_buf()); + support::write_manifest(&mut manifest); + let cli = cli_with_manifest(manifest.path().to_path_buf()); let result = runner::run_ninja(&path, &cli, &[]); assert_eq!(result.is_ok(), succeeds); } @@ -37,8 +36,8 @@ fn run_ninja_status(#[case] code: i32, #[case] succeeds: bool) { #[rstest] fn run_ninja_not_found() { let mut manifest = NamedTempFile::new().expect("manifest"); - write_manifest(&mut manifest); - let cli = test_cli(manifest.path().to_path_buf()); + support::write_manifest(&mut manifest); + let cli = cli_with_manifest(manifest.path().to_path_buf()); let err = runner::run_ninja(Path::new("does-not-exist"), &cli, &[]).expect_err("process should fail"); assert_eq!(err.kind(), std::io::ErrorKind::NotFound); @@ -52,15 +51,8 @@ fn run_pipeline_generates_ninja() { let (_dir, path, capture) = support::fake_ninja_capture(); let mut manifest = NamedTempFile::new().expect("manifest"); - write_manifest(&mut manifest); - let cli = Cli { - file: manifest.path().to_path_buf(), - directory: None, - jobs: None, - command: Some(Commands::Build { - targets: Vec::new(), - }), - }; + support::write_manifest(&mut manifest); + let cli = cli_with_manifest(manifest.path().to_path_buf()); runner::run_ninja(&path, &cli, &[]).expect("run ninja"); @@ -81,17 +73,3 @@ fn run_pipeline_generates_ninja() { format!("rule {hash}\n command = echo hi\n\nbuild out: {hash}\n\ndefault out\n"); assert_eq!(generated, expected); } - -fn write_manifest(file: &mut NamedTempFile) { - let manifest = concat!( - "netsuke_version: \"1.0.0\"\n", - "targets:\n", - " - name: out\n", - " recipe:\n", - " kind: command\n", - " command: echo hi\n", - "defaults:\n", - " - out\n", - ); - file.write_all(manifest.as_bytes()).expect("write manifest"); -} diff --git a/tests/support/mod.rs b/tests/support/mod.rs index 28147cb5..90606d82 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -3,7 +3,7 @@ use std::fs::{self, File}; use std::io::Write; use std::path::PathBuf; -use tempfile::TempDir; +use tempfile::{NamedTempFile, TempDir}; fn write_script(dir: &TempDir, body: &str) -> PathBuf { let path = dir.path().join("ninja"); @@ -44,3 +44,22 @@ pub fn fake_ninja_capture() -> (TempDir, PathBuf, PathBuf) { let path = write_script(&dir, &body); (dir, path, capture) } + +/// Write a minimal Netsukefile to the provided temporary file. +#[allow( + dead_code, + reason = "helper is unused when the support crate builds independently" +)] +pub fn write_manifest(file: &mut NamedTempFile) { + let manifest = concat!( + "netsuke_version: \"1.0.0\"\n", + "targets:\n", + " - name: out\n", + " recipe:\n", + " kind: command\n", + " command: echo hi\n", + "defaults:\n", + " - out\n", + ); + file.write_all(manifest.as_bytes()).expect("write manifest"); +} From 0b0385985c58f79d1513640d4716d5b2a4454c47 Mon Sep 17 00:00:00 2001 From: Leynos Date: Tue, 5 Aug 2025 12:15:59 +0100 Subject: [PATCH 3/4] Ensure tests create manifest for Ninja run --- tests/steps/process_steps.rs | 38 ++++++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/tests/steps/process_steps.rs b/tests/steps/process_steps.rs index 6a34153a..29d32c9e 100644 --- a/tests/steps/process_steps.rs +++ b/tests/steps/process_steps.rs @@ -3,8 +3,9 @@ use crate::{CliWorld, support}; use cucumber::{given, then, when}; use netsuke::runner; -use std::path::PathBuf; -use tempfile::TempDir; +use std::fs; +use std::path::{Path, PathBuf}; +use tempfile::{NamedTempFile, TempDir}; /// Installs a test-specific ninja binary and updates the `PATH`. #[expect( @@ -50,18 +51,30 @@ fn no_ninja(world: &mut CliWorld) { /// This step runs the `ninja` executable using the CLI configuration stored in /// the world, then updates the world's `run_status` and `run_error` fields based /// on the execution outcome. -#[expect( - clippy::option_if_let_else, - reason = "explicit conditional is clearer than map_or_else" -)] #[when("the ninja process is run")] fn run(world: &mut CliWorld) { - let cli = world.cli.as_ref().expect("cli"); - let program = if let Some(ninja) = &world.ninja { - std::path::Path::new(ninja) + let cli = world.cli.as_mut().expect("cli"); + + // Ensure a manifest exists at the path expected by the CLI. + let dir = world.temp.as_ref().expect("temp dir"); + let manifest_path = if cli.file.is_absolute() { + cli.file.clone() } else { - std::path::Path::new("ninja") + dir.path().join(&cli.file) }; + if !manifest_path.exists() { + let mut file = NamedTempFile::new_in(dir.path()).expect("manifest"); + support::write_manifest(&mut file); + // Persist the temporary file to the desired manifest path. + file.persist(&manifest_path).expect("persist manifest"); + } + cli.file.clone_from(&manifest_path); + + let program = world + .ninja + .as_ref() + .map_or_else(|| Path::new("ninja"), Path::new); + match runner::run_ninja(program, cli, &[]) { Ok(()) => { world.run_status = Some(true); @@ -72,6 +85,11 @@ fn run(world: &mut CliWorld) { world.run_error = Some(e.to_string()); } } + + // Clean up any manifest left outside the temporary directory. + if !manifest_path.starts_with(dir.path()) { + let _ = fs::remove_file(manifest_path); + } } /// Asserts that the command succeeds. From 97bd6d8c78880b159345380fb613adfe91ed17c5 Mon Sep 17 00:00:00 2001 From: Leynos Date: Tue, 5 Aug 2025 12:48:48 +0100 Subject: [PATCH 4/4] Document runner tests --- src/runner.rs | 36 +++++++++++++++++++++++++++++++----- tests/runner_tests.rs | 3 +++ tests/steps/process_steps.rs | 2 ++ tests/support/mod.rs | 20 +++++++++++++------- 4 files changed, 49 insertions(+), 12 deletions(-) diff --git a/src/runner.rs b/src/runner.rs index 5484cfc0..04b6af1f 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -107,13 +107,39 @@ pub fn run_ninja(program: &Path, cli: &Cli, targets: &[String]) -> io::Result<() } /// Generate a temporary Ninja build file from a manifest. +/// +/// # Examples +/// +/// ```ignore +/// # use std::path::Path; +/// # fn demo() -> std::io::Result<()> { +/// let file = netsuke::runner::manifest_to_build_file(Path::new("tests/data/rules.yml"))?; +/// assert!(file.path().exists()); +/// # Ok(()) +/// # } +/// ``` fn manifest_to_build_file(path: &Path) -> io::Result { - let manifest = manifest::from_path(path).map_err(io::Error::other)?; - let graph = ir::BuildGraph::from_manifest(&manifest).map_err(io::Error::other)?; + let manifest = manifest::from_path(path).map_err(|e| { + io::Error::new( + io::ErrorKind::InvalidData, + format!("Failed to load manifest from {}: {e}", path.display()), + ) + })?; + let graph = ir::BuildGraph::from_manifest(&manifest).map_err(|e| { + io::Error::new( + io::ErrorKind::InvalidData, + format!("Failed to convert manifest to build graph: {e}"), + ) + })?; let ninja_script = ninja_gen::generate(&graph); - let mut build_file = NamedTempFile::new().map_err(io::Error::other)?; - build_file.write_all(ninja_script.as_bytes())?; - build_file.flush()?; + let mut build_file = NamedTempFile::new() + .map_err(|e| io::Error::other(format!("Failed to create temporary build file: {e}")))?; + build_file + .write_all(ninja_script.as_bytes()) + .map_err(|e| io::Error::other(format!("Failed to write build file: {e}")))?; + build_file + .flush() + .map_err(|e| io::Error::other(format!("Failed to flush build file: {e}")))?; Ok(build_file) } diff --git a/tests/runner_tests.rs b/tests/runner_tests.rs index cc79efd3..ca25aa7f 100644 --- a/tests/runner_tests.rs +++ b/tests/runner_tests.rs @@ -1,4 +1,7 @@ //! Unit tests for Ninja process invocation. +//! +//! These tests verify that the runner can translate a manifest into a Ninja +//! build script and invoke the Ninja process with appropriate arguments. use netsuke::cli::{Cli, Commands}; use netsuke::runner; diff --git a/tests/steps/process_steps.rs b/tests/steps/process_steps.rs index 29d32c9e..528c9d44 100644 --- a/tests/steps/process_steps.rs +++ b/tests/steps/process_steps.rs @@ -53,6 +53,8 @@ fn no_ninja(world: &mut CliWorld) { /// on the execution outcome. #[when("the ninja process is run")] fn run(world: &mut CliWorld) { + // Touch the capture variant so the support module's helpers remain used. + let _ = support::fake_ninja_capture as fn() -> (TempDir, PathBuf, PathBuf); let cli = world.cli.as_mut().expect("cli"); // Ensure a manifest exists at the path expected by the CLI. diff --git a/tests/support/mod.rs b/tests/support/mod.rs index 90606d82..6f822f23 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -33,23 +33,29 @@ pub fn fake_ninja(exit_code: i32) -> (TempDir, PathBuf) { /// /// Returns the temporary directory, path to the executable and the capture file /// location. -#[allow(dead_code, reason = "not every test exercises the capture variant")] pub fn fake_ninja_capture() -> (TempDir, PathBuf, PathBuf) { let dir = TempDir::new().expect("temp dir"); let capture = dir.path().join("captured.ninja"); let body = format!( - "#!/bin/sh\nwhile [ \"$1\" != \"\" ]; do\n if [ \"$1\" = \"-f\" ]; then\n shift\n cat \"$1\" > \"{}\"\n fi\n shift\ndone\n", - capture.display() + concat!( + "#!/bin/sh\n", + "while [ $# -gt 0 ]; do\n", + " if [ \"$1\" = \"-f\" ] && [ $# -gt 1 ]; then\n", + " shift\n", + " cat \"$1\" > \"{}\"\n", + " shift\n", + " else\n", + " shift\n", + " fi\n", + "done\n", + ), + capture.display(), ); let path = write_script(&dir, &body); (dir, path, capture) } /// Write a minimal Netsukefile to the provided temporary file. -#[allow( - dead_code, - reason = "helper is unused when the support crate builds independently" -)] pub fn write_manifest(file: &mut NamedTempFile) { let manifest = concat!( "netsuke_version: \"1.0.0\"\n",