From 05e19e75906b86f608df42ed4be48aef6181aab8 Mon Sep 17 00:00:00 2001 From: leynos Date: Thu, 27 Aug 2026 02:01:32 +0200 Subject: [PATCH 1/5] Use PowerShell for Windows recipes Make Windows legacy recipes invoke powershell.exe regardless of the launching shell, while retaining Git Bash/MSYS2 through an explicit NETSUKE_WINDOWS_SHELL=bash compatibility option. Add a pwsh-launched Windows smoke manifest for scalar, list, script, dependency order, quoting, dollar handling, failure, discovery, and missing-runtime diagnostics. Document the v0.1.x and v0.2.0 boundary. --- .github/workflows/ci.yml | 34 +++++ Cargo.lock | 1 + Cargo.toml | 1 + docs/developers-guide.md | 45 +++--- docs/users-guide.md | 80 +++++++++-- docs/v0-1-0-migration-guide.md | 48 ++++++- scripts/windows-recipe-smoke.ps1 | 127 +++++++++++++++++ src/ast/mod.rs | 12 +- src/ninja_gen/dyndep.rs | 28 +++- src/ninja_gen/mod.rs | 87 ++++++------ src/ninja_gen_display_edge.rs | 43 ++++++ src/ninja_gen_error.rs | 6 +- src/ninja_gen_escape.rs | 21 ++- src/ninja_gen_property_tests.rs | 5 +- src/ninja_gen_recipe_shell.rs | 133 ++++++++++++++++++ src/ninja_gen_tests.rs | 82 ++++++++++- src/ninja_gen_validation.rs | 23 +-- src/runner/dispatch.rs | 4 +- src/runner/dyndep_generation_telemetry.rs | 2 +- src/runner/generation.rs | 9 +- src/runner/mod.rs | 23 ++- src/runner/recipe_shell.rs | 113 +++++++++++++++ src/runner/tests.rs | 10 +- tests/data/windows-recipe-smoke.yml | 40 ++++++ tests/documentation_examples_tests.rs | 1 + ..._command_list_process_integration_tests.rs | 4 +- 26 files changed, 855 insertions(+), 127 deletions(-) create mode 100644 scripts/windows-recipe-smoke.ps1 create mode 100644 src/ninja_gen_display_edge.rs create mode 100644 src/ninja_gen_recipe_shell.rs create mode 100644 src/runner/recipe_shell.rs create mode 100644 tests/data/windows-recipe-smoke.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6ccdecdfd..eb3d4d4f7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -231,6 +231,40 @@ jobs: # blocks the merge. run: make SHELL=bash test + windows-native-recipe-smoke: + # This deliberately has no Git-Bash shell override. It proves that a + # PowerShell-launched Netsuke directs Ninja legacy recipes to the + # PowerShell interpreter selected by the Windows contract (#599). + needs: build-test-windows + runs-on: windows-latest + permissions: + contents: read + env: + CARGO_TERM_COLOR: always + NETSUKE_RUST_TOOLCHAIN: nightly-2026-06-25 + RUSTFLAGS: -D warnings -Zpolonius=next + defaults: + run: + shell: pwsh + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - name: Setup Rust + uses: leynos/shared-actions/.github/actions/setup-rust@8add2d99854a5b77548eae98cca59202e68fefc8 + with: + toolchain: ${{ env.NETSUKE_RUST_TOOLCHAIN }} + rustflags: -D warnings -Zpolonius=next + - name: Install Ninja + uses: seanmiddleditch/gha-setup-ninja@3b1f8f94a2f8254bd26914c4ab9474d4f0015f67 # v6 + - name: Build Netsuke + run: cargo build --locked --bin netsuke + - name: Exercise native Windows recipes + run: >- + ./scripts/windows-recipe-smoke.ps1 + -Netsuke ./target/debug/netsuke.exe + -Manifest ./tests/data/windows-recipe-smoke.yml + kani-smoke: if: github.event_name == 'pull_request' runs-on: ubuntu-latest diff --git a/Cargo.lock b/Cargo.lock index 8eca53c7d..049408282 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1575,6 +1575,7 @@ version = "0.1.0-beta2" dependencies = [ "anyhow", "assert_cmd", + "base64", "camino", "cap-primitives 3.4.4", "cap-std 3.4.4", diff --git a/Cargo.toml b/Cargo.toml index d9021412e..85a23acbf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -104,6 +104,7 @@ camino = "1.2.0" dunce = "1.0.5" semver = { version = "1", features = ["serde"] } anyhow = "1" +base64 = "0.22.1" indicatif = "0.18.4" thiserror = "1" miette = { version = "7.6.0", features = ["fancy"] } diff --git a/docs/developers-guide.md b/docs/developers-guide.md index eaa5a252d..180f9a751 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -288,27 +288,22 @@ The lowering stages have deliberately separate responsibilities: rejected because Netsuke cannot lower it safely; scripts use substitution without command-shaped parsing, so heredocs and comments remain valid. The resulting action contains ordinary command text and no Ninja placeholders. -- `src/ninja_gen/mod.rs` turns completed shell text into a Ninja value exactly - once. That boundary doubles residual dollar signs and rejects control - characters after IR lowering and before file emission. Paths remain distinct - from shell text and are rejected when they contain `$`, spaces, colons, or - control characters. For a list, it puts - each entry in a brace group and joins the groups with `&&`. Each group uses - `eval` with a shell-quoted entry payload. This keeps an inline comment or a - trailing control operator such as `&` inside the entry from consuming the - generated group terminator. Braces run in the current shell, not a subshell, - so directory changes, environment assignments, and shell variables can carry - from one entry to the next. The `&&` chain remains fail-fast. Each entry may - start at most one background job; the generated wrapper waits for that job - before it evaluates a later entry. Ninja generation rejects entries that - start more than one background job. It also rejects entries whose nested - `eval` payload makes the background-job count dynamic because the wrapper - cannot safely determine which jobs to wait for. A direct simple `exec`, - optionally prefixed by shell assignments, is evaluated in a retaining - subshell so its success or failure remains visible to the wrapper; a - successful `exec` ends the remaining chain. Structured or nested `exec` forms - are rejected during Ninja generation because the wrapper cannot supervise - them without changing their shell semantics. +- `src/ninja_gen/mod.rs` delegates completed recipe text to + `src/ninja_gen_recipe_shell.rs`. On Unix, and for the explicit Windows Bash + compatibility route, a scalar remains POSIX shell text. A list puts each + entry in a brace group and joins the groups with `&&`; `eval` receives a + shell-quoted payload, which keeps inline comments and trailing control + operators inside the entry boundary. Braces preserve current-shell state and + the chain remains fail-fast. The existing background-job and `exec` + validation rules apply to this POSIX route. +- On Windows, `RecipeShell::PowerShell` renders scalar commands and scripts as + encoded `powershell.exe` invocations. An ordered list becomes one PowerShell + script that resets and checks `$LASTEXITCODE` after each entry, preserving + PowerShell state while stopping after a failed native program. The POSIX + command-list analyser is deliberately not applied to this route. The runner + resolves `NETSUKE_WINDOWS_SHELL` and preflights `bash.exe` only when the + optional compatibility route is selected; `help targets` stays outside this + execution boundary. - `src/runner/process` forwards the command's output and recognizes the bounded `netsuke command-list failure: action HASH, entry M` marker. A failed list therefore retains the original exit status while adding the fixed-width @@ -355,9 +350,11 @@ accident. `NinjaValue` is the escaped value accepted by a Ninja `command` binding. `escape_ninja_value` is its only constructor and is fallible so control characters fail before emission. -The seam is owned by `src/ninja_gen_escape.rs`. Only the Ninja action writer -may compose a completed command and convert its `ShellText` into a -`NinjaValue`; no lowering code, metadata writer, or future backend may call it. +The seam is owned by `src/ninja_gen_escape.rs`. The Ninja action writer may +compose a completed command and hand it to the selected renderer. POSIX and +Bash routes convert `ShellText` through `escape_ninja_value`; the encoded +PowerShell transport returns a private `NinjaValue` without exposing its +payload to Ninja parsing. No IR or manifest lowering may call either route. Descriptions, `depfile`, `deps`, and `pool` retain their existing raw emission semantics because they are not shell text, although metadata is still checked for control characters. Add a separate, explicitly documented conversion for diff --git a/docs/users-guide.md b/docs/users-guide.md index 4dbc403ea..5402f478b 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -298,7 +298,7 @@ offending key. A rule or target must provide exactly one recipe: - `command`: one shell command, or an ordered list of commands. -- `script`: a multi-line POSIX shell script. +- `script`: a multi-line script for the selected legacy-recipe interpreter. - `rule`: the name of another rule to use. Rules may also provide `description`, text used for Ninja's progress display. @@ -319,8 +319,59 @@ same Jinja context, including `{{ ins }}` and `{{ outs }}`; those two placeholders are resolved later to the concrete target's shell-quoted input and output paths. An empty command list is rejected when the manifest is parsed. -At execution time, each list entry is evaluated inside its own brace group and -the groups are joined with `&&`. The entry is passed to `eval` as a + +### Windows legacy recipe contract + +On Windows, v0.1.x interprets every legacy `command` string, `command` list, +and `script` with **Windows PowerShell** (`powershell.exe`), not with the shell +that launched `netsuke`. Netsuke invokes it explicitly with an encoded, +non-interactive, no-profile command before Ninja executes a recipe. A build +started from PowerShell, `cmd.exe`, an IDE, or Git Bash therefore uses the same +recipe interpreter. This is a Windows PowerShell contract, not a PowerShell +Core (`pwsh`) contract. + +Scalar commands and scripts each receive a fresh PowerShell process. A command +list receives one shared process: entries run in declaration order, later +entries see PowerShell variables, `$env:` assignments, and locations left by an +earlier entry, and Netsuke exits at the first native-program non-zero status. +PowerShell terminating errors also fail the recipe. State does not cross action +or target boundaries. + +Use PowerShell syntax in the default route. `$name` is a PowerShell variable +and `$env:NAME` reads an environment variable; `${VAR:-default}` is POSIX +syntax and is not valid PowerShell. Recipe text is protected from Ninja dollar +expansion, so write ordinary PowerShell dollars rather than `$$`. The rendered +`{{ ins }}` and `{{ outs }}` paths use single-quoted PowerShell arguments, +including paths with spaces. Quote every other path and argument with +PowerShell syntax; arbitrary rendered Jinja text is not shell-quoted. + +Ninja turns a failed recipe into its own non-zero result, and `netsuke` returns +failure after forwarding Ninja's output. The CLI contract distinguishes success +from failure; it does not promise to return the recipe's exact child value. + +To retain POSIX interpretation on Windows, explicitly select a Git +Bash-compatible runtime: + + +```powershell +choco install git --yes --no-progress +$env:PATH = "C:\Program Files\Git\bin;$env:PATH" +$env:NETSUKE_WINDOWS_SHELL = "bash" +netsuke build +``` + +MSYS2 Bash is also supported when `bash.exe` is on `PATH`. Before `build` or +Ninja-tool execution, Netsuke checks this selection. If `bash.exe --version` +cannot run, it stops with instructions to install Git for Windows or MSYS2, add +Bash to `PATH`, or unset `NETSUKE_WINDOWS_SHELL`. `generate` and `help targets` +do not execute recipes, so they do not require Bash. In CI, install Git +explicitly, prepend its `bin` directory to `PATH`, set +`NETSUKE_WINDOWS_SHELL=bash`, and launch Netsuke normally from a `pwsh` step; +do not rely on a workflow-wide `shell: bash` setting. + +For the Unix default and the explicit Bash route, each list entry is evaluated +inside its own brace group and the groups are joined with `&&`. The entry is +passed to `eval` as a shell-quoted payload, so an inline `#` comment or a trailing control operator such as `&` cannot consume the generated group's closing boundary. Brace groups run in the current shell rather than a subshell: a changed working directory, @@ -368,9 +419,12 @@ Prefer a `command` list for a short, ordered sequence of distinct commands. Prefer `script` when the logic needs multi-line structure or shell constructs such as loops, conditionals, or variable assignment. -The v0.1.0-beta2 `script` implementation invokes `/bin/sh -e`; it is not -currently a portable PowerShell abstraction. Prefer `command` or -platform-selected actions when a manifest must work on Windows. +Legacy recipes remain shell strings in v0.1.x. The structured command blocks +and argv templates proposed +in [RFC: structured command blocks and argv templates #573](https://github.com/leynos/netsuke/pull/573) +for v0.2.0 are intended to remove this shell-selection, quoting, path, +variable, and exit-semantics ambiguity. They do not change the v0.1.x contract +described here. ### Targets, inputs, and dependencies @@ -1352,7 +1406,9 @@ Netsuke reduces some common quoting mistakes, but it is not a sandbox: - `{{ ins }}` and `{{ outs }}` are quoted as path arguments. - Arbitrary Jinja values in `command` and `script` are not automatically shell-quoted. -- `script` uses `/bin/sh -e` in v0.1.0-beta2. +- On Windows, legacy recipes use the PowerShell contract above unless + `NETSUKE_WINDOWS_SHELL=bash` selects the explicit Bash compatibility route. + On Unix, scripts use `/bin/sh -e`. - `shell`, `grep`, `fetch`, filesystem helpers, and ordinary recipes interact with the host. - `glob` restricts its filesystem metadata access to a capability handle @@ -1381,11 +1437,11 @@ Netsuke reduces some common quoting mistakes, but it is not a sandbox: structured or nested `exec` forms are rejected during Ninja generation. Failure diagnostics include the action fingerprint and one-based entry position when Netsuke can attribute the failed list entry. -- Write shell dollar expressions normally: `$PATH`, `$RUSTFLAGS`, and - `${CARGO:-cargo}` reach the child shell unchanged. Netsuke performs the - required Ninja escaping after it lowers `$in`, `$out`, `{{ ins }}`, and - `{{ outs }}`. A `$in` or `$out` token inside backticks is rejected because - Netsuke cannot safely lower it there. +- Write shell dollar expressions normally. `$PATH`, `$RUSTFLAGS`, and + `${CARGO:-cargo}` reach POSIX routes unchanged; PowerShell routes use `$name` + or `$env:NAME`. Netsuke performs the required Ninja escaping after it lowers + `$in`, `$out`, `{{ ins }}`, and `{{ outs }}`. A `$in` or `$out` token inside + backticks is rejected because Netsuke cannot safely lower it there. - **Migration:** replace the historical manifest spelling `$$PATH` with `$PATH`. Keeping the extra dollar now asks the shell to interpret `$$` as its process identifier and can change the command's result. Existing script diff --git a/docs/v0-1-0-migration-guide.md b/docs/v0-1-0-migration-guide.md index cf4ca7713..775221050 100644 --- a/docs/v0-1-0-migration-guide.md +++ b/docs/v0-1-0-migration-guide.md @@ -36,7 +36,7 @@ Table: documented v0.1.0 additions, including `netsuke help targets`, and their | Cached CLI configuration API | Breaking for callers of the unstable Rust API: use the opt-in cached discovery flow with `ConfigEnvProvider`; `ConfigStdEnvProvider` supplies process-backed access. | [Users' guide](users-guide.md) | | Timing output | Existing `VerboseTimingReporter::new` keeps its stderr sink; Rust callers can opt into an owned `Write + Send` sink with `with_writer`. | [Users' guide](users-guide.md#capture-verbose-timing-output) | | Glob expansion | Parent-relative patterns such as `glob('../shared/*.h')` now expand. Metadata checks use a capability rooted at the pattern's longest literal directory prefix; missing or non-directory prefixes return no matches, and unresolvable symlink matches are skipped. | [Users' guide](users-guide.md) and [ADR-010](adr-010-scope-glob-capability-to-literal-prefix.md) | -| Command recipes | Existing scalar `command` recipes are unchanged. New YAML command lists are opt-in and run in declaration order with fail-fast semantics. | [Rules and recipes](users-guide.md#rules-and-recipes) | +| Command recipes | On Windows, legacy scalar commands, lists, and scripts use Windows PowerShell by default; YAML command lists remain opt-in, ordered, and fail-fast. | [Windows legacy recipe contract](users-guide.md#windows-legacy-recipe-contract) | | Manifest discovery | Optional target/action `description` values are shown by the new `netsuke help targets` command. Manifests without them and existing build output are unchanged. | [Users' guide](users-guide.md) | | Serial dependencies | New opt-in `dependency_order: serial` runs an action or target's direct `deps` list in declaration order. | [Serial dependency ordering](users-guide.md#run-direct-dependencies-serially) | @@ -63,6 +63,52 @@ non-empty YAML list. The entries run in one shell process and stop at the first non-zero exit. See [Rules and recipes](users-guide.md#rules-and-recipes) for the syntax, shell semantics, and examples. + +## Windows legacy recipe interpreter + +v0.1.x makes Windows legacy-recipe execution explicit. Netsuke starts +`powershell.exe` for every scalar command, ordered list, and script, regardless +of whether the CLI was launched by `pwsh`, `cmd.exe`, an IDE, or Git Bash. The +default is Windows PowerShell, not PowerShell Core. Existing Windows manifests +that contain POSIX-only syntax must either move to PowerShell syntax or opt into +the Bash compatibility route: + +```powershell +choco install git --yes --no-progress +$env:PATH = "C:\Program Files\Git\bin;$env:PATH" +$env:NETSUKE_WINDOWS_SHELL = "bash" +netsuke build +``` + +MSYS2 is equally suitable when its `bash.exe` is on `PATH`. The executable is +checked before `build` and Ninja-tool commands, so an absent selected Bash +runtime produces an actionable Netsuke error instead of a Ninja command-not- +found failure. `generate` and `help targets` do not run recipes and therefore +do not require the optional runtime. + +In the default route, write `$name` for a PowerShell variable and `$env:NAME` +for an environment variable. `${VAR:-default}` is only valid in the explicit +Bash route. The v0.1.0 dollar-escaping fix means these are ordinary, single +dollars, not Ninja-escaped `$$` forms. Ordered lists share one PowerShell +process, so variables, environment assignments, and current-directory changes +persist between entries; a later entry does not run after a terminating error +or non-zero native exit. Each scalar, script, action, and target has a fresh +shell process. `{{ ins }}` and `{{ outs }}` remain path-quoted, including for +spaces; quote any other path or argument with the selected shell's syntax. + +For reproducible Windows CI, use a `pwsh` step and let Netsuke select +PowerShell; do not use a workflow-level `shell: bash` setting as evidence of +recipe behaviour. If selecting Bash, install Git with Chocolatey as above, +prepend `C:\Program Files\Git\bin` to that step's `PATH`, and set +`NETSUKE_WINDOWS_SHELL=bash` explicitly. + +This is deliberately a v0.1.x shell-string compatibility boundary. The +structured command blocks and argv templates in [RFC: structured command +blocks and argv templates #573](https://github.com/leynos/netsuke/pull/573) +are planned for v0.2.0 to remove shell-dependent quoting, paths, variable +expansion, and exit-status ambiguity. They are not backported through an +implicit change to legacy recipes. + ## Opting into an explicit child environment Construct a `CommandEnv`, name the variables to add, and pass it through diff --git a/scripts/windows-recipe-smoke.ps1 b/scripts/windows-recipe-smoke.ps1 new file mode 100644 index 000000000..7b4d8cc1b --- /dev/null +++ b/scripts/windows-recipe-smoke.ps1 @@ -0,0 +1,127 @@ +<# +.SYNOPSIS +Exercises the Windows legacy-recipe contract from a native PowerShell session. + +.DESCRIPTION +Runs a fixture through a supplied Netsuke executable. The caller is expected to +use PowerShell Core (`pwsh`), while the fixture proves that Ninja starts Windows +PowerShell (`powershell.exe`) for legacy recipes. +#> +[CmdletBinding()] +param( + [Parameter(Mandatory)] + [string]$Netsuke, + + [Parameter(Mandatory)] + [string]$Manifest +) + +$ErrorActionPreference = 'Stop' +$Netsuke = (Resolve-Path -LiteralPath $Netsuke).Path +$Manifest = (Resolve-Path -LiteralPath $Manifest).Path + +function Assert-Equal { + param( + [Parameter(Mandatory)] + [string]$Actual, + + [Parameter(Mandatory)] + [string]$Expected, + + [Parameter(Mandatory)] + [string]$Message + ) + + if ($Actual -ne $Expected) { + throw "$Message. Expected '$Expected', got '$Actual'." + } +} + +function Invoke-Netsuke { + param( + [Parameter(Mandatory)] + [string[]]$Arguments + ) + + & $Netsuke @Arguments + if ($LASTEXITCODE -ne 0) { + throw "Netsuke failed for '$($Arguments -join ' ')' with exit code $LASTEXITCODE." + } +} + +if ($PSVersionTable.PSEdition -ne 'Core') { + throw 'This smoke test must be launched by PowerShell Core (pwsh), not Windows PowerShell.' +} + +$workspace = Join-Path $env:RUNNER_TEMP "netsuke-windows-recipe-smoke-$PID" +New-Item -ItemType Directory -Path $workspace | Out-Null +Copy-Item -LiteralPath $Manifest -Destination (Join-Path $workspace 'Netsukefile') + +Push-Location $workspace +try { + $env:NETSUKE_SMOKE_VALUE = 'value with spaces' + + $discovery = & $Netsuke help targets 2>&1 + if ($LASTEXITCODE -ne 0) { + throw "Target discovery failed with exit code $LASTEXITCODE: $discovery" + } + if ($discovery -notmatch 'Confirm target discovery has no recipe side effects') { + throw "Target discovery omitted the fixture target: $discovery" + } + if (Test-Path -LiteralPath 'discovery must not execute.txt') { + throw 'Target discovery executed a recipe.' + } + + Invoke-Netsuke -Arguments @('build', 'scalar') + Assert-Equal -Actual (Get-Content -Raw -LiteralPath 'scalar interpreter with spaces.txt') ` + -Expected 'Desktop' -Message 'A scalar recipe did not run in Windows PowerShell' + + Invoke-Netsuke -Arguments @('build', 'ordered-list') + Assert-Equal -Actual (Get-Content -Raw -LiteralPath 'ordered state.txt') -Expected 'first;second' ` + -Message 'The ordered command list did not preserve state and order' + + $failure = & $Netsuke build first-list-entry-fails 2>&1 + if ($LASTEXITCODE -eq 0) { + throw 'A failed first command-list entry unexpectedly succeeded through Netsuke and Ninja.' + } + if (Test-Path -LiteralPath 'must not exist.txt') { + throw "The second command-list entry ran after the first failed: $failure" + } + + Invoke-Netsuke -Arguments @('build', 'script') + Assert-Equal -Actual (Get-Content -Raw -LiteralPath 'script interpreter.txt') -Expected 'Desktop' ` + -Message 'A script recipe did not run in Windows PowerShell' + + Invoke-Netsuke -Arguments @('build', 'aggregate') + Assert-Equal -Actual (Get-Content -Raw -LiteralPath 'dependency order.txt') -Expected 'first;second;aggregate' ` + -Message 'The aggregate action did not observe serial dependency order' + + Invoke-Netsuke -Arguments @('build', 'automatic path with spaces.txt') + Assert-Equal -Actual (Get-Content -Raw -LiteralPath 'automatic path with spaces.txt') ` + -Expected 'automatic-path-quoting' -Message 'Automatic path quoting did not preserve spaces' + + $savedPath = $env:PATH + $savedNinja = $env:NETSUKE_NINJA + $savedShell = $env:NETSUKE_WINDOWS_SHELL + try { + $env:NETSUKE_NINJA = (Get-Command ninja -CommandType Application).Path + $env:NETSUKE_WINDOWS_SHELL = 'bash' + $env:PATH = $workspace + $bashFailure = & $Netsuke build scalar 2>&1 + if ($LASTEXITCODE -eq 0) { + throw 'Selecting Bash without bash.exe unexpectedly succeeded.' + } + if ($bashFailure -notmatch 'bash.exe.*not found on PATH') { + throw "Missing-Bash diagnostics were not actionable: $bashFailure" + } + } + finally { + $env:PATH = $savedPath + $env:NETSUKE_NINJA = $savedNinja + $env:NETSUKE_WINDOWS_SHELL = $savedShell + } +} +finally { + Pop-Location + Remove-Item -Recurse -Force -LiteralPath $workspace +} diff --git a/src/ast/mod.rs b/src/ast/mod.rs index 491388acb..263277f90 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -152,16 +152,16 @@ pub struct Rule { /// determines the variant. #[derive(Debug, Clone, PartialEq, Serialize)] pub enum Recipe { - /// A shell command, given as a scalar or an ordered list executed by a - /// fail-fast shell chain. + /// A shell command, given as a scalar or an ordered list executed by the + /// selected interpreter's fail-fast sequence. Command { - /// A scalar command passes through unchanged; list entries are - /// evaluated in brace groups joined by a fail-fast `&&` chain. + /// A scalar command and each list entry are rendered for the selected + /// legacy-recipe interpreter. command: StringOrList, }, - /// An embedded multi-line script. + /// An embedded multi-line script for the selected legacy-recipe interpreter. Script { - /// Shell script content rendered into a `printf %b` pipeline. + /// Script content rendered for the selected interpreter. script: String, }, /// Invoke another named rule. diff --git a/src/ninja_gen/dyndep.rs b/src/ninja_gen/dyndep.rs index 955280858..f6dc8c9c0 100644 --- a/src/ninja_gen/dyndep.rs +++ b/src/ninja_gen/dyndep.rs @@ -59,8 +59,8 @@ use crate::hex; use crate::ir::{BuildEdge, BuildGraph}; use crate::localization::{self, keys}; use crate::ninja_gen::{ - NamedAction, NinjaGenError, edge_requires_gates, graph_requires_dyndep, join, path_key, - reject_unsupported_path_characters, validate_action_metadata, validate_action_recipe, + NamedAction, NinjaGenError, RecipeShell, edge_requires_gates, graph_requires_dyndep, join, + path_key, reject_unsupported_path_characters, validate_action_metadata, validate_action_recipe, }; use camino::Utf8PathBuf; use sha2::{Digest, Sha256}; @@ -89,7 +89,20 @@ const DYNDEP_NAMESPACE: &str = ".netsuke/dyndep"; /// `.netsuke/dyndep` namespace, and [`NinjaGenError::MissingAction`] when an /// edge references an unknown action. pub fn generate_bundle(graph: &BuildGraph) -> Result { - generate_bundle_inner(graph) + generate_bundle_for_shell(graph, RecipeShell::host_default()) +} + +/// Generate a complete Ninja bundle for one explicit legacy recipe interpreter. +/// +/// # Errors +/// +/// Returns [`NinjaGenError`] when the graph cannot be represented safely or +/// the generated bundle cannot be written. +pub(crate) fn generate_bundle_for_shell( + graph: &BuildGraph, + shell: RecipeShell, +) -> Result { + generate_bundle_inner(graph, shell) } /// Construct the [`GeneratedNinja`] bundle for `graph`. @@ -108,7 +121,10 @@ pub fn generate_bundle(graph: &BuildGraph) -> Result Result { +fn generate_bundle_inner( + graph: &BuildGraph, + shell: RecipeShell, +) -> Result { reject_unsupported_path_characters(graph)?; reject_reserved_paths(graph)?; let serial_present = graph_requires_dyndep(graph); @@ -121,9 +137,9 @@ fn generate_bundle_inner(graph: &BuildGraph) -> Result = graph.actions.iter().collect(); actions.sort_by_key(|(id, _)| *id); for (zero_based_action_index, (id, action)) in actions.into_iter().enumerate() { - validate_action_recipe(action, zero_based_action_index + 1)?; + validate_action_recipe(action, zero_based_action_index + 1, shell)?; validate_action_metadata(action)?; - NamedAction { id, action }.write_into(&mut out)?; + NamedAction { id, action, shell }.write_into(&mut out)?; } let mut stages = SerialStages::default(); diff --git a/src/ninja_gen/mod.rs b/src/ninja_gen/mod.rs index 65026b3d2..740950edb 100644 --- a/src/ninja_gen/mod.rs +++ b/src/ninja_gen/mod.rs @@ -6,9 +6,12 @@ //! generated Ninja file is written by the runner and `generate` command for //! downstream execution by the Ninja build system. +#[path = "../ninja_gen_display_edge.rs"] +mod display_edge; pub mod dyndep; mod path_syntax; +pub(crate) use display_edge::DisplayEdge; use dyndep::reject_reserved_paths; pub use dyndep::{GeneratedDyndep, GeneratedNinja, generate_bundle}; pub(crate) use path_syntax::{escape_ninja_path, reject_unsupported_path_characters}; @@ -19,7 +22,7 @@ use crate::localization::{self, keys}; use camino::Utf8PathBuf; use itertools::Itertools; use std::collections::HashSet; -use std::fmt::{self, Display, Formatter, Write}; +use std::fmt::Write; #[path = "../ninja_gen_command_list.rs"] pub(crate) mod ninja_gen_command_list; @@ -28,12 +31,15 @@ mod ninja_gen_error; #[path = "../ninja_gen_escape.rs"] mod ninja_gen_escape; +#[path = "../ninja_gen_recipe_shell.rs"] +mod ninja_gen_recipe_shell; #[path = "../ninja_gen_validation.rs"] mod ninja_gen_validation; use ninja_gen_command_list::{ActionId, CommandListEntry, command_list_entry}; pub use ninja_gen_error::NinjaGenError; -use ninja_gen_escape::{ShellText, escape_ninja_value}; +use ninja_gen_escape::ShellText; +pub(crate) use ninja_gen_recipe_shell::RecipeShell; use ninja_gen_validation::{validate_action_metadata, validate_action_recipe}; /// Write `key = value` to a Ninja file when `opt` holds a value. /// @@ -141,6 +147,20 @@ pub fn generate(graph: &BuildGraph) -> Result { /// entry contains a Ninja control character, a graph path uses Netsuke's /// reserved serial-ordering namespace, or writing to the output fails. pub fn generate_into(graph: &BuildGraph, out: &mut W) -> Result<(), NinjaGenError> { + generate_into_with_shell(graph, out, RecipeShell::host_default()) +} + +/// Write a Ninja build file for one explicit legacy recipe interpreter. +/// +/// # Errors +/// +/// Returns [`NinjaGenError`] when the graph cannot be represented safely or +/// writing the generated output fails. +pub(crate) fn generate_into_with_shell( + graph: &BuildGraph, + out: &mut W, + shell: RecipeShell, +) -> Result<(), NinjaGenError> { reject_unsupported_path_characters(graph)?; reject_reserved_paths(graph)?; if graph_requires_dyndep(graph) { @@ -152,9 +172,9 @@ pub fn generate_into(graph: &BuildGraph, out: &mut W) -> Result<(), Ni actions.sort_by_key(|(id, _)| *id); for (zero_based_action_index, (id, action)) in actions.into_iter().enumerate() { let action_index = zero_based_action_index + 1; - validate_action_recipe(action, action_index)?; + validate_action_recipe(action, action_index, shell)?; validate_action_metadata(action)?; - NamedAction { id, action }.write_into(out)?; + NamedAction { id, action, shell }.write_into(out)?; } let mut edges: Vec<_> = graph.targets.values().collect(); @@ -241,6 +261,8 @@ pub(crate) struct NamedAction<'a> { id: &'a str, /// The IR action whose recipe and metadata are rendered. action: &'a crate::ir::Action, + /// The explicit interpreter receiving this action's legacy recipe text. + shell: RecipeShell, } impl NamedAction<'_> { @@ -299,8 +321,10 @@ impl NamedAction<'_> { Recipe::Command { command: StringOrList::String(scalar_command), } => { - Self::assert_shell_command(scalar_command); - scalar_command.clone() + if self.shell != RecipeShell::PowerShell { + Self::assert_shell_command(scalar_command); + } + ShellText::new(scalar_command.clone()) } Recipe::Command { command: StringOrList::List(items), @@ -308,14 +332,17 @@ impl NamedAction<'_> { Recipe::Command { command: StringOrList::Empty, } => return Self::reject_empty_command_recipe(), - Recipe::Script { script } => Self::script_shell_text(script), + Recipe::Script { script } => self.script_shell_text(script), Recipe::Rule { .. } => return Self::reject_rule_recipe(), }; - Ok(ShellText::new(command)) + Ok(command) } /// Wraps a multi-line script in a one-line shell command for Ninja. - fn script_shell_text(script: &str) -> String { + fn script_shell_text(&self, script: &str) -> ShellText { + if self.shell == RecipeShell::PowerShell { + return ShellText::new(script.to_owned()); + } // Ninja commands must be single-line. Encode newlines and reconstruct the // original script with `printf %b` piped into a fresh shell to preserve // expected expansions. @@ -324,11 +351,14 @@ impl NamedAction<'_> { // Scripts are allowed to contain shell constructs such as heredocs and // comments that `shlex` cannot model, so only command recipes use the // debug parser guard. - cmd + ShellText::new(cmd) } /// Write list entries as isolated current-shell groups joined by `&&`. - fn command_list_shell_text(&self, items: &[String]) -> String { + fn command_list_shell_text(&self, items: &[String]) -> ShellText { + if let Some(script) = self.shell.command_list_script(items) { + return ShellText::new(script); + } // Brace groups keep each entry a distinct shell unit, and `eval` // prevents comments or trailing control operators inside an entry // consuming its terminator. Braces run in the current shell (unlike @@ -343,48 +373,17 @@ impl NamedAction<'_> { }) .join(" && "); Self::assert_shell_command(&command_line); - command_line + ShellText::new(command_line) } /// Writes this action's Ninja rule, escaping only the shell-text boundary. fn write_into(&self, output: &mut W) -> Result<(), NinjaGenError> { - let command = escape_ninja_value(self.shell_text()?)?; + let command = self.shell.command_value(&self.shell_text()?)?; writeln!(output, "rule {}", self.id)?; writeln!(output, " command = {command}")?; self.write_metadata(output) } } -/// Wrapper struct to display a build edge. -pub(crate) struct DisplayEdge<'a> { - /// The build edge whose inputs and outputs are rendered. - edge: &'a BuildEdge, - /// Whether the action sets `restat`, suppressing the edge-level override. - action_restat: bool, - /// Dependencies rendered after `|`, either the edge's implicit deps or lowered serial gates. - implicit_deps: &'a [Utf8PathBuf], -} - -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))?; - } - write!(f, ": {}", self.edge.action_id)?; - if !self.edge.inputs.is_empty() { - write!(f, " {}", join(&self.edge.inputs))?; - } - if !self.implicit_deps.is_empty() { - write!(f, " | {}", join(self.implicit_deps))?; - } - if !self.edge.order_only_deps.is_empty() { - write!(f, " || {}", join(&self.edge.order_only_deps))?; - } - writeln!(f)?; - write_flag!(f, "restat", self.edge.always && !self.action_restat); - writeln!(f) - } -} #[cfg(test)] #[path = "../ninja_gen_property_tests.rs"] mod property_tests; diff --git a/src/ninja_gen_display_edge.rs b/src/ninja_gen_display_edge.rs new file mode 100644 index 000000000..cbd87ccdd --- /dev/null +++ b/src/ninja_gen_display_edge.rs @@ -0,0 +1,43 @@ +//! Renders validated build edges into their Ninja syntax. + +use std::fmt::{self, Display, Formatter}; + +use camino::Utf8PathBuf; + +use super::join; +use crate::ir::BuildEdge; + +/// Wraps one build edge and its lowered dependency details for display. +pub(crate) struct DisplayEdge<'a> { + /// The build edge whose inputs and outputs are rendered. + pub(crate) edge: &'a BuildEdge, + /// Whether the action sets `restat`, suppressing the edge-level override. + pub(crate) action_restat: bool, + /// Dependencies rendered after `|`, either source deps or lowered serial gates. + pub(crate) implicit_deps: &'a [Utf8PathBuf], +} + +impl Display for DisplayEdge<'_> { + /// Write the edge in Ninja's build-edge grammar. + fn fmt(&self, formatter: &mut Formatter<'_>) -> fmt::Result { + write!(formatter, "build {}", join(&self.edge.explicit_outputs))?; + if !self.edge.implicit_outputs.is_empty() { + write!(formatter, " | {}", join(&self.edge.implicit_outputs))?; + } + write!(formatter, ": {}", self.edge.action_id)?; + if !self.edge.inputs.is_empty() { + write!(formatter, " {}", join(&self.edge.inputs))?; + } + if !self.implicit_deps.is_empty() { + write!(formatter, " | {}", join(self.implicit_deps))?; + } + if !self.edge.order_only_deps.is_empty() { + write!(formatter, " || {}", join(&self.edge.order_only_deps))?; + } + writeln!(formatter)?; + if self.edge.always && !self.action_restat { + writeln!(formatter, " restat = 1")?; + } + writeln!(formatter) + } +} diff --git a/src/ninja_gen_error.rs b/src/ninja_gen_error.rs index 93d570bbe..a5120c41b 100644 --- a/src/ninja_gen_error.rs +++ b/src/ninja_gen_error.rs @@ -65,6 +65,9 @@ pub enum NinjaGenError { /// One-based stable position in the command list. entry_index: usize, }, + /// Completed recipe text cannot be represented safely in one Ninja binding. + #[error("recipe text contains an unsafe Ninja control character")] + UnsafeNinjaValue, /// A graph with serial dependencies cannot be represented by a single /// build-file string; callers must use [`crate::ninja_gen::generate_bundle`]. #[error("{message}")] @@ -90,9 +93,6 @@ pub enum NinjaGenError { /// Localized error message. message: LocalizedMessage, }, - /// A scalar command or script cannot be represented in one Ninja binding. - #[error("Ninja binding contains an unsafe control character")] - UnsafeNinjaValue, /// A path cannot be represented consistently in a Ninja build edge. #[error("Ninja path contains an unsafe character: {path}")] UnsafeNinjaPath { diff --git a/src/ninja_gen_escape.rs b/src/ninja_gen_escape.rs index 1abc6155c..dd88e4fdd 100644 --- a/src/ninja_gen_escape.rs +++ b/src/ninja_gen_escape.rs @@ -4,7 +4,7 @@ use std::fmt::{self, Display, Formatter}; use super::NinjaGenError; -/// Fully assembled POSIX shell text without Ninja-specific escaping. +/// Fully assembled recipe text without Ninja-specific escaping. pub(super) struct ShellText(String); impl ShellText { @@ -12,6 +12,11 @@ impl ShellText { pub(super) const fn new(text: String) -> Self { Self(text) } + + /// Borrow the completed recipe text before Ninja serialization. + pub(super) fn as_str(&self) -> &str { + &self.0 + } } /// Text safe to emit on the right-hand side of a Ninja binding. @@ -20,6 +25,13 @@ impl ShellText { /// backend escaping boundary exactly once. pub(super) struct NinjaValue(String); +impl NinjaValue { + /// Construct a value already safe for Ninja's binding grammar. + pub(super) const fn from_encoded(value: String) -> Self { + Self(value) + } +} + impl Display for NinjaValue { fn fmt(&self, formatter: &mut Formatter<'_>) -> fmt::Result { formatter.write_str(&self.0) @@ -30,10 +42,9 @@ impl Display for NinjaValue { /// /// Literal dollars become `$$`. Control characters are rejected because they /// could add a new Ninja statement instead of remaining part of the binding. -pub(super) fn escape_ninja_value(text: ShellText) -> Result { - let ShellText(contents) = text; - if contents.contains(['\n', '\r', '\0']) { +pub(super) fn escape_ninja_value(text: &ShellText) -> Result { + if text.as_str().contains(['\n', '\r', '\0']) { return Err(NinjaGenError::UnsafeNinjaValue); } - Ok(NinjaValue(contents.replace('$', "$$"))) + Ok(NinjaValue(text.as_str().replace('$', "$$"))) } diff --git a/src/ninja_gen_property_tests.rs b/src/ninja_gen_property_tests.rs index 6c9120d7c..e1a755b7f 100644 --- a/src/ninja_gen_property_tests.rs +++ b/src/ninja_gen_property_tests.rs @@ -237,7 +237,10 @@ proptest! { .expect("generated action should include a command line"); let mut previous = 0; for entry in &entries { - let expected_entry = format!("eval {}", canonical_shell_single_quote(&format!("echo {entry}"))); + let expected_entry = format!( + "eval {}", + canonical_shell_single_quote(&format!("echo {entry}")).replace('$', "$$") + ); let expected_count = entries.iter().filter(|candidate| *candidate == entry).count(); prop_assert_eq!( command_line.matches(&expected_entry).count(), diff --git a/src/ninja_gen_recipe_shell.rs b/src/ninja_gen_recipe_shell.rs new file mode 100644 index 000000000..cc79bed0f --- /dev/null +++ b/src/ninja_gen_recipe_shell.rs @@ -0,0 +1,133 @@ +//! Renders legacy recipes for the interpreter selected by the host contract. + +use base64::{Engine as _, engine::general_purpose::STANDARD}; + +use super::NinjaGenError; +use super::ninja_gen_escape::{NinjaValue, ShellText, escape_ninja_value}; + +/// Selects the interpreter that receives completed legacy recipe text. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(crate) enum RecipeShell { + /// Uses the host POSIX shell through Ninja's ordinary Unix execution path. + Posix, + /// Uses Windows PowerShell with an encoded script argument. + PowerShell, + /// Uses an explicitly selected Bash compatibility runtime on Windows. + Bash, +} + +impl RecipeShell { + /// Return the interpreter Netsuke selects when no Windows override exists. + pub(crate) const fn host_default() -> Self { + if cfg!(windows) { + Self::PowerShell + } else { + Self::Posix + } + } + + /// Render completed recipe text as a safe Ninja command binding value. + pub(super) fn command_value(self, script: &ShellText) -> Result { + match self { + Self::Posix => escape_ninja_value(script), + Self::PowerShell => { + let power_shell_script = Self::power_shell_script(script); + Self::power_shell_command(&power_shell_script) + } + Self::Bash => Self::bash_command(script), + } + } + + /// Build one shared-scope PowerShell script for an ordered command list. + pub(crate) fn command_list_script(self, entries: &[String]) -> Option { + if self != Self::PowerShell { + return None; + } + let mut script = String::new(); + for entry in entries { + script.push_str("$LASTEXITCODE = 0\n"); + script.push_str(entry); + script.push_str("\nif ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }\n"); + } + Some(script) + } + + /// Add PowerShell's terminating-error and native-process exit policy. + fn power_shell_script(script: &ShellText) -> ShellText { + ShellText::new(format!( + "$ErrorActionPreference = 'Stop'\n$LASTEXITCODE = 0\n{}\nif ($LASTEXITCODE -ne 0) {{ exit $LASTEXITCODE }}", + script.as_str() + )) + } + + /// Encode one PowerShell script without exposing its text to Ninja parsing. + fn power_shell_command(script: &ShellText) -> Result { + if script.as_str().contains('\0') { + return Err(NinjaGenError::UnsafeNinjaValue); + } + let utf16le = script + .as_str() + .encode_utf16() + .flat_map(u16::to_le_bytes) + .collect::>(); + let command = STANDARD.encode(utf16le); + Ok(NinjaValue::from_encoded(format!( + "powershell.exe -NoLogo -NoProfile -NonInteractive -ExecutionPolicy Bypass -EncodedCommand {command}" + ))) + } + + /// Wrap one POSIX recipe in the explicit Windows Bash compatibility runtime. + fn bash_command(script: &ShellText) -> Result { + let command = format!("bash.exe -e -c {}", windows_argument(script.as_str())); + escape_ninja_value(&ShellText::new(command)) + } +} + +/// Quote one argument according to the Windows `CommandLineToArgvW` convention. +fn windows_argument(argument: &str) -> String { + let mut quoted = String::with_capacity(argument.len() + 2); + quoted.push('"'); + let mut backslashes = 0usize; + for character in argument.chars() { + if character == '\\' { + backslashes += 1; + } else if character == '"' { + quoted.push_str(&"\\".repeat(backslashes.saturating_mul(2).saturating_add(1))); + quoted.push('"'); + backslashes = 0; + } else { + quoted.push_str(&"\\".repeat(backslashes)); + quoted.push(character); + backslashes = 0; + } + } + quoted.push_str(&"\\".repeat(backslashes.saturating_mul(2))); + quoted.push('"'); + quoted +} + +#[cfg(test)] +mod tests { + //! Verifies interpreter-specific Ninja command rendering. + + use super::{RecipeShell, windows_argument}; + use crate::ninja_gen::ninja_gen_escape::ShellText; + + #[test] + fn power_shell_command_hides_recipe_dollars_from_ninja() { + let rendered = RecipeShell::PowerShell + .command_value(&ShellText::new("$env:NETSUKE_SMOKE".into())) + .expect("PowerShell encoding should succeed") + .to_string(); + assert!(rendered.starts_with("powershell.exe ")); + assert!(!rendered.contains("NETSUKE_SMOKE")); + } + + #[test] + fn windows_argument_preserves_quotes_and_trailing_backslashes() { + assert_eq!( + windows_argument("a \\\"b\\\"\\\\"), + "\"a \\\\\\\"b\\\\\\\"\\\\\\\\\"" + ); + } +} diff --git a/src/ninja_gen_tests.rs b/src/ninja_gen_tests.rs index 0320fd224..4ecce21b6 100644 --- a/src/ninja_gen_tests.rs +++ b/src/ninja_gen_tests.rs @@ -1,10 +1,11 @@ //! Unit tests for Ninja file generation and rule synthesis. use super::test_support::command_action; -use super::{NamedAction, NinjaGenError, generate, generate_into}; +use super::{NamedAction, NinjaGenError, RecipeShell, generate, generate_into}; use crate::ast::{Recipe, StringOrList}; use crate::ir::{Action, BuildEdge, BuildGraph, DependencyOrder}; use anyhow::{Context, Result, ensure}; +use base64::{Engine as _, engine::general_purpose::STANDARD}; use camino::Utf8PathBuf; use rstest::rstest; @@ -185,6 +186,68 @@ fn generate_command_list_ninja_joins_a_fail_fast_chain() -> Result<()> { Ok(()) } +#[test] +fn power_shell_command_lists_preserve_state_and_stop_on_native_failure() -> Result<()> { + let action = command_action(StringOrList::List(vec![ + "$env:NETSUKE_ORDER = 'first'".into(), + "if ($env:NETSUKE_ORDER -ne 'first') { exit 9 }; cmd.exe /c exit 0".into(), + ])); + let mut rendered = String::new(); + NamedAction { + id: "power_shell_list", + action: &action, + shell: RecipeShell::PowerShell, + } + .write_into(&mut rendered)?; + let encoded = rendered + .split("-EncodedCommand ") + .nth(1) + .context("PowerShell command should include an encoded script")? + .trim(); + let script = decode_power_shell_script(encoded)?; + ensure!(script.contains("$env:NETSUKE_ORDER = 'first'")); + ensure!(script.contains("$LASTEXITCODE = 0")); + ensure!(script.contains("if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }")); + ensure!( + !rendered.contains("NETSUKE_ORDER"), + "Ninja must not parse the PowerShell variable expression: {rendered}" + ); + Ok(()) +} + +#[test] +fn power_shell_scripts_do_not_use_the_posix_script_wrapper() -> Result<()> { + let action = Action { + recipe: Recipe::Script { + script: "$edition = $PSVersionTable.PSEdition\nWrite-Output $edition".into(), + }, + description: None, + depfile: None, + deps_format: None, + pool: None, + restat: false, + }; + let mut rendered = String::new(); + NamedAction { + id: "power_shell_script", + action: &action, + shell: RecipeShell::PowerShell, + } + .write_into(&mut rendered)?; + let encoded = rendered + .split("-EncodedCommand ") + .nth(1) + .context("PowerShell command should include an encoded script")? + .trim(); + let script = decode_power_shell_script(encoded)?; + ensure!(script.contains("$edition = $PSVersionTable.PSEdition")); + ensure!(script.contains("Write-Output $edition")); + ensure!( + !script.contains("/bin/sh"), + "PowerShell script must not traverse the POSIX script wrapper: {script}" + ); + Ok(()) +} #[rstest] #[case::empty(StringOrList::Empty)] #[case::empty_list(StringOrList::List(Vec::new()))] @@ -279,3 +342,20 @@ fn assert_shell_command_tolerates_complex_syntax() { let command = r#"/bin/sh -c "echo 'nested quotes' && echo \"double\" && (echo subshell)""#; NamedAction::assert_shell_command(command); } + +/// Decode the UTF-16LE PowerShell payload emitted in a Ninja command binding. +fn decode_power_shell_script(encoded: &str) -> Result { + let bytes = STANDARD + .decode(encoded) + .context("decode PowerShell command payload")?; + let units = bytes + .chunks_exact(2) + .map(|pair| { + let [low, high]: [u8; 2] = pair + .try_into() + .context("PowerShell UTF-16 unit must contain two bytes")?; + Ok(u16::from(low) | (u16::from(high) << 8)) + }) + .collect::>>()?; + String::from_utf16(&units).context("decode PowerShell UTF-16 payload") +} diff --git a/src/ninja_gen_validation.rs b/src/ninja_gen_validation.rs index 46002b8b6..725179920 100644 --- a/src/ninja_gen_validation.rs +++ b/src/ninja_gen_validation.rs @@ -3,25 +3,32 @@ use super::ninja_gen_command_list::{ CommandListEntry, CommandListEntryError, command_list_entry_error, }; -use super::{ - NinjaGenError, - ninja_gen_escape::{ShellText, escape_ninja_value}, -}; +use super::ninja_gen_escape::{ShellText, escape_ninja_value}; +use super::{NinjaGenError, RecipeShell}; use crate::ast::{Recipe, StringOrList}; /// Reject recipes the generated shell cannot execute with stable semantics. pub(super) fn validate_action_recipe( action: &crate::ir::Action, action_index: usize, + shell: RecipeShell, ) -> Result<(), NinjaGenError> { if let Recipe::Command { command } = &action.recipe && command.is_empty_content() { return Err(NinjaGenError::EmptyCommandRecipe { action_index }); } - if let Recipe::Command { - command: StringOrList::List(entries), - } = &action.recipe + if shell != RecipeShell::PowerShell + && let Recipe::Command { + command: StringOrList::String(command), + } = &action.recipe + { + escape_ninja_value(&ShellText::new(command.clone()))?; + } + if shell != RecipeShell::PowerShell + && let Recipe::Command { + command: StringOrList::List(entries), + } = &action.recipe { for (zero_based_entry_index, entry) in entries.iter().enumerate() { let entry_index = zero_based_entry_index + 1; @@ -68,7 +75,7 @@ pub(super) fn validate_action_metadata(action: &crate::ir::Action) -> Result<(), .into_iter() .flatten() { - escape_ninja_value(ShellText::new(value.clone())).map(|_| ())?; + escape_ninja_value(&ShellText::new(value.clone())).map(|_| ())?; } Ok(()) } diff --git a/src/runner/dispatch.rs b/src/runner/dispatch.rs index 12e2bb26a..a34b046b7 100644 --- a/src/runner/dispatch.rs +++ b/src/runner/dispatch.rs @@ -1,7 +1,7 @@ //! Dispatch parsed commands and emit their successful JSON result documents. use super::{ - ExecutionContext, NinjaContent, NinjaToolSpec, generate_ninja, graph, handle_build, + ExecutionContext, NinjaContent, NinjaToolSpec, generate_ninja_with_shell, graph, handle_build, handle_ninja_tool, help, materialize_dyndep_bundle, process, prune_dyndep_bundle, resolve_output_path, }; @@ -70,7 +70,7 @@ fn execute_generate( output: Option<&std::path::PathBuf>, context: &ExecutionContext<'_>, ) -> Result<()> { - let bundle = generate_ninja(cli, context.reporter, None)?; + let bundle = generate_ninja_with_shell(cli, context.reporter, None, context.recipe_shell)?; let publication = materialize_dyndep_bundle(cli, &bundle)?; prune_dyndep_bundle(cli, bundle.dyndep_files(), &publication)?; let ninja = NinjaContent::new(bundle.into_parts().0); diff --git a/src/runner/dyndep_generation_telemetry.rs b/src/runner/dyndep_generation_telemetry.rs index f833efbe1..20285357a 100644 --- a/src/runner/dyndep_generation_telemetry.rs +++ b/src/runner/dyndep_generation_telemetry.rs @@ -70,11 +70,11 @@ const fn error_category(error: &NinjaGenError) -> &'static str { | NinjaGenError::UnsupportedCommandListExec { .. } | NinjaGenError::UnanalyzableCommandListEval { .. } | NinjaGenError::NinjaControlCharacter { .. } => "command_list", + NinjaGenError::UnsafeNinjaValue => "unsafe_ninja_value", NinjaGenError::Format { .. } => "format", NinjaGenError::DyndepFilesRequired { .. } => "dyndep_files_required", NinjaGenError::ReservedOutputPath { .. } => "reserved_output_path", NinjaGenError::UnsupportedPathCharacter { .. } => "unsupported_path_character", - NinjaGenError::UnsafeNinjaValue => "unsafe_ninja_value", NinjaGenError::UnsafeNinjaPath { .. } => "unsafe_ninja_path", } } diff --git a/src/runner/generation.rs b/src/runner/generation.rs index 5623e57e0..0c1332fdf 100644 --- a/src/runner/generation.rs +++ b/src/runner/generation.rs @@ -101,17 +101,20 @@ pub(super) fn build_graph(manifest: &NetsukeManifest) -> Result { /// # Examples /// /// ```rust,ignore -/// let generated = ninja_text(&graph)?; +/// let generated = ninja_text_for_shell(&graph, RecipeShell::host_default())?; /// let (text, sidecars) = generated.into_parts(); /// assert!(text.contains("build hello:")); /// assert!(sidecars.is_empty()); /// ``` /// +/// Generate Ninja text using the selected legacy-recipe interpreter. +/// /// # Errors /// /// Returns an error when Ninja synthesis fails. -pub(super) fn ninja_text( +pub(super) fn ninja_text_for_shell( graph: &BuildGraph, + shell: crate::ninja_gen::RecipeShell, ) -> Result { - ninja_gen::generate_bundle(graph) + ninja_gen::dyndep::generate_bundle_for_shell(graph, shell) } diff --git a/src/runner/mod.rs b/src/runner/mod.rs index 4537491ef..54162d0ed 100644 --- a/src/runner/mod.rs +++ b/src/runner/mod.rs @@ -44,6 +44,7 @@ mod ninja_content; mod ninja_process_adapter; mod path_helpers; mod process; +mod recipe_shell; pub use ninja_content::NinjaContent; pub use ninja_process_adapter::{run_ninja, run_ninja_tool}; #[cfg(doctest)] @@ -67,6 +68,8 @@ struct ExecutionContext<'a> { /// Keep a native [`Path`]: only `NETSUKE_NINJA` resolution performs UTF-8 /// conversion, preserving valid non-UTF-8 executable paths. ninja_program: &'a Path, + /// Explicit interpreter for generated legacy recipe text. + recipe_shell: ninja_gen::RecipeShell, } /// Target list passed through to Ninja; an empty slice uses IR defaults. @@ -139,10 +142,12 @@ fn run_with_ninja_program_resolver( } let ninja_program = configured_program.map_or_else(|| Cow::Owned(resolve_program()), Cow::Borrowed); + let recipe_shell = recipe_shell::resolve_recipe_shell()?; let context = ExecutionContext { reporter: reporter.as_ref(), progress_enabled, ninja_program: ninja_program.as_ref(), + recipe_shell, }; dispatch::execute(cli, command, &context) } @@ -160,7 +165,13 @@ fn on_task_progress_callback(reporter: &dyn StatusReporter) -> impl FnMut(u32, u /// /// Returns an error if manifest generation or Ninja execution fails. fn handle_build(cli: &Cli, args: &BuildArgs, context: &ExecutionContext<'_>) -> Result<()> { - let bundle = generate_ninja(cli, context.reporter, Some(keys::STATUS_TOOL_BUILD.into()))?; + recipe_shell::validate_recipe_shell(context.recipe_shell)?; + let bundle = generate_ninja_with_shell( + cli, + context.reporter, + Some(keys::STATUS_TOOL_BUILD.into()), + context.recipe_shell, + )?; let publication = materialize_dyndep_bundle(cli, &bundle)?; prune_dyndep_bundle(cli, bundle.dyndep_files(), &publication)?; let ninja = NinjaContent::new(bundle.into_parts().0); @@ -231,7 +242,9 @@ fn handle_ninja_tool( subcommand = tool.name, "Preparing Ninja tool invocation" ); - let bundle = generate_ninja(cli, context.reporter, Some(tool.key))?; + recipe_shell::validate_recipe_shell(context.recipe_shell)?; + let bundle = + generate_ninja_with_shell(cli, context.reporter, Some(tool.key), context.recipe_shell)?; let publication = materialize_dyndep_bundle(cli, &bundle)?; let (ninja_file, dyndep_files) = bundle.into_parts(); let ninja = NinjaContent::new(ninja_file); @@ -285,10 +298,12 @@ fn handle_ninja_tool( /// use netsuke::ninja_gen::GeneratedNinja; /// # let _: Option = None; /// ``` -fn generate_ninja( +/// Generate Ninja output using one selected legacy-recipe interpreter. +pub(super) fn generate_ninja_with_shell( cli: &Cli, reporter: &dyn StatusReporter, tool_key: Option, + recipe_shell: ninja_gen::RecipeShell, ) -> Result { let manifest_path = resolve_manifest_path(cli)?; ensure_manifest_exists_or_error(cli, reporter, &manifest_path)?; @@ -313,7 +328,7 @@ fn generate_ninja( tool_key, ); dyndep_generation_telemetry::instrument_bundle_generation(&graph, || { - generation::ninja_text(&graph) + generation::ninja_text_for_shell(&graph, recipe_shell) }) .context(localization::message(keys::RUNNER_CONTEXT_GENERATE_NINJA)) } diff --git a/src/runner/recipe_shell.rs b/src/runner/recipe_shell.rs new file mode 100644 index 000000000..244586901 --- /dev/null +++ b/src/runner/recipe_shell.rs @@ -0,0 +1,113 @@ +//! Resolves and validates the Windows legacy-recipe interpreter selection. + +use anyhow::{Context, Result, bail}; +use mockable::Env; +use std::ffi::OsString; + +use crate::ninja_gen::RecipeShell; + +/// Names the optional Windows legacy-recipe interpreter override. +pub(super) const WINDOWS_SHELL_ENV: &str = "NETSUKE_WINDOWS_SHELL"; + +/// Resolve the current host's legacy-recipe interpreter selection. +pub(super) fn resolve_recipe_shell() -> Result { + resolve_recipe_shell_with(&mockable::DefaultEnv) +} + +/// Resolve the current host's legacy-recipe interpreter with an injected environment. +pub(super) fn resolve_recipe_shell_with(env: &impl Env) -> Result { + if !cfg!(windows) { + return Ok(RecipeShell::Posix); + } + resolve_windows_recipe_shell(env.os_string(WINDOWS_SHELL_ENV)) +} + +/// Resolve the configured Windows recipe shell from one raw environment value. +fn resolve_windows_recipe_shell(raw_value: Option) -> Result { + let Some(shell_value) = raw_value else { + return Ok(RecipeShell::PowerShell); + }; + let shell_name = shell_value.into_string().map_err(|invalid_value| { + anyhow::anyhow!( + "{WINDOWS_SHELL_ENV} must be valid Unicode, received {}", + invalid_value.to_string_lossy() + ) + })?; + match shell_name.trim().to_ascii_lowercase().as_str() { + "" | "powershell" => Ok(RecipeShell::PowerShell), + "bash" => Ok(RecipeShell::Bash), + _ => bail!( + "{WINDOWS_SHELL_ENV} must be `powershell` or `bash`; \ + omit it to use the Windows PowerShell default" + ), + } +} + +/// Confirm that an explicitly selected external recipe runtime can start. +pub(super) fn validate_recipe_shell(shell: RecipeShell) -> Result<()> { + if !cfg!(windows) || shell != RecipeShell::Bash { + return Ok(()); + } + let status = std::process::Command::new("bash.exe") + .arg("--version") + .status() + .context( + "Windows legacy recipes selected `bash`, but `bash.exe` was not found on PATH; \ + install Git for Windows or MSYS2, add its Bash directory to PATH, or unset \ + NETSUKE_WINDOWS_SHELL to use PowerShell", + )?; + if !status.success() { + bail!( + "Windows legacy recipes selected `bash`, but `bash.exe --version` exited with {status}; \ + repair the Bash runtime or unset NETSUKE_WINDOWS_SHELL to use PowerShell" + ); + } + Ok(()) +} + +#[cfg(test)] +mod tests { + //! Verifies Windows legacy-recipe interpreter selection. + + use super::{WINDOWS_SHELL_ENV, resolve_recipe_shell_with, resolve_windows_recipe_shell}; + use crate::ninja_gen::RecipeShell; + use mockable::MockEnv; + use std::ffi::OsString; + + fn recipe_shell_env(value: Option<&str>) -> MockEnv { + let mut env = MockEnv::new(); + env.expect_os_string() + .times(usize::from(cfg!(windows))) + .withf(|key| key == WINDOWS_SHELL_ENV) + .return_const(value.map(OsString::from)); + env + } + + #[test] + fn defaults_to_the_host_recipe_shell() { + let shell = resolve_recipe_shell_with(&recipe_shell_env(None)) + .expect("default shell resolution should succeed"); + assert_eq!(shell, RecipeShell::host_default()); + } + + #[test] + fn defaults_windows_to_power_shell() { + let shell = resolve_windows_recipe_shell(None) + .expect("Windows default shell resolution should succeed"); + assert_eq!(shell, RecipeShell::PowerShell); + } + + #[test] + fn accepts_the_explicit_bash_compatibility_selection() { + let shell = resolve_windows_recipe_shell(Some(OsString::from("bash"))) + .expect("bash selection should succeed"); + assert_eq!(shell, RecipeShell::Bash); + } + + #[test] + fn rejects_an_unknown_windows_recipe_shell() { + let error = resolve_windows_recipe_shell(Some(OsString::from("cmd"))) + .expect_err("unknown shell selection should fail"); + assert!(error.to_string().contains("powershell` or `bash")); + } +} diff --git a/src/runner/tests.rs b/src/runner/tests.rs index 4689bc607..bc1da1aac 100644 --- a/src/runner/tests.rs +++ b/src/runner/tests.rs @@ -4,7 +4,7 @@ use super::*; use crate::cli::{HelpArgs, HelpTopic}; use crate::ir::{BuildEdge, BuildGraph, DependencyOrder}; use crate::manifest::ManifestLoadStage; -use crate::ninja_gen::NinjaGenError; +use crate::ninja_gen::{NinjaGenError, RecipeShell}; use crate::status::{LocalizationKey, StageNumber, StatusReporter}; use anyhow::{Result, ensure}; use camino::Utf8PathBuf; @@ -84,7 +84,8 @@ fn generation_steps_run_without_reporter() -> anyhow::Result<()> { let manifest = generation::load_manifest(&manifest_path, Some(&mut |stage| stages.push(stage)))?; let graph = generation::build_graph(&manifest)?; - let (ninja_text, _) = generation::ninja_text(&graph)?.into_parts(); + let (ninja_text, _) = + generation::ninja_text_for_shell(&graph, RecipeShell::host_default())?.into_parts(); ensure!( stages == vec![ @@ -187,7 +188,8 @@ fn ninja_text_propagates_typed_generation_errors() { }, ); - let error = generation::ninja_text(&graph).expect_err("missing action should fail generation"); + let error = generation::ninja_text_for_shell(&graph, RecipeShell::host_default()) + .expect_err("missing action should fail generation"); assert!(matches!( error, NinjaGenError::MissingAction { ref id, .. } if id == "missing" @@ -205,7 +207,7 @@ fn runner_reports_the_complete_generation_stage_sequence() -> Result<()> { }; let reporter = StageRecordingReporter::default(); - let generated = generate_ninja(&cli, &reporter, None)?; + let generated = generate_ninja_with_shell(&cli, &reporter, None, RecipeShell::host_default())?; let (ninja_text, _) = generated.into_parts(); let stages = reporter.stages(); diff --git a/tests/data/windows-recipe-smoke.yml b/tests/data/windows-recipe-smoke.yml new file mode 100644 index 000000000..b6f4a95c1 --- /dev/null +++ b/tests/data/windows-recipe-smoke.yml @@ -0,0 +1,40 @@ +netsuke_version: "1.0.0" + +actions: + - name: scalar + description: Prove the scalar recipe uses Windows PowerShell + command: "$edition = $PSVersionTable.PSEdition; if ($env:NETSUKE_SMOKE_VALUE -ne 'value with spaces') { exit 71 }; Set-Content -NoNewline -LiteralPath 'scalar interpreter with spaces.txt' -Value $edition" + - name: ordered-list + description: Preserve PowerShell state through a two-entry command list + command: + - "$env:NETSUKE_ORDER = 'first'; Set-Content -NoNewline -LiteralPath 'ordered state.txt' -Value $env:NETSUKE_ORDER" + - "if ($env:NETSUKE_ORDER -ne 'first') { exit 72 }; Add-Content -NoNewline -LiteralPath 'ordered state.txt' -Value ';second'" + - name: first-list-entry-fails + description: Stop an ordered command list after its first failing entry + command: + - "cmd.exe /d /c exit 27" + - "Set-Content -NoNewline -LiteralPath 'must not exist.txt' -Value second-entry-ran" + - name: script + description: Prove script recipes use Windows PowerShell + script: | + $edition = $PSVersionTable.PSEdition + Set-Content -NoNewline -LiteralPath 'script interpreter.txt' -Value $edition + - name: dependency-first + command: "Set-Content -NoNewline -LiteralPath 'dependency order.txt' -Value first" + - name: dependency-second + command: "if ((Get-Content -Raw -LiteralPath 'dependency order.txt') -ne 'first') { exit 73 }; Add-Content -NoNewline -LiteralPath 'dependency order.txt' -Value ';second'" + - name: aggregate + description: Run direct dependencies in declaration order + command: "if ((Get-Content -Raw -LiteralPath 'dependency order.txt') -ne 'first;second') { exit 74 }; Add-Content -NoNewline -LiteralPath 'dependency order.txt' -Value ';aggregate'" + dependency_order: serial + deps: + - dependency-first + - dependency-second + +targets: + - name: "automatic path with spaces.txt" + description: Discover this target without executing its recipe + command: "Set-Content -NoNewline -LiteralPath {{ outs }} -Value automatic-path-quoting" + - name: "discovery must not execute.txt" + description: Confirm target discovery has no recipe side effects + command: "Set-Content -NoNewline -LiteralPath {{ outs }} -Value recipe-should-not-run" diff --git a/tests/documentation_examples_tests.rs b/tests/documentation_examples_tests.rs index d53adb131..cc9ba703a 100644 --- a/tests/documentation_examples_tests.rs +++ b/tests/documentation_examples_tests.rs @@ -41,6 +41,7 @@ const EXPECTED_EXAMPLE_IDS: &[&str] = &[ "guide-source-install", "guide-utility-commands", "guide-verbose-timing-reporter", + "guide-windows-bash-compatibility", "guide-windows-help", "guide-windows-help-install", "guide-windows-path", diff --git a/tests/ninja_gen_command_list_process_integration_tests.rs b/tests/ninja_gen_command_list_process_integration_tests.rs index 7cefe992d..ec85524b2 100644 --- a/tests/ninja_gen_command_list_process_integration_tests.rs +++ b/tests/ninja_gen_command_list_process_integration_tests.rs @@ -143,8 +143,8 @@ fn command_list_entry_control_flow_cannot_mask_an_earlier_failure( fn command_list_entries_share_one_shell_process( ninja_integration_setup: Option, ) -> Result<()> { - // The backend escapes the shell variable for Ninja, so the shell sees the - // value written by the first entry. + // Final Ninja serialisation escapes this ordinary dollar, so the shell sees + // the value written by the first entry. let Some(run) = run_command_list( ninja_integration_setup, &[ From 4ed2c3fc7d4d1bee11ef8cfe9c676fb4d4274b6d Mon Sep 17 00:00:00 2001 From: leynos Date: Thu, 27 Aug 2026 02:14:33 +0200 Subject: [PATCH 2/5] Strengthen Windows recipe smoke coverage Assert PowerShell dollar-variable handling and Netsuke failure status in the native Windows smoke manifest. Split POSIX recipe validation so the new interpreter selection remains straightforward to maintain. --- scripts/windows-recipe-smoke.ps1 | 8 ++- src/ninja_gen_validation.rs | 92 +++++++++++++++++------------ tests/data/windows-recipe-smoke.yml | 3 + 3 files changed, 63 insertions(+), 40 deletions(-) diff --git a/scripts/windows-recipe-smoke.ps1 b/scripts/windows-recipe-smoke.ps1 index 7b4d8cc1b..7647e21d3 100644 --- a/scripts/windows-recipe-smoke.ps1 +++ b/scripts/windows-recipe-smoke.ps1 @@ -76,13 +76,17 @@ try { Assert-Equal -Actual (Get-Content -Raw -LiteralPath 'scalar interpreter with spaces.txt') ` -Expected 'Desktop' -Message 'A scalar recipe did not run in Windows PowerShell' + Invoke-Netsuke -Arguments @('build', 'dollar-syntax') + Assert-Equal -Actual (Get-Content -Raw -LiteralPath 'dollar value.txt') -Expected 'default value' ` + -Message 'Ninja did not preserve ordinary PowerShell dollar syntax' + Invoke-Netsuke -Arguments @('build', 'ordered-list') Assert-Equal -Actual (Get-Content -Raw -LiteralPath 'ordered state.txt') -Expected 'first;second' ` -Message 'The ordered command list did not preserve state and order' $failure = & $Netsuke build first-list-entry-fails 2>&1 - if ($LASTEXITCODE -eq 0) { - throw 'A failed first command-list entry unexpectedly succeeded through Netsuke and Ninja.' + if ($LASTEXITCODE -ne 1) { + throw "A recipe exit code of 27 should become Netsuke's documented failure exit code 1, got $LASTEXITCODE: $failure" } if (Test-Path -LiteralPath 'must not exist.txt') { throw "The second command-list entry ran after the first failed: $failure" diff --git a/src/ninja_gen_validation.rs b/src/ninja_gen_validation.rs index 725179920..e6d7193ee 100644 --- a/src/ninja_gen_validation.rs +++ b/src/ninja_gen_validation.rs @@ -18,47 +18,63 @@ pub(super) fn validate_action_recipe( { return Err(NinjaGenError::EmptyCommandRecipe { action_index }); } - if shell != RecipeShell::PowerShell - && let Recipe::Command { - command: StringOrList::String(command), - } = &action.recipe - { - escape_ninja_value(&ShellText::new(command.clone()))?; + if shell == RecipeShell::PowerShell { + return Ok(()); } - if shell != RecipeShell::PowerShell - && let Recipe::Command { + match &action.recipe { + Recipe::Command { + command: StringOrList::String(command), + } => validate_scalar_command(command), + Recipe::Command { command: StringOrList::List(entries), - } = &action.recipe - { - for (zero_based_entry_index, entry) in entries.iter().enumerate() { - let entry_index = zero_based_entry_index + 1; - match command_list_entry_error(CommandListEntry(entry)) { - Some(CommandListEntryError::MultipleBackgroundJobs) => { - return Err(NinjaGenError::MultipleBackgroundJobs { - action_index, - entry_index, - }); - } - Some(CommandListEntryError::UnsupportedExec) => { - return Err(NinjaGenError::UnsupportedCommandListExec { - action_index, - entry_index, - }); - } - Some(CommandListEntryError::UnanalyzableEval) => { - return Err(NinjaGenError::UnanalyzableCommandListEval { - action_index, - entry_index, - }); - } - Some(CommandListEntryError::NinjaControlCharacter) => { - return Err(NinjaGenError::NinjaControlCharacter { - action_index, - entry_index, - }); - } - None => {} + } => validate_posix_command_list(entries, action_index), + Recipe::Command { + command: StringOrList::Empty, + } + | Recipe::Script { .. } + | Recipe::Rule { .. } => Ok(()), + } +} + +/// Reject scalar command text that cannot occupy one Ninja command binding. +fn validate_scalar_command(command: &str) -> Result<(), NinjaGenError> { + escape_ninja_value(&ShellText::new(command.into()))?; + Ok(()) +} + +/// Reject POSIX command-list entries that violate the generated wrapper contract. +fn validate_posix_command_list( + entries: &[String], + action_index: usize, +) -> Result<(), NinjaGenError> { + for (zero_based_entry_index, entry) in entries.iter().enumerate() { + let entry_index = zero_based_entry_index + 1; + match command_list_entry_error(CommandListEntry(entry)) { + Some(CommandListEntryError::MultipleBackgroundJobs) => { + return Err(NinjaGenError::MultipleBackgroundJobs { + action_index, + entry_index, + }); + } + Some(CommandListEntryError::UnsupportedExec) => { + return Err(NinjaGenError::UnsupportedCommandListExec { + action_index, + entry_index, + }); + } + Some(CommandListEntryError::UnanalyzableEval) => { + return Err(NinjaGenError::UnanalyzableCommandListEval { + action_index, + entry_index, + }); + } + Some(CommandListEntryError::NinjaControlCharacter) => { + return Err(NinjaGenError::NinjaControlCharacter { + action_index, + entry_index, + }); } + None => {} } } Ok(()) diff --git a/tests/data/windows-recipe-smoke.yml b/tests/data/windows-recipe-smoke.yml index b6f4a95c1..661b25664 100644 --- a/tests/data/windows-recipe-smoke.yml +++ b/tests/data/windows-recipe-smoke.yml @@ -4,6 +4,9 @@ actions: - name: scalar description: Prove the scalar recipe uses Windows PowerShell command: "$edition = $PSVersionTable.PSEdition; if ($env:NETSUKE_SMOKE_VALUE -ne 'value with spaces') { exit 71 }; Set-Content -NoNewline -LiteralPath 'scalar interpreter with spaces.txt' -Value $edition" + - name: dollar-syntax + description: Preserve ordinary PowerShell dollar variables through Ninja + command: "$recipeValue = if ($null -eq $env:NETSUKE_SMOKE_OPTION) { 'default value' } else { $env:NETSUKE_SMOKE_OPTION }; Set-Content -NoNewline -LiteralPath 'dollar value.txt' -Value $recipeValue" - name: ordered-list description: Preserve PowerShell state through a two-entry command list command: From 8debf38470ae796f71ec765ee4976882033ab039 Mon Sep 17 00:00:00 2001 From: leynos Date: Thu, 27 Aug 2026 03:03:45 +0200 Subject: [PATCH 3/5] Test POSIX rendering explicitly Keep host-default Windows PowerShell rendering intact while making POSIX structural tests request RecipeShell::Posix directly. This keeps the compatibility renderer covered on every host. --- src/ninja_gen_property_tests.rs | 28 +++++++++++++++++++++++----- src/ninja_gen_tests.rs | 17 +++++++++++++---- 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/src/ninja_gen_property_tests.rs b/src/ninja_gen_property_tests.rs index e1a755b7f..e3a0f0c56 100644 --- a/src/ninja_gen_property_tests.rs +++ b/src/ninja_gen_property_tests.rs @@ -7,8 +7,8 @@ use proptest::prelude::*; use test_support::ninja_gen::paths_strategy; use super::{ - DisplayEdge, GeneratedDyndep, GeneratedNinja, NinjaGenError, generate, generate_bundle, - test_support::command_action, + DisplayEdge, GeneratedDyndep, GeneratedNinja, NinjaGenError, RecipeShell, generate, + generate_bundle, generate_into_with_shell, test_support::command_action, }; use crate::{ ast::{Recipe, StringOrList}, @@ -68,6 +68,12 @@ fn format_edge(edge: &BuildEdge) -> String { .to_string() } +fn generate_posix(graph: &BuildGraph) -> Result { + let mut ninja = String::new(); + generate_into_with_shell(graph, &mut ninja, RecipeShell::Posix)?; + Ok(ninja) +} + fn build_line(formatted: &str) -> Option<&str> { formatted.lines().next() } @@ -191,7 +197,7 @@ fn command_list_entry_strategy() -> impl Strategy { } fn canonical_shell_single_quote(value: &str) -> String { - format!("'{}'", value.replace('\'', r"'\''").replace('$', "$$")) + format!("'{}'", value.replace('\'', r"'\''")) } proptest! { @@ -232,7 +238,8 @@ proptest! { #[test] fn command_lists_preserve_order_boundaries_and_fail_fast_joins(entries in prop::collection::vec(command_list_entry_strategy(), 1..9)) { - let ninja = generate(&command_list_graph(&entries)).expect("non-empty command list should generate"); + let ninja = generate_posix(&command_list_graph(&entries)) + .expect("non-empty command list should generate"); let command_line = ninja.lines().find(|line| line.starts_with(" command = ")) .expect("generated action should include a command line"); let mut previous = 0; @@ -273,13 +280,24 @@ proptest! { "braced property input must contain a shell braced expansion" ); for candidate in [&command, &braced_command] { - let ninja = generate(&scalar_graph(candidate.clone())) + let ninja = generate_posix(&scalar_graph(candidate.clone())) .expect("scalar command should generate"); let observed = ninja_commands(&ninja)?; prop_assert_eq!(observed.strip_suffix('\n'), Some(candidate.as_str())); } } + #[test] + fn scalar_command_output_retains_the_preexisting_form(command in "echo [a-z]{1,12}") { + let ninja = generate_posix(&scalar_graph(command.clone())) + .expect("scalar command should generate"); + let expected_command_line = format!(" command = {command}\n"); + let retains_scalar_form = ninja.contains(&expected_command_line); + let uses_list_boundary = ninja.contains("_netsuke_background_before=$${!:-}"); + prop_assert!(retains_scalar_form); + prop_assert!(!uses_list_boundary); + } + #[test] fn programmatic_empty_command_recipes_are_rejected( use_empty_list in any::(), diff --git a/src/ninja_gen_tests.rs b/src/ninja_gen_tests.rs index 4ecce21b6..2a8699936 100644 --- a/src/ninja_gen_tests.rs +++ b/src/ninja_gen_tests.rs @@ -1,7 +1,9 @@ //! Unit tests for Ninja file generation and rule synthesis. use super::test_support::command_action; -use super::{NamedAction, NinjaGenError, RecipeShell, generate, generate_into}; +use super::{ + NamedAction, NinjaGenError, RecipeShell, generate, generate_into, generate_into_with_shell, +}; use crate::ast::{Recipe, StringOrList}; use crate::ir::{Action, BuildEdge, BuildGraph, DependencyOrder}; use anyhow::{Context, Result, ensure}; @@ -37,7 +39,7 @@ fn generate_simple_ninja() -> Result<()> { graph.targets.insert(Utf8PathBuf::from("out"), edge); graph.default_targets.push(Utf8PathBuf::from("out")); - let ninja = generate(&graph)?; + let ninja = generate_posix(&graph)?; let expected = concat!( "rule a\n", " command = echo hi\n\n", @@ -125,7 +127,7 @@ fn generate_script_ninja_round_trips() -> Result<()> { graph.actions.insert("a".into(), action); graph.targets.insert(Utf8PathBuf::from("out"), edge); - let ninja = generate(&graph)?; + let ninja = generate_posix(&graph)?; ensure!(ninja.contains("rule a")); ensure!(ninja.contains("command = /bin/sh -e -c")); ensure!(ninja.contains("echo '\"'\"'a b'\"'\"'")); @@ -158,7 +160,7 @@ fn generate_command_list_ninja_joins_a_fail_fast_chain() -> Result<()> { graph.actions.insert("a".into(), action); graph.targets.insert(Utf8PathBuf::from("out"), edge); - let ninja = generate(&graph)?; + let ninja = generate_posix(&graph)?; ensure!( ninja.contains("command = { _netsuke_background_before=$${!:-};"), "first list boundary should start the generated command:\n{ninja}" @@ -359,3 +361,10 @@ fn decode_power_shell_script(encoded: &str) -> Result { .collect::>>()?; String::from_utf16(&units).context("decode PowerShell UTF-16 payload") } + +/// Generate Ninja text using the explicit POSIX compatibility renderer. +fn generate_posix(graph: &BuildGraph) -> Result { + let mut ninja = String::new(); + generate_into_with_shell(graph, &mut ninja, RecipeShell::Posix)?; + Ok(ninja) +} From 3dd5485a48de357fee6a895b6d4b02efab71564b Mon Sep 17 00:00:00 2001 From: leynos Date: Thu, 27 Aug 2026 03:48:31 +0200 Subject: [PATCH 4/5] Align POSIX property test with escape boundary Preserve the explicit POSIX renderer assertion after the backend-dollar escaping rebase, and keep the property module within Whitaker's size limit. --- src/ninja_gen_property_tests.rs | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/src/ninja_gen_property_tests.rs b/src/ninja_gen_property_tests.rs index e3a0f0c56..b9c227d08 100644 --- a/src/ninja_gen_property_tests.rs +++ b/src/ninja_gen_property_tests.rs @@ -73,11 +73,9 @@ fn generate_posix(graph: &BuildGraph) -> Result { generate_into_with_shell(graph, &mut ninja, RecipeShell::Posix)?; Ok(ninja) } - fn build_line(formatted: &str) -> Option<&str> { formatted.lines().next() } - fn dependency_side(line: &str) -> Option<&str> { line.split_once(": ").map(|(_, deps)| deps) } @@ -238,8 +236,7 @@ proptest! { #[test] fn command_lists_preserve_order_boundaries_and_fail_fast_joins(entries in prop::collection::vec(command_list_entry_strategy(), 1..9)) { - let ninja = generate_posix(&command_list_graph(&entries)) - .expect("non-empty command list should generate"); + let ninja = generate_posix(&command_list_graph(&entries)).expect("command list should generate"); let command_line = ninja.lines().find(|line| line.starts_with(" command = ")) .expect("generated action should include a command line"); let mut previous = 0; @@ -280,8 +277,7 @@ proptest! { "braced property input must contain a shell braced expansion" ); for candidate in [&command, &braced_command] { - let ninja = generate_posix(&scalar_graph(candidate.clone())) - .expect("scalar command should generate"); + let ninja = generate_posix(&scalar_graph(candidate.clone())).expect("scalar command should generate"); let observed = ninja_commands(&ninja)?; prop_assert_eq!(observed.strip_suffix('\n'), Some(candidate.as_str())); } @@ -289,15 +285,13 @@ proptest! { #[test] fn scalar_command_output_retains_the_preexisting_form(command in "echo [a-z]{1,12}") { - let ninja = generate_posix(&scalar_graph(command.clone())) - .expect("scalar command should generate"); + let ninja = generate_posix(&scalar_graph(command.clone())).expect("scalar command should generate"); let expected_command_line = format!(" command = {command}\n"); let retains_scalar_form = ninja.contains(&expected_command_line); let uses_list_boundary = ninja.contains("_netsuke_background_before=$${!:-}"); prop_assert!(retains_scalar_form); prop_assert!(!uses_list_boundary); } - #[test] fn programmatic_empty_command_recipes_are_rejected( use_empty_list in any::(), @@ -311,7 +305,6 @@ proptest! { let is_stable_empty_recipe_error = matches!(error, NinjaGenError::EmptyCommandRecipe { action_index: 1 }); prop_assert!(is_stable_empty_recipe_error); } - #[test] fn short_serial_lists_need_no_staging(dependencies in paths_strategy("dep", 0..2)) { let bundle = generate_serial_bundle(dependencies.clone())?; @@ -321,7 +314,6 @@ proptest! { prop_assert!(bundle.build_file().contains(dependency.as_str())); } } - #[test] fn staged_serial_lists_preserve_declaration_order(dependencies in paths_strategy("dep", 2..6)) { let bundle = generate_serial_bundle(dependencies.clone())?; From b9e969da9ec6d0b903cc2be87163ea8030bf9508 Mon Sep 17 00:00:00 2001 From: leynos Date: Thu, 27 Aug 2026 04:05:56 +0200 Subject: [PATCH 5/5] Fix merged Windows documentation spacing Remove duplicate blank lines introduced while replaying the Windows recipe documentation onto the refreshed parent branch. --- docs/users-guide.md | 1 - docs/v0-1-0-migration-guide.md | 1 - 2 files changed, 2 deletions(-) diff --git a/docs/users-guide.md b/docs/users-guide.md index 5402f478b..2faea4ad2 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -319,7 +319,6 @@ same Jinja context, including `{{ ins }}` and `{{ outs }}`; those two placeholders are resolved later to the concrete target's shell-quoted input and output paths. An empty command list is rejected when the manifest is parsed. - ### Windows legacy recipe contract On Windows, v0.1.x interprets every legacy `command` string, `command` list, diff --git a/docs/v0-1-0-migration-guide.md b/docs/v0-1-0-migration-guide.md index 775221050..7c4799216 100644 --- a/docs/v0-1-0-migration-guide.md +++ b/docs/v0-1-0-migration-guide.md @@ -63,7 +63,6 @@ non-empty YAML list. The entries run in one shell process and stop at the first non-zero exit. See [Rules and recipes](users-guide.md#rules-and-recipes) for the syntax, shell semantics, and examples. - ## Windows legacy recipe interpreter v0.1.x makes Windows legacy-recipe execution explicit. Netsuke starts