diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dbe43610..8a556e38 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,7 +17,7 @@ jobs: steps: - uses: actions/checkout@v4 - name: Setup Rust - uses: leynos/shared-actions/.github/actions/setup-rust@v1.1.0 + uses: leynos/shared-actions/.github/actions/setup-rust@c6559452842af6a83b83429129dccaf910e34562 - name: Show Ninja version run: ninja --version - name: Format @@ -26,17 +26,17 @@ jobs: run: make lint - name: Test run: make test - - name: Install cargo-tarpaulin - run: cargo install cargo-tarpaulin - - name: Run coverage - run: cargo tarpaulin --out lcov + - name: Test and Measure Coverage + uses: leynos/shared-actions/.github/actions/generate-coverage@c6559452842af6a83b83429129dccaf910e34562 + with: + output-path: lcov.info + format: lcov - name: Upload coverage data to CodeScene env: CS_ACCESS_TOKEN: ${{ secrets.CS_ACCESS_TOKEN }} - if: ${{ env.CS_ACCESS_TOKEN != '' }} - uses: leynos/shared-actions/.github/actions/upload-codescene-coverage@v1.2.1 + if: ${{ env.CS_ACCESS_TOKEN }} + uses: leynos/shared-actions/.github/actions/upload-codescene-coverage@c6559452842af6a83b83429129dccaf910e34562 with: format: lcov access-token: ${{ env.CS_ACCESS_TOKEN }} installer-checksum: ${{ vars.CODESCENE_CLI_SHA256 }} - diff --git a/.gitignore b/.gitignore index 324c57f7..221eb3b3 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ target/ **/*.rs.bk +.crush diff --git a/CRUSH.md b/CRUSH.md new file mode 120000 index 00000000..47dc3e3d --- /dev/null +++ b/CRUSH.md @@ -0,0 +1 @@ +AGENTS.md \ No newline at end of file diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index 7237cce3..6df4ae66 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -1155,6 +1155,12 @@ The command construction will follow this pattern: streamed to the user's console, potentially with additional formatting or status updates from Netsuke itself. +In the initial implementation a small helper wraps `Command::new` to forward +the `-j` and `-C` flags and any explicit build targets. Standard output and +error are piped and written back to Netsuke's own streams so users see Ninja's +messages in order. A non-zero exit status or failure to spawn the process is +reported as an `io::Error` for the CLI to surface. + ### 6.2 The Criticality of Shell Escaping A primary security responsibility for Netsuke is the prevention of command diff --git a/docs/roadmap.md b/docs/roadmap.md index dd08fcde..22038c39 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -61,8 +61,8 @@ compilation pipeline from parsing to execution. - [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`. + - [x] Implement the process management logic in `main.rs` to invoke the ninja + executable as a subprocess using `std::process::Command`. *(done)* - **Success Criterion:** diff --git a/src/ast.rs b/src/ast.rs index 42d5e500..640b428d 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -10,7 +10,7 @@ //! use netsuke::ast::NetsukeManifest; //! use netsuke::ast::StringOrList; //! -//! let yaml = r#"netsuke_version: \"1.0.0\"\ntargets:\n - name: hello\n recipe:\n kind: command\n command: \"echo hi\""#; +//! let yaml = "netsuke_version: \"1.0.0\"\ntargets:\n - name: hello\n recipe:\n kind: command\n command: \"echo hi\""; //! let manifest: NetsukeManifest = serde_yml::from_str(yaml).expect("parse"); //! if let StringOrList::String(name) = &manifest.targets[0].name { //! assert_eq!(name, "hello"); @@ -50,7 +50,7 @@ use std::collections::HashMap; /// ```rust /// use netsuke::ast::NetsukeManifest; /// # fn main() -> Result<(), Box> { -/// let yaml = "netsuke_version: 1.0.0\ntargets:\n - name: hello\n recipe:\n kind: command\n command: echo hi"; +/// let yaml = "netsuke_version: \"1.0.0\"\ntargets:\n - name: hello\n recipe:\n kind: command\n command: echo hi"; /// let manifest: NetsukeManifest = serde_yml::from_str(yaml)?; /// assert_eq!(manifest.targets.len(), 1); /// # Ok(()) } diff --git a/src/ir.rs b/src/ir.rs index 9fb84c35..296a2ef4 100644 --- a/src/ir.rs +++ b/src/ir.rs @@ -322,11 +322,11 @@ fn find_cycle(targets: &HashMap) -> Option> { stack.push(node.clone()); - if let Some(edge) = targets.get(node) { - let deps_result = visit_dependencies(targets, &edge.inputs, stack, states); - if let Some(cycle) = deps_result { - return Some(cycle); - } + if let Some(cycle) = targets + .get(node) + .and_then(|edge| visit_dependencies(targets, &edge.inputs, stack, states)) + { + return Some(cycle); } stack.pop(); @@ -341,11 +341,10 @@ fn find_cycle(targets: &HashMap) -> Option> { states: &mut HashMap, ) -> Option> { for dep in deps { - if targets.contains_key(dep) { - let visit_result = visit(targets, dep, stack, states); - if let Some(cycle) = visit_result { - return Some(cycle); - } + if targets.contains_key(dep) + && let Some(cycle) = visit(targets, dep, stack, states) + { + return Some(cycle); } } None @@ -355,6 +354,7 @@ fn find_cycle(targets: &HashMap) -> Option> { let mut stack = Vec::new(); for node in targets.keys() { + // Skip nodes we've already processed to avoid redundant traversal. if states.contains_key(node) { continue; } diff --git a/src/main.rs b/src/main.rs index f72c2a2e..61d6c077 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,6 +1,17 @@ +//! Application entry point. +//! +//! Parses command-line arguments and delegates execution to [`runner::run`]. + use netsuke::{cli::Cli, runner}; +use std::process::ExitCode; -fn main() { +fn main() -> ExitCode { let cli = Cli::parse_with_default(); - runner::run(cli); + match runner::run(&cli) { + Ok(()) => ExitCode::SUCCESS, + Err(err) => { + eprintln!("{err}"); + ExitCode::FAILURE + } + } } diff --git a/src/runner.rs b/src/runner.rs index 1abdf777..a90b92d7 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -1,23 +1,96 @@ //! CLI execution and command dispatch logic. //! //! This module keeps [`main`] minimal by providing a single entry point that -//! handles command execution. It currently prints which command was invoked. +//! 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 std::io::{self, BufRead, BufReader, Write}; +use std::path::Path; +use std::process::{Command, Stdio}; +use std::thread; /// Execute the parsed [`Cli`] commands. -pub fn run(cli: Cli) { - match cli.command.unwrap_or(Commands::Build { +/// +/// # Errors +/// +/// Returns an [`io::Error`] if the Ninja process fails to spawn or exits with a +/// non-zero status code. +pub fn run(cli: &Cli) -> io::Result<()> { + let command = cli.command.clone().unwrap_or(Commands::Build { targets: Vec::new(), - }) { - Commands::Build { targets } => { - println!("Building targets: {targets:?}"); - } + }); + match command { + Commands::Build { targets } => run_ninja(Path::new("ninja"), cli, &targets), Commands::Clean => { println!("Clean requested"); + Ok(()) } Commands::Graph => { println!("Graph requested"); + Ok(()) + } + } +} + +/// 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. +/// +/// # Errors +/// +/// Returns an [`io::Error`] if the Ninja process fails to spawn or reports a +/// non-zero exit status. +/// +/// # Panics +/// +/// Panics if the child's output streams cannot be captured. +pub fn run_ninja(program: &Path, cli: &Cli, targets: &[String]) -> io::Result<()> { + let mut cmd = Command::new(program); + if let Some(dir) = &cli.directory { + cmd.current_dir(dir).arg("-C").arg(dir); + } + if let Some(jobs) = cli.jobs { + cmd.arg("-j").arg(jobs.to_string()); + } + cmd.args(targets); + cmd.stdout(Stdio::piped()); + cmd.stderr(Stdio::piped()); + + let mut child = cmd.spawn()?; + let stdout = child.stdout.take().expect("child stdout"); + let stderr = child.stderr.take().expect("child stderr"); + + let out_handle = thread::spawn(move || { + let reader = BufReader::new(stdout); + let mut handle = io::stdout(); + for line in reader.lines().map_while(Result::ok) { + let _ = writeln!(handle, "{line}"); } + }); + let err_handle = thread::spawn(move || { + let reader = BufReader::new(stderr); + let mut handle = io::stderr(); + for line in reader.lines().map_while(Result::ok) { + let _ = writeln!(handle, "{line}"); + } + }); + + let status = child.wait()?; + let _ = out_handle.join(); + let _ = err_handle.join(); + + if status.success() { + Ok(()) + } else { + #[expect( + clippy::io_other_error, + reason = "use explicit error kind for compatibility with older Rust" + )] + Err(io::Error::new( + io::ErrorKind::Other, + format!("ninja exited with {status}"), + )) } } diff --git a/tests/cucumber.rs b/tests/cucumber.rs index 56c422d8..76e7885d 100644 --- a/tests/cucumber.rs +++ b/tests/cucumber.rs @@ -1,5 +1,8 @@ +//! Cucumber test runner and world state. + use cucumber::World; +/// Shared state for Cucumber scenarios. #[derive(Debug, Default, World)] pub struct CliWorld { pub cli: Option, @@ -7,10 +10,32 @@ pub struct CliWorld { pub manifest: Option, pub manifest_error: Option, pub build_graph: Option, + /// Generated Ninja file content. pub ninja: Option, + /// Status of the last process execution (true for success, false for + /// failure). + pub run_status: Option, + /// Error message from the last failed process execution. + pub run_error: Option, + /// Temporary directory handle for test isolation. + pub temp: Option, + /// Original `PATH` value restored after each scenario. + pub original_path: Option, +} + +impl Drop for CliWorld { + fn drop(&mut self) { + if let Some(path) = self.original_path.take() { + // SAFETY: nightly marks `set_var` as unsafe; restore path for isolation. + unsafe { + std::env::set_var("PATH", path); + } + } + } } mod steps; +mod support; #[tokio::main] async fn main() { diff --git a/tests/features/ninja_process.feature b/tests/features/ninja_process.feature new file mode 100644 index 00000000..c8502804 --- /dev/null +++ b/tests/features/ninja_process.feature @@ -0,0 +1,19 @@ +Feature: Ninja process execution + + Scenario: Ninja succeeds + Given a fake ninja executable that exits with 0 + And the CLI is parsed with "" + When the ninja process is run + Then the command should succeed + + Scenario: Ninja fails + Given a fake ninja executable that exits with 1 + And the CLI is parsed with "" + When the ninja process is run + Then the command should fail with error "ninja exited with exit status: 1" + + Scenario: Ninja missing + Given no ninja executable is available + And the CLI is parsed with "" + When the ninja process is run + Then the command should fail with error "No such file or directory" diff --git a/tests/runner_tests.rs b/tests/runner_tests.rs new file mode 100644 index 00000000..79cb9a82 --- /dev/null +++ b/tests/runner_tests.rs @@ -0,0 +1,38 @@ +//! Unit tests for Ninja process invocation. + +use netsuke::cli::{Cli, Commands}; +use netsuke::runner; +use rstest::rstest; +use std::path::{Path, PathBuf}; + +/// Creates a default CLI configuration for testing Ninja invocation. +fn test_cli() -> Cli { + Cli { + file: PathBuf::from("Netsukefile"), + directory: None, + jobs: None, + command: Some(Commands::Build { + targets: Vec::new(), + }), + } +} + +mod support; + +#[rstest] +#[case(0, true)] +#[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 result = runner::run_ninja(&path, &cli, &[]); + assert_eq!(result.is_ok(), succeeds); +} + +#[rstest] +fn run_ninja_not_found() { + let cli = test_cli(); + let err = + runner::run_ninja(Path::new("does-not-exist"), &cli, &[]).expect_err("process should fail"); + assert_eq!(err.kind(), std::io::ErrorKind::NotFound); +} diff --git a/tests/steps/cli_steps.rs b/tests/steps/cli_steps.rs index f31643d8..984e5b82 100644 --- a/tests/steps/cli_steps.rs +++ b/tests/steps/cli_steps.rs @@ -5,7 +5,7 @@ use crate::CliWorld; use clap::Parser; -use cucumber::{then, when}; +use cucumber::{given, then, when}; use netsuke::cli::{Cli, Commands}; use std::path::PathBuf; @@ -42,6 +42,7 @@ fn extract_build(world: &CliWorld) -> Option<&Vec> { clippy::needless_pass_by_value, reason = "Cucumber requires owned String arguments" )] +#[given(expr = "the CLI is parsed with {string}")] #[when(expr = "the CLI is parsed with {string}")] fn parse_cli(world: &mut CliWorld, args: String) { apply_cli(world, &args); @@ -51,6 +52,7 @@ fn parse_cli(world: &mut CliWorld, args: String) { clippy::needless_pass_by_value, reason = "Cucumber requires owned String arguments" )] +#[given(expr = "the CLI is parsed with invalid arguments {string}")] #[when(expr = "the CLI is parsed with invalid arguments {string}")] fn parse_cli_invalid(world: &mut CliWorld, args: String) { apply_cli(world, &args); diff --git a/tests/steps/mod.rs b/tests/steps/mod.rs index cb30bcdf..3be671dc 100644 --- a/tests/steps/mod.rs +++ b/tests/steps/mod.rs @@ -2,3 +2,4 @@ mod cli_steps; mod ir_steps; mod manifest_steps; mod ninja_steps; +mod process_steps; diff --git a/tests/steps/process_steps.rs b/tests/steps/process_steps.rs new file mode 100644 index 00000000..6a34153a --- /dev/null +++ b/tests/steps/process_steps.rs @@ -0,0 +1,109 @@ +//! Step definitions for Ninja process execution. + +use crate::{CliWorld, support}; +use cucumber::{given, then, when}; +use netsuke::runner; +use std::path::PathBuf; +use tempfile::TempDir; + +/// Installs a test-specific ninja binary and updates the `PATH`. +#[expect( + clippy::needless_pass_by_value, + reason = "helper owns path for simplicity" +)] +fn install_test_ninja(world: &mut CliWorld, dir: TempDir, ninja_path: PathBuf) { + let original = world + .original_path + .get_or_insert_with(|| std::env::var_os("PATH").unwrap_or_default()); + + let new_path = format!("{}:{}", dir.path().display(), original.to_string_lossy()); + // SAFETY: nightly marks `set_var` as unsafe; override path for test isolation. + unsafe { + std::env::set_var("PATH", &new_path); + } + + world.ninja = Some(ninja_path.to_string_lossy().into_owned()); + world.temp = Some(dir); +} + +/// Creates a fake ninja executable that exits with the given status code. +#[given(expr = "a fake ninja executable that exits with {int}")] +fn fake_ninja(world: &mut CliWorld, code: i32) { + let (dir, path) = support::fake_ninja(code); + install_test_ninja(world, dir, path); +} + +/// Sets up a scenario where no ninja executable is available. +/// +/// This step creates a temporary directory and records the path to a +/// non-existent `ninja` binary within that directory, allowing tests to verify +/// behaviour when the executable is missing. +#[given("no ninja executable is available")] +fn no_ninja(world: &mut CliWorld) { + let dir = TempDir::new().expect("temp dir"); + let path = dir.path().join("ninja"); + install_test_ninja(world, dir, path); +} + +/// Executes the ninja process and captures the result in the test world. +/// +/// 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) + } else { + std::path::Path::new("ninja") + }; + match runner::run_ninja(program, cli, &[]) { + Ok(()) => { + world.run_status = Some(true); + world.run_error = None; + } + Err(e) => { + world.run_status = Some(false); + world.run_error = Some(e.to_string()); + } + } +} + +/// Asserts that the command succeeds. +#[then("the command should succeed")] +fn command_should_succeed(world: &mut CliWorld) { + assert_eq!(world.run_status, Some(true)); +} + +/// Asserts that the command fails and records an error message. +#[then("the command should fail")] +fn command_should_fail(world: &mut CliWorld) { + assert_eq!(world.run_status, Some(false)); + assert!( + world.run_error.is_some(), + "Expected an error message, but none was found", + ); +} + +/// Asserts that the command failed and the error message matches the expected value. +#[expect( + clippy::needless_pass_by_value, + reason = "cucumber step parameters require owned Strings" +)] +#[then(expr = "the command should fail with error {string}")] +fn command_should_fail_with_error(world: &mut CliWorld, expected: String) { + assert_eq!(world.run_status, Some(false)); + let actual = world + .run_error + .as_ref() + .expect("Expected an error message, but none was found"); + assert!( + actual.contains(&expected), + "Expected error message to contain '{expected}', but got '{actual}'", + ); +} diff --git a/tests/support/mod.rs b/tests/support/mod.rs new file mode 100644 index 00000000..722f3cae --- /dev/null +++ b/tests/support/mod.rs @@ -0,0 +1,24 @@ +//! Test utilities for process management. + +use std::fs::{self, File}; +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"); + let path = dir.path().join("ninja"); + let mut file = File::create(&path).expect("script"); + writeln!(file, "#!/bin/sh\nexit {exit_code}").expect("write script"); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mut perms = fs::metadata(&path).expect("meta").permissions(); + perms.set_mode(0o755); + fs::set_permissions(&path, perms).expect("perms"); + } + (dir, path) +}