diff --git a/.gitignore b/.gitignore index f6e3d5c4b..e6c99d614 100644 --- a/.gitignore +++ b/.gitignore @@ -4,6 +4,8 @@ target/ *.swp .crush/ .claude/ +.vtcode/ +vtcode.toml .memdb/ .grepai/ build.ninja @@ -17,5 +19,4 @@ __pycache__/ .pytest_cache/ .typos-oxendict-base.json .typos-oxendict-base.toml -.vtcode/ *.swo diff --git a/CHANGELOG.md b/CHANGELOG.md index bd95570b1..228b2fbe8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,13 @@ reproduces the existing behaviour, so `run_ninja` and `run_ninja_tool` keep their signatures and no embedder needs to change ([#490](https://github.com/leynos/netsuke/issues/490)) +- Accept a non-empty ordered list of commands for a rule or target `command` + recipe, executed as a single fail-fast `&&` shell chain, so the build stops + at the first non-zero exit; an empty command list is rejected at parse time, + and entries with multiple background jobs or unsupported `exec` structures + are rejected during Ninja generation as `MultipleBackgroundJobs` or + `UnsupportedCommandListExec` + ([#550](https://github.com/leynos/netsuke/issues/550)) ### Changed diff --git a/Cargo.lock b/Cargo.lock index a4e2378d0..3f9249bb2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1465,6 +1465,12 @@ dependencies = [ "syn 1.0.109", ] +[[package]] +name = "monotony" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c07971c6281a9a50979e8426fa6c6c618a42b729180847cb4363a794fdc4e607" + [[package]] name = "netsuke-build" version = "0.1.0-beta1" @@ -1493,6 +1499,7 @@ dependencies = [ "minijinja", "mockable", "mockall", + "monotony", "ortho_config", "predicates 3.1.3", "proptest", diff --git a/Cargo.toml b/Cargo.toml index 2ba91d200..f327091b3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -117,6 +117,7 @@ glob = "0.3.3" hashbrown = "0.17.1" walkdir = "2.5" metrics = "0.24.6" +monotony = "0.1.0" mockable = "3.0" tracing = "0.1" tracing-subscriber = { version = "0.3", features = ["fmt"] } @@ -153,6 +154,7 @@ predicates = "3" # global recorder; constrained to the family that pairs with metrics 0.24. metrics-util = { version = "0.20", features = ["debugging"] } mockable = { version = "3.0", features = ["mock"] } +monotony = { version = "0.1.0", features = ["test-util"] } serial_test = "3" mockall = "0.11" camino = "1.2.0" diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 1add5ea61..2da36302e 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -196,6 +196,84 @@ they are per-invocation arguments tagged `#[serde(skip)]` on would silently change the artefact destination — a footgun the design avoids by construction. +## Command and recipe lowering + +Command recipes use the `StringOrList` AST type. A scalar command remains one +shell-text value; a YAML sequence is an ordered list of entries. The same +recipe path handles commands declared on reusable rules, direct targets, and +actions. Manifest deserialization rejects an empty command list. Code that +constructs the IR directly must also reject both `StringOrList::Empty` and an +empty `StringOrList::List(Vec::new())` during Ninja generation rather than +emitting an unusable rule. + +The lowering stages have deliberately separate responsibilities: + +- `src/manifest/render.rs` renders a scalar or each list entry independently. + Every entry sees the same cloned recipe context, including target variables + and delayed `ins`/`outs` markers. A rendering error for a list includes its + one-based entry position. +- `src/ir/from_manifest_support.rs` prepares one shell-quoted input/output + binding set for the recipe, then interpolates every scalar or list entry with + that set. `{{ ins }}` and `{{ outs }}` markers and standalone `$in` and + `$out` tokens are resolved per entry; tokens inside backticks are preserved. + The resulting action contains ordinary command text and no Ninja + placeholders. +- `src/ninja_gen.rs` emits a scalar command unchanged. 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/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 + hashed action fingerprint and one-based entry index to the Ninja failure + error. + +Failure attribution is private to Ninja process execution: +`FailureAttributionWriter` parses only Ninja's stderr. Because Ninja relays a +failed subcommand's stderr on its own stdout, build runs retain only a fixed +512-byte stdout tail and use its parsed marker only after a non-zero exit. +Ordinary child stdout streams forward directly and must not use this tail. + +The lowest-layer POSIX shell-word quoting used for input/output paths during IR +lowering is `shell_quote::QuoteRefExt::quoted(Sh)`. It performs minimal, +fragmented shell quoting, which is appropriate for a literal shell word but not +for the command-list `eval` payload. That renderer requires a canonical +single-quoted payload so existing generated Ninja list text remains +byte-for-byte stable, and the delimiter/boundary tests continue to hold. Keep +that quoting in the deliberately local `shell_single_quote` function; it is +not a general-purpose helper. Neither quoting path is the platform-specific +`src/stdlib/command/quote.rs` implementation behind the `command.quote` +template wrapper, which must retain its `cmd.exe` quoting behaviour on Windows. + +Attributed list failures emit the bounded tracing fields +`command_list_action` (a fixed-width action fingerprint) and +`command_list_entry` (the one-based entry index), plus the matching +`command_list_failure` marker. The process boundary records +`netsuke_ninja_command_list_failures_total` and +`netsuke_ninja_command_list_failure_duration_seconds`, with an `outcome` +label of `failure`. Elapsed failure duration is measured through the injected +`monotony::MonotonicClock`; production uses `StdMonotonicClock`, while tests use +deterministic test clocks. These diagnostics and metrics contain no command +text. + +Changes to this pipeline must preserve the scalar/list distinction, per-entry +rendering, current-shell state sharing, and failure attribution. The focused +rendering, lowering, Ninja-generation, and real-Ninja integration tests are +the behavioural contract for these boundaries. + ## Package and target naming The crates.io package is `netsuke-build`; the library target, the binary diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index 08161b06e..2b3c0c2fa 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -223,7 +223,7 @@ erDiagram bool always } RECIPE { - string command + StringOrList command string script StringOrList rule } @@ -245,18 +245,28 @@ Each entry in the `rules` list is a mapping that defines a reusable action. - `name`: A unique string identifier for the rule. -- `command`: A single command string to be executed. It may include the - placeholders `{{ ins }}` and `{{ outs }}` to represent input and output - files. Netsuke expands these placeholders to space-separated lists of file - paths quoted for POSIX `/bin/sh` using the +- `command`: A command string, or a non-empty ordered list of command strings, + to be executed. `StringOrList` is also used for direct target and action + commands, so the rule and target forms have the same scalar/list semantics. + Each entry may include the placeholders `{{ ins }}` and `{{ outs }}`. Jinja + renders a scalar or each list entry separately with the same recipe context; + the placeholders are delayed until IR lowering, then replaced in every entry + with space-separated, POSIX-shell-quoted input and output paths using the [`shell-quote`](https://docs.rs/shell-quote/latest/shell_quote/) crate (Sh - mode) before hashing the action. The IR stores the fully expanded command; - Ninja executes this text verbatim. After interpolation, the command must be - parsable by [shlex](https://docs.rs/shlex/latest/shlex/) (POSIX mode). - Automatic shell escaping applies only where the schema has enough structure - to identify argument boundaries. Plain command strings remain shell text; - authors should use structured recipes or explicit quoting helpers for - arbitrary variables. + mode) before hashing the action. Standalone `$in` and `$out` tokens are + resolved at the same boundary, while tokens inside backticks are preserved. + A scalar command is emitted unchanged. A list is lowered to brace groups + that evaluate each entry through a shell-quoted `eval` payload and are joined + by `&&`. The groups run in declaration order in one shell process and stop + at the first non-zero exit, so working directory, environment, and shell + variables carry forward. The `eval` boundary keeps an entry's inline + comments or trailing control operators from consuming the generated group + terminator. A failed entry emits a bounded action/entry marker for the + runner to include in the failure diagnostic. The resulting command must be + parsable by [shlex](https://docs.rs/shlex/latest/shlex/) (POSIX mode). An + empty command list is rejected during manifest deserialization. Plain command + strings remain shell text; authors should use structured recipes or explicit + quoting helpers for arbitrary variables. - `script`: A multi-line script declared with the YAML `|` block style. The entire block is passed to an interpreter. If the first line begins with `#!` @@ -326,7 +336,10 @@ rule: - clean-up ``` -- `command`: A single command string to run directly for this target. +- `command`: A command string or non-empty ordered list of command strings to + run directly for this target. Direct target lists follow the same per-entry + Jinja rendering, delayed `ins`/`outs` interpolation, and shell lowering as + rule lists. - `script`: A multi-line script passed to the interpreter. When present, it is defined using the YAML `|` block style. @@ -711,7 +724,7 @@ pub struct Rule { /// A union of execution styles for both rules and targets. #[serde(untagged)] pub enum Recipe { - Command { command: String }, + Command { command: StringOrList }, Script { script: String }, Rule { rule: StringOrList }, // FUTURE: planned Recipe::Exec extension; not present in src/ast.rs yet. @@ -787,9 +800,11 @@ pub enum StringOrList { } ``` -*Note: The* `StringOrList` *enum with* `#[serde(untagged)]` *provides the -flexibility for users to specify single sources, dependencies, and rule names -as a simple string and multiple as a list, enhancing user-friendliness.* +*Note: The* `StringOrList` *enum with* `#[serde(untagged)]` *preserves whether +the manifest supplied one string or an ordered list. The same type represents +command recipes, sources, dependencies, order-only dependencies, and rule +selectors; command lists are executed in order, while path-like fields are +interpreted only at the manifest-to-IR boundary.* `StringOrList` owns the conversions that only need to know its own shape: `map_each` applies a function to every contained string, and `to_string_vec` @@ -1956,17 +1971,19 @@ This transformation involves several steps: Current behaviour: For each expanded target, resolve the referenced rule template, merge - rule-level and target-level execution metadata, interpolate its command with - the target's input and output paths, and register the resulting `ir::Action` - in the `actions` map. Actions are hashed on the fully resolved recipe and - file set, so identical rule templates yield distinct actions when their - paths differ. Create a corresponding `ir::BuildEdge` linking the target to - the action identifier and transfer the `phony` and `always` flags. `sources` - are lowered into the edge's explicit input list so recipe interpolation and - Ninja `$in` see only material inputs. `deps` are lowered into a separate - `implicit_deps` list, which maps to Ninja's implicit dependency syntax (`|`) - so Ninja orders and rebuilds them without exposing them as recipe arguments; - `order_only_deps` remains separate and maps to Ninja's `||` class. + rule-level and target-level execution metadata, and interpolate every + command entry with the target's input and output paths. Direct target and + action commands use the same path. Register the resulting scalar or ordered + `StringOrList` recipe in the `ir::Action` map. Actions are hashed on the + fully resolved recipe and file set, so identical rule templates yield + distinct actions when their paths differ. Create a corresponding + `ir::BuildEdge` linking the target to the action identifier and transfer the + `phony` and `always` flags. `sources` are lowered into the edge's explicit + input list so recipe interpolation and Ninja `$in` see only material inputs. + `deps` are lowered into a separate `implicit_deps` list, which maps to Ninja's + implicit dependency syntax (`|`) so Ninja orders and rebuilds them without + exposing them as recipe arguments; `order_only_deps` remains separate and + maps to Ninja's `||` class. FUTURE: @@ -2010,9 +2027,12 @@ structures to the Ninja file syntax. be written at the top of the file (e.g., `msvc_deps_prefix` for Windows 2. **Write Rules:** Iterate through the `graph.actions` map. For each - `ir::Action`, write a corresponding Ninja `rule` statement. The input and - output lists stored in the action replace the `ins` and `outs` placeholders. - These lists are then rewritten as Ninja's `$in` and `$out`. + `ir::Action`, write a corresponding Ninja `rule` statement. The IR already + contains ordinary command text: its input and output paths have replaced + Netsuke's `ins`/`outs` and `$in`/`$out` placeholders during lowering. Scalar + commands are emitted as-is. List commands are emitted as the brace-group, + `eval`, and `&&` chain described in §2.3, including the bounded failure + marker for each one-based entry. When an action's `recipe` is a script, the generated rule wraps the script in an invocation of `/bin/sh -e -c` so that multi-line scripts execute @@ -2184,33 +2204,21 @@ catastrophic consequences. For this critical task, the recommended crate is `shell-quote`. While other crates like `shlex` exist, `shell-quote` offers a more robust and -flexible API specifically designed for this purpose.[^22] It supports quoting -for multiple shell flavours (e.g., Bash, sh, Fish), which is vital for a -cross-platform build tool. It also correctly handles a wide variety of input -types, including byte strings and OS-native strings, which is essential for -dealing with non-UTF8 file paths. The - -`QuoteExt` trait provided by the crate offers an ergonomic and safe method for -building command strings by pushing quoted components into a buffer: -`script.push_quoted(Bash, "foo bar")`. +flexible API specifically designed for this purpose.[^22] The current lowering +path uses its `QuoteRefExt::quoted` method with `Sh` mode, producing +POSIX-compatible quoted path arguments before the command is hashed. `shlex` +remains a validation parser; it does not perform the quoting. ### 6.3 Implementation Strategy -The command generation logic within the `ninja_gen.rs` module must not use -simple string formatting (like `format!`) to construct the final command -strings. Instead, parse the Netsuke command template (e.g., -`{{ cc }} -c {{ ins }} -o` `{{ outs }}`) and build the final command string -step by step. The placeholders `{{ ins }}` and `{{ outs }}` are expanded to -space-separated lists of file paths within Netsuke itself, each path being -shell-escaped using the `shell-quote` API. Netsuke uses the `Sh` quoting mode -to emit POSIX-compliant single-quoted strings and scans the template for -standalone `$in` and `$out` tokens to avoid rewriting unrelated variables. -Substitution happens during IR generation and the fully expanded command is -emitted to `build.ninja` unchanged. After substitution, the command is -validated with \[`shlex`\]() to ensure it -parses correctly. This approach guarantees that every dynamic part of the -command is securely quoted, albeit at the cost of deduplicating only actions -with identical file sets. +The command interpolation logic in `src/ir/cmd_interpolate.rs` prepares one +quoted input/output binding set per recipe and applies it to each scalar or +list entry. It replaces the delayed `{{ ins }}`/`{{ outs }}` markers and +standalone `$in`/`$out` tokens outside backticks, preserving longer identifiers +and backtick-delimited text. Unbalanced backticks or text that `shlex` cannot +parse produce an IR error before an action is hashed. Ninja generation then +receives fully expanded command text and is responsible only for preserving the +scalar form or constructing the list-entry shell boundaries. ### 6.4 Automatic Security as a "Friendliness" Feature @@ -2220,10 +2228,11 @@ user to trivial security vulnerabilities is fundamentally unfriendly. In many build systems, the burden of correct shell quoting falls on the user, an error-prone task that requires specialized knowledge. -Netsuke's design elevates security to a core feature by making it automatic and -transparent. The user writes a simple, unquoted command template, and Netsuke -performs the complex and critical task of making it secure behind the scenes. -By integrating `shell-quote` directly into the Ninja file synthesis stage, +Netsuke's design makes identified path substitution safe by default. Netsuke +quotes the `ins`/`outs` path values before action hashing and Ninja synthesis; +arbitrary Jinja values and handwritten shell fragments remain the manifest +author's responsibility. By integrating `shell-quote` into IR command +lowering, before action hashing and Ninja file synthesis, Netsuke protects users from a common and dangerous class of errors by default. This approach embodies a deeper form of user-friendliness: one that anticipates and mitigates risks on the user's behalf. diff --git a/docs/users-guide.md b/docs/users-guide.md index fe10f83ad..49c15d959 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -290,12 +290,71 @@ offending key. A rule or target must provide exactly one recipe: -- `command`: one shell command. +- `command`: one shell command, or an ordered list of commands. - `script`: a multi-line POSIX shell script. - `rule`: the name of another rule to use. Rules may also provide `description`, text used for Ninja's progress display. +A `command` list runs its entries in declaration order and stops at the first +non-zero exit, so entries share the fail-fast behaviour of a handwritten +`&&` chain. The command field is a `StringOrList`: a scalar remains one shell +command, while a YAML sequence is rendered and lowered one entry at a time. +This applies equally to rules, direct targets, and actions. Each entry sees the +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 +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, environment assignment, or shell variable can therefore be used by +later entries. A failed entry stops the chain, and the diagnostic identifies +the generated action and one-based list-entry positions, for example +`netsuke command-list failure: action HASH, entry 2`. + + + +```yaml +netsuke_version: "1.0.0" + +rules: + - name: comprehensive-check + description: Run the required checks sequentially + command: + - echo "check-fmt" + - echo "lint" + - echo "test" + +targets: + - name: done + rule: comprehensive-check +``` + +The same list form can be attached directly to a target. Jinja rendering and +`{{ outs }}` interpolation apply independently to each entry: + + + +```yaml +netsuke_version: "1.0.0" + +targets: + - name: report.txt + vars: + heading: Report + command: + - "printf '{{ heading }}\\n' > {{ outs }}" + - "printf 'complete\\n' >> {{ outs }}" +``` + +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-beta1 `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. @@ -1059,6 +1118,23 @@ Netsuke reduces some common quoting mistakes, but it is not a sandbox: may retain the original input so invalid patterns can be explained. - `raw` template output and handwritten shell fragments remain the manifest author's responsibility. +- Each `command` list entry is joined into a single shell chain; a later + entry inherits the working directory, environment, and shell variables + left by an earlier entry, and runs only when that earlier entry exits with + status zero. A failed entry may still leave side effects behind before it + halts the chain. The generated brace/eval boundary keeps comments and + trailing control operators inside an entry from changing the chain's + structure. An entry may start at most one background job; Netsuke waits for + that job before moving to a later entry, and rejects an entry that starts + more than one background job during Ninja generation. It also rejects an + entry 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 supervised so + its success or failure retains the list's status semantics: a successful + `exec` ends the remaining chain, while 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. - Literal shell dollar expressions currently require Ninja-aware escaping, such as `$$PATH`. diff --git a/docs/v0-1-0-migration-guide.md b/docs/v0-1-0-migration-guide.md index bc8865330..e4675700e 100644 --- a/docs/v0-1-0-migration-guide.md +++ b/docs/v0-1-0-migration-guide.md @@ -24,6 +24,7 @@ Table: v0.1.0 child-environment API additions and their impact | Child environment | New opt-in `netsuke::runner::CommandEnv` carries additive variable overrides and an injected `PATH` for Ninja child processes. | [Users' guide](users-guide.md) | | Request types | New `netsuke::runner::NinjaBuildRequest` and `netsuke::runner::NinjaToolRequest` name the program, build file, and targets or tool for the `*_with` run functions. | [Users' guide](users-guide.md) | | 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) | ## Nothing to change for existing callers @@ -31,6 +32,14 @@ The convenience wrappers keep their signatures and their behaviour: the child inherits the calling process's environment, and Ninja is resolved exactly as before. No caller needs to change to adopt this release. +## Opting into ordered command lists + +Existing scalar `command` recipes remain valid, so no migration is required. +To run a short sequence of commands in declaration order, change a recipe to a +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. + ## Opting into an explicit child environment Construct a `CommandEnv`, name the variables to add, and pass it through diff --git a/locales/ar/messages.ftl b/locales/ar/messages.ftl index 3a45931e0..47bbeafa0 100644 --- a/locales/ar/messages.ftl +++ b/locales/ar/messages.ftl @@ -149,6 +149,7 @@ manifest.glob.invalid_pattern = نمط glob غير صالح «{ $pattern }»: { manifest.glob.unknown_pattern_error = خطأ نمط غير معروف. manifest.glob.io_failed = فشل glob للنمط «{ $pattern }»: { $detail }. manifest.glob.unknown_io_error = خطأ إدخال/إخراج غير معروف. +manifest.command_list_empty = يجب ألّا تكون قائمة الأوامر فارغة؛ قدِّم سلسلة أمر أو قائمة غير فارغة. # أخطاء التمثيل الوسيط. ir.rule_not_found = تعذّر العثور على القاعدة «{ $rule }» التي يشير إليها الهدف «{ $target }». diff --git a/locales/cs/messages.ftl b/locales/cs/messages.ftl index c16798e8c..4e852fdac 100644 --- a/locales/cs/messages.ftl +++ b/locales/cs/messages.ftl @@ -149,6 +149,7 @@ manifest.glob.invalid_pattern = Neplatný vzor glob „{ $pattern }“: { $detai manifest.glob.unknown_pattern_error = neznámá chyba vzoru. manifest.glob.io_failed = Glob selhal pro „{ $pattern }“: { $detail }. manifest.glob.unknown_io_error = neznámá vstupně-výstupní chyba. +manifest.command_list_empty = Pole „command“ nesmí být prázdné: zadejte řetězec s příkazem nebo neprázdný seznam. # Chyby mezikódu. ir.rule_not_found = Pravidlo „{ $rule }“, na které odkazuje cíl „{ $target }“, nebylo nalezeno. diff --git a/locales/cy/messages.ftl b/locales/cy/messages.ftl index 8f2ba112e..5cd1c875c 100644 --- a/locales/cy/messages.ftl +++ b/locales/cy/messages.ftl @@ -149,6 +149,7 @@ manifest.glob.invalid_pattern = Patrwm glob annilys ‘{ $pattern }’: { $detai manifest.glob.unknown_pattern_error = gwall patrwm anhysbys. manifest.glob.io_failed = Methodd glob ar gyfer ‘{ $pattern }’: { $detail }. manifest.glob.unknown_io_error = gwall mewnbwn/allbwn anhysbys. +manifest.command_list_empty = Rhaid i’r maes ‘command’ beidio â bod yn wag: rhowch linyn gorchymyn neu restr nad yw’n wag. # Gwallau'r cynrychioliad canolradd. ir.rule_not_found = Ni chafwyd hyd i'r rheol ‘{ $rule }’ y cyfeirir ati gan y targed ‘{ $target }’. diff --git a/locales/da/messages.ftl b/locales/da/messages.ftl index 3a672d7c2..610a479ce 100644 --- a/locales/da/messages.ftl +++ b/locales/da/messages.ftl @@ -149,6 +149,7 @@ manifest.glob.invalid_pattern = Ugyldigt glob-mønster "{ $pattern }": { $detail manifest.glob.unknown_pattern_error = ukendt mønsterfejl. manifest.glob.io_failed = Glob mislykkedes for "{ $pattern }": { $detail }. manifest.glob.unknown_io_error = ukendt I/O-fejl. +manifest.command_list_empty = Feltet "command" må ikke være tomt: angiv en kommandostreng eller en ikke-tom liste. # Fejl i den interne repræsentation. ir.rule_not_found = Reglen "{ $rule }", som målet "{ $target }" henviser til, blev ikke fundet. diff --git a/locales/de/messages.ftl b/locales/de/messages.ftl index cd9fcf049..0314f12e1 100644 --- a/locales/de/messages.ftl +++ b/locales/de/messages.ftl @@ -149,6 +149,7 @@ manifest.glob.invalid_pattern = Ungültiges Glob-Muster „{ $pattern }“: { $d manifest.glob.unknown_pattern_error = unbekannter Musterfehler. manifest.glob.io_failed = Glob für „{ $pattern }“ fehlgeschlagen: { $detail }. manifest.glob.unknown_io_error = unbekannter E/A-Fehler. +manifest.command_list_empty = Das Feld „command“ darf nicht leer sein: Geben Sie eine Befehlszeichenkette oder eine nicht leere Liste an. # Fehler der Zwischendarstellung. ir.rule_not_found = Die vom Ziel „{ $target }“ referenzierte Regel „{ $rule }“ wurde nicht gefunden. diff --git a/locales/el/messages.ftl b/locales/el/messages.ftl index 2fb9cd0bf..f6413b904 100644 --- a/locales/el/messages.ftl +++ b/locales/el/messages.ftl @@ -150,6 +150,7 @@ manifest.glob.invalid_pattern = Μη έγκυρο μοτίβο glob «{ $pattern manifest.glob.unknown_pattern_error = άγνωστο σφάλμα μοτίβου. manifest.glob.io_failed = Το glob απέτυχε για «{ $pattern }»: { $detail }. manifest.glob.unknown_io_error = άγνωστο σφάλμα εισόδου/εξόδου. +manifest.command_list_empty = Το πεδίο «command» δεν πρέπει να είναι κενό: δώστε μια συμβολοσειρά εντολής ή μια μη κενή λίστα. # Σφάλματα της ενδιάμεσης αναπαράστασης. ir.rule_not_found = Ο κανόνας «{ $rule }» στον οποίο παραπέμπει ο στόχος «{ $target }» δεν βρέθηκε. diff --git a/locales/en-GB/messages.ftl b/locales/en-GB/messages.ftl index 0b6b23116..279abc6ee 100644 --- a/locales/en-GB/messages.ftl +++ b/locales/en-GB/messages.ftl @@ -149,6 +149,7 @@ manifest.glob.invalid_pattern = Invalid glob pattern '{ $pattern }': { $detail } manifest.glob.unknown_pattern_error = unknown pattern error. manifest.glob.io_failed = Glob failed for '{ $pattern }': { $detail }. manifest.glob.unknown_io_error = unknown I/O error. +manifest.command_list_empty = The 'command' field must not be empty: provide a command string or a non-empty list. # IR errors. ir.rule_not_found = Rule '{ $rule }' referenced by target '{ $target }' was not found. diff --git a/locales/en-US/messages.ftl b/locales/en-US/messages.ftl index 3066a7331..add74180e 100644 --- a/locales/en-US/messages.ftl +++ b/locales/en-US/messages.ftl @@ -149,6 +149,7 @@ manifest.glob.invalid_pattern = Invalid glob pattern '{ $pattern }': { $detail } manifest.glob.unknown_pattern_error = unknown pattern error. manifest.glob.io_failed = Glob failed for '{ $pattern }': { $detail }. manifest.glob.unknown_io_error = unknown IO error. +manifest.command_list_empty = The 'command' field must not be empty: provide a command string or a non-empty list. # IR errors. ir.rule_not_found = Rule '{ $rule }' referenced by target '{ $target }' was not found. diff --git a/locales/es-419/messages.ftl b/locales/es-419/messages.ftl index 6c49acff1..ea92ca583 100644 --- a/locales/es-419/messages.ftl +++ b/locales/es-419/messages.ftl @@ -150,6 +150,7 @@ manifest.glob.invalid_pattern = Patrón glob no válido '{ $pattern }': { $detai manifest.glob.unknown_pattern_error = error de patrón desconocido. manifest.glob.io_failed = El glob falló para '{ $pattern }': { $detail }. manifest.glob.unknown_io_error = error de E/S desconocido. +manifest.command_list_empty = El campo 'command' no debe estar vacío: proporcione una cadena de comando o una lista no vacía. # Errores de la representación intermedia. ir.rule_not_found = No se encontró la regla '{ $rule }' referenciada por el objetivo '{ $target }'. diff --git a/locales/es-ES/messages.ftl b/locales/es-ES/messages.ftl index 8f9f4a018..685d5ad58 100644 --- a/locales/es-ES/messages.ftl +++ b/locales/es-ES/messages.ftl @@ -149,6 +149,7 @@ manifest.glob.invalid_pattern = Patrón glob inválido '{ $pattern }': { $detail manifest.glob.unknown_pattern_error = error de patrón desconocido. manifest.glob.io_failed = Falló el glob para '{ $pattern }': { $detail }. manifest.glob.unknown_io_error = error de E/S desconocido. +manifest.command_list_empty = El campo 'command' no debe estar vacío: proporcione una cadena de comando o una lista no vacía. # Errores de IR. ir.rule_not_found = No se encontró la regla '{ $rule }' referenciada por el objetivo '{ $target }'. diff --git a/locales/fa/messages.ftl b/locales/fa/messages.ftl index 2c2022719..b393f4a05 100644 --- a/locales/fa/messages.ftl +++ b/locales/fa/messages.ftl @@ -149,6 +149,7 @@ manifest.glob.invalid_pattern = الگوی glob نامعتبر «{ $pattern }»: manifest.glob.unknown_pattern_error = خطای الگوی ناشناخته. manifest.glob.io_failed = ‏glob برای «{ $pattern }» ناکام ماند: { $detail }. manifest.glob.unknown_io_error = خطای ورودی/خروجی ناشناخته. +manifest.command_list_empty = فیلد «command» نباید خالی باشد: یک رشتهٔ فرمان یا فهرستی ناتهی ارائه دهید. # خطاهای بازنمایی میانی. ir.rule_not_found = قاعدهٔ «{ $rule }» که هدف «{ $target }» به آن ارجاع می‌دهد یافت نشد. diff --git a/locales/fi/messages.ftl b/locales/fi/messages.ftl index e2a95e57b..5867e496f 100644 --- a/locales/fi/messages.ftl +++ b/locales/fi/messages.ftl @@ -149,6 +149,7 @@ manifest.glob.invalid_pattern = Virheellinen glob-hahmo ”{ $pattern }”: { $d manifest.glob.unknown_pattern_error = tuntematon hahmovirhe. manifest.glob.io_failed = Glob epäonnistui hahmolle ”{ $pattern }”: { $detail }. manifest.glob.unknown_io_error = tuntematon siirräntävirhe. +manifest.command_list_empty = Kenttä ”command” ei saa olla tyhjä: anna komentomerkkijono tai ei-tyhjä luettelo. # Välimuotoesityksen virheet. ir.rule_not_found = Sääntöä ”{ $rule }”, johon kohde ”{ $target }” viittaa, ei löytynyt. diff --git a/locales/fr/messages.ftl b/locales/fr/messages.ftl index b9f28c335..a629f6261 100644 --- a/locales/fr/messages.ftl +++ b/locales/fr/messages.ftl @@ -150,6 +150,7 @@ manifest.glob.invalid_pattern = Motif glob non valide « { $pattern } » : { $de manifest.glob.unknown_pattern_error = erreur de motif inconnue. manifest.glob.io_failed = Échec du glob pour « { $pattern } » : { $detail }. manifest.glob.unknown_io_error = erreur d'E/S inconnue. +manifest.command_list_empty = Le champ « command » ne doit pas être vide : indiquez une chaîne de commande ou une liste non vide. # Erreurs de la représentation intermédiaire. ir.rule_not_found = La règle « { $rule } » référencée par la cible « { $target } » est introuvable. diff --git a/locales/gd/messages.ftl b/locales/gd/messages.ftl index 5cf48185b..cde202b08 100644 --- a/locales/gd/messages.ftl +++ b/locales/gd/messages.ftl @@ -149,6 +149,7 @@ manifest.glob.invalid_pattern = Pàtran glob mì-dhligheach “{ $pattern }”: manifest.glob.unknown_pattern_error = mearachd phàtrain neo-aithnichte. manifest.glob.io_failed = Dh'fhàillig glob airson “{ $pattern }”: { $detail }. manifest.glob.unknown_io_error = mearachd ion-chuir/às-chuir neo-aithnichte. +manifest.command_list_empty = Chan fhaod an raon “command” a bhith falamh: thoir seachad sreang àithne no liosta nach eil falamh. # Mearachdan an riochdachaidh mheadhanaich. ir.rule_not_found = Cha deach an riaghailt “{ $rule }” air a bheil an targaid “{ $target }” a' toirt iomradh a lorg. diff --git a/locales/he/messages.ftl b/locales/he/messages.ftl index 1d5a64ec4..ec19b5843 100644 --- a/locales/he/messages.ftl +++ b/locales/he/messages.ftl @@ -149,6 +149,7 @@ manifest.glob.invalid_pattern = תבנית glob לא תקינה „{ $pattern } manifest.glob.unknown_pattern_error = שגיאת תבנית לא ידועה. manifest.glob.io_failed = ‏glob נכשל עבור „{ $pattern }”: { $detail }. manifest.glob.unknown_io_error = שגיאת קלט/פלט לא ידועה. +manifest.command_list_empty = השדה „command” אינו יכול להיות ריק: יש לספק מחרוזת פקודה או רשימה שאינה ריקה. # שגיאות הייצוג הביניימי. ir.rule_not_found = הכלל „{ $rule }” שאליו מפנה היעד „{ $target }” לא נמצא. diff --git a/locales/hi/messages.ftl b/locales/hi/messages.ftl index 8a8b1c6f8..b380c2036 100644 --- a/locales/hi/messages.ftl +++ b/locales/hi/messages.ftl @@ -149,6 +149,7 @@ manifest.glob.invalid_pattern = अमान्य glob प्रतिरूप manifest.glob.unknown_pattern_error = अज्ञात प्रतिरूप त्रुटि। manifest.glob.io_failed = “{ $pattern }” के लिए glob विफल रहा: { $detail }। manifest.glob.unknown_io_error = अज्ञात इनपुट/आउटपुट त्रुटि। +manifest.command_list_empty = “command” फ़ील्ड रिक्त नहीं होना चाहिए: कोई कमांड स्ट्रिंग या ग़ैर-रिक्त सूची दें। # मध्यवर्ती निरूपण की त्रुटियाँ। ir.rule_not_found = लक्ष्य “{ $target }” जिस नियम “{ $rule }” का संदर्भ देता है वह नहीं मिला। diff --git a/locales/hu/messages.ftl b/locales/hu/messages.ftl index 3cdf92115..fa93f43ea 100644 --- a/locales/hu/messages.ftl +++ b/locales/hu/messages.ftl @@ -149,6 +149,7 @@ manifest.glob.invalid_pattern = Érvénytelen glob-minta („{ $pattern }”): { manifest.glob.unknown_pattern_error = ismeretlen mintahiba. manifest.glob.io_failed = A glob sikertelen ehhez: „{ $pattern }”: { $detail }. manifest.glob.unknown_io_error = ismeretlen be- és kiviteli hiba. +manifest.command_list_empty = A „command” mező nem lehet üres: adjon meg egy parancs-karakterláncot vagy egy nem üres listát. # A köztes ábrázolás hibái. ir.rule_not_found = A(z) „{ $target }” cél által hivatkozott „{ $rule }” szabály nem található. diff --git a/locales/id/messages.ftl b/locales/id/messages.ftl index 733e136ac..4cb266021 100644 --- a/locales/id/messages.ftl +++ b/locales/id/messages.ftl @@ -149,6 +149,7 @@ manifest.glob.invalid_pattern = Pola glob tidak sah "{ $pattern }": { $detail }. manifest.glob.unknown_pattern_error = galat pola yang tidak dikenal. manifest.glob.io_failed = Glob gagal untuk "{ $pattern }": { $detail }. manifest.glob.unknown_io_error = galat masukan/keluaran yang tidak dikenal. +manifest.command_list_empty = Bidang "command" tidak boleh kosong: berikan string perintah atau daftar yang tidak kosong. # Galat representasi antara. ir.rule_not_found = Aturan "{ $rule }" yang dirujuk target "{ $target }" tidak ditemukan. diff --git a/locales/it/messages.ftl b/locales/it/messages.ftl index a94120b04..730d32970 100644 --- a/locales/it/messages.ftl +++ b/locales/it/messages.ftl @@ -150,6 +150,7 @@ manifest.glob.invalid_pattern = Pattern glob non valido «{ $pattern }»: { $det manifest.glob.unknown_pattern_error = errore di pattern sconosciuto. manifest.glob.io_failed = Glob non riuscito per «{ $pattern }»: { $detail }. manifest.glob.unknown_io_error = errore di I/O sconosciuto. +manifest.command_list_empty = Il campo «command» non deve essere vuoto: fornire una stringa di comando o un elenco non vuoto. # Errori della rappresentazione intermedia. ir.rule_not_found = La regola «{ $rule }» referenziata dal target «{ $target }» non è stata trovata. diff --git a/locales/ja/messages.ftl b/locales/ja/messages.ftl index 1408cee8e..fdfc9868d 100644 --- a/locales/ja/messages.ftl +++ b/locales/ja/messages.ftl @@ -149,6 +149,7 @@ manifest.glob.invalid_pattern = 無効な glob パターン「{ $pattern }」: { manifest.glob.unknown_pattern_error = 不明なパターンエラー。 manifest.glob.io_failed = 「{ $pattern }」の glob に失敗しました: { $detail }。 manifest.glob.unknown_io_error = 不明な入出力エラー。 +manifest.command_list_empty = 「command」フィールドは空にできません: コマンド文字列または空でないリストを指定してください。 # 中間表現のエラー。 ir.rule_not_found = ターゲット「{ $target }」が参照する規則「{ $rule }」が見つかりません。 diff --git a/locales/ko/messages.ftl b/locales/ko/messages.ftl index ab0850a8f..2973b31fc 100644 --- a/locales/ko/messages.ftl +++ b/locales/ko/messages.ftl @@ -149,6 +149,7 @@ manifest.glob.invalid_pattern = 잘못된 glob 패턴 '{ $pattern }': { $detail manifest.glob.unknown_pattern_error = 알 수 없는 패턴 오류. manifest.glob.io_failed = '{ $pattern }'에 대한 glob이 실패했습니다: { $detail }. manifest.glob.unknown_io_error = 알 수 없는 입출력 오류. +manifest.command_list_empty = 'command' 필드는 비어 있을 수 없습니다: 명령 문자열 또는 비어 있지 않은 목록을 지정하십시오. # 중간 표현 오류. ir.rule_not_found = 대상 '{ $target }'이(가) 참조하는 규칙 '{ $rule }'을(를) 찾을 수 없습니다. diff --git a/locales/nb/messages.ftl b/locales/nb/messages.ftl index 3519a18f0..3c1e98bd0 100644 --- a/locales/nb/messages.ftl +++ b/locales/nb/messages.ftl @@ -149,6 +149,7 @@ manifest.glob.invalid_pattern = Ugyldig glob-mønster «{ $pattern }»: { $detai manifest.glob.unknown_pattern_error = ukjent mønsterfeil. manifest.glob.io_failed = Glob mislyktes for «{ $pattern }»: { $detail }. manifest.glob.unknown_io_error = ukjent I/U-feil. +manifest.command_list_empty = Feltet «command» kan ikke være tomt: oppgi en kommandostreng eller en ikke-tom liste. # Feil i den interne representasjonen. ir.rule_not_found = Regelen «{ $rule }» som målet «{ $target }» viser til, ble ikke funnet. diff --git a/locales/nl/messages.ftl b/locales/nl/messages.ftl index bae00c43e..d402bacfa 100644 --- a/locales/nl/messages.ftl +++ b/locales/nl/messages.ftl @@ -149,6 +149,7 @@ manifest.glob.invalid_pattern = Ongeldig glob-patroon ‘{ $pattern }’: { $det manifest.glob.unknown_pattern_error = onbekende patroonfout. manifest.glob.io_failed = Glob is mislukt voor ‘{ $pattern }’: { $detail }. manifest.glob.unknown_io_error = onbekende I/O-fout. +manifest.command_list_empty = Het veld ‘command’ mag niet leeg zijn: geef een opdrachtreeks of een niet-lege lijst op. # Fouten in de tussenrepresentatie. ir.rule_not_found = De regel ‘{ $rule }’ waarnaar doel ‘{ $target }’ verwijst, is niet gevonden. diff --git a/locales/pl/messages.ftl b/locales/pl/messages.ftl index 195e136e3..77c4fe8e4 100644 --- a/locales/pl/messages.ftl +++ b/locales/pl/messages.ftl @@ -149,6 +149,7 @@ manifest.glob.invalid_pattern = Nieprawidłowy wzorzec glob „{ $pattern }”: manifest.glob.unknown_pattern_error = nieznany błąd wzorca. manifest.glob.io_failed = Wzorzec glob „{ $pattern }” zawiódł: { $detail }. manifest.glob.unknown_io_error = nieznany błąd wejścia/wyjścia. +manifest.command_list_empty = Pole „command” nie może być puste: podaj łańcuch polecenia lub niepustą listę. # Błędy reprezentacji pośredniej. ir.rule_not_found = Nie znaleziono reguły „{ $rule }”, do której odwołuje się cel „{ $target }”. diff --git a/locales/pt-BR/messages.ftl b/locales/pt-BR/messages.ftl index 959455679..2ced9ce27 100644 --- a/locales/pt-BR/messages.ftl +++ b/locales/pt-BR/messages.ftl @@ -150,6 +150,7 @@ manifest.glob.invalid_pattern = Padrão glob inválido "{ $pattern }": { $detail manifest.glob.unknown_pattern_error = erro de padrão desconhecido. manifest.glob.io_failed = O glob falhou para "{ $pattern }": { $detail }. manifest.glob.unknown_io_error = erro de E/S desconhecido. +manifest.command_list_empty = O campo "command" não pode estar vazio: forneça uma cadeia de comando ou uma lista não vazia. # Erros da representação intermediária. ir.rule_not_found = A regra "{ $rule }" referenciada pelo alvo "{ $target }" não foi encontrada. diff --git a/locales/pt-PT/messages.ftl b/locales/pt-PT/messages.ftl index b3245c3b7..394a77930 100644 --- a/locales/pt-PT/messages.ftl +++ b/locales/pt-PT/messages.ftl @@ -150,6 +150,7 @@ manifest.glob.invalid_pattern = Padrão glob inválido «{ $pattern }»: { $deta manifest.glob.unknown_pattern_error = erro de padrão desconhecido. manifest.glob.io_failed = O glob falhou para «{ $pattern }»: { $detail }. manifest.glob.unknown_io_error = erro de E/S desconhecido. +manifest.command_list_empty = O campo «command» não pode estar vazio: forneça uma cadeia de comando ou uma lista não vazia. # Erros da representação intermédia. ir.rule_not_found = A regra «{ $rule }» referenciada pelo alvo «{ $target }» não foi encontrada. diff --git a/locales/ro/messages.ftl b/locales/ro/messages.ftl index 692639afd..9cc7901b1 100644 --- a/locales/ro/messages.ftl +++ b/locales/ro/messages.ftl @@ -149,6 +149,7 @@ manifest.glob.invalid_pattern = Tipar glob nevalid „{ $pattern }”: { $detail manifest.glob.unknown_pattern_error = eroare de tipar necunoscută. manifest.glob.io_failed = Glob a eșuat pentru „{ $pattern }”: { $detail }. manifest.glob.unknown_io_error = eroare de intrare/ieșire necunoscută. +manifest.command_list_empty = Câmpul „command” nu trebuie să fie gol: furnizați un șir de comandă sau o listă nevidă. # Erori ale reprezentării intermediare. ir.rule_not_found = Regula „{ $rule }” la care face referire ținta „{ $target }” nu a fost găsită. diff --git a/locales/ru/messages.ftl b/locales/ru/messages.ftl index a9988ce66..ca9ccd562 100644 --- a/locales/ru/messages.ftl +++ b/locales/ru/messages.ftl @@ -149,6 +149,7 @@ manifest.glob.invalid_pattern = Некорректный шаблон glob «{ $ manifest.glob.unknown_pattern_error = неизвестная ошибка шаблона. manifest.glob.io_failed = Сбой glob для «{ $pattern }»: { $detail }. manifest.glob.unknown_io_error = неизвестная ошибка ввода-вывода. +manifest.command_list_empty = Поле «command» не должно быть пустым: укажите строку команды или непустой список. # Ошибки промежуточного представления. ir.rule_not_found = Правило «{ $rule }», на которое ссылается цель «{ $target }», не найдено. diff --git a/locales/sv/messages.ftl b/locales/sv/messages.ftl index 336a47b85..ad1126a0f 100644 --- a/locales/sv/messages.ftl +++ b/locales/sv/messages.ftl @@ -149,6 +149,7 @@ manifest.glob.invalid_pattern = Ogiltigt glob-mönster ”{ $pattern }”: { $de manifest.glob.unknown_pattern_error = okänt mönsterfel. manifest.glob.io_failed = Glob misslyckades för ”{ $pattern }”: { $detail }. manifest.glob.unknown_io_error = okänt I/O-fel. +manifest.command_list_empty = Fältet ”command” får inte vara tomt: ange en kommandosträng eller en icke-tom lista. # Fel i den interna representationen. ir.rule_not_found = Regeln ”{ $rule }” som målet ”{ $target }” hänvisar till hittades inte. diff --git a/locales/th/messages.ftl b/locales/th/messages.ftl index 7f4ed54fe..5afd113ae 100644 --- a/locales/th/messages.ftl +++ b/locales/th/messages.ftl @@ -149,6 +149,7 @@ manifest.glob.invalid_pattern = รูปแบบ glob ไม่ถูกต้ manifest.glob.unknown_pattern_error = ข้อผิดพลาดของรูปแบบที่ไม่รู้จัก manifest.glob.io_failed = glob ล้มเหลวสำหรับ “{ $pattern }”: { $detail } manifest.glob.unknown_io_error = ข้อผิดพลาดรับส่งข้อมูลที่ไม่รู้จัก +manifest.command_list_empty = ฟิลด์ “command” ต้องไม่ว่าง: ระบุสตริงคำสั่งหรือรายการที่ไม่ว่าง # ข้อผิดพลาดของรูปแทนระดับกลาง ir.rule_not_found = ไม่พบกฎ “{ $rule }” ที่เป้าหมาย “{ $target }” อ้างถึง diff --git a/locales/tr/messages.ftl b/locales/tr/messages.ftl index cc69bcfc4..8af7246e7 100644 --- a/locales/tr/messages.ftl +++ b/locales/tr/messages.ftl @@ -149,6 +149,7 @@ manifest.glob.invalid_pattern = Geçersiz glob deseni "{ $pattern }": { $detail manifest.glob.unknown_pattern_error = bilinmeyen desen hatası. manifest.glob.io_failed = "{ $pattern }" için glob başarısız oldu: { $detail }. manifest.glob.unknown_io_error = bilinmeyen G/Ç hatası. +manifest.command_list_empty = "command" alanı boş olmamalıdır: bir komut dizesi veya boş olmayan bir liste verin. # Ara gösterim hataları. ir.rule_not_found = "{ $target }" hedefinin başvurduğu "{ $rule }" kuralı bulunamadı. diff --git a/locales/uk/messages.ftl b/locales/uk/messages.ftl index 45884abba..260d0188d 100644 --- a/locales/uk/messages.ftl +++ b/locales/uk/messages.ftl @@ -149,6 +149,7 @@ manifest.glob.invalid_pattern = Некоректний шаблон glob «{ $pa manifest.glob.unknown_pattern_error = невідома помилка шаблону. manifest.glob.io_failed = Збій glob для «{ $pattern }»: { $detail }. manifest.glob.unknown_io_error = невідома помилка вводу-виводу. +manifest.command_list_empty = Поле «command» не має бути порожнім: укажіть рядок команди або непорожній список. # Помилки проміжного подання. ir.rule_not_found = Правило «{ $rule }», на яке посилається ціль «{ $target }», не знайдено. diff --git a/locales/vi/messages.ftl b/locales/vi/messages.ftl index b9389371a..06a083ba7 100644 --- a/locales/vi/messages.ftl +++ b/locales/vi/messages.ftl @@ -149,6 +149,7 @@ manifest.glob.invalid_pattern = Mẫu glob không hợp lệ “{ $pattern }”: manifest.glob.unknown_pattern_error = lỗi mẫu không xác định. manifest.glob.io_failed = Glob thất bại với “{ $pattern }”: { $detail }. manifest.glob.unknown_io_error = lỗi vào/ra không xác định. +manifest.command_list_empty = Trường “command” không được để trống: hãy cung cấp một chuỗi lệnh hoặc một danh sách không rỗng. # Lỗi của biểu diễn trung gian. ir.rule_not_found = Không tìm thấy quy tắc “{ $rule }” mà đích “{ $target }” tham chiếu. diff --git a/locales/zh-Hans/messages.ftl b/locales/zh-Hans/messages.ftl index a49f41fa9..dc92f76fa 100644 --- a/locales/zh-Hans/messages.ftl +++ b/locales/zh-Hans/messages.ftl @@ -148,6 +148,7 @@ manifest.glob.invalid_pattern = 无效的 glob 模式“{ $pattern }”：{ $det manifest.glob.unknown_pattern_error = 未知的模式错误。 manifest.glob.io_failed = 对“{ $pattern }”执行 glob 失败：{ $detail }。 manifest.glob.unknown_io_error = 未知的输入输出错误。 +manifest.command_list_empty = “command”字段不能为空：请提供命令字符串或非空列表。 # 中间表示的错误。 ir.rule_not_found = 找不到目标“{ $target }”引用的规则“{ $rule }”。 diff --git a/locales/zh-Hant/messages.ftl b/locales/zh-Hant/messages.ftl index 1dbbe3f5f..663e321b9 100644 --- a/locales/zh-Hant/messages.ftl +++ b/locales/zh-Hant/messages.ftl @@ -148,6 +148,7 @@ manifest.glob.invalid_pattern = 無效的 glob 樣式「{ $pattern }」：{ $det manifest.glob.unknown_pattern_error = 未知的樣式錯誤。 manifest.glob.io_failed = 對「{ $pattern }」執行 glob 失敗：{ $detail }。 manifest.glob.unknown_io_error = 未知的輸入輸出錯誤。 +manifest.command_list_empty = 「command」欄位不得為空：請提供命令字串或非空清單。 # 中介表示法的錯誤。 ir.rule_not_found = 找不到目標「{ $target }」所參照的規則「{ $rule }」。 diff --git a/src/ast.rs b/src/ast.rs index 9c69e5e38..bc540a7b1 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -43,6 +43,8 @@ pub type Vars = HashMap; /// Map type for `vars` blocks under Kani. #[cfg(kani)] pub type Vars = HashMap>; +/// Stable schema error that the manifest adapter translates for its users. +pub(crate) const EMPTY_COMMAND_LIST_ERROR: &str = "command list must not be empty"; fn deserialize_actions<'de, D>(deserializer: D) -> Result, D::Error> where @@ -141,10 +143,12 @@ pub struct Rule { /// determines the variant. #[derive(Debug, Clone, PartialEq, Serialize)] pub enum Recipe { - /// A single shell command. + /// A shell command, given as a scalar or an ordered list executed by a + /// fail-fast shell chain. Command { - /// Shell command executed verbatim by Ninja. - command: String, + /// A scalar command passes through unchanged; list entries are + /// evaluated in brace groups joined by a fail-fast `&&` chain. + command: StringOrList, }, /// An embedded multi-line script. Script { @@ -161,7 +165,7 @@ pub enum Recipe { #[derive(Debug, Deserialize)] #[serde(deny_unknown_fields)] struct RawRecipe { - command: Option, + command: Option, script: Option, rule: Option, } @@ -178,7 +182,14 @@ impl<'de> Deserialize<'de> for Recipe { rule: rule_field, } = raw; match (command_field, script_field, rule_field) { - (Some(command), None, None) => Ok(Self::Command { command }), + (Some(command), None, None) => match command { + empty if empty.is_empty_content() => { + Err(serde::de::Error::custom(EMPTY_COMMAND_LIST_ERROR)) + } + command_value => Ok(Self::Command { + command: command_value, + }), + }, (None, Some(script), None) => Ok(Self::Script { script }), (None, None, Some(rule)) => Ok(Self::Rule { rule }), (None, None, None) => Err(serde::de::Error::custom( @@ -345,4 +356,45 @@ impl StringOrList { _ => None, } } + + /// Whether the value carries no string content. + /// + /// `Empty` and an empty `List` both yield `true`; a `String` (even an + /// empty string) and a non-empty `List` yield `false`. + /// + /// # Examples + /// + /// ``` + /// use netsuke::ast::StringOrList; + /// + /// assert!(StringOrList::Empty.is_empty_content()); + /// assert!(StringOrList::List(Vec::new()).is_empty_content()); + /// assert!(!StringOrList::String(String::new()).is_empty_content()); + /// ``` + #[must_use] + pub const fn is_empty_content(&self) -> bool { + match self { + Self::Empty => true, + Self::String(_) => false, + Self::List(v) => v.is_empty(), + } + } +} + +impl From<&str> for StringOrList { + fn from(value: &str) -> Self { + Self::String(value.to_owned()) + } +} + +impl From for StringOrList { + fn from(value: String) -> Self { + Self::String(value) + } +} + +impl From> for StringOrList { + fn from(value: Vec) -> Self { + Self::List(value) + } } diff --git a/src/ir/cmd_interpolate.rs b/src/ir/cmd_interpolate.rs index 144844fb6..0ee1c4d86 100644 --- a/src/ir/cmd_interpolate.rs +++ b/src/ir/cmd_interpolate.rs @@ -9,8 +9,74 @@ use crate::localization::{self, keys}; use camino::Utf8PathBuf; use shell_quote::{QuoteRefExt, Sh}; +#[cfg(test)] +use std::cell::Cell; + use super::IrGenError; +/// Quoted `$in` and `$out` substitutions prepared for one recipe. +/// +/// A rule command list shares its input/output bindings, so lowering creates +/// this once and reuses it for every entry rather than re-quoting paths for +/// each command. +#[derive(Debug, Clone)] +pub(crate) struct CommandBindings { + ins: String, + outs: String, +} + +impl CommandBindings { + /// Quote the paths once for every command in one recipe. + #[must_use] + pub(crate) fn new(inputs: &[Utf8PathBuf], outputs: &[Utf8PathBuf]) -> Self { + record_binding_preparation(); + Self { + ins: quote_paths(inputs).join(" "), + outs: quote_paths(outputs).join(" "), + } + } +} + +#[cfg(test)] +thread_local! { + static BINDING_PREPARATIONS: Cell = const { Cell::new(0) }; +} + +#[cfg(test)] +fn record_binding_preparation() { + BINDING_PREPARATIONS.with(|count| count.set(count.get() + 1)); +} + +#[cfg(not(test))] +const fn record_binding_preparation() {} + +#[cfg(test)] +pub(crate) fn reset_binding_preparations() { + BINDING_PREPARATIONS.with(|count| count.set(0)); +} + +#[cfg(test)] +pub(crate) fn binding_preparations() -> usize { + BINDING_PREPARATIONS.with(Cell::get) +} + +fn quote_paths(paths: &[Utf8PathBuf]) -> Vec { + paths + .iter() + .map(|path| { + // Utf8PathBuf guarantees UTF-8, and shell quoting should preserve it. + let bytes: Vec = path.as_str().quoted(Sh); + match String::from_utf8(bytes) { + Ok(text) => text, + Err(err) => { + debug_assert!(false, "shell quoting produced non UTF-8 bytes: {err}"); + String::from_utf8_lossy(err.as_bytes()).into_owned() + } + } + }) + .collect() +} + /// Returns `true` when the command contains an odd number of backticks. /// /// # Examples @@ -22,31 +88,22 @@ fn has_unmatched_backticks(s: &str) -> bool { s.chars().filter(|&c| c == '`').count().rem_euclid(2) != 0 } +#[cfg(test)] pub(crate) fn interpolate_command( template: &str, inputs: &[Utf8PathBuf], outputs: &[Utf8PathBuf], ) -> Result { - fn quote_paths(paths: &[Utf8PathBuf]) -> Vec { - paths - .iter() - .map(|p| { - // Utf8PathBuf guarantees UTF-8, and shell quoting should preserve it. - let bytes: Vec = p.as_str().quoted(Sh); - match String::from_utf8(bytes) { - Ok(text) => text, - Err(err) => { - debug_assert!(false, "shell quoting produced non UTF-8 bytes: {err}"); - String::from_utf8_lossy(err.as_bytes()).into_owned() - } - } - }) - .collect() - } + let bindings = CommandBindings::new(inputs, outputs); + interpolate_command_with_bindings(template, &bindings) +} - let ins = quote_paths(inputs); - let outs = quote_paths(outputs); - let interpolated = substitute(template, &ins, &outs); +/// Interpolate `template` with bindings prepared for its enclosing recipe. +pub(crate) fn interpolate_command_with_bindings( + template: &str, + bindings: &CommandBindings, +) -> Result { + let interpolated = substitute(template, &bindings.ins, &bindings.outs); if has_unmatched_backticks(&interpolated) || shlex::split(&interpolated).is_none() { let snippet = interpolated.chars().take(160).collect(); let message = localization::message(keys::IR_INVALID_COMMAND).with_arg("snippet", &snippet); @@ -175,9 +232,7 @@ fn try_match_token<'a>( Some((replacement, matched_len)) } -fn substitute(template: &str, ins: &[String], outs: &[String]) -> String { - let ins_joined = ins.join(" "); - let outs_joined = outs.join(" "); +fn substitute(template: &str, ins: &str, outs: &str) -> String { let chars: Vec = template.chars().collect(); let mut out = String::with_capacity(template.len()); let mut in_backticks = false; @@ -196,7 +251,7 @@ fn substitute(template: &str, ins: &[String], outs: &[String]) -> String { continue; } - if let Some((replacement, skip)) = find_substitution(&chars, i, &ins_joined, &outs_joined) { + if let Some((replacement, skip)) = find_substitution(&chars, i, ins, outs) { out.push_str(replacement); i += skip; } else { diff --git a/src/ir/from_manifest_support.rs b/src/ir/from_manifest_support.rs index c911922a7..43dfbd672 100644 --- a/src/ir/from_manifest_support.rs +++ b/src/ir/from_manifest_support.rs @@ -13,7 +13,7 @@ use crate::hasher::ActionHasher; use crate::localization::{self, keys}; use super::super::{ - cmd_interpolate::interpolate_command, + cmd_interpolate::{CommandBindings, interpolate_command_with_bindings}, graph::{Action, BuildEdge, IrGenError, IrHashMap}, }; @@ -31,7 +31,22 @@ pub(super) fn register_action( ) -> Result { let resolved_recipe = match recipe { Recipe::Command { command } => { - let interpolated = interpolate_command(&command, bindings.inputs, bindings.outputs)?; + let command_bindings = CommandBindings::new(bindings.inputs, bindings.outputs); + let interpolated = match command { + StringOrList::String(cmd) => StringOrList::String( + interpolate_command_with_bindings(&cmd, &command_bindings)?, + ), + StringOrList::List(items) => { + let mut rendered = Vec::with_capacity(items.len()); + for item in items { + rendered.push(interpolate_command_with_bindings(&item, &command_bindings)?); + } + StringOrList::List(rendered) + } + // An empty command list cannot deserialize (the manifest + // parser rejects it), so nothing needs interpolating here. + StringOrList::Empty => StringOrList::Empty, + }; Recipe::Command { command: interpolated, } @@ -335,3 +350,7 @@ pub(super) fn get_target_display_name(paths: &[Utf8PathBuf]) -> String { .map(|p: &Utf8PathBuf| p.to_string()) .unwrap_or_default() } + +#[cfg(test)] +#[path = "from_manifest_support_tests.rs"] +mod tests; diff --git a/src/ir/from_manifest_support_tests.rs b/src/ir/from_manifest_support_tests.rs new file mode 100644 index 000000000..3fe1f7171 --- /dev/null +++ b/src/ir/from_manifest_support_tests.rs @@ -0,0 +1,63 @@ +//! Regression tests for command-list manifest-to-IR lowering. + +use super::*; +use crate::ir::cmd_interpolate::{binding_preparations, reset_binding_preparations}; +use proptest::prelude::*; + +#[test] +fn large_command_list_prepares_path_bindings_once() { + reset_binding_preparations(); + let entries = (0..64) + .map(|index| format!("printf {index} $in $out")) + .collect(); + let mut actions = IrHashMap::default(); + register_action( + &mut actions, + Recipe::Command { + command: StringOrList::List(entries), + }, + None, + ActionBindings { + inputs: &[Utf8PathBuf::from("input")], + outputs: &[Utf8PathBuf::from("output")], + }, + ) + .expect("shell-safe command list should lower"); + assert_eq!( + binding_preparations(), + 1, + "all entries in one recipe must reuse one prepared input/output binding set" + ); +} + +proptest! { + #[test] + fn command_list_placeholder_interpolation_preserves_entry_order( + labels in prop::collection::vec("[a-z]{1,10}", 1..9), + ) { + let entries: Vec = labels + .iter() + .map(|label| format!("echo {label} $in $out")) + .collect(); + let mut actions = IrHashMap::default(); + let action_id = register_action( + &mut actions, + Recipe::Command { command: StringOrList::List(entries) }, + None, + ActionBindings { + inputs: &[Utf8PathBuf::from("input")], + outputs: &[Utf8PathBuf::from("output")], + }, + ).expect("shell-safe generated entries should interpolate"); + let action = actions.get(&action_id).expect("registered action should be available"); + let Recipe::Command { command } = &action.recipe else { + prop_assert!(false, "registered command list should remain a command recipe"); + return Ok(()); + }; + let expected: Vec = labels + .iter() + .map(|label| format!("echo {label} input output")) + .collect(); + prop_assert_eq!(command.to_string_vec(), expected); + } +} diff --git a/src/localization/keys.rs b/src/localization/keys.rs index d019e3c1c..b91622878 100644 --- a/src/localization/keys.rs +++ b/src/localization/keys.rs @@ -132,6 +132,7 @@ define_keys! { MANIFEST_GLOB_UNKNOWN_PATTERN_ERROR => "manifest.glob.unknown_pattern_error", MANIFEST_GLOB_IO_FAILED => "manifest.glob.io_failed", MANIFEST_GLOB_UNKNOWN_IO_ERROR => "manifest.glob.unknown_io_error", + MANIFEST_COMMAND_LIST_EMPTY => "manifest.command_list_empty", IR_RULE_NOT_FOUND => "ir.rule_not_found", IR_MULTIPLE_RULES => "ir.multiple_rules", IR_EMPTY_RULE => "ir.empty_rule", diff --git a/src/manifest/mod.rs b/src/manifest/mod.rs index bad26f40a..04b986a20 100644 --- a/src/manifest/mod.rs +++ b/src/manifest/mod.rs @@ -23,7 +23,7 @@ //! single namespace. use crate::{ - ast::NetsukeManifest, + ast::{EMPTY_COMMAND_LIST_ERROR, NetsukeManifest}, localization::{self, keys}, stdlib::{NetworkPolicy, StdlibConfig}, }; @@ -147,14 +147,25 @@ fn from_str_named( notify_stage(on_stage, ManifestLoadStage::FinalRendering); let manifest: NetsukeManifest = - serde_json::from_value(doc).map_err(|e| ManifestError::Parse { - source: map_data_error(e, name), + serde_json::from_value(doc).map_err(|error| ManifestError::Parse { + source: map_data_error(localize_recipe_error(error), name), message: localization::message(keys::MANIFEST_PARSE), })?; render_manifest(manifest, &jinja) } +/// Translate schema-only recipe errors at the manifest adapter boundary. +fn localize_recipe_error(error: serde_json::Error) -> serde_json::Error { + if error.to_string().starts_with(EMPTY_COMMAND_LIST_ERROR) { + serde_json::Error::custom( + localization::message(keys::MANIFEST_COMMAND_LIST_EMPTY).to_string(), + ) + } else { + error + } +} + /// Names the manifest loader registers as Jinja helper functions. /// /// `MiniJinja` keeps functions and global variables in a single namespace, so a @@ -260,7 +271,7 @@ pub fn from_str(yaml: &str) -> Result { /// /// assert!(matches!( /// &manifest.targets[0].recipe, -/// Recipe::Command { command } if command == "echo release" +/// Recipe::Command { command } if command.as_single() == Some("echo release") /// )); /// ``` pub fn from_str_with_env(yaml: &str, env_reader: &EnvReader) -> Result { @@ -346,7 +357,7 @@ pub fn from_path_with_policy( /// /// assert!(matches!( /// &manifest.targets[0].recipe, -/// Recipe::Command { command } if command == "echo offline" +/// Recipe::Command { command } if command.as_single() == Some("echo offline") /// )); /// ``` pub fn from_path_with_policy_and_env( diff --git a/src/manifest/render.rs b/src/manifest/render.rs index 39ac16fae..54caced25 100644 --- a/src/manifest/render.rs +++ b/src/manifest/render.rs @@ -1,7 +1,7 @@ //! Renders manifest templates using `MiniJinja` before IR lowering. //! //! Provides [`render_manifest`], which evaluates Jinja2-style template -//! expressions in target and rule fields. [`render_recipe_str_with`] ensures +//! expressions in target and rule fields. Recipe rendering ensures //! `ins`/`outs` context keys are always present, inserting //! `__NETSUKE_INS_PLACEHOLDER__`/`__NETSUKE_OUTS_PLACEHOLDER__` when absent //! so that [`crate::ir::cmd_interpolate`] can substitute them later. @@ -12,6 +12,9 @@ use crate::ir::{INS_TOKEN, OUTS_TOKEN}; use anyhow::{Context, Result}; use minijinja::Environment; +#[cfg(test)] +use std::cell::Cell; + /// Render manifest targets and rules by evaluating template expressions. /// /// # Errors @@ -41,7 +44,7 @@ fn render_rule(rule: &mut crate::ast::Rule, env: &Environment, vars: &Vars) -> R } match &mut rule.recipe { Recipe::Command { command } => { - *command = render_recipe_str_with(env, command, vars, || "render rule command".into())?; + render_recipe_string_or_list(command, env, vars, || "render rule command".into())?; } Recipe::Script { script } => { *script = render_str_with(env, script, vars, || "render rule script".into())?; @@ -59,7 +62,7 @@ fn render_target(target: &mut Target, env: &Environment) -> Result<()> { render_string_or_list(&mut target.order_only_deps, env, &target.vars)?; match &mut target.recipe { Recipe::Command { command } => { - *command = render_recipe_str_with(env, command, &target.vars, || { + render_recipe_string_or_list(command, env, &target.vars, || { "render target command".into() })?; } @@ -96,29 +99,47 @@ fn render_string_or_list(value: &mut StringOrList, env: &Environment, ctx: &Vars Ok(()) } -fn render_str_with( +/// Render a recipe `command` field, injecting the `ins`/`outs` placeholders +/// for every entry. +/// +/// A scalar command renders as today; each entry of a list command is +/// rendered independently so `{{ ins }}`/`{{ outs }}` expand per entry during +/// IR interpolation. The `what` label is computed once and shared by every +/// entry. A scalar failure names the recipe stage alone; a list failure also +/// names the one-based position of the entry that failed to render. +fn render_recipe_string_or_list( + value: &mut StringOrList, env: &Environment, - tpl: &str, - ctx: &impl serde::Serialize, + ctx: &Vars, what: impl FnOnce() -> String, -) -> Result { - render_template(env, tpl, ctx).with_context(what) +) -> Result<()> { + let label = what(); + let recipe_ctx = recipe_render_context(ctx); + let render_entry = |entry: &mut String, position: Option| -> Result<()> { + *entry = render_str_with(env, entry, &recipe_ctx, || { + position.map_or_else(|| label.clone(), |index| format!("{label} entry {index}")) + })?; + Ok(()) + }; + match value { + StringOrList::String(s) => render_entry(s, None)?, + StringOrList::List(list) => { + for (index, item) in list.iter_mut().enumerate() { + render_entry(item, Some(index + 1))?; + } + } + StringOrList::Empty => {} + } + Ok(()) } -/// Clones the supplied template context (`Vars`) and guarantees `ins` and `outs` -/// entries exist before invoking `MiniJinja` rendering. +/// Clone a recipe context once, adding the delayed path placeholders. /// -/// If `ins` or `outs` are absent, they are populated with the placeholders -/// `__NETSUKE_INS_PLACEHOLDER__` and `__NETSUKE_OUTS_PLACEHOLDER__` so -/// downstream logic can rely on those variables being present before later -/// `Ninja` substitution. Rendering is performed by -/// calling `render_str_with`. -fn render_recipe_str_with( - env: &Environment, - tpl: &str, - ctx: &Vars, - what: impl FnOnce() -> String, -) -> Result { +/// Every list entry sees the same Jinja bindings. Keeping this preparation +/// outside the entry loop avoids cloning a target's complete `vars` map for +/// each item while retaining the scalar rendering contract. +fn recipe_render_context(ctx: &Vars) -> Vars { + record_recipe_context_preparation(); let mut recipe_ctx = ctx.clone(); recipe_ctx .entry("ins".into()) @@ -126,7 +147,39 @@ fn render_recipe_str_with( recipe_ctx .entry("outs".into()) .or_insert_with(|| ManifestValue::String(OUTS_TOKEN.into())); - render_str_with(env, tpl, &recipe_ctx, what) + recipe_ctx +} + +#[cfg(test)] +thread_local! { + static RECIPE_CONTEXT_PREPARATIONS: Cell = const { Cell::new(0) }; +} + +#[cfg(test)] +fn record_recipe_context_preparation() { + RECIPE_CONTEXT_PREPARATIONS.with(|count| count.set(count.get() + 1)); +} + +#[cfg(not(test))] +const fn record_recipe_context_preparation() {} + +#[cfg(test)] +pub(super) fn reset_recipe_context_preparations() { + RECIPE_CONTEXT_PREPARATIONS.with(|count| count.set(0)); +} + +#[cfg(test)] +pub(super) fn recipe_context_preparations() -> usize { + RECIPE_CONTEXT_PREPARATIONS.with(Cell::get) +} + +fn render_str_with( + env: &Environment, + tpl: &str, + ctx: &impl serde::Serialize, + what: impl FnOnce() -> String, +) -> Result { + render_template(env, tpl, ctx).with_context(what) } #[cfg(test)] @@ -212,7 +265,10 @@ mod tests { #[expect(clippy::panic, reason = "panic for clearer test failures")] fn expect_command(recipe: &Recipe, label: impl std::fmt::Display) -> &str { match recipe { - Recipe::Command { command } => command, + Recipe::Command { command } => match command { + StringOrList::String(item) => item, + other => panic!("expected {label} command as a scalar, got {other:?}"), + }, other => panic!("expected {label} command recipe, got {other:?}"), } } @@ -234,7 +290,7 @@ mod tests { fn assert_rendered_rule(rule: &Rule) { assert_eq!(rule.description.as_deref(), Some("2")); match &rule.recipe { - Recipe::Command { command } => assert_eq!(command, "4"), + Recipe::Command { command } => assert_eq!(command.as_single(), Some("4")), other => panic!("expected command recipe, got {other:?}"), } } @@ -253,4 +309,71 @@ mod tests { assert_rendered_rule(rendered_rule); Ok(()) } + + #[test] + fn command_list_renders_each_entry_with_ins_outs_placeholders() -> Result<()> { + let env = Environment::new(); + let manifest = NetsukeManifest { + netsuke_version: Version::parse("1.0.0")?, + vars: Vars::new(), + macros: Vec::new(), + rules: vec![Rule { + name: "check".into(), + recipe: Recipe::Command { + command: StringOrList::List(vec![ + "echo {{ 1 + 1 }}".into(), + "{{ ins }}".into(), + "{{ outs }}".into(), + ]), + }, + description: None, + }], + actions: Vec::new(), + targets: Vec::new(), + defaults: Vec::new(), + }; + let rendered = render_manifest(manifest, &env)?; + let rule = rendered.rules.first().context("rendered rule missing")?; + let Recipe::Command { command } = &rule.recipe else { + anyhow::bail!("expected command recipe, got {:?}", rule.recipe); + }; + anyhow::ensure!( + command.to_string_vec() == ["echo 2", crate::ir::INS_TOKEN, crate::ir::OUTS_TOKEN], + "unexpected rendered command list: {command:?}" + ); + Ok(()) + } + + #[test] + fn command_list_render_failure_names_the_failing_entry() -> Result<()> { + let env = Environment::new(); + let manifest = NetsukeManifest { + netsuke_version: Version::parse("1.0.0")?, + vars: Vars::new(), + macros: Vec::new(), + rules: vec![Rule { + name: "check".into(), + recipe: Recipe::Command { + command: StringOrList::List(vec!["echo ok".into(), "echo {{ 1 + }}".into()]), + }, + description: None, + }], + actions: Vec::new(), + targets: Vec::new(), + defaults: Vec::new(), + }; + let error = render_manifest(manifest, &env) + .err() + .context("expected the malformed entry to fail rendering")?; + let report = format!("{error:#}"); + anyhow::ensure!( + report.contains("render rule command entry 2"), + "error should name the failing list position, got: {report}" + ); + Ok(()) + } } + +#[cfg(test)] +#[path = "render_command_list_tests.rs"] +mod command_list_tests; diff --git a/src/manifest/render_command_list_tests.rs b/src/manifest/render_command_list_tests.rs new file mode 100644 index 000000000..3b9d00111 --- /dev/null +++ b/src/manifest/render_command_list_tests.rs @@ -0,0 +1,35 @@ +//! Regression tests for rendering command-list entries. + +use super::*; + +#[test] +fn large_command_list_prepares_the_jinja_context_once() { + reset_recipe_context_preparations(); + let mut command = StringOrList::List( + (0..64) + .map(|index| format!("echo {{{{ label }}}} {index} {{{{ ins }}}}")) + .collect(), + ); + let mut vars = Vars::new(); + vars.insert("label".into(), ManifestValue::String("rendered".into())); + + render_recipe_string_or_list(&mut command, &Environment::new(), &vars, || { + "render command list".into() + }) + .expect("shell-safe command list should render"); + + assert_eq!( + recipe_context_preparations(), + 1, + "one recipe must prepare its Jinja context once regardless of entry count" + ); + let rendered_entries = command.to_string_vec(); + assert_eq!( + rendered_entries.first().map(String::as_str), + Some("echo rendered 0 __NETSUKE_INS_PLACEHOLDER__") + ); + assert_eq!( + rendered_entries.last().map(String::as_str), + Some("echo rendered 63 __NETSUKE_INS_PLACEHOLDER__") + ); +} diff --git a/src/manifest/tests/workspace.rs b/src/manifest/tests/workspace.rs index 72c4f847f..ec915d149 100644 --- a/src/manifest/tests/workspace.rs +++ b/src/manifest/tests/workspace.rs @@ -195,8 +195,8 @@ fn from_path_uses_manifest_directory_for_caches() -> AnyResult<()> { let first_target = manifest.targets.first().context("target missing")?; match &first_target.recipe { Recipe::Command { command } => anyhow::ensure!( - command == "workspace-body", - "unexpected recipe output: {command}" + command.as_single() == Some("workspace-body"), + "unexpected recipe output: {command:?}" ), other => anyhow::bail!("expected command recipe, got {other:?}"), } diff --git a/src/ninja_gen.rs b/src/ninja_gen.rs index 0450d043b..2353e6fba 100644 --- a/src/ninja_gen.rs +++ b/src/ninja_gen.rs @@ -6,46 +6,24 @@ //! generated Ninja file is written by the runner and `generate` command for //! downstream execution by the Ninja build system. -use crate::ast::Recipe; +use crate::ast::{Recipe, StringOrList}; use crate::ir::{BuildEdge, BuildGraph}; -use crate::localization::{self, LocalizedMessage, keys}; +use crate::localization::{self, keys}; use camino::Utf8PathBuf; use itertools::Itertools; use std::collections::HashSet; use std::fmt::{self, Display, Formatter, Write}; -use thiserror::Error; -/// Errors produced while rendering Ninja manifests. -#[derive(Debug, Error)] -pub enum NinjaGenError { - /// The build graph referenced an action that was not defined. - #[error("{message}")] - MissingAction { - /// Identifier of the missing action referenced by a build edge. - id: String, - /// Localized error message. - message: LocalizedMessage, - }, - /// Formatting the Ninja output failed. - #[error("{message}")] - Format { - /// Underlying formatting error. - #[source] - source: fmt::Error, - /// Localized error message. - message: LocalizedMessage, - }, -} - -impl From for NinjaGenError { - fn from(source: fmt::Error) -> Self { - Self::Format { - message: localization::message(keys::NINJA_GEN_FORMAT), - source, - } - } -} +#[path = "ninja_gen_command_list.rs"] +pub(crate) mod ninja_gen_command_list; +#[path = "ninja_gen_error.rs"] +mod ninja_gen_error; +#[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_validation::validate_action_recipe; macro_rules! write_kv { ($f:expr, $key:expr, $opt:expr) => { if let Some(val) = $opt { @@ -92,8 +70,11 @@ macro_rules! write_flag { /// /// # Errors /// -/// Returns [`NinjaGenError`] if a build edge references an unknown action or -/// writing to the output fails. +/// Returns [`NinjaGenError`] if a build edge references an unknown action, a +/// programmatic action has an empty command recipe, a command-list entry starts +/// multiple background jobs, a command-list entry uses an unsupported `exec` +/// structure, a command-list `eval` payload cannot be analysed, a command-list +/// entry contains a Ninja control character, or writing to the output fails. pub fn generate(graph: &BuildGraph) -> Result { let mut out = String::new(); generate_into(graph, &mut out)?; @@ -131,11 +112,17 @@ pub fn generate(graph: &BuildGraph) -> Result { /// /// # Errors /// -/// Returns [`NinjaGenError`] if a build edge references an unknown action or writing to the output fails. +/// Returns [`NinjaGenError`] if a build edge references an unknown action, a +/// programmatic action has an empty command recipe, a command-list entry starts +/// multiple background jobs, a command-list entry uses an unsupported `exec` +/// structure, a command-list `eval` payload cannot be analysed, a command-list +/// entry contains a Ninja control character, or writing to the output fails. pub fn generate_into(graph: &BuildGraph, out: &mut W) -> Result<(), NinjaGenError> { let mut actions: Vec<_> = graph.actions.iter().collect(); actions.sort_by_key(|(id, _)| *id); - for (id, action) in actions { + 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)?; write!(out, "{}", NamedAction { id, action })?; } @@ -214,10 +201,18 @@ struct NamedAction<'a> { impl NamedAction<'_> { fn write_recipe(&self, f: &mut Formatter<'_>) -> fmt::Result { match &self.action.recipe { - Recipe::Command { command } => { - Self::assert_shell_command(command); - writeln!(f, " command = {command}") + Recipe::Command { + command: StringOrList::String(scalar_command), + } => { + Self::assert_shell_command(scalar_command); + writeln!(f, " command = {scalar_command}") } + Recipe::Command { + command: StringOrList::List(items), + } => self.write_command_list(f, items), + Recipe::Command { + command: StringOrList::Empty, + } => Self::reject_empty_command_recipe(), Recipe::Script { script } => Self::write_script_command(f, script), Recipe::Rule { .. } => Self::reject_rule_recipe(), } @@ -233,6 +228,25 @@ impl NamedAction<'_> { writeln!(f, " command = {cmd}") } + /// Write list entries as isolated current-shell groups joined by `&&`. + fn write_command_list(&self, f: &mut Formatter<'_>, items: &[String]) -> fmt::Result { + // 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 + // `( ... )`), so working directory, environment, and variables set by + // one entry still carry into the next, and the `&&` chain stays + // fail-fast. + let command_line = items + .iter() + .enumerate() + .map(|(entry_index, item)| { + command_list_entry(CommandListEntry(item), ActionId(self.id), entry_index + 1) + }) + .join(" && "); + Self::assert_shell_command(&command_line); + writeln!(f, " command = {command_line}") + } + fn write_metadata(&self, f: &mut Formatter<'_>) -> fmt::Result { write_kv!(f, "description", &self.action.description); write_kv!(f, "depfile", &self.action.depfile); @@ -266,6 +280,17 @@ impl NamedAction<'_> { } Err(fmt::Error) } + + /// Reject a command recipe that carries no entries. + /// + /// Deserialization rejects empty command recipes, so reaching here means an + /// earlier stage constructed one directly. `Display::to_string` turns the + /// returned error into a panic, so the fault still surfaces loudly without + /// a hand-rolled debug-only panic. + #[cold] + const fn reject_empty_command_recipe() -> fmt::Result { + Err(fmt::Error) + } } impl Display for NamedAction<'_> { @@ -307,94 +332,8 @@ impl Display for DisplayEdge<'_> { #[path = "ninja_gen_property_tests.rs"] mod property_tests; #[cfg(test)] -mod tests { - //! Unit tests for Ninja file generation and rule synthesis. - use super::*; - use crate::ir::{Action, BuildEdge, BuildGraph}; - use anyhow::{Result, ensure}; - use rstest::rstest; - #[rstest] - fn generate_simple_ninja() -> Result<()> { - let action = Action { - recipe: Recipe::Command { - command: "echo hi".into(), - }, - description: None, - depfile: None, - deps_format: None, - pool: None, - restat: false, - }; - let edge = BuildEdge { - action_id: "a".into(), - inputs: vec![Utf8PathBuf::from("in")], - implicit_deps: Vec::new(), - explicit_outputs: vec![Utf8PathBuf::from("out")], - implicit_outputs: Vec::new(), - order_only_deps: Vec::new(), - phony: false, - always: false, - }; - let mut graph = BuildGraph::default(); - graph.actions.insert("a".into(), action); - graph.targets.insert(Utf8PathBuf::from("out"), edge); - graph.default_targets.push(Utf8PathBuf::from("out")); - - let ninja = generate(&graph)?; - let expected = concat!( - "rule a\n", - " command = echo hi\n\n", - "build out: a in\n\n", - "default out\n" - ); - ensure!( - ninja == expected, - "expected Ninja manifest:\n{expected}\nactual:\n{ninja}" - ); - Ok(()) - } - - #[rstest] - fn generate_script_ninja_round_trips() -> Result<()> { - let script = "echo 'a b' && echo \"$HOME\" && printf %s \"`whoami`\"\n# line"; - let action = Action { - recipe: Recipe::Script { - script: script.into(), - }, - description: None, - depfile: None, - deps_format: None, - pool: None, - restat: false, - }; - let edge = BuildEdge { - action_id: "a".into(), - inputs: Vec::new(), - implicit_deps: Vec::new(), - explicit_outputs: vec![Utf8PathBuf::from("out")], - implicit_outputs: Vec::new(), - order_only_deps: Vec::new(), - phony: false, - always: false, - }; - let mut graph = BuildGraph::default(); - graph.actions.insert("a".into(), action); - graph.targets.insert(Utf8PathBuf::from("out"), edge); - - let ninja = generate(&graph)?; - ensure!(ninja.contains("rule a")); - ensure!(ninja.contains("command = /bin/sh -e -c")); - ensure!(ninja.contains("echo '\"'\"'a b'\"'\"'")); - ensure!(ninja.contains("\\\"\\$HOME\\\"")); - ensure!(ninja.contains("\\`whoami\\`")); - ensure!(ninja.contains("printf %b")); - ensure!(ninja.contains("\\n# line' | /bin/sh -e")); - Ok(()) - } - - #[test] - 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); - } -} +#[path = "ninja_gen_test_support.rs"] +mod test_support; +#[cfg(test)] +#[path = "ninja_gen_tests.rs"] +mod tests; diff --git a/src/ninja_gen_command_list.rs b/src/ninja_gen_command_list.rs new file mode 100644 index 000000000..607a41e08 --- /dev/null +++ b/src/ninja_gen_command_list.rs @@ -0,0 +1,379 @@ +//! Shell-safe rendering for ordered Ninja command-list entries. + +use sha2::{Digest, Sha256}; + +use crate::hex::to_lower_hex; + +#[path = "ninja_gen_command_list_scanner.rs"] +mod scanner; + +use scanner::background_operator_count; + +/// Prefix used to carry bounded list-entry failure attribution through Ninja. +pub(crate) const COMMAND_LIST_FAILURE_PREFIX: &str = "netsuke command-list failure: action "; + +/// A command-list entry cannot preserve the ordered execution contract. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(crate) enum CommandListEntryError { + /// An entry starts multiple background jobs. + MultipleBackgroundJobs, + /// An `exec` occurs in a shell structure the list wrapper cannot supervise. + UnsupportedExec, + /// An `eval` payload cannot be analysed for attributable background jobs. + UnanalyzableEval, + /// An entry cannot be represented safely in one Ninja command binding. + NinjaControlCharacter, +} + +/// One rendered shell command-list entry. +#[derive(Clone, Copy)] +pub(super) struct CommandListEntry<'a>(pub(super) &'a str); + +/// An internal action identifier before it is converted to a safe fingerprint. +#[derive(Clone, Copy)] +pub(super) struct ActionId<'a>(pub(super) &'a str); + +/// One shell word parsed from a command-list entry. +#[derive(Clone, Copy)] +struct ShellWord<'a>(&'a str); + +/// The shell-word sequence parsed from one command-list entry. +struct ShellWords(Vec); + +/// Signals that static inspection cannot account for an `eval` payload. +#[derive(Clone, Copy)] +struct UnanalyzableEval; + +/// Return the unsupported boundary, if any, for one command-list entry. +pub(super) fn command_list_entry_error( + command: CommandListEntry<'_>, +) -> Option { + // Manifest validation normally rejects syntax that `shlex` cannot parse, + // but programmatic IR can bypass it. Preserve the direct scan on parse + // failure: it can still prove multiple direct background jobs, while + // nested `eval` and `exec` analysis remain unavailable. + let direct_background_jobs = background_operator_count(command); + if command.has_ninja_control_character() { + Some(CommandListEntryError::NinjaControlCharacter) + } else if let Some(words) = ShellWords::parse(command) { + let Ok(nested_jobs) = words.background_job_count() else { + return Some(CommandListEntryError::UnanalyzableEval); + }; + if direct_background_jobs + .checked_add(nested_jobs) + .is_none_or(|background_jobs| background_jobs > 1) + { + Some(CommandListEntryError::MultipleBackgroundJobs) + } else if exec_boundary(command) == ExecBoundary::Unsupported { + Some(CommandListEntryError::UnsupportedExec) + } else { + None + } + } else if direct_background_jobs > 1 { + Some(CommandListEntryError::MultipleBackgroundJobs) + } else if exec_boundary(command) == ExecBoundary::Unsupported { + Some(CommandListEntryError::UnsupportedExec) + } else { + None + } +} + +/// Render one entry so it fails atomically without exposing command content. +/// +/// Brace groups deliberately run in the current shell, so the EXIT trap must +/// be cleared on both the success and failure paths before leaving the group. +/// `$$!` records only the latest background PID: validation rejects entries +/// with multiple or dynamically generated background jobs before rendering. +/// The `_netsuke_*` variables are reserved because user assignments to them +/// can corrupt status propagation or failure attribution. Finally, a direct +/// successful `exec` sets `_netsuke_exec_succeeded=1` and exits with status +/// zero, preserving process replacement by preventing later entries from +/// running. +pub(super) fn command_list_entry( + command: CommandListEntry<'_>, + action_id: ActionId<'_>, + entry_index: usize, +) -> String { + let identity = action_identity(action_id); + let context = format!("{COMMAND_LIST_FAILURE_PREFIX}{identity}, entry {entry_index}"); + let evaluator = command_evaluator(command); + format!( + concat!( + "{{ _netsuke_background_before=$${{!:-}}; _netsuke_exec_succeeded=0; ", + "trap '_netsuke_command_status=$$?; printf \"%s\\n\" \"{}\" >&2; ", + "trap - EXIT; exit \"$$_netsuke_command_status\"' EXIT; ", + "if {}; then _netsuke_command_status=0;{} else _netsuke_command_status=$$?; fi; ", + "_netsuke_background_after=$${{!:-}}; ", + "if [ -n \"$$_netsuke_background_after\" ] && ", + "[ \"$$_netsuke_background_after\" != \"$$_netsuke_background_before\" ]; then ", + "if wait \"$$_netsuke_background_after\"; then :; ", + "else _netsuke_background_status=$$?; ", + "if [ \"$$_netsuke_command_status\" -eq 0 ]; then ", + "_netsuke_command_status=$$_netsuke_background_status; fi; fi; fi; ", + "if [ \"$$_netsuke_command_status\" -eq 0 ]; then trap - EXIT; ", + "if [ \"$$_netsuke_exec_succeeded\" -eq 1 ]; then exit 0; else :; fi; ", + "else trap - EXIT; printf '%s\\n' '{}' >&2; ", + "exit \"$$_netsuke_command_status\"; fi; }}" + ), + context, evaluator.shell_expression, evaluator.exec_success_fragment, context, + ) +} + +/// Evaluate a supported direct `exec` in a retaining subshell. +/// +/// A direct `exec` replaces its subshell, allowing the brace group to observe +/// its status. A successful replacement then exits the command chain without +/// emitting a marker, as an in-shell `exec` would. +struct CommandEvaluator { + /// Shell expression that evaluates one list entry. + shell_expression: String, + /// Fragment that records a successful retaining-subshell `exec`. + exec_success_fragment: &'static str, +} + +fn command_evaluator(command: CommandListEntry<'_>) -> CommandEvaluator { + let quoted = shell_single_quote(command); + if exec_boundary(command) == ExecBoundary::Direct { + CommandEvaluator { + shell_expression: format!("(eval {quoted})"), + exec_success_fragment: " _netsuke_exec_succeeded=1;", + } + } else { + CommandEvaluator { + shell_expression: format!("eval {quoted}"), + exec_success_fragment: "", + } + } +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum ExecBoundary { + /// The entry does not contain `exec` in a shell command position. + None, + /// `exec` is the entry's first simple command after leading assignments. + Direct, + /// `exec` occurs in a later or wrapped command position the wrapper cannot supervise. + Unsupported, +} + +/// Classify `exec` only when it begins a simple command after assignments. +fn exec_boundary(command: CommandListEntry<'_>) -> ExecBoundary { + ShellWords::parse(command).map_or(ExecBoundary::None, |words| words.exec_boundary()) +} + +impl ShellWords { + /// Parse the shell words that make up one command-list entry. + fn parse(command: CommandListEntry<'_>) -> Option { + shlex::split(command.0).map(Self) + } + + /// Classify `exec` only when it begins a simple command after assignments. + fn exec_boundary(&self) -> ExecBoundary { + let direct_index = self.first_non_assignment_index(); + self.0 + .iter() + .map(|word| ShellWord(word)) + .enumerate() + .find_map(|(index, word)| self.exec_boundary_at(index, word, direct_index)) + .unwrap_or(ExecBoundary::None) + } + + fn exec_boundary_at( + &self, + index: usize, + word: ShellWord<'_>, + direct_index: Option, + ) -> Option { + if !word.is_exec() { + return None; + } + if Some(index) == direct_index { + return Some(ExecBoundary::Direct); + } + (self.is_command_word(index) || self.is_exec_wrapper(index)) + .then_some(ExecBoundary::Unsupported) + } + + /// Return the index of the first word that is not a leading assignment. + fn first_non_assignment_index(&self) -> Option { + self.0 + .iter() + .position(|word| !ShellWord(word).is_assignment()) + } + + /// Return whether this word begins a simple shell command. + fn is_command_word(&self, index: usize) -> bool { + let Some(words_before) = self.0.get(..index) else { + return false; + }; + let preceding_word = words_before + .iter() + .rev() + .find(|word| !ShellWord(word).is_assignment()); + preceding_word.is_none_or(|word| ShellWord(word).ends_command()) + } + + /// Return whether `command exec` wraps a process-replacing built-in. + fn is_exec_wrapper(&self, index: usize) -> bool { + let is_wrapper = index + .checked_sub(1) + .and_then(|previous_index| self.0.get(previous_index)) + .is_some_and(|word| ShellWord(word).is_exec_wrapper()); + is_wrapper + && index + .checked_sub(1) + .is_some_and(|previous| self.is_command_word(previous)) + } + + /// Count background jobs launched by the entry, including static nested + /// `eval` payloads. An error means an `eval` payload cannot be analysed + /// without potentially hiding background jobs. + fn background_job_count(&self) -> Result { + self.background_job_count_at_depth(0) + } + + fn background_job_count_at_depth(&self, depth: usize) -> Result { + self.0 + .iter() + .map(|word| ShellWord(word)) + .enumerate() + .filter(|(index, word)| word.is_eval() && self.is_command_word(*index)) + .try_fold(0_usize, |count, (index, _)| { + count + .checked_add(self.background_jobs_from_eval(index, depth)?) + .ok_or(UnanalyzableEval) + }) + } + + fn background_jobs_from_eval( + &self, + index: usize, + depth: usize, + ) -> Result { + const MAX_EVAL_NESTING: usize = 16; + if depth == MAX_EVAL_NESTING { + return Err(UnanalyzableEval); + } + let source = self.eval_source(index); + if source.is_empty() { + return Ok(0); + } + if ShellWord(&source).has_dynamic_expansion() { + return Err(UnanalyzableEval); + } + let nested = CommandListEntry(&source); + background_operator_count(nested) + .checked_add( + Self::parse(nested) + .ok_or(UnanalyzableEval)? + .background_job_count_at_depth(depth + 1)?, + ) + .ok_or(UnanalyzableEval) + } + + /// Reconstruct the static words that the `eval` command will evaluate. + fn eval_source(&self, index: usize) -> String { + index + .checked_add(1) + .and_then(|first_argument| self.0.get(first_argument..)) + .unwrap_or_default() + .iter() + .take_while(|word| !ShellWord(word).is_list_operator()) + .cloned() + .collect::>() + .join(" ") + } +} + +impl ShellWord<'_> { + /// Whether this word is `exec`. + fn is_exec(self) -> bool { + self.0 == "exec" + } + + /// Whether this word invokes `eval` as a simple shell command. + fn is_eval(self) -> bool { + self.0 == "eval" + } + + /// Whether this word can invoke `exec` outside the direct supported boundary. + fn is_exec_wrapper(self) -> bool { + matches!(self.0, "if" | "command") + } + + /// Whether this word ends one simple command and starts another. + fn ends_command(self) -> bool { + matches!( + self.0, + "&&" | "||" + | "|" + | "&" + | "(" + | "{" + | "if" + | "then" + | "do" + | "else" + | "elif" + | "while" + | "until" + ) || self.0.ends_with(';') + || self.0.ends_with(')') + } + + /// Whether this word terminates an `eval` command's argument sequence. + fn is_list_operator(self) -> bool { + matches!(self.0, "&&" | "||" | "|" | "&" | ";") || self.0.ends_with(';') + } + + /// Whether this shell source can expand into arbitrary syntax at runtime. + fn has_dynamic_expansion(self) -> bool { + self.0 + .chars() + .any(|character| matches!(character, '$' | '`' | '*' | '?' | '[')) + } + + /// Whether this word is a valid POSIX shell assignment word. + fn is_assignment(self) -> bool { + let Some((name, _)) = self.0.split_once('=') else { + return false; + }; + let mut chars = name.chars(); + chars + .next() + .is_some_and(|first| first == '_' || first.is_ascii_alphabetic()) + && chars.all(|character| character == '_' || character.is_ascii_alphanumeric()) + } +} + +impl CommandListEntry<'_> { + /// Whether this entry contains a control character Ninja cannot retain in + /// one `command =` binding. + fn has_ninja_control_character(self) -> bool { + self.0.chars().any(char::is_control) + } +} + +/// Return a fixed-width fingerprint for an action identifier. +/// +/// IR-generated identifiers are already hashes, but hashing again prevents a +/// programmatically supplied identifier from disclosing arbitrary content. +fn action_identity(action_id: ActionId<'_>) -> String { + to_lower_hex(&Sha256::digest(action_id.0.as_bytes())) +} + +/// Quote `value` as one literal POSIX shell argument. +/// +/// The command-list renderer passes each entry to `eval` so an inline comment +/// or trailing control operator cannot consume the brace-group terminator. +fn shell_single_quote(command: CommandListEntry<'_>) -> String { + // `shell_quote::QuoteRefExt::quoted(Sh)` produces minimally quoted + // fragments, while the `eval` wrapper requires this canonical enclosing + // form to preserve its generated Ninja text and delimiter contract. + let escaped = command.0.replace('\'', r"'\''"); + format!("'{escaped}'") +} + +#[cfg(test)] +#[path = "ninja_gen_command_list_tests.rs"] +mod tests; diff --git a/src/ninja_gen_command_list_scanner.rs b/src/ninja_gen_command_list_scanner.rs new file mode 100644 index 000000000..dc350e8df --- /dev/null +++ b/src/ninja_gen_command_list_scanner.rs @@ -0,0 +1,124 @@ +//! Lexical detection of command-list background operators. + +use super::CommandListEntry; + +/// Count unquoted background operators without mistaking `&&` for one. +pub(super) fn background_operator_count(command: CommandListEntry<'_>) -> usize { + let mut state = ShellScanState::new(); + let mut count = 0; + let mut characters = command.0.chars().peekable(); + while let Some(character) = characters.next() { + if state.consume_escaped() { + continue; + } + if state.consume_quoted(character) { + continue; + } + if state.starts_comment(character) { + break; + } + count += state.count_unquoted_background_operator(character, &mut characters); + } + count +} + +/// Minimal shell scanner state used only to detect background operators. +struct ShellScanState { + quote: Option, + escaped: bool, + word_boundary: bool, + pending_redirection_ampersand: bool, +} + +impl ShellScanState { + const fn new() -> Self { + Self { + quote: None, + escaped: false, + word_boundary: true, + pending_redirection_ampersand: false, + } + } + + const fn consume_escaped(&mut self) -> bool { + if self.escaped { + self.escaped = false; + self.word_boundary = false; + true + } else { + false + } + } + + const fn consume_quoted(&mut self, character: char) -> bool { + let Some(delimiter) = self.quote else { + return false; + }; + if character == delimiter { + self.quote = None; + } else if character == '\\' && delimiter == '"' { + self.escaped = true; + } + self.word_boundary = false; + true + } + + const fn starts_comment(&self, character: char) -> bool { + character == '#' && self.word_boundary + } + + /// Count one unquoted background operator and advance this scanner state. + fn count_unquoted_background_operator( + &mut self, + character: char, + characters: &mut std::iter::Peekable>, + ) -> usize { + match character { + '\\' => { + self.escaped = true; + 0 + } + '\'' | '"' => { + self.quote = Some(character); + self.word_boundary = false; + 0 + } + '&' if characters.peek() == Some(&'&') => { + characters.next(); + self.pending_redirection_ampersand = false; + self.word_boundary = true; + 0 + } + '&' if self.pending_redirection_ampersand => { + self.pending_redirection_ampersand = false; + self.word_boundary = false; + 0 + } + '&' => { + self.pending_redirection_ampersand = false; + self.word_boundary = true; + 1 + } + '<' | '>' => { + self.pending_redirection_ampersand = true; + self.word_boundary = true; + 0 + } + ';' | '|' | '(' | ')' => { + self.pending_redirection_ampersand = false; + self.word_boundary = true; + 0 + } + whitespace if whitespace.is_whitespace() => { + self.pending_redirection_ampersand = false; + self.word_boundary = true; + 0 + } + _ => { + self.pending_redirection_ampersand = false; + self.word_boundary = false; + 0 + } + } + } +} diff --git a/src/ninja_gen_command_list_tests.rs b/src/ninja_gen_command_list_tests.rs new file mode 100644 index 000000000..fc15cb206 --- /dev/null +++ b/src/ninja_gen_command_list_tests.rs @@ -0,0 +1,92 @@ +//! Unit tests for private command-list shell boundaries. + +use super::{ + ActionId, CommandListEntry, CommandListEntryError, ExecBoundary, action_identity, + background_operator_count, command_list_entry, command_list_entry_error, exec_boundary, + shell_single_quote, +}; +use rstest::rstest; + +#[rstest] +#[case::direct_assignment_prefixed("FOO=1 exec false", ExecBoundary::Direct)] +#[case::conditional_body("if true; then exec false; fi", ExecBoundary::Unsupported)] +#[case::loop_body("while true; do exec false; done", ExecBoundary::Unsupported)] +#[case::case_body("case x in x) exec false;; esac", ExecBoundary::Unsupported)] +#[case::and_list("true && exec false", ExecBoundary::Unsupported)] +#[case::argument("echo exec", ExecBoundary::None)] +#[case::printf_argument("printf '%s' exec", ExecBoundary::None)] +#[case::command_wrapper("command exec false", ExecBoundary::Unsupported)] +fn classifies_direct_and_unsupported_exec_entries( + #[case] command: &str, + #[case] expected: ExecBoundary, +) { + assert_eq!(exec_boundary(CommandListEntry(command)), expected); +} + +#[rstest] +#[case::single_background("sleep 1 &", 1)] +#[case::multiple_backgrounds("sleep 1 & true &", 2)] +#[case::quoted_and_comment("echo '&' # &", 0)] +#[case::redirect_then_background("cmd 2>&1 &", 1)] +#[case::two_output_redirects("cmd 2>&1 1>&2", 0)] +#[case::output_redirect("cmd 1>&2", 0)] +fn counts_only_unquoted_background_operators_before_comments( + #[case] command: &str, + #[case] expected: usize, +) { + assert_eq!( + background_operator_count(CommandListEntry(command)), + expected + ); +} + +#[rstest] +#[case::single_static_eval_job("eval 'true &'", None)] +#[case::nested_multiple_jobs( + "eval 'false & true &'", + Some(CommandListEntryError::MultipleBackgroundJobs) +)] +#[case::nested_and_outer_job( + "eval 'true &' &", + Some(CommandListEntryError::MultipleBackgroundJobs) +)] +#[case::unsupported_exec( + "if true; then exec false; fi", + Some(CommandListEntryError::UnsupportedExec) +)] +#[case::dynamic_eval_source("eval '$jobs'", Some(CommandListEntryError::UnanalyzableEval))] +#[case::glob_eval_source("eval 'cp *.c build/'", Some(CommandListEntryError::UnanalyzableEval))] +#[case::variable_eval_source( + "eval \"$CC -c main.c\"", + Some(CommandListEntryError::UnanalyzableEval) +)] +fn rejects_unattributable_eval_background_jobs( + #[case] command: &str, + #[case] expected: Option, +) { + assert_eq!( + command_list_entry_error(CommandListEntry(command)), + expected + ); +} + +#[test] +fn shell_quotes_each_entry_as_one_literal_argument() { + assert_eq!( + shell_single_quote(CommandListEntry("echo 'quoted'")), + "'echo '\\''quoted'\\'''" + ); +} + +#[test] +fn rendered_entry_uses_a_hashed_action_identity_and_one_based_index() { + let rendered = command_list_entry(CommandListEntry("false"), ActionId("example"), 3); + let expected_identity = "50d858e0985ecc7f60418aaf0cc5ab587f42c2570a884095a9e8ccacd0f6545c"; + assert_eq!(action_identity(ActionId("example")), expected_identity); + assert!( + rendered.contains(&format!( + "netsuke command-list failure: action {expected_identity}, entry 3" + )), + "entry must use the hashed identity and its one-based index: {rendered}" + ); +} diff --git a/src/ninja_gen_error.rs b/src/ninja_gen_error.rs new file mode 100644 index 000000000..714a195eb --- /dev/null +++ b/src/ninja_gen_error.rs @@ -0,0 +1,86 @@ +//! Errors produced while rendering Ninja manifests. + +use crate::localization::{self, LocalizedMessage, keys}; +use std::fmt; +use thiserror::Error; + +/// Errors produced while rendering Ninja manifests. +#[derive(Debug, Error)] +pub enum NinjaGenError { + /// The build graph referenced an action that was not defined. + #[error("{message}")] + MissingAction { + /// Identifier of the missing action referenced by a build edge. + id: String, + /// Localized error message. + message: LocalizedMessage, + }, + /// An action built outside manifest deserialization has no command entries. + #[error("command-list action {action_index} has no command entries")] + EmptyCommandRecipe { + /// One-based stable position in generated action order. + action_index: usize, + }, + /// A list entry starts multiple background jobs, which cannot be + /// attributed reliably by a shared POSIX shell. + #[error( + "command-list action {action_index}, entry {entry_index} has unsupported background jobs" + )] + MultipleBackgroundJobs { + /// One-based stable position in generated action order. + action_index: usize, + /// One-based stable position in the command list. + entry_index: usize, + }, + /// A list entry uses `exec` in a shell structure the wrapper cannot + /// supervise without changing its semantics. + #[error( + "command-list action {action_index}, entry {entry_index} has unsupported exec structure" + )] + UnsupportedCommandListExec { + /// One-based stable position in generated action order. + action_index: usize, + /// One-based stable position in the command list. + entry_index: usize, + }, + /// A list entry contains a dynamic `eval` payload whose background jobs + /// cannot be attributed reliably. + #[error( + "command-list action {action_index}, entry {entry_index} has an unanalyzable eval payload" + )] + UnanalyzableCommandListEval { + /// One-based stable position in generated action order. + action_index: usize, + /// One-based stable position in the command list. + entry_index: usize, + }, + /// A list entry contains a control character that cannot be serialized in + /// one Ninja command binding. + #[error( + "command-list action {action_index}, entry {entry_index} contains an unsafe Ninja control character" + )] + NinjaControlCharacter { + /// One-based stable position in generated action order. + action_index: usize, + /// One-based stable position in the command list. + entry_index: usize, + }, + /// Formatting the Ninja output failed. + #[error("{message}")] + Format { + /// Underlying formatting error. + #[source] + source: fmt::Error, + /// Localized error message. + message: LocalizedMessage, + }, +} + +impl From for NinjaGenError { + fn from(source: fmt::Error) -> Self { + Self::Format { + message: localization::message(keys::NINJA_GEN_FORMAT), + source, + } + } +} diff --git a/src/ninja_gen_property_tests.rs b/src/ninja_gen_property_tests.rs index fb3c2d8d7..facb02e57 100644 --- a/src/ninja_gen_property_tests.rs +++ b/src/ninja_gen_property_tests.rs @@ -8,8 +8,11 @@ use proptest::prelude::*; use test_support::ninja_gen::paths_strategy; -use super::DisplayEdge; -use crate::ir::BuildEdge; +use super::{DisplayEdge, NinjaGenError, generate, test_support::command_action}; +use crate::{ + ast::StringOrList, + ir::{BuildEdge, BuildGraph}, +}; fn edge_strategy_with_ranges( input_range: std::ops::Range, @@ -67,6 +70,46 @@ fn bare_pipe_position(line: &str) -> Option { line.match_indices(" | ").map(|(index, _)| index).next() } +/// Build the one-action graph used by command recipe generation properties. +fn command_graph(recipe: StringOrList) -> BuildGraph { + let mut graph = BuildGraph::default(); + graph + .actions + .insert("action".into(), command_action(recipe)); + graph +} + +fn command_list_graph(entries: &[String]) -> BuildGraph { + command_graph(StringOrList::List( + entries + .iter() + .map(|entry| format!("echo {entry}")) + .collect(), + )) +} + +fn scalar_graph(command: String) -> BuildGraph { + command_graph(StringOrList::String(command)) +} + +fn command_list_entry_strategy() -> impl Strategy { + prop_oneof![ + Just("plain"), + Just("two words"), + Just("apostrophe's"), + Just("dollar$value"), + Just("hash # comment"), + Just("semi;colon"), + Just("double\"quote"), + Just("parentheses()"), + ] + .prop_map(str::to_owned) +} + +fn canonical_shell_single_quote(value: &str) -> String { + format!("'{}'", value.replace('\'', r"'\''")) +} + proptest! { #[test] fn implicit_deps_separator_precedes_order_only_separator(edge in edge_strategy_with_ranges(1..5, 1..5, 1..5)) { @@ -100,4 +143,57 @@ proptest! { prop_assert!(bare_pipe_position(deps).is_none()); prop_assert!(deps.contains(" || ")); } + + #[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 command_line = ninja.lines().find(|line| line.starts_with(" command = ")) + .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_count = entries.iter().filter(|candidate| *candidate == entry).count(); + prop_assert_eq!( + command_line.matches(&expected_entry).count(), + expected_count, + "every entry should retain one independently quoted evaluator" + ); + let position = command_line + .get(previous..) + .and_then(|remaining| remaining.find(&expected_entry)) + .expect("entries should retain their declaration order"); + previous += position + expected_entry.len(); + } + prop_assert_eq!( + command_line + .matches("{ _netsuke_background_before=$${!:-};") + .count(), + entries.len() + ); + prop_assert_eq!(command_line.matches("} && {").count(), entries.len() - 1); + } + + #[test] + fn scalar_command_output_retains_the_preexisting_form(command in "echo [a-z]{1,12}") { + let ninja = generate(&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::(), + ) { + let graph = command_graph(if use_empty_list { + StringOrList::List(Vec::new()) + } else { + StringOrList::Empty + }); + let error = generate(&graph).expect_err("empty command recipe should be rejected"); + let is_stable_empty_recipe_error = matches!(error, NinjaGenError::EmptyCommandRecipe { action_index: 1 }); + prop_assert!(is_stable_empty_recipe_error); + } } diff --git a/src/ninja_gen_test_support.rs b/src/ninja_gen_test_support.rs new file mode 100644 index 000000000..baf156f69 --- /dev/null +++ b/src/ninja_gen_test_support.rs @@ -0,0 +1,18 @@ +//! Shared test constructors for Ninja generation modules. + +use crate::{ + ast::{Recipe, StringOrList}, + ir::Action, +}; + +/// Construct a command action with the stable default metadata used in tests. +pub(super) const fn command_action(command: StringOrList) -> Action { + Action { + recipe: Recipe::Command { command }, + description: None, + depfile: None, + deps_format: None, + pool: None, + restat: false, + } +} diff --git a/src/ninja_gen_tests.rs b/src/ninja_gen_tests.rs new file mode 100644 index 000000000..1844ca094 --- /dev/null +++ b/src/ninja_gen_tests.rs @@ -0,0 +1,230 @@ +//! Unit tests for Ninja file generation and rule synthesis. + +use super::test_support::command_action; +use super::*; +use crate::ir::{Action, BuildEdge, BuildGraph}; +use anyhow::{Result, ensure}; +use rstest::rstest; + +#[rstest] +fn generate_simple_ninja() -> Result<()> { + let action = Action { + recipe: Recipe::Command { + command: "echo hi".into(), + }, + description: None, + depfile: None, + deps_format: None, + pool: None, + restat: false, + }; + let edge = BuildEdge { + action_id: "a".into(), + inputs: vec![Utf8PathBuf::from("in")], + implicit_deps: Vec::new(), + explicit_outputs: vec![Utf8PathBuf::from("out")], + implicit_outputs: Vec::new(), + order_only_deps: Vec::new(), + phony: false, + always: false, + }; + let mut graph = BuildGraph::default(); + graph.actions.insert("a".into(), action); + graph.targets.insert(Utf8PathBuf::from("out"), edge); + graph.default_targets.push(Utf8PathBuf::from("out")); + + let ninja = generate(&graph)?; + let expected = concat!( + "rule a\n", + " command = echo hi\n\n", + "build out: a in\n\n", + "default out\n" + ); + ensure!( + ninja == expected, + "expected Ninja manifest:\n{expected}\nactual:\n{ninja}" + ); + Ok(()) +} + +#[rstest] +fn generate_script_ninja_round_trips() -> Result<()> { + let script = "echo 'a b' && echo \"$HOME\" && printf %s \"`whoami`\"\n# line"; + let action = Action { + recipe: Recipe::Script { + script: script.into(), + }, + description: None, + depfile: None, + deps_format: None, + pool: None, + restat: false, + }; + let edge = BuildEdge { + action_id: "a".into(), + inputs: Vec::new(), + implicit_deps: Vec::new(), + explicit_outputs: vec![Utf8PathBuf::from("out")], + implicit_outputs: Vec::new(), + order_only_deps: Vec::new(), + phony: false, + always: false, + }; + let mut graph = BuildGraph::default(); + graph.actions.insert("a".into(), action); + graph.targets.insert(Utf8PathBuf::from("out"), edge); + + let ninja = generate(&graph)?; + ensure!(ninja.contains("rule a")); + ensure!(ninja.contains("command = /bin/sh -e -c")); + ensure!(ninja.contains("echo '\"'\"'a b'\"'\"'")); + ensure!(ninja.contains("\\\"\\$HOME\\\"")); + ensure!(ninja.contains("\\`whoami\\`")); + ensure!(ninja.contains("printf %b")); + ensure!(ninja.contains("\\n# line' | /bin/sh -e")); + Ok(()) +} + +#[rstest] +fn generate_command_list_ninja_joins_a_fail_fast_chain() -> Result<()> { + let action = command_action(StringOrList::List(vec![ + "echo one".into(), + "echo two".into(), + "echo three".into(), + ])); + let edge = BuildEdge { + action_id: "a".into(), + inputs: Vec::new(), + implicit_deps: Vec::new(), + explicit_outputs: vec![Utf8PathBuf::from("out")], + implicit_outputs: Vec::new(), + order_only_deps: Vec::new(), + phony: false, + always: false, + }; + let mut graph = BuildGraph::default(); + graph.actions.insert("a".into(), action); + graph.targets.insert(Utf8PathBuf::from("out"), edge); + + let ninja = generate(&graph)?; + ensure!( + ninja.contains("command = { _netsuke_background_before=$${!:-};"), + "first list boundary should start the generated command:\n{ninja}" + ); + ensure!( + ninja.contains("if eval 'echo one'"), + "first command should retain its evaluator:\n{ninja}" + ); + ensure!( + ninja.contains("if eval 'echo two'"), + "second command should retain its evaluator:\n{ninja}" + ); + ensure!( + ninja.contains("if eval 'echo three'"), + "third command should retain its evaluator:\n{ninja}" + ); + ensure!( + ninja.contains("if wait \"$$_netsuke_background_after\"; then :;"), + "list boundary should wait for its one supported background job:\n{ninja}" + ); + ensure!( + ninja.matches("} && {").count() == 2, + "three list boundaries should be joined by exactly two && operators:\n{ninja}" + ); + Ok(()) +} + +#[rstest] +#[case::empty(StringOrList::Empty)] +#[case::empty_list(StringOrList::List(Vec::new()))] +fn programmatic_empty_command_recipe_returns_a_typed_generation_error( + #[case] command: StringOrList, +) { + let action = command_action(command); + let mut graph = BuildGraph::default(); + graph.actions.insert("empty".into(), action); + + let error = generate(&graph).expect_err("empty command recipe should not generate Ninja"); + assert!( + matches!(error, NinjaGenError::EmptyCommandRecipe { action_index: 1 }), + "empty command recipe should produce the stable typed error, got {error:?}" + ); +} + +#[test] +fn nested_command_list_exec_returns_a_typed_generation_error() { + let action = command_action(StringOrList::List(vec![ + "if true; then exec false; fi".into(), + ])); + let mut graph = BuildGraph::default(); + graph.actions.insert("nested-exec".into(), action); + + let error = generate(&graph).expect_err("nested exec should not generate Ninja"); + assert!( + matches!( + error, + NinjaGenError::UnsupportedCommandListExec { + action_index: 1, + entry_index: 1, + } + ), + "nested exec should produce the stable typed error, got {error:?}" + ); +} + +#[rstest] +#[case::dynamic_eval( + "eval '$jobs'", + NinjaGenError::UnanalyzableCommandListEval { + action_index: 1, + entry_index: 1, + } +)] +#[case::newline( + "echo safe\nbuild injected: phony", + NinjaGenError::NinjaControlCharacter { + action_index: 1, + entry_index: 1, + } +)] +fn unsafe_command_list_entries_return_typed_generation_errors( + #[case] entry: &str, + #[case] expected: NinjaGenError, +) { + let action = command_action(StringOrList::List(vec![entry.into()])); + let mut graph = BuildGraph::default(); + graph.actions.insert("unsafe".into(), action); + let mut ninja = String::new(); + + let error = generate_into(&graph, &mut ninja) + .expect_err("unsafe command-list entries should not generate Ninja"); + assert!( + matches!( + (error, expected), + ( + NinjaGenError::UnanalyzableCommandListEval { + action_index: 1, + entry_index: 1, + }, + NinjaGenError::UnanalyzableCommandListEval { .. } + ) | ( + NinjaGenError::NinjaControlCharacter { + action_index: 1, + entry_index: 1, + }, + NinjaGenError::NinjaControlCharacter { .. } + ) + ), + "unsafe command-list entry should return its stable typed error" + ); + assert!( + ninja.is_empty(), + "validation must reject the entry before it can inject Ninja output: {ninja}" + ); +} + +#[test] +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); +} diff --git a/src/ninja_gen_validation.rs b/src/ninja_gen_validation.rs new file mode 100644 index 000000000..05fb507e8 --- /dev/null +++ b/src/ninja_gen_validation.rs @@ -0,0 +1,55 @@ +//! Validation for command-list boundaries before Ninja rendering. + +use super::NinjaGenError; +use super::ninja_gen_command_list::{ + CommandListEntry, CommandListEntryError, command_list_entry_error, +}; +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, +) -> 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 + { + 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/src/runner/process/child_exit.rs b/src/runner/process/child_exit.rs new file mode 100644 index 000000000..4f70da4fe --- /dev/null +++ b/src/runner/process/child_exit.rs @@ -0,0 +1,103 @@ +//! Child-process shutdown and Ninja non-zero exit conversion helpers. + +use monotony::MonotonicClock; +use std::{ + io, + process::{Child, ExitStatus}, + thread, + time::Instant, +}; + +use super::{ + command_list_telemetry, + command_logging::{CommandLogContext, log_command_exit_failure}, + failure_attribution::CommandListFailure, + streaming::ForwardStats, +}; + +/// Context retained until the child process has completed. +#[derive(Clone, Copy)] +pub(super) struct ExitFailureContext<'failure, 'clock, Clock> { + pub(super) operation: &'failure str, + pub(super) suppress_stderr: bool, + pub(super) command_list_failure: Option<&'failure CommandListFailure>, + pub(super) clock: &'clock Clock, + pub(super) started_at: Instant, +} + +/// Return a child-process failure after recording any bounded command-list context. +pub(super) fn check_exit_status_with_context( + status: ExitStatus, + context: &CommandLogContext, + failure_context: &ExitFailureContext<'_, '_, Clock>, +) -> io::Result<()> { + if status.success() { + Ok(()) + } else { + tracing::Span::current().record("failure_category", "exit_status"); + log_command_exit_failure( + context, + failure_context.operation, + failure_context.suppress_stderr, + status, + ); + if let Some(failure) = failure_context.command_list_failure { + command_list_telemetry::record_failure( + failure, + failure_context + .clock + .now() + .duration_since(failure_context.started_at), + ); + } + ninja_exit_error(status, failure_context.command_list_failure) + } +} + +/// Terminate a partially configured child and reap it before returning an error. +pub(super) fn terminate_child(child: &mut Child, context: &str) { + if let Err(error) = child.kill() { + tracing::debug!("failed to kill child after {context}: {error}"); + } + if let Err(error) = child.wait() { + tracing::debug!("failed to reap child after {context}: {error}"); + } +} + +/// Convert a Ninja exit status into an error with optional bounded attribution. +pub(super) fn ninja_exit_error( + status: ExitStatus, + command_list_failure: Option<&CommandListFailure>, +) -> io::Result<()> { + let message = command_list_failure.map_or_else( + || format!("ninja exited with {status}"), + |failure| format!("ninja exited with {status}: {failure}"), + ); + Err(io::Error::other(message)) +} + +/// Join stderr forwarding and surface the child's wait result. +pub(super) fn finalize_streaming( + wait_result: io::Result, + stdout_stats: ForwardStats, + err_handle: thread::JoinHandle<(ForwardStats, Option)>, +) -> io::Result<(ExitStatus, Option)> { + handle_forwarding_stats(stdout_stats, "stdout"); + let command_list_failure = match err_handle.join() { + Ok((stats, context)) => { + handle_forwarding_stats(stats, "stderr"); + context + } + Err(error) => { + tracing::warn!("stderr forwarding thread panicked: {error:?}"); + None + } + }; + wait_result.map(|status| (status, command_list_failure)) +} + +fn handle_forwarding_stats(stats: ForwardStats, stream_name: &str) { + if stats.write_failed { + tracing::debug!("{stream_name} forwarding encountered closed pipe; output truncated"); + } +} diff --git a/src/runner/process/command_list_telemetry.rs b/src/runner/process/command_list_telemetry.rs new file mode 100644 index 000000000..c66d9b6e5 --- /dev/null +++ b/src/runner/process/command_list_telemetry.rs @@ -0,0 +1,84 @@ +//! Bounded metrics and tracing for attributed command-list failures. + +use super::failure_attribution::CommandListFailure; +use metrics::{counter, describe_counter, describe_histogram, histogram}; +use std::{sync::Once, time::Duration}; + +const COMMAND_LIST_FAILURES_TOTAL: &str = "netsuke_ninja_command_list_failures_total"; +pub(super) const COMMAND_LIST_FAILURE_DURATION: &str = + "netsuke_ninja_command_list_failure_duration_seconds"; + +/// Record the only observable per-entry outcome: a safely attributed failure. +pub(super) fn record_failure(failure: &CommandListFailure, elapsed: Duration) { + describe_metrics(); + tracing::warn!( + command_list_action = failure.action_identity(), + command_list_entry = failure.entry_index(), + command_list_failure = %failure, + "Ninja command-list entry failed" + ); + counter!(COMMAND_LIST_FAILURES_TOTAL, "outcome" => "failure").increment(1); + histogram!(COMMAND_LIST_FAILURE_DURATION, "outcome" => "failure").record(elapsed); +} + +fn describe_metrics() { + static DESCRIBE: Once = Once::new(); + DESCRIBE.call_once(|| { + describe_counter!( + COMMAND_LIST_FAILURES_TOTAL, + "Counts attributed Ninja command-list entry failures." + ); + describe_histogram!( + COMMAND_LIST_FAILURE_DURATION, + "Measures elapsed Ninja build time before an attributed command-list failure." + ); + }); +} + +#[cfg(test)] +mod tests { + //! Metric contracts for bounded command-list failure telemetry. + + use super::*; + use crate::runner::process::failure_attribution::FailureAttributionWriter; + use metrics_util::{ + MetricKind, + debugging::{DebugValue, DebuggingRecorder}, + }; + use std::io::Write; + + #[test] + fn attributed_failure_records_bounded_outcome_and_duration() { + let mut writer = FailureAttributionWriter::new(Vec::new()); + writer + .write_all( + concat!( + "netsuke command-list failure: action ", + "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef, entry 2\n" + ) + .as_bytes(), + ) + .expect("marker should parse"); + let failure = writer + .into_failure() + .expect("marker should produce attribution"); + let recorder = DebuggingRecorder::new(); + let snapshotter = recorder.snapshotter(); + metrics::with_local_recorder(&recorder, || { + record_failure(&failure, Duration::from_millis(1)); + }); + let snapshot = snapshotter.snapshot().into_vec(); + let has_counter = snapshot.iter().any(|(key, _, _, value)| { + key.kind() == MetricKind::Counter + && key.key().name() == COMMAND_LIST_FAILURES_TOTAL + && matches!(value, DebugValue::Counter(1)) + }); + let has_duration = snapshot.iter().any(|(key, _, _, value)| { + key.kind() == MetricKind::Histogram + && key.key().name() == COMMAND_LIST_FAILURE_DURATION + && matches!(value, DebugValue::Histogram(samples) if samples.len() == 1) + }); + assert!(has_counter, "failure counter should record exactly once"); + assert!(has_duration, "failure duration should record one sample"); + } +} diff --git a/src/runner/process/failure_attribution.rs b/src/runner/process/failure_attribution.rs new file mode 100644 index 000000000..940abfbe5 --- /dev/null +++ b/src/runner/process/failure_attribution.rs @@ -0,0 +1,285 @@ +//! Bounded extraction of command-list failure attribution from Ninja output. + +use crate::ninja_gen::ninja_gen_command_list::COMMAND_LIST_FAILURE_PREFIX; +use std::io::{self, Write}; + +use super::streaming::{ForwardStats, forward_child_output}; + +/// Forward stderr while retaining only the bounded command-list failure marker. +pub(super) fn forward_stderr_with_attribution( + reader: R, + output: W, +) -> (ForwardStats, Option) +where + R: io::Read, + W: Write, +{ + let mut attribution_writer = FailureAttributionWriter::new(output); + let stats = forward_child_output(reader, &mut attribution_writer, "stderr"); + (stats, attribution_writer.into_failure()) +} + +/// Retain only Ninja's trailing output, where it relays failed subcommand +/// diagnostics after the command itself has completed. +/// +/// Ninja merges a subcommand's stderr into its own stdout. Retaining a small +/// tail lets the process boundary recover the generated failure marker after a +/// non-zero exit without examining or line-buffering ordinary command output. +pub(super) struct NinjaFailureOutputTail { + inner: W, + tail: Vec, +} + +impl NinjaFailureOutputTail { + const MAX_TAIL_BYTES: usize = 512; + + pub(super) fn new(inner: W) -> Self { + Self { + inner, + tail: Vec::with_capacity(Self::MAX_TAIL_BYTES), + } + } + + /// Extract a bounded marker only after Ninja has reported a failure. + pub(super) fn into_failure(self) -> Option { + self.tail + .split(|byte| *byte == b'\n') + .filter_map(parse_marker) + .next_back() + } + + #[cfg(test)] + const fn tail_len(&self) -> usize { + self.tail.len() + } + + fn retain_tail(&mut self, bytes: &[u8]) { + if bytes.len() >= Self::MAX_TAIL_BYTES { + self.tail.clear(); + let suffix = bytes + .get(bytes.len().saturating_sub(Self::MAX_TAIL_BYTES)..) + .unwrap_or_default(); + self.tail.extend_from_slice(suffix); + return; + } + + let retained = self.tail.len().saturating_add(bytes.len()); + if retained > Self::MAX_TAIL_BYTES { + self.tail.drain(..retained - Self::MAX_TAIL_BYTES); + } + self.tail.extend_from_slice(bytes); + } +} + +impl Write for NinjaFailureOutputTail { + fn write(&mut self, bytes: &[u8]) -> io::Result { + let count = self.inner.write(bytes)?; + let Some(written) = bytes.get(..count) else { + return Err(io::Error::other("writer reported an invalid byte count")); + }; + self.retain_tail(written); + Ok(count) + } + + fn flush(&mut self) -> io::Result<()> { + self.inner.flush() + } +} + +pub(super) struct FailureAttributionWriter { + inner: W, + pending: Vec, + failure: Option, +} + +/// Safe, fixed-shape failure details emitted by command-list lowering. +#[derive(Clone, Debug, Eq, PartialEq)] +pub(super) struct CommandListFailure { + action_identity: String, + entry_index: usize, +} + +impl CommandListFailure { + /// Stable hashed action identity, never the manifest command content. + pub(super) fn action_identity(&self) -> &str { + &self.action_identity + } + + /// One-based command-list entry position. + pub(super) const fn entry_index(&self) -> usize { + self.entry_index + } +} + +impl std::fmt::Display for CommandListFailure { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + formatter, + "{COMMAND_LIST_FAILURE_PREFIX}{}, entry {}", + self.action_identity, self.entry_index + ) + } +} + +impl FailureAttributionWriter { + const MAX_LINE_BYTES: usize = 128; + + pub(super) const fn new(inner: W) -> Self { + Self { + inner, + pending: Vec::new(), + failure: None, + } + } + + pub(super) fn into_failure(self) -> Option { + self.failure + } + + fn observe(&mut self, bytes: &[u8]) { + for byte in bytes { + if *byte == b'\n' { + self.record_line(); + self.pending.clear(); + } else if self.pending.len() < Self::MAX_LINE_BYTES { + self.pending.push(*byte); + } + } + } + + fn record_line(&mut self) { + if let Some(failure) = parse_marker(&self.pending) { + self.failure = Some(failure); + } + } +} + +fn parse_marker(bytes: &[u8]) -> Option { + let line = std::str::from_utf8(bytes).ok()?; + let (action, entry_text) = line + .strip_prefix(COMMAND_LIST_FAILURE_PREFIX)? + .split_once(", entry ")?; + let entry = entry_text.parse::().ok()?; + if is_action_identity(action) && entry > 0 { + Some(CommandListFailure { + action_identity: action.to_owned(), + entry_index: entry, + }) + } else { + None + } +} + +fn is_action_identity(value: &str) -> bool { + value.len() == 64 && value.bytes().all(|byte| byte.is_ascii_hexdigit()) +} + +impl Write for FailureAttributionWriter { + fn write(&mut self, bytes: &[u8]) -> io::Result { + let count = self.inner.write(bytes)?; + let Some(written) = bytes.get(..count) else { + return Err(io::Error::other("writer reported an invalid byte count")); + }; + self.observe(written); + Ok(count) + } + + fn flush(&mut self) -> io::Result<()> { + self.inner.flush() + } +} + +#[cfg(test)] +mod tests { + //! Tests for bounded, chunk-independent failure attribution. + + use super::*; + + const ACTION_IDENTITY: &str = + "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; + + #[test] + fn extracts_a_valid_marker_split_across_writes() { + let mut writer = FailureAttributionWriter::new(Vec::new()); + writer + .write_all(b"ninja output\nnetsuke command-list fail") + .expect("first chunk should write"); + writer + .write_all(format!("ure: action {ACTION_IDENTITY}, entry 3\n").as_bytes()) + .expect("second chunk should write"); + + let failure = writer.into_failure().map(|failure| failure.to_string()); + assert_eq!( + failure.as_deref(), + Some( + "netsuke command-list failure: action 0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef, entry 3" + ) + ); + } + + #[test] + fn retains_a_marker_when_later_output_has_no_marker() { + let mut writer = FailureAttributionWriter::new(Vec::new()); + writer + .write_all( + format!("netsuke command-list failure: action {ACTION_IDENTITY}, entry 3\n") + .as_bytes(), + ) + .expect("failure marker should write"); + writer + .write_all(b"ordinary command output\n") + .expect("ordinary output should write"); + + assert_eq!( + writer.into_failure().map(|failure| failure.to_string()), + Some(format!( + "netsuke command-list failure: action {ACTION_IDENTITY}, entry 3" + )) + ); + } + + #[test] + fn ignores_malformed_or_unbounded_markers() { + let mut writer = FailureAttributionWriter::new(Vec::new()); + writer + .write_all(b"netsuke command-list failure: action zero, entry 2\n") + .expect("malformed marker should write"); + writer + .write_all(&[b'x'; FailureAttributionWriter::>::MAX_LINE_BYTES + 1]) + .expect("unbounded marker should write"); + writer + .write_all( + format!("netsuke command-list failure: action {ACTION_IDENTITY}, entry 3\n") + .as_bytes(), + ) + .expect("valid marker after unbounded content should write"); + + assert!(writer.into_failure().is_none()); + } + + #[test] + fn retains_a_bounded_ninja_output_tail_for_failure_attribution() { + let mut writer = NinjaFailureOutputTail::new(Vec::new()); + writer + .write_all(&vec![b'x'; 256 * 1024]) + .expect("large command output should forward"); + writer + .write_all( + format!("\nnetsuke command-list failure: action {ACTION_IDENTITY}, entry 3\n") + .as_bytes(), + ) + .expect("Ninja failure marker should forward"); + + assert!( + writer.tail_len() <= NinjaFailureOutputTail::>::MAX_TAIL_BYTES, + "Ninja failure attribution must retain a fixed-size output tail" + ); + assert_eq!( + writer.into_failure().map(|failure| failure.to_string()), + Some( + "netsuke command-list failure: action 0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef, entry 3" + .into() + ) + ); + } +} diff --git a/src/runner/process/mod.rs b/src/runner/process/mod.rs index 417ff36d6..81175c359 100644 --- a/src/runner/process/mod.rs +++ b/src/runner/process/mod.rs @@ -3,27 +3,26 @@ use super::BuildTargets; use crate::cli::Cli; -use std::{ - io::{self, BufReader}, - path::Path, - process::{Child, Command, ExitStatus}, - thread, -}; -use tracing::{debug, warn}; +use monotony::{MonotonicClock, StdMonotonicClock}; +use std::{io, path::Path, process::Command}; +mod child_exit; +mod command_list_telemetry; mod command_logging; +mod failure_attribution; mod file_io; mod ninja_program; mod ninja_status; +mod output_forwarding; mod paths; mod redaction; mod streaming; #[cfg(test)] mod tests; +use child_exit::{ExitFailureContext, check_exit_status_with_context}; use command_logging::{ - CommandLogContext, command_span, log_command_execution, log_command_exit_failure, - log_command_spawn_failure, + CommandLogContext, command_span, log_command_execution, log_command_spawn_failure, }; pub use file_io::*; pub use ninja_program::resolve_ninja_program; @@ -31,6 +30,7 @@ pub use ninja_program::resolve_ninja_program; pub use ninja_program::resolve_ninja_program_utf8; #[cfg(test)] use ninja_program::{resolve_ninja_program_utf8_with, resolve_ninja_program_with}; +use output_forwarding::{StatusObserver, spawn_and_stream_output}; mod command_env; mod configure; @@ -39,15 +39,14 @@ pub use command_env::CommandEnv; use configure::{configure_ninja_build_command, configure_ninja_tool_command}; pub use paths::*; pub use request::{NinjaBuildRequest, NinjaToolRequest}; -use streaming::{ForwardStats, forward_child_output, forward_child_output_with_ninja_status}; -/// Callback contract for task-progress updates from parsed Ninja status lines. -/// -/// Accepts `(current, total, description)` where `current` and `total` are -/// progress counters and `description` is a human-readable status string. -/// This alias appears in `pub(crate)` function signatures and borrows a mutable -/// callback for the call duration, so callers can retain state across updates. -type StatusObserver<'a> = &'a mut dyn FnMut(u32, u32, &str); +/// Per-invocation process settings passed only from Ninja setup to execution. +struct CommandExecutionContext<'a, Clock> { + operation: &'a str, + suppress_stderr: bool, + captures_ninja_failure_output: bool, + clock: &'a Clock, +} // Public helpers for doctests only. This exposes internal helpers as a stable // testing surface without exporting them in release builds. @@ -69,38 +68,40 @@ pub mod doc { }; } -fn check_exit_status_with_context( - status: ExitStatus, - context: &CommandLogContext, - operation: &str, - suppress_stderr: bool, -) -> io::Result<()> { - if status.success() { - Ok(()) - } else { - tracing::Span::current().record("failure_category", "exit_status"); - log_command_exit_failure(context, operation, suppress_stderr, status); - ninja_exit_error(status) - } -} - -fn run_command_and_stream_with_context( +fn run_command_and_stream_with_context( mut cmd: Command, status_observer: Option>, - suppress_stderr: bool, - operation: &str, + execution: &CommandExecutionContext<'_, Clock>, ) -> io::Result<()> { let context = CommandLogContext::from_command(&cmd); - let span = command_span(&context, operation, suppress_stderr); + let span = command_span(&context, execution.operation, execution.suppress_stderr); let _entered = span.enter(); - log_command_execution(&context, operation, suppress_stderr); + log_command_execution(&context, execution.operation, execution.suppress_stderr); + let started_at = execution.clock.now(); let child = cmd.spawn().inspect_err(|err| { tracing::Span::current().record("failure_category", "spawn"); - log_command_spawn_failure(&context, operation, suppress_stderr, err); + log_command_spawn_failure( + &context, + execution.operation, + execution.suppress_stderr, + err, + ); })?; - let status = spawn_and_stream_output(child, status_observer, suppress_stderr)?; - check_exit_status_with_context(status, &context, operation, suppress_stderr) + let (status, command_list_failure) = spawn_and_stream_output( + child, + status_observer, + execution.suppress_stderr, + execution.captures_ninja_failure_output, + )?; + let failure_context = ExitFailureContext { + operation: execution.operation, + suppress_stderr: execution.suppress_stderr, + command_list_failure: command_list_failure.as_ref(), + clock: execution.clock, + started_at, + }; + check_exit_status_with_context(status, &context, &failure_context) } /// Invoke the Ninja executable with the provided CLI settings. @@ -165,7 +166,14 @@ pub fn run_ninja( /// Returns an [`io::Error`] if the Ninja process fails to spawn, the standard /// streams are unavailable, or when Ninja reports a non-zero exit status. pub fn run_ninja_with(request: &NinjaBuildRequest<'_>) -> io::Result<()> { - run_ninja_build_internal(*request, None) + run_ninja_with_clock(request, &StdMonotonicClock) +} + +fn run_ninja_with_clock( + request: &NinjaBuildRequest<'_>, + clock: &impl MonotonicClock, +) -> io::Result<()> { + run_ninja_build_internal(*request, None, clock) } /// Invoke a Ninja tool (e.g., `ninja -t clean`) with the provided CLI settings. @@ -213,7 +221,7 @@ pub fn run_ninja_tool(program: &Path, cli: &Cli, build_file: &Path, tool: &str) /// Returns an [`io::Error`] if the Ninja process fails to spawn, the standard /// streams are unavailable, or when Ninja reports a non-zero exit status. pub fn run_ninja_tool_with(request: &NinjaToolRequest<'_>) -> io::Result<()> { - run_ninja_tool_internal(*request, None) + run_ninja_tool_internal(*request, None, &StdMonotonicClock) } struct NinjaInternalRequest<'request, 'observer> { @@ -221,24 +229,32 @@ struct NinjaInternalRequest<'request, 'observer> { cli: &'request Cli, status_observer: Option>, operation: &'request str, + captures_ninja_failure_output: bool, } -fn run_ninja_internal(request: NinjaInternalRequest<'_, '_>, configure: F) -> io::Result<()> +fn run_ninja_internal( + request: NinjaInternalRequest<'_, '_>, + clock: &Clock, + configure: F, +) -> io::Result<()> where F: FnOnce(&mut Command) -> io::Result<()>, + Clock: MonotonicClock, { let mut cmd = Command::new(request.program); configure(&mut cmd)?; - run_command_and_stream_with_context( - cmd, - request.status_observer, - request.cli.json, - request.operation, - ) + let execution = CommandExecutionContext { + operation: request.operation, + suppress_stderr: request.cli.json, + captures_ninja_failure_output: request.captures_ninja_failure_output, + clock, + }; + run_command_and_stream_with_context(cmd, request.status_observer, &execution) } fn run_ninja_build_internal( request: NinjaBuildRequest<'_>, status_observer: Option>, + clock: &impl MonotonicClock, ) -> io::Result<()> { run_ninja_internal( NinjaInternalRequest { @@ -246,7 +262,9 @@ fn run_ninja_build_internal( cli: request.cli, status_observer, operation: "build", + captures_ninja_failure_output: true, }, + clock, |cmd| configure_ninja_build_command(cmd, &request), ) } @@ -254,6 +272,7 @@ fn run_ninja_build_internal( fn run_ninja_tool_internal( request: NinjaToolRequest<'_>, status_observer: Option>, + clock: &impl MonotonicClock, ) -> io::Result<()> { run_ninja_internal( NinjaInternalRequest { @@ -261,7 +280,9 @@ fn run_ninja_tool_internal( cli: request.cli, status_observer, operation: request.tool, + captures_ninja_failure_output: false, }, + clock, |cmd| configure_ninja_tool_command(cmd, &request), ) } @@ -276,7 +297,7 @@ pub(crate) fn run_ninja_with_status( request: NinjaBuildRequest<'_>, status_observer: StatusObserver<'_>, ) -> io::Result<()> { - run_ninja_build_internal(request, Some(status_observer)) + run_ninja_build_internal(request, Some(status_observer), &StdMonotonicClock) } /// Invoke `ninja -t` and stream parsed task updates from status lines. @@ -289,105 +310,5 @@ pub(crate) fn run_ninja_tool_with_status( request: NinjaToolRequest<'_>, status_observer: StatusObserver<'_>, ) -> io::Result<()> { - run_ninja_tool_internal(request, Some(status_observer)) -} - -fn handle_forwarding_stats(stats: ForwardStats, stream_name: &str) { - if stats.write_failed { - debug!("{stream_name} forwarding encountered closed pipe; output truncated"); - } -} - -fn handle_forwarding_thread_result(result: thread::Result, stream_name: &str) { - match result { - Ok(stats) => handle_forwarding_stats(stats, stream_name), - Err(err) => { - warn!("{stream_name} forwarding thread panicked: {err:?}"); - } - } -} - -fn forward_stdout( - stdout: impl io::Read, - output: &mut impl io::Write, - status_observer: Option>, -) -> ForwardStats { - match status_observer { - Some(observer) => forward_child_output_with_ninja_status( - BufReader::new(stdout), - output, - observer, - "stdout", - ), - None => forward_child_output(BufReader::new(stdout), output, "stdout"), - } -} -fn spawn_and_stream_output( - mut child: Child, - status_observer: Option>, - suppress_stderr: bool, -) -> io::Result { - let Some(stdout) = child.stdout.take() else { - terminate_child(&mut child, "stdout pipe unavailable"); - return Err(io::Error::other("child process missing stdout pipe")); - }; - let Some(stderr) = child.stderr.take() else { - terminate_child(&mut child, "stderr pipe unavailable"); - return Err(io::Error::other("child process missing stderr pipe")); - }; - - let err_handle = thread::spawn(move || { - // Avoid a long-lived stderr lock: status observers invoked while - // draining stdout may emit task updates to stderr, and that path must - // not block behind stderr forwarding. In JSON diagnostics mode we still - // drain child stderr, but discard it to keep stderr machine-readable. - if suppress_stderr { - forward_child_output(BufReader::new(stderr), io::sink(), "stderr") - } else { - forward_child_output(BufReader::new(stderr), io::stderr(), "stderr") - } - }); - - // Intentionally drain stdout on the main thread when `status_observer` is - // present so forwarding and callback-driven status updates keep a stable - // ordering; moving this elsewhere can regress output timing/interleaving. - let stdout_stats = if suppress_stderr { - let mut output = io::sink(); - forward_stdout(stdout, &mut output, status_observer) - } else { - let mut output = io::stdout().lock(); - forward_stdout(stdout, &mut output, status_observer) - }; - - // Capture the wait result without `?` so the stderr forwarding thread is - // joined on every exit path. Returning early on a `wait()` error would - // otherwise detach the thread, leaking it and discarding its result. - let wait_result = child.wait(); - finalize_streaming(wait_result, stdout_stats, err_handle) -} - -/// Drain forwarding bookkeeping and join the stderr thread, then surface the -/// child's wait result. The stderr thread is always joined first so a failed -/// `wait()` cannot detach background work. -fn finalize_streaming( - wait_result: io::Result, - stdout_stats: ForwardStats, - err_handle: thread::JoinHandle, -) -> io::Result { - handle_forwarding_stats(stdout_stats, "stdout"); - handle_forwarding_thread_result(err_handle.join(), "stderr"); - wait_result -} - -fn terminate_child(child: &mut Child, context: &str) { - if let Err(err) = child.kill() { - tracing::debug!("failed to kill child after {context}: {err}"); - } - if let Err(err) = child.wait() { - tracing::debug!("failed to reap child after {context}: {err}"); - } -} - -fn ninja_exit_error(status: ExitStatus) -> io::Result<()> { - Err(io::Error::other(format!("ninja exited with {status}"))) + run_ninja_tool_internal(request, Some(status_observer), &StdMonotonicClock) } diff --git a/src/runner/process/output_forwarding.rs b/src/runner/process/output_forwarding.rs new file mode 100644 index 000000000..ecc1ab2c9 --- /dev/null +++ b/src/runner/process/output_forwarding.rs @@ -0,0 +1,119 @@ +//! Forward Ninja output while preserving bounded command-list attribution. + +use super::{ + child_exit::{finalize_streaming, terminate_child}, + failure_attribution::{ + CommandListFailure, NinjaFailureOutputTail, forward_stderr_with_attribution, + }, + streaming::{ForwardStats, forward_child_output, forward_child_output_with_ninja_status}, +}; +use std::{ + io::{self, BufReader}, + process::{Child, ExitStatus}, + thread, +}; + +/// Callback contract for task-progress updates from parsed Ninja status lines. +/// +/// Accepts `(current, total, description)` where `current` and `total` are +/// progress counters and `description` is a human-readable status string. +/// This alias appears in `pub(crate)` function signatures and borrows a mutable +/// callback for the call duration, so callers can retain state across updates. +pub(super) type StatusObserver<'a> = &'a mut dyn FnMut(u32, u32, &str); + +fn forward_stdout( + stdout: impl io::Read, + output: &mut W, + status_observer: Option>, + captures_ninja_failure_output: bool, +) -> (ForwardStats, Option) +where + W: io::Write, +{ + if captures_ninja_failure_output { + let mut tail_writer = NinjaFailureOutputTail::new(output); + let stats = match status_observer { + Some(observer) => forward_child_output_with_ninja_status( + BufReader::new(stdout), + &mut tail_writer, + observer, + "stdout", + ), + None => forward_child_output(BufReader::new(stdout), &mut tail_writer, "stdout"), + }; + return (stats, tail_writer.into_failure()); + } + + let stats = match status_observer { + Some(observer) => forward_child_output_with_ninja_status( + BufReader::new(stdout), + output, + observer, + "stdout", + ), + None => forward_child_output(BufReader::new(stdout), output, "stdout"), + }; + (stats, None) +} + +/// Stream a Ninja child and return its exit status and bounded failure marker. +pub(super) fn spawn_and_stream_output( + mut child: Child, + status_observer: Option>, + suppress_stderr: bool, + captures_ninja_failure_output: bool, +) -> io::Result<(ExitStatus, Option)> { + let Some(stdout) = child.stdout.take() else { + terminate_child(&mut child, "stdout pipe unavailable"); + return Err(io::Error::other("child process missing stdout pipe")); + }; + let Some(stderr) = child.stderr.take() else { + terminate_child(&mut child, "stderr pipe unavailable"); + return Err(io::Error::other("child process missing stderr pipe")); + }; + + let err_handle = thread::spawn(move || { + // Avoid a long-lived stderr lock: status observers invoked while + // draining stdout may emit task updates to stderr, and that path must + // not block behind stderr forwarding. In JSON diagnostics mode we still + // drain child stderr, but discard it to keep stderr machine-readable. + if suppress_stderr { + forward_stderr_with_attribution(BufReader::new(stderr), io::sink()) + } else { + forward_stderr_with_attribution(BufReader::new(stderr), io::stderr()) + } + }); + + // Intentionally drain stdout on the main thread when `status_observer` is + // present so forwarding and callback-driven status updates keep a stable + // ordering; moving this elsewhere can regress output timing/interleaving. + let (stdout_stats, stdout_failure) = if suppress_stderr { + let mut output = io::sink(); + forward_stdout( + stdout, + &mut output, + status_observer, + captures_ninja_failure_output, + ) + } else { + let mut output = io::stdout().lock(); + forward_stdout( + stdout, + &mut output, + status_observer, + captures_ninja_failure_output, + ) + }; + + // Capture the wait result without `?` so the stderr forwarding thread is + // joined on every exit path. Returning early on a `wait()` error would + // otherwise detach the thread, leaking it and discarding its result. + let wait_result = child.wait(); + let (status, stderr_failure) = finalize_streaming(wait_result, stdout_stats, err_handle)?; + let failure = if status.success() { + stderr_failure + } else { + stderr_failure.or(stdout_failure) + }; + Ok((status, failure)) +} diff --git a/src/runner/process/tests.rs b/src/runner/process/tests.rs index 880ab3afa..9925f0230 100644 --- a/src/runner/process/tests.rs +++ b/src/runner/process/tests.rs @@ -1,14 +1,30 @@ //! Unit and property tests for Ninja process helpers. use super::super::{NINJA_ENV, NINJA_PROGRAM}; +use super::child_exit::finalize_streaming; +#[cfg(unix)] +use super::command_list_telemetry::COMMAND_LIST_FAILURE_DURATION; +use super::streaming::ForwardStats; use super::*; use camino::Utf8PathBuf; +#[cfg(unix)] +use metrics_util::{ + MetricKind, + debugging::{DebugValue, DebuggingRecorder}, +}; use mockable::MockEnv; +#[cfg(unix)] +use monotony::{StdMonotonicClock, test_util::FixedMonotonicClock}; use proptest::prelude::*; use rstest::{fixture, rstest}; use std::ffi::OsString; #[cfg(unix)] use std::path::PathBuf; +#[cfg(unix)] +use std::process::Stdio; +use std::thread; +#[cfg(unix)] +use std::time::Duration; /// A `MockEnv` answering exactly one `os_string` read of `NETSUKE_NINJA`. /// @@ -111,7 +127,7 @@ fn finalize_streaming_joins_stderr_thread_when_wait_fails() { let err_handle = thread::spawn(move || { thread::sleep(Duration::from_millis(100)); worker_flag.store(true, Ordering::SeqCst); - ForwardStats::default() + (ForwardStats::default(), None) }); let wait_result = Err(io::Error::other("simulated wait failure")); @@ -127,6 +143,94 @@ fn finalize_streaming_joins_stderr_thread_when_wait_fails() { ); } +#[cfg(unix)] +#[test] +fn command_list_failure_duration_uses_the_injected_monotonic_clock() { + let duration = Duration::from_millis(7); + let clock = FixedMonotonicClock::with_elapsed(duration); + let mut command = Command::new("sh"); + command + .args([ + "-c", + concat!( + "printf '%s\\n' 'netsuke command-list failure: action ", + "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef, entry 2' >&2; ", + "exit 1" + ), + ]) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + let recorder = DebuggingRecorder::new(); + let snapshotter = recorder.snapshotter(); + + let execution = CommandExecutionContext { + operation: "build", + suppress_stderr: true, + captures_ninja_failure_output: false, + clock: &clock, + }; + let result = metrics::with_local_recorder(&recorder, || { + run_command_and_stream_with_context(command, None, &execution) + }); + + assert!(result.is_err(), "the attributed command should fail"); + let snapshot = snapshotter.snapshot().into_vec(); + let recorded_durations = snapshot + .iter() + .filter(|(key, _, _, value)| { + key.kind() == MetricKind::Histogram + && key.key().name() == COMMAND_LIST_FAILURE_DURATION + && matches!( + value, + DebugValue::Histogram(samples) + if samples.as_slice() == [duration.as_secs_f64()] + ) + }) + .count(); + assert_eq!( + recorded_durations, 1, + "the failure duration must use the injected clock exactly once" + ); +} + +#[cfg(unix)] +#[test] +fn large_stdout_cannot_supply_command_list_attribution() -> anyhow::Result<()> { + let mut command = Command::new("sh"); + command + .args([ + "-c", + concat!( + "yes x | head -c 262144; ", + "printf '%s\\n' 'netsuke command-list failure: action ", + "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef, entry 2'; ", + "exit 1" + ), + ]) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + let execution = CommandExecutionContext { + operation: "build", + suppress_stderr: true, + // Only a Ninja build can relay a subcommand's stderr through stdout. + // An arbitrary command's large stdout must be forwarded untouched. + captures_ninja_failure_output: false, + clock: &StdMonotonicClock, + }; + + let Err(error) = run_command_and_stream_with_context(command, None, &execution) else { + anyhow::bail!("a failing command should return an error"); + }; + + if error + .to_string() + .contains("netsuke command-list failure: action ") + { + anyhow::bail!("stdout must not supply command-list attribution: {error}"); + } + Ok(()) +} + // As above, the fixture is called directly because `proptest!` generates the // function signature and leaves no parameter for rstest to inject. #[cfg(unix)] diff --git a/tests/ast_tests.rs b/tests/ast_tests.rs index 2554bf100..d6b36d97a 100644 --- a/tests/ast_tests.rs +++ b/tests/ast_tests.rs @@ -11,6 +11,8 @@ mod macros; mod manifest_files; #[path = "ast_tests/parsing.rs"] mod parsing; +#[path = "ast_tests/recipe.rs"] +mod recipe; #[path = "ast_tests/string_or_list.rs"] mod string_or_list; #[path = "ast_tests/support.rs"] diff --git a/tests/ast_tests/parsing.rs b/tests/ast_tests/parsing.rs index d73931459..8c01cf77e 100644 --- a/tests/ast_tests/parsing.rs +++ b/tests/ast_tests/parsing.rs @@ -36,7 +36,10 @@ targets: ensure!(name == "hello", "unexpected target name: {name}"); if let Recipe::Command { command } = &first.recipe { - ensure!(command == "echo hi", "unexpected command: {command}"); + ensure!( + *command == StringOrList::String("echo hi".into()), + "unexpected command: {command:?}" + ); } else { bail!("Expected command recipe, got: {:?}", first.recipe); } @@ -186,7 +189,10 @@ fn vars_section_allows_non_reserved_names() -> Result<()> { let Recipe::Command { command } = &first.recipe else { bail!("expected a command recipe, got {:?}", first.recipe); }; - ensure!(command == "echo hi", "unexpected command: {command}"); + ensure!( + *command == StringOrList::String("echo hi".into()), + "unexpected command: {command:?}" + ); Ok(()) } diff --git a/tests/ast_tests/recipe.rs b/tests/ast_tests/recipe.rs new file mode 100644 index 000000000..91648b25e --- /dev/null +++ b/tests/ast_tests/recipe.rs @@ -0,0 +1,106 @@ +//! Tests for recipe deserialization: the scalar and list forms of `command`, +//! and the rejection of an empty command list. + +use anyhow::{Context, Result, bail, ensure}; +use netsuke::ast::{Recipe, StringOrList}; +use netsuke::localization::{self, keys}; +use test_support::display_error_chain; + +use super::support::parse_manifest; + +#[test] +fn command_accepts_scalar_and_list_forms() -> Result<()> { + { + let yaml = r#" + netsuke_version: "1.0.0" + rules: + - name: lint + command: cargo clippy + targets: + - name: hello + rule: lint + "#; + let manifest = parse_manifest(yaml)?; + let rule = manifest.rules.first().context("expected one rule")?; + let Recipe::Command { command } = &rule.recipe else { + bail!("expected command recipe, got {:?}", rule.recipe); + }; + ensure!( + command == &StringOrList::String("cargo clippy".into()), + "unexpected scalar command: {command:?}" + ); + } + + { + let yaml = r#" + netsuke_version: "1.0.0" + rules: + - name: comprehensive-check + command: + - cargo fmt + - cargo clippy + - cargo test + targets: + - name: hello + rule: comprehensive-check + "#; + let manifest = parse_manifest(yaml)?; + let rule = manifest.rules.first().context("expected one rule")?; + let Recipe::Command { command } = &rule.recipe else { + bail!("expected command recipe, got {:?}", rule.recipe); + }; + ensure!( + command + == &StringOrList::List( + ["cargo fmt", "cargo clippy", "cargo test"] + .map(str::to_owned) + .to_vec() + ), + "unexpected list command: {command:?}" + ); + } + Ok(()) +} + +#[test] +fn empty_command_list_is_rejected() -> Result<()> { + let yaml = r#" + netsuke_version: "1.0.0" + rules: + - name: none + command: [] + targets: + - name: hello + rule: none + "#; + let err = parse_manifest(yaml) + .err() + .context("an empty command list should fail to parse")?; + let chain = display_error_chain(err.as_ref()); + let expected = localization::message(keys::MANIFEST_COMMAND_LIST_EMPTY).to_string(); + ensure!( + chain.contains(&expected), + "unexpected error message: {chain}" + ); + Ok(()) +} + +#[test] +fn direct_ast_deserialization_uses_a_schema_error() -> Result<()> { + let yaml = r#" + netsuke_version: "1.0.0" + rules: + - name: none + command: [] + targets: + - name: hello + rule: none + "#; + let error = serde_saphyr::from_str::(yaml) + .expect_err("an empty command list should fail AST deserialization"); + ensure!( + error.to_string().contains("command list must not be empty"), + "direct AST deserialization should expose the neutral schema error: {error}" + ); + Ok(()) +} diff --git a/tests/ast_tests/string_or_list.rs b/tests/ast_tests/string_or_list.rs index 003f953a0..e1bc736f4 100644 --- a/tests/ast_tests/string_or_list.rs +++ b/tests/ast_tests/string_or_list.rs @@ -99,6 +99,25 @@ fn string_or_list_variants() -> Result<()> { Ok(()) } +#[rstest] +#[case("cc", StringOrList::String("cc".into()))] +#[case("", StringOrList::String(String::new()))] +fn string_or_list_from_str(#[case] value: &str, #[case] expected: StringOrList) { + assert_eq!(StringOrList::from(value), expected); +} + +#[rstest] +fn string_or_list_from_string_and_vec() { + assert_eq!( + StringOrList::from("cc".to_owned()), + StringOrList::String("cc".into()) + ); + assert_eq!( + StringOrList::from(vec!["a".to_owned(), "b".to_owned()]), + StringOrList::List(vec!["a".into(), "b".into()]) + ); +} + #[rstest] #[case(StringOrList::Empty, &[])] #[case(StringOrList::String("cc".into()), &["cc"])] diff --git a/tests/bdd/steps/manifest/mod.rs b/tests/bdd/steps/manifest/mod.rs index 694ed0a13..cb049d331 100644 --- a/tests/bdd/steps/manifest/mod.rs +++ b/tests/bdd/steps/manifest/mod.rs @@ -318,8 +318,8 @@ fn action_command_n(world: &TestWorld, index: usize, command: &str) -> Result<() with_action(world, index, |action| match &action.recipe { Recipe::Command { command: actual } => { ensure!( - actual == command.as_str(), - "expected action {index} command '{command}', got '{actual}'" + actual.as_single() == Some(command.as_str()), + "expected action {index} command '{command}', got '{actual:?}'" ); Ok(()) } diff --git a/tests/bdd/steps/manifest/targets.rs b/tests/bdd/steps/manifest/targets.rs index 3df784894..03990feda 100644 --- a/tests/bdd/steps/manifest/targets.rs +++ b/tests/bdd/steps/manifest/targets.rs @@ -70,7 +70,10 @@ fn first_target_command(world: &TestWorld, command: &str) -> Result<()> { let result = world.manifest.with_ref(|m| { let target = m.targets.first().context("missing target 1")?; match &target.recipe { - Recipe::Command { command: actual } => assert_target_command_eq(1, actual, &command), + Recipe::Command { command: actual } => { + let actual = actual.as_single().context("command is a scalar")?; + assert_target_command_eq(1, actual, &command) + } other => bail!("Expected command recipe, got: {other:?}"), } }); @@ -161,7 +164,10 @@ fn target_name_n(world: &TestWorld, index: usize, name: &str) -> Result<()> { fn target_command_n(world: &TestWorld, index: usize, command: &str) -> Result<()> { let command = CommandText::new(command); with_target(world, index, |target| match &target.recipe { - Recipe::Command { command: actual } => assert_target_command_eq(index, actual, &command), + Recipe::Command { command: actual } => { + let actual = actual.as_single().context("command is a scalar")?; + assert_target_command_eq(index, actual, &command) + } other => bail!("Expected command recipe, got: {other:?}"), }) } diff --git a/tests/command_env_ui_tests.rs b/tests/command_env_ui_tests.rs index 07394aad3..0dbe45517 100644 --- a/tests/command_env_ui_tests.rs +++ b/tests/command_env_ui_tests.rs @@ -28,13 +28,30 @@ use std::{ /// The embedder fixture type-checks against the public API. #[test] fn command_env_embedder_fixture_compiles() -> io::Result<()> { + compile_public_api_fixture( + "tests/ui/command_env_embedder_pass.rs", + "the embedder fixture should compile against the public API", + ) +} + +/// The public command-list constructors compile for an external embedder. +#[test] +fn command_list_public_api_fixture_compiles() -> io::Result<()> { + compile_public_api_fixture( + "tests/ui/command_list_public_api_pass.rs", + "the command-list public API fixture should compile", + ) +} + +/// Compile one external public-API fixture through the direct-rustc harness. +fn compile_public_api_fixture(source: &str, failure_message: &str) -> io::Result<()> { let rlib = NetsukeRlib::build()?; - let output = rlib.compile("tests/ui/command_env_embedder_pass.rs")?; + let output = rlib.compile(source)?; if !output.status.success() { return Err(io::Error::other(format!( - "the embedder fixture should compile against the public API:\n{}", - stderr(&output), + "{failure_message}:\n{}", + stderr(&output) ))); } Ok(()) diff --git a/tests/command_escaping_tests.rs b/tests/command_escaping_tests.rs index 71e6c8d41..70a34cabb 100644 --- a/tests/command_escaping_tests.rs +++ b/tests/command_escaping_tests.rs @@ -33,7 +33,8 @@ fn command_words(body: &str) -> Result> { let Recipe::Command { command } = &action.recipe else { bail!("expected command recipe, got: {:?}", action.recipe); }; - shlex::split(command).context("split command into words") + let command_str = command.as_single().context("command should be a scalar")?; + shlex::split(command_str).context("split command into words") } #[rstest] diff --git a/tests/data/multi_command.yml b/tests/data/multi_command.yml new file mode 100644 index 000000000..2b4f6e2a4 --- /dev/null +++ b/tests/data/multi_command.yml @@ -0,0 +1,14 @@ +netsuke_version: "1.0.0" +rules: + - name: comprehensive-check + description: Run the required checks sequentially + command: + - echo check-fmt + - echo lint + - echo test +targets: + - name: done + rule: comprehensive-check +actions: + - name: aggregate + rule: comprehensive-check \ No newline at end of file diff --git a/tests/documentation_examples_tests.rs b/tests/documentation_examples_tests.rs index 68712e885..0863a272f 100644 --- a/tests/documentation_examples_tests.rs +++ b/tests/documentation_examples_tests.rs @@ -20,8 +20,10 @@ const EXPECTED_EXAMPLE_IDS: &[&str] = &[ "guide-binstall-install", "guide-cli-usage", "guide-command-available-manifest", + "guide-command-list", "guide-complete-manifest", "guide-crates-io-install", + "guide-direct-command-list", "guide-env-reader-snippet", "guide-first-build-commands", "guide-first-build-manifest", @@ -155,6 +157,8 @@ fn every_documented_fence_has_a_known_unique_identifier() -> Result<()> { #[case("guide-complete-manifest")] #[case("guide-foreach-manifest")] #[case("guide-macro-manifest")] +#[case("guide-command-list")] +#[case("guide-direct-command-list")] #[case("guide-command-available-manifest")] #[case("stdlib-yaml-syntax-manifest")] #[case("stdlib-jinja-syntax-manifest")] diff --git a/tests/hasher_tests.rs b/tests/hasher_tests.rs index 4c8b472b4..08187e380 100644 --- a/tests/hasher_tests.rs +++ b/tests/hasher_tests.rs @@ -31,7 +31,9 @@ use rstest::rstest; )] #[case( Action { - recipe: Recipe::Command { command: String::new() }, + recipe: Recipe::Command { + command: StringOrList::String(String::new()), + }, description: None, depfile: None, deps_format: None, diff --git a/tests/ir_from_manifest_tests.rs b/tests/ir_from_manifest_tests.rs index 7c63f0f1b..9f26a1bfa 100644 --- a/tests/ir_from_manifest_tests.rs +++ b/tests/ir_from_manifest_tests.rs @@ -33,6 +33,37 @@ fn minimal_manifest_to_ir() -> Result<()> { Ok(()) } +#[rstest] +fn command_list_entries_are_interpolated_in_order() -> Result<()> { + let yaml = r#" + netsuke_version: "1.0.0" + rules: + - name: build + command: + - echo first $in + - echo second $out + targets: + - name: out/app + sources: src/main.c + rule: build + "#; + let manifest = manifest::from_str(yaml)?; + let graph = BuildGraph::from_manifest(&manifest).context("expected graph generation")?; + let action = graph + .actions + .values() + .next() + .context("expected one action")?; + let Recipe::Command { command } = &action.recipe else { + bail!("expected a command recipe, got {:?}", action.recipe); + }; + ensure!( + command.to_string_vec() == ["echo first src/main.c", "echo second out/app"], + "each list entry should be interpolated in declaration order: {command:?}" + ); + Ok(()) +} + #[rstest] fn duplicate_rules_emit_distinct_actions() -> Result<()> { let manifest = manifest::from_path("tests/data/duplicate_rules.yml")?; @@ -220,8 +251,8 @@ fn manifest_deps_do_not_contribute_to_recipe_inputs() -> Result<()> { }; ensure!( - command == "echo src/main.c src/main.c > out/app", - "deps should not appear in recipe interpolation: {command}" + command.as_single() == Some("echo src/main.c src/main.c > out/app"), + "deps should not appear in recipe interpolation: {command:?}" ); ensure!( edge.inputs == vec![Utf8PathBuf::from("src/main.c")], diff --git a/tests/ir_tests.rs b/tests/ir_tests.rs index 3e58758f9..d059f7619 100644 --- a/tests/ir_tests.rs +++ b/tests/ir_tests.rs @@ -79,7 +79,7 @@ fn build_graph_duplicate_action_ids() { panic!("expected action for id 'a'"); }; if let Recipe::Command { command } = &action.recipe { - assert_eq!(command, "two"); + assert_eq!(command.as_single(), Some("two")); } else { panic!("unexpected recipe type"); } diff --git a/tests/logging_stderr/command_list_failure.rs b/tests/logging_stderr/command_list_failure.rs new file mode 100644 index 000000000..40ef65d7f --- /dev/null +++ b/tests/logging_stderr/command_list_failure.rs @@ -0,0 +1,138 @@ +//! Runtime diagnostics for failed entries in command-list recipes. + +use super::support::open_workspace; +use anyhow::{Context, Result, ensure}; +use cap_std::fs_utf8::Dir; +use netsuke::runner::NINJA_ENV; +use serde_json::Value; +use tempfile::TempDir; +use test_support::ninja::ninja_integration_workspace; + +const FAILURE_PREFIX: &str = "netsuke command-list failure: action "; + +fn identifies_entry(message: &str, entry: usize) -> bool { + message.contains(FAILURE_PREFIX) && message.contains(&format!(", entry {entry}")) +} + +fn failing_command_list_workspace(first_entry: &str) -> Result> { + let temp = match ninja_integration_workspace() { + Ok(temp) => temp, + Err(error) => { + tracing::warn!(%error, "skipping command-list failure attribution test: Ninja unavailable"); + return Ok(None); + } + }; + let workspace: Dir = open_workspace(&temp)?; + workspace.write( + "Netsukefile", + format!( + r#" +netsuke_version: "1.0.0" +targets: + - name: result.txt + command: + - "{first_entry}" + - "false" + - "echo unexpected >> $out" +"#, + ) + .as_bytes(), + )?; + Ok(Some(temp)) +} + +fn run_failing_build(temp: &TempDir, arguments: &[&str]) -> Result { + assert_cmd::cargo::cargo_bin_cmd!("netsuke") + .current_dir(temp.path()) + .env(NINJA_ENV, "ninja") + .args(arguments) + .output() + .context("run failing command-list build") +} + +#[test] +fn failed_command_list_entry_is_attributed_in_human_output() -> Result<()> { + let Some(temp) = failing_command_list_workspace("echo first > $out")? else { + return Ok(()); + }; + let output = run_failing_build(&temp, &["--progress", "never", "build"])?; + ensure!( + !output.status.success(), + "a failing list entry must fail the build" + ); + let stderr = String::from_utf8(output.stderr).context("stderr should be valid UTF-8")?; + ensure!( + identifies_entry(&stderr, 2), + "human diagnostics should name the bounded failing entry: {stderr}" + ); + let output_file = open_workspace(&temp)? + .read_to_string("result.txt") + .context("read the partial command-list output")?; + ensure!( + output_file == "first\n", + "a failure must prevent subsequent command-list entries from running, got {output_file:?}" + ); + Ok(()) +} + +#[test] +fn failed_command_list_entry_is_attributed_in_json_diagnostics() -> Result<()> { + let Some(temp) = failing_command_list_workspace("echo first > $out")? else { + return Ok(()); + }; + let output = run_failing_build(&temp, &["--json", "build"])?; + ensure!( + !output.status.success(), + "a failing list entry must fail the build" + ); + let stderr = String::from_utf8(output.stderr).context("stderr should be valid UTF-8")?; + let diagnostics: Value = serde_json::from_str(&stderr).context("stderr should be JSON")?; + ensure!( + identifies_entry(&diagnostics.to_string(), 2), + "JSON diagnostics should retain bounded entry attribution: {diagnostics}" + ); + ensure!( + !stderr.contains("false"), + "JSON attribution must not expose the command text: {stderr}" + ); + Ok(()) +} + +#[test] +fn failed_command_list_entry_is_attributed_in_tracing_output() -> Result<()> { + let Some(temp) = failing_command_list_workspace("echo first > $out")? else { + return Ok(()); + }; + let output = run_failing_build(&temp, &["--verbose", "--progress", "never", "build"])?; + ensure!( + !output.status.success(), + "a failing list entry must fail the build" + ); + let stderr = String::from_utf8(output.stderr).context("stderr should be valid UTF-8")?; + ensure!( + stderr.contains("command_list_failure") && identifies_entry(&stderr, 2), + "tracing should record the bounded command-list failure context: {stderr}" + ); + Ok(()) +} + +#[test] +fn large_command_stdout_retains_command_list_failure_attribution() -> Result<()> { + let Some(temp) = failing_command_list_workspace( + "echo first > $out && i=0; while [ $$i -lt 65536 ]; do printf 'xxxx\\\\n'; i=$$((i + 1)); done", + )? + else { + return Ok(()); + }; + let output = run_failing_build(&temp, &["--progress", "never", "build"])?; + ensure!( + !output.status.success(), + "a failing list entry must fail the build" + ); + let stderr = String::from_utf8(output.stderr).context("stderr should be valid UTF-8")?; + ensure!( + identifies_entry(&stderr, 2), + "large command stdout must not hide the bounded failing entry: {stderr}" + ); + Ok(()) +} diff --git a/tests/logging_stderr_tests.rs b/tests/logging_stderr_tests.rs index d220366ba..c1cf2cf92 100644 --- a/tests/logging_stderr_tests.rs +++ b/tests/logging_stderr_tests.rs @@ -1,5 +1,8 @@ //! Integration tests for stderr logging and JSON output contracts. +#[cfg(unix)] +#[path = "logging_stderr/command_list_failure.rs"] +mod command_list_failure; #[path = "logging_stderr/config_tracing.rs"] mod config_tracing; #[path = "logging_stderr/json.rs"] diff --git a/tests/manifest_env_tests.rs b/tests/manifest_env_tests.rs index 99d01f511..f0567960e 100644 --- a/tests/manifest_env_tests.rs +++ b/tests/manifest_env_tests.rs @@ -38,7 +38,10 @@ fn rendered_command(value: Result) -> Result { let Recipe::Command { command } = &target.recipe else { return Err(anyhow!("expected command recipe, got {:?}", target.recipe)); }; - Ok(command.clone()) + command + .as_single() + .map(str::to_owned) + .context("command should be a scalar") } #[rstest] diff --git a/tests/manifest_jinja_tests.rs b/tests/manifest_jinja_tests.rs index 3d8527194..a83ff208a 100644 --- a/tests/manifest_jinja_tests.rs +++ b/tests/manifest_jinja_tests.rs @@ -103,7 +103,10 @@ fn extract_target_names(manifest: &NetsukeManifest) -> Result> { fn extract_target_commands(manifest: &NetsukeManifest) -> Result> { extract_target_field(manifest, |target| match &target.recipe { - Recipe::Command { command } => Ok(command.clone()), + Recipe::Command { command } => command + .as_single() + .map(str::to_owned) + .context("command should be a scalar"), other => bail!("expected command recipe, got {other:?}"), }) } @@ -122,7 +125,10 @@ fn renders_global_vars() -> Result<()> { let Recipe::Command { command } = &first.recipe else { bail!("expected command recipe, got {:?}", first.recipe); }; - ensure!(command == "echo world", "unexpected command: {command}"); + ensure!( + command.as_single() == Some("echo world"), + "unexpected command: {command:?}" + ); Ok(()) } @@ -145,7 +151,10 @@ fn renders_env_function() -> Result<()> { let Recipe::Command { command } = &first.recipe else { bail!("expected command recipe, got {:?}", first.recipe); }; - ensure!(command == "echo 42", "unexpected command: {command}"); + ensure!( + command.as_single() == Some("echo 42"), + "unexpected command: {command:?}" + ); ensure!( reader("NETSUKE_WRONG_ENV").is_err(), "the reader should reject a variable not named by the manifest" @@ -220,7 +229,10 @@ fn registers_manifest_macros() -> Result<()> { .context("manifest should contain at least one target")?; match &target.recipe { Recipe::Command { command } => { - ensure!(command == "HELLO WORLD!", "unexpected command: {command}"); + ensure!( + command.as_single() == Some("HELLO WORLD!"), + "unexpected command: {command:?}" + ); } other => bail!("expected command recipe, got {other:?}"), } @@ -285,7 +297,10 @@ fn registers_manifest_macro_argument_variants( .context("manifest should contain at least one target")?; match &target.recipe { Recipe::Command { command } => { - ensure!(command == expected, "unexpected command: {command}"); + ensure!( + command.as_single() == Some(expected), + "unexpected command: {command:?}" + ); } other => bail!("expected command recipe, got {other:?}"), } @@ -361,7 +376,10 @@ fn renders_if_blocks(#[case] flag: bool, #[case] expected: &str) -> Result<()> { let Recipe::Command { command } = &first.recipe else { bail!("expected command recipe, got {:?}", first.recipe); }; - ensure!(command == expected, "unexpected command: {command}"); + ensure!( + command.as_single() == Some(expected), + "unexpected command: {command:?}" + ); Ok(()) } @@ -483,7 +501,10 @@ fn expands_single_item_foreach_targets() -> Result<()> { let Recipe::Command { command } = &first.recipe else { bail!("expected command recipe, got {:?}", first.recipe); }; - ensure!(command == "echo 'only'", "unexpected command: {command}"); + ensure!( + command.as_single() == Some("echo 'only'"), + "unexpected command: {command:?}" + ); Ok(()) } @@ -569,7 +590,10 @@ fn renders_target_fields_command() -> Result<()> { let Recipe::Command { command } = &target.recipe else { bail!("expected command recipe, got {:?}", target.recipe); }; - ensure!(command == "echo 'base1'", "unexpected command: {command}"); + ensure!( + command.as_single() == Some("echo 'base1'"), + "unexpected command: {command:?}" + ); Ok(()) } diff --git a/tests/ninja_gen_command_list_integration_tests.rs b/tests/ninja_gen_command_list_integration_tests.rs new file mode 100644 index 000000000..c2c2c4728 --- /dev/null +++ b/tests/ninja_gen_command_list_integration_tests.rs @@ -0,0 +1,335 @@ +//! Real-Ninja regressions for command-list shell boundaries. +//! +//! These tests cover syntax which would escape a directly interpolated brace +//! group and therefore require the generated command to evaluate each entry as +//! a complete shell unit. + +use anyhow::{Context, Result, ensure}; +use camino::Utf8PathBuf; +use cap_std::{ambient_authority, fs_utf8::Dir}; +use netsuke::ast::{Recipe, StringOrList}; +use netsuke::ir::{Action, BuildEdge, BuildGraph}; +use netsuke::ninja_gen::{NinjaGenError, generate}; +use rstest::rstest; +use std::process::Command; +use tempfile::TempDir; +use test_support::ninja_gen::ninja_integration_setup; + +fn run_command_list( + dir: &TempDir, + entries: Vec, + expected_file: &str, + expected_content: &str, +) -> Result<()> { + let dir_path = temp_workspace_path(dir)?; + let action = command_list_action(entries); + let target = Utf8PathBuf::from("out"); + let edge = BuildEdge { + action_id: "chain".into(), + inputs: Vec::new(), + implicit_deps: Vec::new(), + explicit_outputs: vec![target.clone()], + implicit_outputs: Vec::new(), + order_only_deps: Vec::new(), + phony: false, + always: false, + }; + let mut graph = BuildGraph::default(); + graph.actions.insert("chain".into(), action); + graph.targets.insert(target.clone(), edge); + graph.default_targets.push(target); + + let ninja = generate(&graph)?; + let handle = open_temp_workspace(dir)?; + handle + .write("build.ninja", ninja.as_bytes()) + .context("write ninja build file")?; + let ninja_output = Command::new("ninja") + .arg("out") + .current_dir(dir_path.as_std_path()) + .output() + .context("invoke ninja")?; + ensure!( + ninja_output.status.success(), + "command list should run successfully: {ninja_output:?}" + ); + let content = handle + .read_to_string(expected_file) + .with_context(|| format!("read {expected_file} written by the second entry"))?; + ensure!( + content.trim() == expected_content, + "expected {expected_file} to contain '{expected_content}', got '{content}'" + ); + Ok(()) +} + +#[test] +fn command_list_entry_with_inline_comment_preserves_the_next_boundary() -> Result<()> { + let Some(dir) = ninja_integration_setup() else { + return Ok(()); + }; + run_command_list( + &dir, + vec![ + "echo first # a comment that formerly consumed the closing brace".into(), + "echo second > after-comment.txt".into(), + ], + "after-comment.txt", + "second", + ) +} + +#[test] +fn command_list_entry_ending_in_background_operator_preserves_the_next_boundary() -> Result<()> { + let Some(dir) = ninja_integration_setup() else { + return Ok(()); + }; + run_command_list( + &dir, + vec![ + "(sleep 1 && echo waited > waited-background-job.txt) &".into(), + "echo second > after-background.txt".into(), + ], + "after-background.txt", + "second", + )?; + let workspace = open_temp_workspace(&dir)?; + ensure!( + workspace.exists("waited-background-job.txt"), + "Ninja must wait for a successful background job before running the next entry" + ); + Ok(()) +} + +const fn command_list_action(entries: Vec) -> Action { + Action { + recipe: Recipe::Command { + command: StringOrList::List(entries), + }, + description: None, + depfile: None, + deps_format: None, + pool: None, + restat: false, + } +} + +fn command_list_command_line(entries: Vec) -> Result { + let action = command_list_action(entries); + let mut graph = BuildGraph::default(); + graph.actions.insert("chain".into(), action); + let ninja = generate(&graph)?; + ninja + .lines() + .find_map(|line| line.strip_prefix(" command = ")) + .map(str::to_owned) + .context("generated command-list action missing") +} + +fn assert_multiple_background_jobs_are_rejected( + entries: Vec, + expectation: &str, +) -> Result<()> { + let Err(error) = command_list_command_line(entries) else { + anyhow::bail!("{expectation}"); + }; + ensure!( + matches!( + error.downcast_ref::(), + Some(NinjaGenError::MultipleBackgroundJobs { + action_index: 1, + entry_index: 1, + }) + ), + "multiple background jobs should return a stable typed error: {error:?}" + ); + Ok(()) +} + +fn open_temp_workspace(dir: &TempDir) -> Result { + let dir_path = temp_workspace_path(dir)?; + Dir::open_ambient_dir(&dir_path, ambient_authority()).context("open command-list workspace") +} + +fn temp_workspace_path(dir: &TempDir) -> Result { + let dir_path = Utf8PathBuf::from_path_buf(dir.path().to_path_buf()) + .map_err(|path| anyhow::anyhow!("temp dir path {path:?} is not UTF-8"))?; + Ok(dir_path) +} + +fn run_generated_command_with_ninja(dir: &TempDir, command: &str) -> Result { + let workspace = open_temp_workspace(dir)?; + workspace.write( + "build.ninja", + format!("rule chain\n command = {command}\nbuild out: chain\n").as_bytes(), + )?; + Command::new("ninja") + .arg("out") + .current_dir(dir.path()) + .output() + .context("run generated command-list with Ninja") +} + +#[test] +fn command_list_exit_entry_preserves_status_and_emits_attribution() -> Result<()> { + let Some(dir) = ninja_integration_setup() else { + return Ok(()); + }; + let command = command_list_command_line(vec![ + "exit 23".into(), + "echo unexpected > continued-after-exit.txt".into(), + ])?; + let shell_command = command.replace("$$", "$"); + let output = Command::new("sh") + .args(["-c", &shell_command]) + .current_dir(dir.path()) + .output() + .context("run generated command-list shell")?; + ensure!( + output.status.code() == Some(23), + "exit command should retain status 23, got {:?}", + output.status + ); + let stderr = String::from_utf8(output.stderr).context("shell stderr should be UTF-8")?; + ensure!( + stderr.contains("netsuke command-list failure: action ") && stderr.contains(", entry 1"), + "exit command should emit the first-entry marker: {stderr}" + ); + let workspace = open_temp_workspace(&dir)?; + ensure!( + !workspace.exists("continued-after-exit.txt"), + "an exit failure must not run a later entry" + ); + let ninja_output = run_generated_command_with_ninja(&dir, &command)?; + ensure!( + !ninja_output.status.success(), + "Ninja should report the generated command failure, got {:?}", + ninja_output.status + ); + let ninja_diagnostics = format!( + "{}{}", + String::from_utf8(ninja_output.stdout).context("Ninja stdout should be UTF-8")?, + String::from_utf8(ninja_output.stderr).context("Ninja stderr should be UTF-8")?, + ); + ensure!( + ninja_diagnostics.contains(", entry 1"), + "Ninja should retain the first-entry marker: {ninja_diagnostics}" + ); + Ok(()) +} + +#[test] +fn command_list_exec_entries_preserve_attribution_and_success() -> Result<()> { + let Some(dir) = ninja_integration_setup() else { + return Ok(()); + }; + let successful_command = command_list_command_line(vec![ + "exec true".into(), + "echo unexpected > continued-after-successful-exec.txt".into(), + ])?; + let successful_output = run_generated_command_with_ninja(&dir, &successful_command)?; + ensure!( + successful_output.status.success(), + "a successful process-replacing entry must succeed" + ); + let successful_diagnostics = format!( + "{}{}", + String::from_utf8(successful_output.stdout).context("Ninja stdout should be UTF-8")?, + String::from_utf8(successful_output.stderr).context("Ninja stderr should be UTF-8")?, + ); + ensure!( + !successful_diagnostics + .lines() + .any(|line| line.starts_with("netsuke command-list failure: action ")), + "successful exec must not emit failure attribution: {successful_diagnostics}" + ); + let workspace = open_temp_workspace(&dir)?; + ensure!( + !workspace.exists("continued-after-successful-exec.txt"), + "a successful exec must retain process-replacement semantics" + ); + let command = command_list_command_line(vec![ + "FOO=1 exec false".into(), + "echo unexpected > continued-after-exec.txt".into(), + ])?; + let output = run_generated_command_with_ninja(&dir, &command)?; + ensure!( + !output.status.success(), + "a process-replacing entry must fail the Ninja build" + ); + let stdout = String::from_utf8(output.stdout).context("Ninja stdout should be UTF-8")?; + let stderr = String::from_utf8(output.stderr).context("Ninja stderr should be UTF-8")?; + let diagnostics = format!("{stdout}{stderr}"); + ensure!( + diagnostics.contains("netsuke command-list failure: action ") + && diagnostics.contains(", entry 1"), + "assignment-prefixed exec failure should emit the first-entry marker: {diagnostics}" + ); + ensure!( + !workspace.exists("continued-after-exec.txt"), + "an exec failure must not run a later entry" + ); + Ok(()) +} + +#[test] +fn command_list_background_failure_waits_before_the_next_entry() -> Result<()> { + let Some(dir) = ninja_integration_setup() else { + return Ok(()); + }; + let command = command_list_command_line(vec![ + "sh -c 'sleep 0.1; exit 1' &".into(), + "echo unexpected > continued-after-background.txt".into(), + ])?; + let shell_command = command.replace("$$", "$"); + let output = Command::new("sh") + .args(["-c", &shell_command]) + .current_dir(dir.path()) + .output() + .context("run generated command-list shell")?; + ensure!( + !output.status.success(), + "failing background work must fail its command-list entry" + ); + let stderr = String::from_utf8(output.stderr).context("shell stderr should be UTF-8")?; + ensure!( + stderr.contains(", entry 1"), + "background failure should identify the first entry: {stderr}" + ); + let workspace = open_temp_workspace(&dir)?; + ensure!( + !workspace.exists("continued-after-background.txt"), + "a background failure must stop later entries" + ); + let ninja_output = run_generated_command_with_ninja(&dir, &command)?; + ensure!( + !ninja_output.status.success(), + "Ninja must fail when a backgrounded entry fails: {ninja_output:?}" + ); + Ok(()) +} + +#[rstest] +#[case::multiple_background_jobs( + "true & sh -c 'sleep 0.1; exit 1' &", + "echo unexpected > continued-after-multiple-background-jobs.txt", + "multiple background jobs should be rejected before Ninja runs" +)] +#[case::nested_eval_background_jobs( + "eval 'false & true &'", + "echo unexpected > continued-after-nested-eval.txt", + "nested eval background jobs should be rejected before Ninja runs" +)] +fn command_list_rejects_unattributable_background_jobs( + #[case] entry: &str, + #[case] later_entry: &str, + #[case] expectation: &str, +) -> Result<()> { + assert_multiple_background_jobs_are_rejected( + vec![entry.into(), later_entry.into()], + expectation, + ) +} + +#[path = "support/ninja_gen_direct_target_command_list.rs"] +mod direct_target_tests; diff --git a/tests/ninja_gen_integration_tests.rs b/tests/ninja_gen_integration_tests.rs index f5adc3fc5..b134f322a 100644 --- a/tests/ninja_gen_integration_tests.rs +++ b/tests/ninja_gen_integration_tests.rs @@ -7,7 +7,7 @@ use anyhow::{Context, Result, bail, ensure}; use camino::Utf8PathBuf; use cap_std::{ambient_authority, fs_utf8::Dir}; -use netsuke::ast::Recipe; +use netsuke::ast::{Recipe, StringOrList}; use netsuke::ir::{Action, BuildEdge, BuildGraph}; use netsuke::ninja_gen::{NinjaGenError, generate, generate_into}; use rstest::{fixture, rstest}; @@ -193,6 +193,205 @@ fn ninja_integration_tests( Ok(()) } +#[rstest] +fn command_list_fails_fast_at_first_nonzero_exit( + ninja_integration_setup: Option, +) -> Result<()> { + let Some(dir) = ninja_integration_setup else { + return Ok(()); + }; + let dir_path = Utf8PathBuf::from_path_buf(dir.path().to_path_buf()) + .map_err(|path| anyhow::anyhow!("temp dir path {:?} is not UTF-8", path))?; + + let action = Action { + recipe: Recipe::Command { + command: StringOrList::List(vec![ + "echo one > first.txt".into(), + "false".into(), + "echo never > last.txt".into(), + ]), + }, + description: None, + depfile: None, + deps_format: None, + pool: None, + restat: false, + }; + let edge = BuildEdge { + action_id: "chain".into(), + inputs: Vec::new(), + implicit_deps: Vec::new(), + explicit_outputs: vec![Utf8PathBuf::from("out")], + implicit_outputs: Vec::new(), + order_only_deps: Vec::new(), + phony: false, + always: false, + }; + let mut graph = BuildGraph::default(); + graph.actions.insert("chain".into(), action); + graph.targets.insert(Utf8PathBuf::from("out"), edge); + graph.default_targets.push(Utf8PathBuf::from("out")); + + let ninja = generate(&graph)?; + let handle = Dir::open_ambient_dir(&dir_path, ambient_authority()) + .with_context(|| format!("open ambient dir for temp workspace at {dir_path}"))?; + handle + .write("build.ninja", ninja.as_bytes()) + .context("write ninja build file")?; + let output = Command::new("ninja") + .arg("out") + .current_dir(dir_path.as_std_path()) + .output() + .context("invoke ninja")?; + ensure!( + !output.status.success(), + "command chain should fail when an entry exits non-zero" + ); + let first = handle + .read_to_string("first.txt") + .context("first entry should have run and written first.txt")?; + ensure!( + first.trim() == "one", + "first entry should have written its output, got '{first}'" + ); + ensure!( + !handle.try_exists("last.txt").context("check last.txt")?, + "fail-fast chain should skip entries after the first non-zero exit" + ); + Ok(()) +} + +#[rstest] +fn command_list_entry_control_flow_cannot_mask_an_earlier_failure( + ninja_integration_setup: Option, +) -> Result<()> { + let Some(dir) = ninja_integration_setup else { + return Ok(()); + }; + let dir_path = Utf8PathBuf::from_path_buf(dir.path().to_path_buf()) + .map_err(|path| anyhow::anyhow!("temp dir path {:?} is not UTF-8", path))?; + + // Without per-entry isolation, the second entry's `||` would join the + // raw chain as `false && false || echo recovered > recovered.txt`, which + // POSIX evaluates as `(false && false) || echo ...`, running the echo and + // reporting success despite the first entry failing. + let action = Action { + recipe: Recipe::Command { + command: StringOrList::List(vec![ + "false".into(), + "false || echo recovered > recovered.txt".into(), + ]), + }, + description: None, + depfile: None, + deps_format: None, + pool: None, + restat: false, + }; + let edge = BuildEdge { + action_id: "chain".into(), + inputs: Vec::new(), + implicit_deps: Vec::new(), + explicit_outputs: vec![Utf8PathBuf::from("out")], + implicit_outputs: Vec::new(), + order_only_deps: Vec::new(), + phony: false, + always: false, + }; + let mut graph = BuildGraph::default(); + graph.actions.insert("chain".into(), action); + graph.targets.insert(Utf8PathBuf::from("out"), edge); + graph.default_targets.push(Utf8PathBuf::from("out")); + + let ninja = generate(&graph)?; + let handle = Dir::open_ambient_dir(&dir_path, ambient_authority()) + .with_context(|| format!("open ambient dir for temp workspace at {dir_path}"))?; + handle + .write("build.ninja", ninja.as_bytes()) + .context("write ninja build file")?; + let output = Command::new("ninja") + .arg("out") + .current_dir(dir_path.as_std_path()) + .output() + .context("invoke ninja")?; + ensure!( + !output.status.success(), + "the first entry's failure must not be masked by a later '||': {output:?}" + ); + ensure!( + !handle + .try_exists("recovered.txt") + .context("check recovered.txt")?, + "the second entry should not run after the first entry fails" + ); + Ok(()) +} + +#[rstest] +fn command_list_entries_share_one_shell_process( + ninja_integration_setup: Option, +) -> Result<()> { + let Some(dir) = ninja_integration_setup else { + return Ok(()); + }; + let dir_path = Utf8PathBuf::from_path_buf(dir.path().to_path_buf()) + .map_err(|path| anyhow::anyhow!("temp dir path {:?} is not UTF-8", path))?; + + let action = Action { + recipe: Recipe::Command { + // `$$` escapes Ninja's variable expansion so the shell sees a + // literal `$NETSUKE_SHARED` written by the first entry. + command: StringOrList::List(vec![ + "export NETSUKE_SHARED=yes".into(), + "test \"$$NETSUKE_SHARED\" = yes && echo ok > shared.txt".into(), + ]), + }, + description: None, + depfile: None, + deps_format: None, + pool: None, + restat: false, + }; + let edge = BuildEdge { + action_id: "chain".into(), + inputs: Vec::new(), + implicit_deps: Vec::new(), + explicit_outputs: vec![Utf8PathBuf::from("out")], + implicit_outputs: Vec::new(), + order_only_deps: Vec::new(), + phony: false, + always: false, + }; + let mut graph = BuildGraph::default(); + graph.actions.insert("chain".into(), action); + graph.targets.insert(Utf8PathBuf::from("out"), edge); + graph.default_targets.push(Utf8PathBuf::from("out")); + + let ninja = generate(&graph)?; + let handle = Dir::open_ambient_dir(&dir_path, ambient_authority()) + .with_context(|| format!("open ambient dir for temp workspace at {dir_path}"))?; + handle + .write("build.ninja", ninja.as_bytes()) + .context("write ninja build file")?; + let output = Command::new("ninja") + .arg("out") + .current_dir(dir_path.as_std_path()) + .output() + .context("invoke ninja")?; + ensure!( + output.status.success(), + "command chain should succeed when every entry succeeds: {output:?}" + ); + let shared = handle + .read_to_string("shared.txt") + .context("later entries should see the environment set by an earlier entry")?; + ensure!( + shared.trim() == "ok", + "unexpected shared.txt content: {shared}" + ); + Ok(()) +} + #[rstest] fn errors_when_action_missing() -> Result<()> { let mut graph = BuildGraph::default(); diff --git a/tests/ninja_snapshot_tests.rs b/tests/ninja_snapshot_tests.rs index 91d356009..c477012b6 100644 --- a/tests/ninja_snapshot_tests.rs +++ b/tests/ninja_snapshot_tests.rs @@ -6,6 +6,7 @@ //! fast and deterministic. use anyhow::{Context, Result, ensure}; +use cap_std::{ambient_authority, fs_utf8::Dir}; use insta::{Settings, assert_snapshot}; use netsuke::{ir::BuildGraph, manifest, ninja_gen}; use std::{fs, process::Command}; @@ -130,9 +131,48 @@ fn conditional_manifest_ninja_snapshot() -> Result<()> { Ok(()) } +#[test] +fn multi_command_manifest_ninja_snapshot() -> Result<()> { + let fixture_dir = Dir::open_ambient_dir(env!("CARGO_MANIFEST_DIR"), ambient_authority()) + .context("open repository root to read tests/data/multi_command.yml")?; + let manifest_yaml = fixture_dir + .read_to_string("tests/data/multi_command.yml") + .context("read tests/data/multi_command.yml")?; + + let manifest = manifest::from_str(&manifest_yaml)?; + let ir = BuildGraph::from_manifest(&manifest)?; + let ninja_content = ninja_gen::generate(&ir)?; + + ensure!( + ninja_content.contains("if eval 'echo check-fmt'") + && ninja_content.contains("if eval 'echo lint'") + && ninja_content.contains("if eval 'echo test'") + && ninja_content.matches("} && {").count() == 2, + "expected the command list joined into a fail-fast chain:\n{ninja_content}" + ); + ensure!( + ninja_content.contains("build done:") && ninja_content.contains("build aggregate:"), + "the multi-command rule should be referenced by both a target and an action:\n{ninja_content}" + ); + + let mut settings = Settings::new(); + settings.set_snapshot_path(concat!( + env!("CARGO_MANIFEST_DIR"), + "/tests/snapshots/ninja" + )); + settings.bind(|| { + assert_snapshot!("multi_command_manifest_ninja", ninja_content); + }); + + Ok(()) +} + #[test] fn implicit_deps_manifest_ninja_snapshot() -> Result<()> { - let manifest_yaml = std::fs::read_to_string("tests/data/implicit_deps.yml") + let fixture_dir = Dir::open_ambient_dir(env!("CARGO_MANIFEST_DIR"), ambient_authority()) + .context("open repository root to read tests/data/implicit_deps.yml")?; + let manifest_yaml = fixture_dir + .read_to_string("tests/data/implicit_deps.yml") .context("read tests/data/implicit_deps.yml")?; let manifest = manifest::from_str(&manifest_yaml)?; diff --git a/tests/snapshots/ninja/ninja_snapshot_tests__multi_command_manifest_ninja.snap b/tests/snapshots/ninja/ninja_snapshot_tests__multi_command_manifest_ninja.snap new file mode 100644 index 000000000..34ebff72d --- /dev/null +++ b/tests/snapshots/ninja/ninja_snapshot_tests__multi_command_manifest_ninja.snap @@ -0,0 +1,11 @@ +--- +source: tests/ninja_snapshot_tests.rs +expression: ninja_content +--- +rule 2ba9145cfe6f2224b0cbcf527eea1371cf3cc742c8790e332ddfb7ba0c51e3da + command = { _netsuke_background_before=$${!:-}; _netsuke_exec_succeeded=0; trap '_netsuke_command_status=$$?; printf "%s\n" "netsuke command-list failure: action aad603e884fb9196bd1cfe656b4b72baa75d7b3eae7cb3f30a56e840ae134bc3, entry 1" >&2; trap - EXIT; exit "$$_netsuke_command_status"' EXIT; if eval 'echo check-fmt'; then _netsuke_command_status=0; else _netsuke_command_status=$$?; fi; _netsuke_background_after=$${!:-}; if [ -n "$$_netsuke_background_after" ] && [ "$$_netsuke_background_after" != "$$_netsuke_background_before" ]; then if wait "$$_netsuke_background_after"; then :; else _netsuke_background_status=$$?; if [ "$$_netsuke_command_status" -eq 0 ]; then _netsuke_command_status=$$_netsuke_background_status; fi; fi; fi; if [ "$$_netsuke_command_status" -eq 0 ]; then trap - EXIT; if [ "$$_netsuke_exec_succeeded" -eq 1 ]; then exit 0; else :; fi; else trap - EXIT; printf '%s\n' 'netsuke command-list failure: action aad603e884fb9196bd1cfe656b4b72baa75d7b3eae7cb3f30a56e840ae134bc3, entry 1' >&2; exit "$$_netsuke_command_status"; fi; } && { _netsuke_background_before=$${!:-}; _netsuke_exec_succeeded=0; trap '_netsuke_command_status=$$?; printf "%s\n" "netsuke command-list failure: action aad603e884fb9196bd1cfe656b4b72baa75d7b3eae7cb3f30a56e840ae134bc3, entry 2" >&2; trap - EXIT; exit "$$_netsuke_command_status"' EXIT; if eval 'echo lint'; then _netsuke_command_status=0; else _netsuke_command_status=$$?; fi; _netsuke_background_after=$${!:-}; if [ -n "$$_netsuke_background_after" ] && [ "$$_netsuke_background_after" != "$$_netsuke_background_before" ]; then if wait "$$_netsuke_background_after"; then :; else _netsuke_background_status=$$?; if [ "$$_netsuke_command_status" -eq 0 ]; then _netsuke_command_status=$$_netsuke_background_status; fi; fi; fi; if [ "$$_netsuke_command_status" -eq 0 ]; then trap - EXIT; if [ "$$_netsuke_exec_succeeded" -eq 1 ]; then exit 0; else :; fi; else trap - EXIT; printf '%s\n' 'netsuke command-list failure: action aad603e884fb9196bd1cfe656b4b72baa75d7b3eae7cb3f30a56e840ae134bc3, entry 2' >&2; exit "$$_netsuke_command_status"; fi; } && { _netsuke_background_before=$${!:-}; _netsuke_exec_succeeded=0; trap '_netsuke_command_status=$$?; printf "%s\n" "netsuke command-list failure: action aad603e884fb9196bd1cfe656b4b72baa75d7b3eae7cb3f30a56e840ae134bc3, entry 3" >&2; trap - EXIT; exit "$$_netsuke_command_status"' EXIT; if eval 'echo test'; then _netsuke_command_status=0; else _netsuke_command_status=$$?; fi; _netsuke_background_after=$${!:-}; if [ -n "$$_netsuke_background_after" ] && [ "$$_netsuke_background_after" != "$$_netsuke_background_before" ]; then if wait "$$_netsuke_background_after"; then :; else _netsuke_background_status=$$?; if [ "$$_netsuke_command_status" -eq 0 ]; then _netsuke_command_status=$$_netsuke_background_status; fi; fi; fi; if [ "$$_netsuke_command_status" -eq 0 ]; then trap - EXIT; if [ "$$_netsuke_exec_succeeded" -eq 1 ]; then exit 0; else :; fi; else trap - EXIT; printf '%s\n' 'netsuke command-list failure: action aad603e884fb9196bd1cfe656b4b72baa75d7b3eae7cb3f30a56e840ae134bc3, entry 3' >&2; exit "$$_netsuke_command_status"; fi; } + description = Run the required checks sequentially + +build aggregate: 2ba9145cfe6f2224b0cbcf527eea1371cf3cc742c8790e332ddfb7ba0c51e3da + +build done: 2ba9145cfe6f2224b0cbcf527eea1371cf3cc742c8790e332ddfb7ba0c51e3da diff --git a/tests/support/ninja_gen_direct_target_command_list.rs b/tests/support/ninja_gen_direct_target_command_list.rs new file mode 100644 index 000000000..72565c5df --- /dev/null +++ b/tests/support/ninja_gen_direct_target_command_list.rs @@ -0,0 +1,110 @@ +//! Real-Ninja coverage for direct target command-list lowering. + +use super::{open_temp_workspace, temp_workspace_path}; +use anyhow::{Context, Result, ensure}; +use minijinja::Environment; +use netsuke::ast::{NetsukeManifest, Recipe}; +use netsuke::ir::BuildGraph; +use netsuke::manifest::{self, render_manifest}; +use netsuke::ninja_gen::generate; +use std::process::Command; +use tempfile::TempDir; +use test_support::ninja_gen::ninja_integration_setup; + +fn rendered_direct_target_manifest() -> Result { + let manifest = manifest::from_str( + r#" +netsuke_version: "1.0.0" +targets: + - name: result.txt + sources: input.txt + vars: + first: rendered-first + second: rendered-second + command: + - "test -f $in && echo '{{ first }}' > $out" + - "echo '{{ second }}' >> {{ outs }}" +"#, + )?; + render_manifest(manifest, &Environment::new()) +} + +fn assert_rendered_direct_target(manifest: &NetsukeManifest) -> Result<()> { + let target = manifest + .targets + .first() + .context("rendered direct target missing")?; + let Recipe::Command { command } = &target.recipe else { + anyhow::bail!("direct target should retain its command recipe"); + }; + ensure!( + command.to_string_vec() + == [ + "test -f $in && echo 'rendered-first' > $out", + "echo 'rendered-second' >> __NETSUKE_OUTS_PLACEHOLDER__", + ], + "rendered direct-target command entries should preserve declaration order: {command:?}" + ); + Ok(()) +} + +fn direct_target_command_list_graph() -> Result { + let rendered = rendered_direct_target_manifest()?; + assert_rendered_direct_target(&rendered)?; + let graph = BuildGraph::from_manifest(&rendered)?; + let action = graph + .actions + .values() + .next() + .context("direct target action missing")?; + let Recipe::Command { + command: lowered_command, + } = &action.recipe + else { + anyhow::bail!("lowered direct target should retain a command recipe"); + }; + ensure!( + lowered_command.to_string_vec() + == [ + "test -f input.txt && echo 'rendered-first' > result.txt", + "echo 'rendered-second' >> result.txt", + ], + "IR should interpolate every direct-target entry independently in order: {lowered_command:?}" + ); + Ok(graph) +} + +fn execute_direct_target_command_list(dir: &TempDir, graph: &BuildGraph) -> Result<()> { + let dir_path = temp_workspace_path(dir)?; + let handle = open_temp_workspace(dir)?; + handle + .write("input.txt", b"input") + .context("write direct-target input")?; + handle + .write("build.ninja", generate(graph)?.as_bytes()) + .context("write generated Ninja file")?; + let ninja_output = Command::new("ninja") + .arg("result.txt") + .current_dir(dir_path.as_std_path()) + .output() + .context("run real Ninja for direct target command list")?; + ensure!( + ninja_output.status.success(), + "direct target command list should succeed: {ninja_output:?}" + ); + let result = handle.read_to_string("result.txt")?; + ensure!( + result == "rendered-first\nrendered-second\n", + "target output should prove both entries executed in declaration order, got {result:?}" + ); + Ok(()) +} + +#[test] +fn direct_target_command_list_renders_lowers_and_executes_in_order() -> Result<()> { + let Some(dir) = ninja_integration_setup() else { + return Ok(()); + }; + let graph = direct_target_command_list_graph()?; + execute_direct_target_command_list(&dir, &graph) +} diff --git a/tests/ui/command_list_public_api_pass.rs b/tests/ui/command_list_public_api_pass.rs new file mode 100644 index 000000000..8d7e84be6 --- /dev/null +++ b/tests/ui/command_list_public_api_pass.rs @@ -0,0 +1,25 @@ +//! Compile-pass fixture for the public command-list AST surface. + +use netsuke::ast::{Recipe, Recipe::Command, StringOrList}; + +fn command(recipe: Recipe) -> StringOrList { + let Recipe::Command { command } = recipe else { + unreachable!("fixture constructs only command recipes"); + }; + command +} + +fn main() { + let borrowed = StringOrList::from("borrowed"); + let owned = StringOrList::from(String::from("owned")); + let listed = StringOrList::from(vec![String::from("first"), String::from("second")]); + + assert!(matches!(borrowed, StringOrList::String(value) if value == "borrowed")); + assert!(matches!(owned, StringOrList::String(value) if value == "owned")); + assert!(matches!(listed, StringOrList::List(values) if values == ["first", "second"])); + + let constructed: StringOrList = command(Command { + command: StringOrList::from("recipe"), + }); + assert!(matches!(constructed, StringOrList::String(value) if value == "recipe")); +}