From f91402434534dd05502b50a5176337238aa4fc1b Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 31 May 2026 22:53:52 +0200 Subject: [PATCH] Document Ninja executable override (#83) Document `NETSUKE_NINJA` in the user and runner documentation so users can find the portable Ninja executable override. Add binary-level logging coverage for the default `ninja` fallback and an explicit `NETSUKE_NINJA` path so verbose command traces show the resolved program in both cases. --- docs/netsuke-design.md | 8 +-- docs/test-isolation-with-ninja-env.md | 34 ++++++------ docs/users-guide.md | 7 +++ src/runner/mod.rs | 4 +- src/runner/process/mod.rs | 7 ++- tests/logging_stderr_tests.rs | 75 +++++++++++++++++++++++++++ 6 files changed, 111 insertions(+), 24 deletions(-) diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index 953f17c1..5a1f7378 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -2508,10 +2508,10 @@ and fail build] Figure: Build script localization audit flow for Fluent key validation. -The Ninja executable may be overridden via the `NINJA_ENV` environment -variable. For example, `NINJA_ENV=/opt/ninja/bin/ninja netsuke build` forces -Netsuke to execute the specified binary while preserving the default when the -variable is unset or invalid. +The Ninja executable may be overridden via the `NETSUKE_NINJA` environment +variable. For example, `NETSUKE_NINJA=/opt/ninja/bin/ninja netsuke build` +forces Netsuke to execute the specified binary while preserving the default +when the variable is unset or invalid. ### 8.4.1 Configuration File Discovery diff --git a/docs/test-isolation-with-ninja-env.md b/docs/test-isolation-with-ninja-env.md index 80285de8..d061cd37 100644 --- a/docs/test-isolation-with-ninja-env.md +++ b/docs/test-isolation-with-ninja-env.md @@ -1,23 +1,23 @@ -# Test isolation with `NINJA_ENV` +# Test isolation with `NETSUKE_NINJA` -Netsuke resolves the Ninja binary from the `NINJA_ENV` environment variable -before falling back to `ninja` on `PATH`. Tests should override `NINJA_ENV` -instead of mutating `PATH` so they can execute in parallel without stepping on -each other's environment. +Netsuke resolves the Ninja binary from the `NETSUKE_NINJA` environment variable +before falling back to `ninja` on `PATH`. Tests should override +`NETSUKE_NINJA` instead of mutating `PATH` so they can execute in parallel +without stepping on each other's environment. -## Why prefer `NINJA_ENV` +## Why prefer `NETSUKE_NINJA` - Mutating `PATH` is global and risks races when tests run concurrently. - `override_ninja_env` scopes changes via an `EnvGuard`, restoring the previous value even if the test fails. - Keeping `PATH` untouched avoids coupling to the developer's shell setup. - `override_ninja_env` also holds a process-wide lock for the guard's lifetime, - preventing parallel tests from interleaving `NINJA_ENV` mutations. + preventing parallel tests from interleaving `NETSUKE_NINJA` mutations. ## Fixture pattern -Use a fixture to create a fake Ninja executable and point `NINJA_ENV` at it. -The fixture keeps the temporary directory alive for the test duration and +Use a fixture to create a fake Ninja executable and point `NETSUKE_NINJA` at +it. The fixture keeps the temporary directory alive for the test duration and automatically restores the environment on drop. ```rust @@ -41,15 +41,15 @@ Inject the fixture into tests that need a controlled Ninja binary: fn run_build_uses_fake_ninja( (_, _guard): (tempfile::TempDir, NinjaEnvGuard), ) { - // run the command-line interface (CLI) here; the guard restores NINJA_ENV - // on drop + // run the command-line interface (CLI) here; the guard restores + // NETSUKE_NINJA on drop } ``` ## Dos and don'ts -- Do keep the guard alive until after the CLI invocation so `NINJA_ENV` stays - set. +- Do keep the guard alive until after the CLI invocation so `NETSUKE_NINJA` + stays set. - Do avoid explicit `drop` calls for `PathBuf` values; they do not own external resources. - Don't add `#[serial]` purely to protect `PATH` mutations; prefer the fixture @@ -57,7 +57,7 @@ fn run_build_uses_fake_ninja( ## Precedence over `PATH` -`NINJA_ENV` should override any `ninja` found on `PATH`. When asserting this in -tests, place a failing fake Ninja on `PATH` with `prepend_dir_to_path` and set -`NINJA_ENV` to a working fake Ninja via `override_ninja_env`. The test should -pass only if `NINJA_ENV` is respected. +`NETSUKE_NINJA` should override any `ninja` found on `PATH`. When asserting +this in tests, place a failing fake Ninja on `PATH` with `prepend_dir_to_path` +and set `NETSUKE_NINJA` to a working fake Ninja via `override_ninja_env`. The +test should pass only if `NETSUKE_NINJA` is respected. diff --git a/docs/users-guide.md b/docs/users-guide.md index 02a9c763..9ea1a413 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -687,6 +687,12 @@ Environment variables use the `NETSUKE_` prefix (for example, `NETSUKE_JOBS=8`). Use `__` to separate nested keys when matching structured configuration. +`NETSUKE_NINJA` overrides the Ninja executable used for `build`, `clean`, and +`graph` commands. Leave it unset to use the default `ninja` command on `PATH`, +or set it to another executable name such as `ninja-build` or to an absolute +path such as `/opt/ninja/bin/ninja` when the binary is installed outside the +default search path. + Use `--locale `, `NETSUKE_LOCALE`, or a `locale = "..."` entry in a configuration file to select localized CLI copy and error messages. Locale precedence is: command-line flag, environment variable, configuration file, @@ -1228,6 +1234,7 @@ names to screaming snake case: - `--verbose` → `NETSUKE_VERBOSE=true` - `--colour-policy` → `NETSUKE_COLOUR_POLICY=always` - `--spinner-mode` → `NETSUKE_SPINNER_MODE=disabled` +- Ninja executable override → `NETSUKE_NINJA=/opt/ninja/bin/ninja` For nested fields or indexed lists, use double underscore separators: diff --git a/src/runner/mod.rs b/src/runner/mod.rs index f8814cbe..af3308e0 100644 --- a/src/runner/mod.rs +++ b/src/runner/mod.rs @@ -2,7 +2,9 @@ //! //! This module keeps `main` minimal by providing a single entry point that //! handles command execution. It now delegates build requests to the Ninja -//! subprocess, streaming its output back to the user. +//! subprocess, streaming its output back to the user. The executable defaults +//! to `ninja` and may be overridden with `NETSUKE_NINJA` for systems that use a +//! different binary name or require a full path. mod error; diff --git a/src/runner/process/mod.rs b/src/runner/process/mod.rs index 5e5bb19a..87612855 100644 --- a/src/runner/process/mod.rs +++ b/src/runner/process/mod.rs @@ -1,5 +1,8 @@ -//! Process helpers for Ninja file lifecycle, argument redaction, and subprocess I/O. -//! Internal to `runner`; public API is defined in `runner.rs`. +//! Process helpers for Ninja file lifecycle, argument redaction, and subprocess +//! I/O. Internal to `runner`; public API is defined in `runner.rs`. +//! +//! Ninja executable resolution checks `NETSUKE_NINJA` first, then falls back to +//! `ninja` on `PATH` so existing installations keep their default behaviour. use super::{BuildTargets, NINJA_PROGRAM}; use crate::cli::Cli; diff --git a/tests/logging_stderr_tests.rs b/tests/logging_stderr_tests.rs index c3065b19..44805ec2 100644 --- a/tests/logging_stderr_tests.rs +++ b/tests/logging_stderr_tests.rs @@ -4,6 +4,7 @@ //! binary and asserting log messages appear on stderr rather than stdout. use anyhow::{Context, Result, ensure}; +use ninja_env::NINJA_ENV; use predicates::prelude::*; use rstest::{fixture, rstest}; use serde_json::Value; @@ -81,6 +82,39 @@ fn write_fake_ninja_script( make_script_executable(path) } +fn fake_ninja_name(stem: &str) -> String { + if cfg!(windows) { + format!("{stem}.cmd") + } else { + stem.to_owned() + } +} + +fn path_containing(dir: &Path) -> Result { + std::env::join_paths([dir]).context("build PATH containing fake ninja") +} + +fn run_verbose_build_with_ninja_env( + current_dir: &Path, + path_env: std::ffi::OsString, + ninja_env: Option<&Path>, +) -> Result { + let mut command = assert_cmd::cargo::cargo_bin_cmd!("netsuke"); + command + .current_dir(current_dir) + .env("PATH", path_env) + .env_remove(NINJA_ENV) + .arg("--verbose") + .arg("build"); + if let Some(ninja) = ninja_env { + command.env(NINJA_ENV, ninja); + } + + let output = command.output().context("run verbose netsuke build")?; + ensure!(output.status.success(), "expected verbose build to succeed"); + String::from_utf8(output.stderr).context("stderr should be valid UTF-8") +} + /// Verifies that runner errors are logged to stderr. /// /// The test creates an empty temporary directory (no manifest) and runs the @@ -206,6 +240,47 @@ fn diag_json_passthrough_uses_normal_clap_output( assert_diag_json_passthrough(flag, ctx, stdout_marker) } +#[rstest] +fn verbose_build_logs_default_ninja_command( + temp_with_minimal_manifest: Result, +) -> Result<()> { + let temp = temp_with_minimal_manifest?; + let ninja_temp = tempdir().context("create fake ninja dir")?; + let ninja_path = ninja_temp.path().join(fake_ninja_name("ninja")); + write_fake_ninja_script(&ninja_path, &[], None)?; + + let stderr = + run_verbose_build_with_ninja_env(temp.path(), path_containing(ninja_temp.path())?, None)?; + + ensure!( + stderr.contains("Executing command: ninja "), + "default build should log the fallback ninja program, got:\n{stderr}" + ); + Ok(()) +} + +#[rstest] +fn verbose_build_logs_ninja_env_override( + temp_with_minimal_manifest: Result, +) -> Result<()> { + let temp = temp_with_minimal_manifest?; + let ninja_temp = tempdir().context("create fake ninja dir")?; + let ninja_path = ninja_temp.path().join(fake_ninja_name("custom-ninja")); + write_fake_ninja_script(&ninja_path, &[], None)?; + + let stderr = run_verbose_build_with_ninja_env( + temp.path(), + path_containing(ninja_temp.path())?, + Some(ninja_path.as_path()), + )?; + + ensure!( + stderr.contains(&format!("Executing command: {} ", ninja_path.display())), + "override build should log the resolved ninja program, got:\n{stderr}" + ); + Ok(()) +} + #[test] fn config_driven_diag_json_formats_merge_failures_as_json() -> Result<()> { let temp = tempdir().context("create temp dir")?;