Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions docs/netsuke-design.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
34 changes: 17 additions & 17 deletions docs/test-isolation-with-ninja-env.md
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -41,23 +41,23 @@ 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
above to keep tests parallel-friendly.

## 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.
7 changes: 7 additions & 0 deletions docs/users-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 <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,
Expand Down Expand Up @@ -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:

Expand Down
4 changes: 3 additions & 1 deletion src/runner/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
7 changes: 5 additions & 2 deletions src/runner/process/mod.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
75 changes: 75 additions & 0 deletions tests/logging_stderr_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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::ffi::OsString> {
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<String> {
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
Expand Down Expand Up @@ -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<TempDir>,
) -> 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(())
}
Comment thread
leynos marked this conversation as resolved.

#[rstest]
fn verbose_build_logs_ninja_env_override(
temp_with_minimal_manifest: Result<TempDir>,
) -> 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")?;
Expand Down
Loading