From 8fb1e3213fa2bdb81a9b021a673930ab2bfe2ec6 Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 9 Aug 2026 20:08:47 +0200 Subject: [PATCH 01/14] Allow rule commands to be non-empty ordered lists Widen the manifest command schema from a single string to a scalar or a non-empty ordered list. Each list entry is Jinja-rendered and interpolated independently, then emitted as a single fail-fast '&&' shell chain so the build stops at the first non-zero exit. An empty command list is rejected during deserialization with a localized diagnostic. The scalar form serializes byte-identically, so existing action hashes and snapshots stay unchanged. Reuse StringOrList for the field and add From impls so existing construction sites keep compiling. Co-Authored-By: Claude --- locales/ar/messages.ftl | 1 + locales/cs/messages.ftl | 1 + locales/cy/messages.ftl | 1 + locales/da/messages.ftl | 1 + locales/de/messages.ftl | 1 + locales/el/messages.ftl | 1 + locales/en-GB/messages.ftl | 1 + locales/en-US/messages.ftl | 1 + locales/es-419/messages.ftl | 1 + locales/es-ES/messages.ftl | 1 + locales/fa/messages.ftl | 1 + locales/fi/messages.ftl | 1 + locales/fr/messages.ftl | 1 + locales/gd/messages.ftl | 1 + locales/he/messages.ftl | 1 + locales/hi/messages.ftl | 1 + locales/hu/messages.ftl | 1 + locales/id/messages.ftl | 1 + locales/it/messages.ftl | 1 + locales/ja/messages.ftl | 1 + locales/ko/messages.ftl | 1 + locales/nb/messages.ftl | 1 + locales/nl/messages.ftl | 1 + locales/pl/messages.ftl | 1 + locales/pt-BR/messages.ftl | 1 + locales/pt-PT/messages.ftl | 1 + locales/ro/messages.ftl | 1 + locales/ru/messages.ftl | 1 + locales/sv/messages.ftl | 1 + locales/th/messages.ftl | 1 + locales/tr/messages.ftl | 1 + locales/uk/messages.ftl | 1 + locales/vi/messages.ftl | 1 + locales/zh-Hans/messages.ftl | 1 + locales/zh-Hant/messages.ftl | 1 + src/ast.rs | 58 +++++++- src/ir/from_manifest_support.rs | 22 ++- src/localization/keys.rs | 1 + src/manifest/mod.rs | 4 +- src/manifest/render.rs | 76 ++++++++++- src/manifest/tests/workspace.rs | 4 +- src/ninja_gen.rs | 120 ++++------------ src/ninja_gen_tests.rs | 129 ++++++++++++++++++ tests/ast_tests.rs | 2 + tests/ast_tests/parsing.rs | 10 +- tests/ast_tests/recipe.rs | 86 ++++++++++++ tests/ast_tests/string_or_list.rs | 19 +++ tests/bdd/steps/manifest/mod.rs | 4 +- tests/bdd/steps/manifest/targets.rs | 10 +- tests/command_escaping_tests.rs | 3 +- tests/data/multi_command.yml | 14 ++ tests/hasher_tests.rs | 4 +- tests/ir_from_manifest_tests.rs | 35 ++++- tests/ir_tests.rs | 2 +- tests/manifest_env_tests.rs | 5 +- tests/manifest_jinja_tests.rs | 40 ++++-- tests/ninja_gen_integration_tests.rs | 70 +++++++++- tests/ninja_snapshot_tests.rs | 30 ++++ ...t_tests__multi_command_manifest_ninja.snap | 11 ++ 59 files changed, 666 insertions(+), 128 deletions(-) create mode 100644 src/ninja_gen_tests.rs create mode 100644 tests/ast_tests/recipe.rs create mode 100644 tests/data/multi_command.yml create mode 100644 tests/snapshots/ninja/ninja_snapshot_tests__multi_command_manifest_ninja.snap diff --git a/locales/ar/messages.ftl b/locales/ar/messages.ftl index 3a45931e0..ad67a582c 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 = ‏The 'command' field must not be empty: provide a command string or a non-empty list. # أخطاء التمثيل الوسيط. ir.rule_not_found = تعذّر العثور على القاعدة «{ $rule }» التي يشير إليها الهدف «{ $target }». diff --git a/locales/cs/messages.ftl b/locales/cs/messages.ftl index c16798e8c..533363ebc 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 = The 'command' field must not be empty: provide a command string or a non-empty list. # 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..e8810a8a2 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 = The 'command' field must not be empty: provide a command string or a non-empty list. # 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..03420912d 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 = The 'command' field must not be empty: provide a command string or a non-empty list. # 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..f13f688a9 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 = The 'command' field must not be empty: provide a command string or a non-empty list. # 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..53786656f 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 = The 'command' field must not be empty: provide a command string or a non-empty list. # Σφάλματα της ενδιάμεσης αναπαράστασης. 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..88dd05895 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 = The 'command' field must not be empty: provide a command string or a non-empty list. # 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..e7e0ac13d 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 = The 'command' field must not be empty: provide a command string or a non-empty list. # 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..a489bca7c 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 = ‏The 'command' field must not be empty: provide a command string or a non-empty list. # خطاهای بازنمایی میانی. ir.rule_not_found = قاعدهٔ «{ $rule }» که هدف «{ $target }» به آن ارجاع می‌دهد یافت نشد. diff --git a/locales/fi/messages.ftl b/locales/fi/messages.ftl index e2a95e57b..075b1e3af 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 = The 'command' field must not be empty: provide a command string or a non-empty list. # 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..dc19e82bc 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 = The 'command' field must not be empty: provide a command string or a non-empty list. # 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..740ec889e 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 = The 'command' field must not be empty: provide a command string or a non-empty list. # 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..20af419f7 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 = ‏The 'command' field must not be empty: provide a command string or a non-empty list. # שגיאות הייצוג הביניימי. ir.rule_not_found = הכלל „{ $rule }” שאליו מפנה היעד „{ $target }” לא נמצא. diff --git a/locales/hi/messages.ftl b/locales/hi/messages.ftl index 8a8b1c6f8..ff67c22d2 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 = The 'command' field must not be empty: provide a command string or a non-empty list. # मध्यवर्ती निरूपण की त्रुटियाँ। ir.rule_not_found = लक्ष्य “{ $target }” जिस नियम “{ $rule }” का संदर्भ देता है वह नहीं मिला। diff --git a/locales/hu/messages.ftl b/locales/hu/messages.ftl index 3cdf92115..f4a33cb77 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 = The 'command' field must not be empty: provide a command string or a non-empty list. # 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..f7128d8b0 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 = The 'command' field must not be empty: provide a command string or a non-empty list. # 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..c4efa2e3e 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 = The 'command' field must not be empty: provide a command string or a non-empty list. # 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..69ba29882 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 = The 'command' field must not be empty: provide a command string or a non-empty list. # 中間表現のエラー。 ir.rule_not_found = ターゲット「{ $target }」が参照する規則「{ $rule }」が見つかりません。 diff --git a/locales/ko/messages.ftl b/locales/ko/messages.ftl index ab0850a8f..073d96fc3 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 = The 'command' field must not be empty: provide a command string or a non-empty list. # 중간 표현 오류. ir.rule_not_found = 대상 '{ $target }'이(가) 참조하는 규칙 '{ $rule }'을(를) 찾을 수 없습니다. diff --git a/locales/nb/messages.ftl b/locales/nb/messages.ftl index 3519a18f0..2a556b993 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 = The 'command' field must not be empty: provide a command string or a non-empty list. # 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..9f98143ca 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 = The 'command' field must not be empty: provide a command string or a non-empty list. # 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..37435aa8d 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 = The 'command' field must not be empty: provide a command string or a non-empty 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..833d12bd4 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 = The 'command' field must not be empty: provide a command string or a non-empty list. # 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..2941a4f85 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 = The 'command' field must not be empty: provide a command string or a non-empty list. # 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..488d39c7a 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 = The 'command' field must not be empty: provide a command string or a non-empty list. # 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..88cb2d457 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 = The 'command' field must not be empty: provide a command string or a non-empty list. # Ошибки промежуточного представления. ir.rule_not_found = Правило «{ $rule }», на которое ссылается цель «{ $target }», не найдено. diff --git a/locales/sv/messages.ftl b/locales/sv/messages.ftl index 336a47b85..aa8babc58 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 = The 'command' field must not be empty: provide a command string or a non-empty list. # 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..8be17d812 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 = The 'command' field must not be empty: provide a command string or a non-empty list. # ข้อผิดพลาดของรูปแทนระดับกลาง ir.rule_not_found = ไม่พบกฎ “{ $rule }” ที่เป้าหมาย “{ $target }” อ้างถึง diff --git a/locales/tr/messages.ftl b/locales/tr/messages.ftl index cc69bcfc4..0246ae304 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 = The 'command' field must not be empty: provide a command string or a non-empty list. # 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..5ccbc2bff 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 = The 'command' field must not be empty: provide a command string or a non-empty list. # Помилки проміжного подання. ir.rule_not_found = Правило «{ $rule }», на яке посилається ціль «{ $target }», не знайдено. diff --git a/locales/vi/messages.ftl b/locales/vi/messages.ftl index b9389371a..14e180b69 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 = The 'command' field must not be empty: provide a command string or a non-empty list. # 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..4df49f6ed 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 = The 'command' field must not be empty: provide a command string or a non-empty list. # 中间表示的错误。 ir.rule_not_found = 找不到目标“{ $target }”引用的规则“{ $rule }”。 diff --git a/locales/zh-Hant/messages.ftl b/locales/zh-Hant/messages.ftl index 1dbbe3f5f..ff86bf5fc 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 = The 'command' field must not be empty: provide a command string or a non-empty list. # 中介表示法的錯誤。 ir.rule_not_found = 找不到目標「{ $target }」所參照的規則「{ $rule }」。 diff --git a/src/ast.rs b/src/ast.rs index 9c69e5e38..286fb4260 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -29,6 +29,7 @@ //! assert_eq!(manifest.targets.len(), 1); //! ``` +use crate::localization::{self, keys}; use semver::Version; use serde::{Deserialize, Serialize, de::Deserializer}; use std::collections::HashMap; @@ -141,10 +142,11 @@ 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, + command: StringOrList, }, /// An embedded multi-line script. Script { @@ -161,7 +163,7 @@ pub enum Recipe { #[derive(Debug, Deserialize)] #[serde(deny_unknown_fields)] struct RawRecipe { - command: Option, + command: Option, script: Option, rule: Option, } @@ -178,7 +180,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( + localization::message(keys::MANIFEST_COMMAND_LIST_EMPTY).to_string(), + )), + 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 +354,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/from_manifest_support.rs b/src/ir/from_manifest_support.rs index c911922a7..f8840cace 100644 --- a/src/ir/from_manifest_support.rs +++ b/src/ir/from_manifest_support.rs @@ -31,7 +31,27 @@ pub(super) fn register_action( ) -> Result { let resolved_recipe = match recipe { Recipe::Command { command } => { - let interpolated = interpolate_command(&command, bindings.inputs, bindings.outputs)?; + let interpolated = match command { + StringOrList::String(cmd) => StringOrList::String(interpolate_command( + &cmd, + bindings.inputs, + bindings.outputs, + )?), + StringOrList::List(items) => { + let mut rendered = Vec::with_capacity(items.len()); + for item in items { + rendered.push(interpolate_command( + &item, + bindings.inputs, + bindings.outputs, + )?); + } + 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, } 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 32bc1a894..509d2dd3f 100644 --- a/src/manifest/mod.rs +++ b/src/manifest/mod.rs @@ -256,7 +256,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 { @@ -342,7 +342,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..ae55e0a8b 100644 --- a/src/manifest/render.rs +++ b/src/manifest/render.rs @@ -41,7 +41,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 +59,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,6 +96,37 @@ fn render_string_or_list(value: &mut StringOrList, env: &Environment, ctx: &Vars Ok(()) } +/// 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, so a rendering failure names the recipe stage rather than the list +/// position. +fn render_recipe_string_or_list( + value: &mut StringOrList, + env: &Environment, + ctx: &Vars, + what: impl FnOnce() -> String, +) -> Result<()> { + let label = what(); + let render_entry = |entry: &mut String| -> Result<()> { + *entry = render_recipe_str_with(env, entry, ctx, || label.clone())?; + Ok(()) + }; + match value { + StringOrList::String(s) => render_entry(s)?, + StringOrList::List(list) => { + for item in list { + render_entry(item)?; + } + } + StringOrList::Empty => {} + } + Ok(()) +} + fn render_str_with( env: &Environment, tpl: &str, @@ -212,7 +243,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 +268,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 +287,38 @@ 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(()) + } } 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..b195af7ad 100644 --- a/src/ninja_gen.rs +++ b/src/ninja_gen.rs @@ -6,7 +6,7 @@ //! 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 camino::Utf8PathBuf; @@ -215,8 +215,13 @@ 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}") + let command_line = match command { + StringOrList::String(cmd) => cmd.clone(), + StringOrList::List(items) => items.iter().map(String::as_str).join(" && "), + StringOrList::Empty => return Self::reject_empty_command_recipe(), + }; + Self::assert_shell_command(&command_line); + writeln!(f, " command = {command_line}") } Recipe::Script { script } => Self::write_script_command(f, script), Recipe::Rule { .. } => Self::reject_rule_recipe(), @@ -266,6 +271,22 @@ impl NamedAction<'_> { } Err(fmt::Error) } + + #[cold] + #[expect( + clippy::panic_in_result_fn, + reason = "debug builds intentionally panic to expose empty command recipes" + )] + #[expect( + clippy::manual_assert, + reason = "debug-only guard escalates to panic for visibility" + )] + fn reject_empty_command_recipe() -> fmt::Result { + if cfg!(debug_assertions) { + panic!("empty command recipes are rejected while deserializing the manifest"); + } + Err(fmt::Error) + } } impl Display for NamedAction<'_> { @@ -307,94 +328,5 @@ 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_tests.rs"] +mod tests; diff --git a/src/ninja_gen_tests.rs b/src/ninja_gen_tests.rs new file mode 100644 index 000000000..807c63e02 --- /dev/null +++ b/src/ninja_gen_tests.rs @@ -0,0 +1,129 @@ +//! 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(()) +} + +#[rstest] +fn generate_command_list_ninja_joins_a_fail_fast_chain() -> Result<()> { + let action = Action { + recipe: Recipe::Command { + command: StringOrList::List(vec![ + "echo one".into(), + "echo two".into(), + "echo three".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("command = echo one && echo two && echo three"), + "command list should be joined into a fail-fast chain:\n{ninja}" + ); + 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); +} 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..6bf2d84bf 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.as_single() == Some("echo hi"), + "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.as_single() == Some("echo hi"), + "unexpected command: {command:?}" + ); Ok(()) } diff --git a/tests/ast_tests/recipe.rs b/tests/ast_tests/recipe.rs new file mode 100644 index 000000000..347c0908b --- /dev/null +++ b/tests/ast_tests/recipe.rs @@ -0,0 +1,86 @@ +//! 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(()) +} 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_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/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/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_integration_tests.rs b/tests/ninja_gen_integration_tests.rs index f5adc3fc5..77494f09d 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,74 @@ 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 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..cdf744eaf 100644 --- a/tests/ninja_snapshot_tests.rs +++ b/tests/ninja_snapshot_tests.rs @@ -130,6 +130,36 @@ fn conditional_manifest_ninja_snapshot() -> Result<()> { Ok(()) } +#[test] +fn multi_command_manifest_ninja_snapshot() -> Result<()> { + let manifest_yaml = std::fs::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("echo check-fmt && echo lint && echo test"), + "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") 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..24d5096cf --- /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 = echo check-fmt && echo lint && echo test + description = Run the required checks sequentially + +build aggregate: 2ba9145cfe6f2224b0cbcf527eea1371cf3cc742c8790e332ddfb7ba0c51e3da + +build done: 2ba9145cfe6f2224b0cbcf527eea1371cf3cc742c8790e332ddfb7ba0c51e3da From e5e16bb5a93169412d79136c4fb8c1c3b9a0d8d6 Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 9 Aug 2026 20:10:05 +0200 Subject: [PATCH 02/14] Document command lists in the guide and design doc The users' guide now describes the scalar-or-list command recipe, its declaration-order and fail-fast shell-chain semantics, the shared-shell state caveat, and a documented example, alongside when to prefer 'script'. The design doc records the same schema and shell semantics, and the changelog notes the new manifest form. Co-Authored-By: Claude --- CHANGELOG.md | 4 ++++ docs/netsuke-design.md | 23 +++++++++++------- docs/users-guide.md | 34 ++++++++++++++++++++++++++- tests/documentation_examples_tests.rs | 2 ++ 4 files changed, 53 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 05ee9da01..957ee2095 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,10 @@ 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 + ([#550](https://github.com/leynos/netsuke/issues/550)) ### Changed diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index c13fd8a03..6c7080419 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -245,14 +245,19 @@ 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 - [`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). +- `command`: A command string, or a non-empty ordered list of command strings, + to be executed. Each entry 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 [`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). A list command is emitted as a single fail-fast `&&` chain, + so entries run in declaration order and the chain stops at the first + non-zero exit; all entries share one shell process, carrying working + directory, environment, and exit-code state forward like a `script` block. + An empty command list is rejected during manifest deserialization. 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 @@ -711,7 +716,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. diff --git a/docs/users-guide.md b/docs/users-guide.md index e21cb04cb..19cc45613 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -290,12 +290,41 @@ 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. All entries run in one shell process, so working directory, +environment, and exit-code state set by an earlier entry carry into later +entries, exactly as they do for `script`. An empty command list is rejected +when the manifest is parsed. + + + +```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 +``` + +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. @@ -1037,6 +1066,9 @@ Netsuke reduces some common quoting mistakes, but it is not a sandbox: with the host. - `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 even if the earlier entry only partially succeeded. - Literal shell dollar expressions currently require Ninja-aware escaping, such as `$$PATH`. diff --git a/tests/documentation_examples_tests.rs b/tests/documentation_examples_tests.rs index 68712e885..dc81a3de3 100644 --- a/tests/documentation_examples_tests.rs +++ b/tests/documentation_examples_tests.rs @@ -20,6 +20,7 @@ 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-env-reader-snippet", @@ -155,6 +156,7 @@ 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-command-available-manifest")] #[case("stdlib-yaml-syntax-manifest")] #[case("stdlib-jinja-syntax-manifest")] From 33fd8b617689bb90e4a076f8b4b4f45b63ba8436 Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 9 Aug 2026 20:37:55 +0200 Subject: [PATCH 03/14] Isolate each command list entry before chaining Concatenating entries with '&&' alone let a later entry's own '||', ';' or '&' escape the entry boundary and mask an earlier failure: entries 'false' and 'false || echo recovered' became 'false && false || echo recovered', which POSIX evaluates as (false && false) || echo, reporting success after the first entry failed. Wrap each entry in a brace group so it forms a distinct shell unit before the fail-fast '&&' between entries. Braces run in the current shell (unlike '( ... )'), so working directory, environment, and variables set by one entry still carry into the next, keeping the documented shared-shell-state semantics. Add integration tests for the masking scenario and for environment state carrying across entries. Co-Authored-By: Claude --- src/ninja_gen.rs | 11 +- src/ninja_gen_tests.rs | 4 +- tests/ninja_gen_integration_tests.rs | 131 ++++++++++++++++++ tests/ninja_snapshot_tests.rs | 2 +- ...t_tests__multi_command_manifest_ninja.snap | 4 +- 5 files changed, 146 insertions(+), 6 deletions(-) diff --git a/src/ninja_gen.rs b/src/ninja_gen.rs index b195af7ad..1203c475c 100644 --- a/src/ninja_gen.rs +++ b/src/ninja_gen.rs @@ -217,7 +217,16 @@ impl NamedAction<'_> { Recipe::Command { command } => { let command_line = match command { StringOrList::String(cmd) => cmd.clone(), - StringOrList::List(items) => items.iter().map(String::as_str).join(" && "), + // Brace groups keep each entry a distinct shell unit so its + // own `||`, `;`, or `&` cannot escape the entry boundary + // and mask an earlier failure. 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. + StringOrList::List(items) => items + .iter() + .map(|item| format!("{{ {item}; }}")) + .join(" && "), StringOrList::Empty => return Self::reject_empty_command_recipe(), }; Self::assert_shell_command(&command_line); diff --git a/src/ninja_gen_tests.rs b/src/ninja_gen_tests.rs index 807c63e02..20de55984 100644 --- a/src/ninja_gen_tests.rs +++ b/src/ninja_gen_tests.rs @@ -116,8 +116,8 @@ fn generate_command_list_ninja_joins_a_fail_fast_chain() -> Result<()> { let ninja = generate(&graph)?; ensure!( - ninja.contains("command = echo one && echo two && echo three"), - "command list should be joined into a fail-fast chain:\n{ninja}" + ninja.contains("command = { echo one; } && { echo two; } && { echo three; }"), + "command list entries should be isolated brace groups joined by &&:\n{ninja}" ); Ok(()) } diff --git a/tests/ninja_gen_integration_tests.rs b/tests/ninja_gen_integration_tests.rs index 77494f09d..b134f322a 100644 --- a/tests/ninja_gen_integration_tests.rs +++ b/tests/ninja_gen_integration_tests.rs @@ -261,6 +261,137 @@ fn command_list_fails_fast_at_first_nonzero_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 cdf744eaf..78f7a2b5d 100644 --- a/tests/ninja_snapshot_tests.rs +++ b/tests/ninja_snapshot_tests.rs @@ -140,7 +140,7 @@ fn multi_command_manifest_ninja_snapshot() -> Result<()> { let ninja_content = ninja_gen::generate(&ir)?; ensure!( - ninja_content.contains("echo check-fmt && echo lint && echo test"), + ninja_content.contains("{ echo check-fmt; } && { echo lint; } && { echo test; }"), "expected the command list joined into a fail-fast chain:\n{ninja_content}" ); ensure!( 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 index 24d5096cf..2b115c82e 100644 --- a/tests/snapshots/ninja/ninja_snapshot_tests__multi_command_manifest_ninja.snap +++ b/tests/snapshots/ninja/ninja_snapshot_tests__multi_command_manifest_ninja.snap @@ -3,9 +3,9 @@ source: tests/ninja_snapshot_tests.rs expression: ninja_content --- rule 2ba9145cfe6f2224b0cbcf527eea1371cf3cc742c8790e332ddfb7ba0c51e3da - command = echo check-fmt && echo lint && echo test + command = { echo check-fmt; } && { echo lint; } && { echo test; } description = Run the required checks sequentially build aggregate: 2ba9145cfe6f2224b0cbcf527eea1371cf3cc742c8790e332ddfb7ba0c51e3da -build done: 2ba9145cfe6f2224b0cbcf527eea1371cf3cc742c8790e332ddfb7ba0c51e3da +build done: 2ba9145cfe6f2224b0cbcf527eea1371cf3cc742c8790e332ddfb7ba0c51e3da \ No newline at end of file From 60df417a3af9bb9435d6f4e0aafae7af14a50f91 Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 9 Aug 2026 20:55:52 +0200 Subject: [PATCH 04/14] Address review feedback on command lists Translate the manifest.command_list_empty diagnostic into all 33 non-English catalogues, following each locale's quotation conventions; only the en-GB and en-US catalogues keep the English source text. Name the failing list position when a command list entry fails to render, so a Jinja error identifies the entry rather than only the recipe stage. Drop the debug-only panic and its two clippy expectations from reject_empty_command_recipe; Display::to_string already escalates the returned fmt::Error, so the fault still surfaces loudly. Compare scalar command assertions against StringOrList::String directly, since as_single also accepts a single-element list and so does not prove the scalar variant was preserved. Correct the users' guide and design doc: a command list is fail-fast, so a later entry runs only when the preceding entry exits zero. Only the working directory, environment, and shell variables carry forward, and a failed entry may leave side effects behind. Reflow the shell-quote link paragraph within 80 columns. Co-Authored-By: Claude Opus 5 (1M context) --- docs/netsuke-design.md | 26 ++++++++++----------- docs/users-guide.md | 13 +++++++---- locales/ar/messages.ftl | 2 +- locales/cs/messages.ftl | 2 +- locales/cy/messages.ftl | 2 +- locales/da/messages.ftl | 2 +- locales/de/messages.ftl | 2 +- locales/el/messages.ftl | 2 +- locales/es-419/messages.ftl | 2 +- locales/es-ES/messages.ftl | 2 +- locales/fa/messages.ftl | 2 +- locales/fi/messages.ftl | 2 +- locales/fr/messages.ftl | 2 +- locales/gd/messages.ftl | 2 +- locales/he/messages.ftl | 2 +- locales/hi/messages.ftl | 2 +- locales/hu/messages.ftl | 2 +- locales/id/messages.ftl | 2 +- locales/it/messages.ftl | 2 +- locales/ja/messages.ftl | 2 +- locales/ko/messages.ftl | 2 +- locales/nb/messages.ftl | 2 +- locales/nl/messages.ftl | 2 +- locales/pl/messages.ftl | 2 +- locales/pt-BR/messages.ftl | 2 +- locales/pt-PT/messages.ftl | 2 +- locales/ro/messages.ftl | 2 +- locales/ru/messages.ftl | 2 +- locales/sv/messages.ftl | 2 +- locales/th/messages.ftl | 2 +- locales/tr/messages.ftl | 2 +- locales/uk/messages.ftl | 2 +- locales/vi/messages.ftl | 2 +- locales/zh-Hans/messages.ftl | 2 +- locales/zh-Hant/messages.ftl | 2 +- src/manifest/render.rs | 45 ++++++++++++++++++++++++++++++------ src/ninja_gen.rs | 19 ++++++--------- tests/ast_tests/parsing.rs | 4 ++-- 38 files changed, 101 insertions(+), 72 deletions(-) diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index 6c7080419..f4cb1dbcd 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -248,20 +248,20 @@ Each entry in the `rules` list is a mapping that defines a reusable action. - `command`: A command string, or a non-empty ordered list of command strings, to be executed. Each entry 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 [`shell-quote`](https://docs.rs/shell-quote/latest/shell_quote/) + placeholders to space-separated lists of file paths quoted for POSIX `/bin/sh` + 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). A list command is emitted as a single fail-fast `&&` chain, - so entries run in declaration order and the chain stops at the first - non-zero exit; all entries share one shell process, carrying working - directory, environment, and exit-code state forward like a `script` block. - An empty command list is rejected during manifest deserialization. - 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. + command; Ninja executes this text verbatim. After interpolation, the command + must be parsable by [shlex](https://docs.rs/shlex/latest/shlex/) (POSIX mode). + A list command is emitted as a single fail-fast `&&` chain, so entries run in + declaration order and the chain stops at the first non-zero exit; all entries + share one shell process, carrying working directory, environment, and shell + variables forward like a `script` block, and each later entry starts only + when the preceding entry exits with status zero. An empty command list is + rejected during manifest deserialization. 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. - `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 `#!` diff --git a/docs/users-guide.md b/docs/users-guide.md index 19cc45613..a5bb8b741 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -299,8 +299,9 @@ 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. All entries run in one shell process, so working directory, -environment, and exit-code state set by an earlier entry carry into later -entries, exactly as they do for `script`. An empty command list is rejected +environment, and shell variables set by an earlier entry carry into later +entries, exactly as they do for `script`. Each later entry starts only when +the preceding entry exits with status zero. An empty command list is rejected when the manifest is parsed. @@ -1066,9 +1067,11 @@ Netsuke reduces some common quoting mistakes, but it is not a sandbox: with the host. - `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 even if the earlier entry only partially succeeded. +- 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. - Literal shell dollar expressions currently require Ninja-aware escaping, such as `$$PATH`. diff --git a/locales/ar/messages.ftl b/locales/ar/messages.ftl index ad67a582c..1669593ce 100644 --- a/locales/ar/messages.ftl +++ b/locales/ar/messages.ftl @@ -149,7 +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 = ‏The 'command' field must not be empty: provide a command string or a non-empty list. +manifest.command_list_empty = حقل «command» يجب ألا يكون فارغًا: قدِّم سلسلة أمر أو قائمة غير فارغة. # أخطاء التمثيل الوسيط. ir.rule_not_found = تعذّر العثور على القاعدة «{ $rule }» التي يشير إليها الهدف «{ $target }». diff --git a/locales/cs/messages.ftl b/locales/cs/messages.ftl index 533363ebc..4e852fdac 100644 --- a/locales/cs/messages.ftl +++ b/locales/cs/messages.ftl @@ -149,7 +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 = The 'command' field must not be empty: provide a command string or a non-empty list. +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 e8810a8a2..5cd1c875c 100644 --- a/locales/cy/messages.ftl +++ b/locales/cy/messages.ftl @@ -149,7 +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 = The 'command' field must not be empty: provide a command string or a non-empty list. +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 03420912d..610a479ce 100644 --- a/locales/da/messages.ftl +++ b/locales/da/messages.ftl @@ -149,7 +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 = The 'command' field must not be empty: provide a command string or a non-empty list. +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 f13f688a9..0314f12e1 100644 --- a/locales/de/messages.ftl +++ b/locales/de/messages.ftl @@ -149,7 +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 = The 'command' field must not be empty: provide a command string or a non-empty list. +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 53786656f..f6413b904 100644 --- a/locales/el/messages.ftl +++ b/locales/el/messages.ftl @@ -150,7 +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 = The 'command' field must not be empty: provide a command string or a non-empty list. +manifest.command_list_empty = Το πεδίο «command» δεν πρέπει να είναι κενό: δώστε μια συμβολοσειρά εντολής ή μια μη κενή λίστα. # Σφάλματα της ενδιάμεσης αναπαράστασης. ir.rule_not_found = Ο κανόνας «{ $rule }» στον οποίο παραπέμπει ο στόχος «{ $target }» δεν βρέθηκε. diff --git a/locales/es-419/messages.ftl b/locales/es-419/messages.ftl index 88dd05895..ea92ca583 100644 --- a/locales/es-419/messages.ftl +++ b/locales/es-419/messages.ftl @@ -150,7 +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 = The 'command' field must not be empty: provide a command string or a non-empty list. +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 e7e0ac13d..685d5ad58 100644 --- a/locales/es-ES/messages.ftl +++ b/locales/es-ES/messages.ftl @@ -149,7 +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 = The 'command' field must not be empty: provide a command string or a non-empty list. +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 a489bca7c..b393f4a05 100644 --- a/locales/fa/messages.ftl +++ b/locales/fa/messages.ftl @@ -149,7 +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 = ‏The 'command' field must not be empty: provide a command string or a non-empty list. +manifest.command_list_empty = فیلد «command» نباید خالی باشد: یک رشتهٔ فرمان یا فهرستی ناتهی ارائه دهید. # خطاهای بازنمایی میانی. ir.rule_not_found = قاعدهٔ «{ $rule }» که هدف «{ $target }» به آن ارجاع می‌دهد یافت نشد. diff --git a/locales/fi/messages.ftl b/locales/fi/messages.ftl index 075b1e3af..5867e496f 100644 --- a/locales/fi/messages.ftl +++ b/locales/fi/messages.ftl @@ -149,7 +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 = The 'command' field must not be empty: provide a command string or a non-empty list. +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 dc19e82bc..a629f6261 100644 --- a/locales/fr/messages.ftl +++ b/locales/fr/messages.ftl @@ -150,7 +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 = The 'command' field must not be empty: provide a command string or a non-empty list. +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 740ec889e..cde202b08 100644 --- a/locales/gd/messages.ftl +++ b/locales/gd/messages.ftl @@ -149,7 +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 = The 'command' field must not be empty: provide a command string or a non-empty list. +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 20af419f7..ec19b5843 100644 --- a/locales/he/messages.ftl +++ b/locales/he/messages.ftl @@ -149,7 +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 = ‏The 'command' field must not be empty: provide a command string or a non-empty list. +manifest.command_list_empty = השדה „command” אינו יכול להיות ריק: יש לספק מחרוזת פקודה או רשימה שאינה ריקה. # שגיאות הייצוג הביניימי. ir.rule_not_found = הכלל „{ $rule }” שאליו מפנה היעד „{ $target }” לא נמצא. diff --git a/locales/hi/messages.ftl b/locales/hi/messages.ftl index ff67c22d2..b380c2036 100644 --- a/locales/hi/messages.ftl +++ b/locales/hi/messages.ftl @@ -149,7 +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 = The 'command' field must not be empty: provide a command string or a non-empty list. +manifest.command_list_empty = “command” फ़ील्ड रिक्त नहीं होना चाहिए: कोई कमांड स्ट्रिंग या ग़ैर-रिक्त सूची दें। # मध्यवर्ती निरूपण की त्रुटियाँ। ir.rule_not_found = लक्ष्य “{ $target }” जिस नियम “{ $rule }” का संदर्भ देता है वह नहीं मिला। diff --git a/locales/hu/messages.ftl b/locales/hu/messages.ftl index f4a33cb77..fa93f43ea 100644 --- a/locales/hu/messages.ftl +++ b/locales/hu/messages.ftl @@ -149,7 +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 = The 'command' field must not be empty: provide a command string or a non-empty list. +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 f7128d8b0..4cb266021 100644 --- a/locales/id/messages.ftl +++ b/locales/id/messages.ftl @@ -149,7 +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 = The 'command' field must not be empty: provide a command string or a non-empty list. +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 c4efa2e3e..730d32970 100644 --- a/locales/it/messages.ftl +++ b/locales/it/messages.ftl @@ -150,7 +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 = The 'command' field must not be empty: provide a command string or a non-empty list. +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 69ba29882..fdfc9868d 100644 --- a/locales/ja/messages.ftl +++ b/locales/ja/messages.ftl @@ -149,7 +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 = The 'command' field must not be empty: provide a command string or a non-empty list. +manifest.command_list_empty = 「command」フィールドは空にできません: コマンド文字列または空でないリストを指定してください。 # 中間表現のエラー。 ir.rule_not_found = ターゲット「{ $target }」が参照する規則「{ $rule }」が見つかりません。 diff --git a/locales/ko/messages.ftl b/locales/ko/messages.ftl index 073d96fc3..2973b31fc 100644 --- a/locales/ko/messages.ftl +++ b/locales/ko/messages.ftl @@ -149,7 +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 = The 'command' field must not be empty: provide a command string or a non-empty list. +manifest.command_list_empty = 'command' 필드는 비어 있을 수 없습니다: 명령 문자열 또는 비어 있지 않은 목록을 지정하십시오. # 중간 표현 오류. ir.rule_not_found = 대상 '{ $target }'이(가) 참조하는 규칙 '{ $rule }'을(를) 찾을 수 없습니다. diff --git a/locales/nb/messages.ftl b/locales/nb/messages.ftl index 2a556b993..3c1e98bd0 100644 --- a/locales/nb/messages.ftl +++ b/locales/nb/messages.ftl @@ -149,7 +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 = The 'command' field must not be empty: provide a command string or a non-empty list. +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 9f98143ca..d402bacfa 100644 --- a/locales/nl/messages.ftl +++ b/locales/nl/messages.ftl @@ -149,7 +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 = The 'command' field must not be empty: provide a command string or a non-empty list. +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 37435aa8d..77c4fe8e4 100644 --- a/locales/pl/messages.ftl +++ b/locales/pl/messages.ftl @@ -149,7 +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 = The 'command' field must not be empty: provide a command string or a non-empty list. +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 833d12bd4..2ced9ce27 100644 --- a/locales/pt-BR/messages.ftl +++ b/locales/pt-BR/messages.ftl @@ -150,7 +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 = The 'command' field must not be empty: provide a command string or a non-empty list. +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 2941a4f85..394a77930 100644 --- a/locales/pt-PT/messages.ftl +++ b/locales/pt-PT/messages.ftl @@ -150,7 +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 = The 'command' field must not be empty: provide a command string or a non-empty list. +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 488d39c7a..9cc7901b1 100644 --- a/locales/ro/messages.ftl +++ b/locales/ro/messages.ftl @@ -149,7 +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 = The 'command' field must not be empty: provide a command string or a non-empty list. +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 88cb2d457..ca9ccd562 100644 --- a/locales/ru/messages.ftl +++ b/locales/ru/messages.ftl @@ -149,7 +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 = The 'command' field must not be empty: provide a command string or a non-empty list. +manifest.command_list_empty = Поле «command» не должно быть пустым: укажите строку команды или непустой список. # Ошибки промежуточного представления. ir.rule_not_found = Правило «{ $rule }», на которое ссылается цель «{ $target }», не найдено. diff --git a/locales/sv/messages.ftl b/locales/sv/messages.ftl index aa8babc58..ad1126a0f 100644 --- a/locales/sv/messages.ftl +++ b/locales/sv/messages.ftl @@ -149,7 +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 = The 'command' field must not be empty: provide a command string or a non-empty list. +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 8be17d812..5afd113ae 100644 --- a/locales/th/messages.ftl +++ b/locales/th/messages.ftl @@ -149,7 +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 = The 'command' field must not be empty: provide a command string or a non-empty list. +manifest.command_list_empty = ฟิลด์ “command” ต้องไม่ว่าง: ระบุสตริงคำสั่งหรือรายการที่ไม่ว่าง # ข้อผิดพลาดของรูปแทนระดับกลาง ir.rule_not_found = ไม่พบกฎ “{ $rule }” ที่เป้าหมาย “{ $target }” อ้างถึง diff --git a/locales/tr/messages.ftl b/locales/tr/messages.ftl index 0246ae304..8af7246e7 100644 --- a/locales/tr/messages.ftl +++ b/locales/tr/messages.ftl @@ -149,7 +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 = The 'command' field must not be empty: provide a command string or a non-empty list. +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 5ccbc2bff..260d0188d 100644 --- a/locales/uk/messages.ftl +++ b/locales/uk/messages.ftl @@ -149,7 +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 = The 'command' field must not be empty: provide a command string or a non-empty list. +manifest.command_list_empty = Поле «command» не має бути порожнім: укажіть рядок команди або непорожній список. # Помилки проміжного подання. ir.rule_not_found = Правило «{ $rule }», на яке посилається ціль «{ $target }», не знайдено. diff --git a/locales/vi/messages.ftl b/locales/vi/messages.ftl index 14e180b69..06a083ba7 100644 --- a/locales/vi/messages.ftl +++ b/locales/vi/messages.ftl @@ -149,7 +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 = The 'command' field must not be empty: provide a command string or a non-empty list. +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 4df49f6ed..dc92f76fa 100644 --- a/locales/zh-Hans/messages.ftl +++ b/locales/zh-Hans/messages.ftl @@ -148,7 +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 = The 'command' field must not be empty: provide a command string or a non-empty list. +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 ff86bf5fc..663e321b9 100644 --- a/locales/zh-Hant/messages.ftl +++ b/locales/zh-Hant/messages.ftl @@ -148,7 +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 = The 'command' field must not be empty: provide a command string or a non-empty list. +manifest.command_list_empty = 「command」欄位不得為空:請提供命令字串或非空清單。 # 中介表示法的錯誤。 ir.rule_not_found = 找不到目標「{ $target }」所參照的規則「{ $rule }」。 diff --git a/src/manifest/render.rs b/src/manifest/render.rs index ae55e0a8b..9b2d8fdd2 100644 --- a/src/manifest/render.rs +++ b/src/manifest/render.rs @@ -102,8 +102,8 @@ fn render_string_or_list(value: &mut StringOrList, env: &Environment, ctx: &Vars /// 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, so a rendering failure names the recipe stage rather than the list -/// position. +/// 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, @@ -111,15 +111,17 @@ fn render_recipe_string_or_list( what: impl FnOnce() -> String, ) -> Result<()> { let label = what(); - let render_entry = |entry: &mut String| -> Result<()> { - *entry = render_recipe_str_with(env, entry, ctx, || label.clone())?; + let render_entry = |entry: &mut String, position: Option| -> Result<()> { + *entry = render_recipe_str_with(env, entry, ctx, || { + position.map_or_else(|| label.clone(), |index| format!("{label} entry {index}")) + })?; Ok(()) }; match value { - StringOrList::String(s) => render_entry(s)?, + StringOrList::String(s) => render_entry(s, None)?, StringOrList::List(list) => { - for item in list { - render_entry(item)?; + for (index, item) in list.iter_mut().enumerate() { + render_entry(item, Some(index + 1))?; } } StringOrList::Empty => {} @@ -321,4 +323,33 @@ mod tests { ); 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(()) + } } diff --git a/src/ninja_gen.rs b/src/ninja_gen.rs index 1203c475c..ce8c212d7 100644 --- a/src/ninja_gen.rs +++ b/src/ninja_gen.rs @@ -281,19 +281,14 @@ 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] - #[expect( - clippy::panic_in_result_fn, - reason = "debug builds intentionally panic to expose empty command recipes" - )] - #[expect( - clippy::manual_assert, - reason = "debug-only guard escalates to panic for visibility" - )] - fn reject_empty_command_recipe() -> fmt::Result { - if cfg!(debug_assertions) { - panic!("empty command recipes are rejected while deserializing the manifest"); - } + const fn reject_empty_command_recipe() -> fmt::Result { Err(fmt::Error) } } diff --git a/tests/ast_tests/parsing.rs b/tests/ast_tests/parsing.rs index 6bf2d84bf..8c01cf77e 100644 --- a/tests/ast_tests/parsing.rs +++ b/tests/ast_tests/parsing.rs @@ -37,7 +37,7 @@ targets: if let Recipe::Command { command } = &first.recipe { ensure!( - command.as_single() == Some("echo hi"), + *command == StringOrList::String("echo hi".into()), "unexpected command: {command:?}" ); } else { @@ -190,7 +190,7 @@ fn vars_section_allows_non_reserved_names() -> Result<()> { bail!("expected a command recipe, got {:?}", first.recipe); }; ensure!( - command.as_single() == Some("echo hi"), + *command == StringOrList::String("echo hi".into()), "unexpected command: {command:?}" ); Ok(()) From ac521cfc1d927d5c30d450e2421fba81be645a95 Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 10 Aug 2026 11:08:40 +0200 Subject: [PATCH 05/14] Ignore local VTCode tooling artifacts The .vtcode/ directory holds transient session tool-output logs and vtcode.toml is a machine-specific agent configuration referencing a local API key environment variable. Neither belongs in the repository; follow the existing convention that already ignores .claude/, .crush/, .grepai/, and .memdb/. --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index e153a98c7..c8c1f4352 100644 --- a/.gitignore +++ b/.gitignore @@ -4,6 +4,8 @@ target/ *.swp .crush/ .claude/ +.vtcode/ +vtcode.toml .memdb/ .grepai/ build.ninja From 3ead23506a8a74104a024b7504eef8e8ff46fe4e Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 12 Aug 2026 00:15:06 +0200 Subject: [PATCH 06/14] Fix command-list shell boundaries (#550) Evaluate list entries as safely quoted shell text within their brace groups. This prevents inline comments and trailing background operators from swallowing the chain delimiter while preserving order, fail-fast behaviour, and shared shell state. Align the related schema documentation, Arabic diagnostic, fixture capability access, and Ninja snapshots. --- CHANGELOG.md | 2 +- docs/netsuke-design.md | 29 ++--- locales/ar/messages.ftl | 2 +- src/ast.rs | 3 +- src/ninja_gen.rs | 24 ++-- src/ninja_gen_tests.rs | 4 +- ...inja_gen_command_list_integration_tests.rs | 103 ++++++++++++++++++ tests/ninja_snapshot_tests.rs | 15 ++- ...t_tests__multi_command_manifest_ninja.snap | 4 +- 9 files changed, 156 insertions(+), 30 deletions(-) create mode 100644 tests/ninja_gen_command_list_integration_tests.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 957ee2095..3530e1704 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,7 +23,7 @@ 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 + 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 ([#550](https://github.com/leynos/netsuke/issues/550)) diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index f4cb1dbcd..7aa98e5e5 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 } @@ -250,18 +250,18 @@ Each entry in the `rules` list is a mapping that defines a reusable action. `{{ 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 [`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). - A list command is emitted as a single fail-fast `&&` chain, so entries run in - declaration order and the chain stops at the first non-zero exit; all entries - share one shell process, carrying working directory, environment, and shell - variables forward like a `script` block, and each later entry starts only - when the preceding entry exits with status zero. An empty command list is - rejected during manifest deserialization. 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. + crate (Sh mode) before hashing the action. After interpolation, a scalar + command passes through unchanged, while a list is lowered to brace groups + that evaluate each entry and are joined by `&&` (for example, + `{ eval 'first'; } && { eval 'second'; }`). 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 while each entry + remains a separate shell unit. 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. 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. - `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 `#!` @@ -331,7 +331,8 @@ 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. - `script`: A multi-line script passed to the interpreter. When present, it is defined using the YAML `|` block style. diff --git a/locales/ar/messages.ftl b/locales/ar/messages.ftl index 1669593ce..47bbeafa0 100644 --- a/locales/ar/messages.ftl +++ b/locales/ar/messages.ftl @@ -149,7 +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» يجب ألا يكون فارغًا: قدِّم سلسلة أمر أو قائمة غير فارغة. +manifest.command_list_empty = يجب ألّا تكون قائمة الأوامر فارغة؛ قدِّم سلسلة أمر أو قائمة غير فارغة. # أخطاء التمثيل الوسيط. ir.rule_not_found = تعذّر العثور على القاعدة «{ $rule }» التي يشير إليها الهدف «{ $target }». diff --git a/src/ast.rs b/src/ast.rs index 286fb4260..c8cdfa5ea 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -145,7 +145,8 @@ pub enum Recipe { /// 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. + /// 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. diff --git a/src/ninja_gen.rs b/src/ninja_gen.rs index ce8c212d7..68c812a4b 100644 --- a/src/ninja_gen.rs +++ b/src/ninja_gen.rs @@ -205,6 +205,15 @@ fn escape_script(script: &str) -> String { .replace('\n', "\\n") } +/// 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(value: &str) -> String { + let escaped = value.replace('\'', r"'\\''"); + format!("'{escaped}'") +} + /// Wrapper struct to display a rule with its identifier. struct NamedAction<'a> { id: &'a str, @@ -217,15 +226,16 @@ impl NamedAction<'_> { Recipe::Command { command } => { let command_line = match command { StringOrList::String(cmd) => cmd.clone(), - // Brace groups keep each entry a distinct shell unit so its - // own `||`, `;`, or `&` cannot escape the entry boundary - // and mask an earlier failure. 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. + // 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. StringOrList::List(items) => items .iter() - .map(|item| format!("{{ {item}; }}")) + .map(|item| format!("{{ eval {}; }}", shell_single_quote(item))) .join(" && "), StringOrList::Empty => return Self::reject_empty_command_recipe(), }; diff --git a/src/ninja_gen_tests.rs b/src/ninja_gen_tests.rs index 20de55984..8d733122c 100644 --- a/src/ninja_gen_tests.rs +++ b/src/ninja_gen_tests.rs @@ -116,7 +116,9 @@ fn generate_command_list_ninja_joins_a_fail_fast_chain() -> Result<()> { let ninja = generate(&graph)?; ensure!( - ninja.contains("command = { echo one; } && { echo two; } && { echo three; }"), + ninja.contains( + "command = { eval 'echo one'; } && { eval 'echo two'; } && { eval 'echo three'; }" + ), "command list entries should be isolated brace groups joined by &&:\n{ninja}" ); 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..4c9522927 --- /dev/null +++ b/tests/ninja_gen_command_list_integration_tests.rs @@ -0,0 +1,103 @@ +//! 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::generate; +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 = Utf8PathBuf::from_path_buf(dir.path().to_path_buf()) + .map_err(|path| anyhow::anyhow!("temp dir path {path:?} is not UTF-8"))?; + let action = Action { + recipe: Recipe::Command { + command: StringOrList::List(entries), + }, + description: None, + depfile: None, + deps_format: None, + pool: None, + restat: false, + }; + 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 = 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 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!["true &".into(), "echo second > after-background.txt".into()], + "after-background.txt", + "second", + ) +} diff --git a/tests/ninja_snapshot_tests.rs b/tests/ninja_snapshot_tests.rs index 78f7a2b5d..7b6c7e165 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}; @@ -132,7 +133,10 @@ fn conditional_manifest_ninja_snapshot() -> Result<()> { #[test] fn multi_command_manifest_ninja_snapshot() -> Result<()> { - let manifest_yaml = std::fs::read_to_string("tests/data/multi_command.yml") + 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)?; @@ -140,7 +144,9 @@ fn multi_command_manifest_ninja_snapshot() -> Result<()> { let ninja_content = ninja_gen::generate(&ir)?; ensure!( - ninja_content.contains("{ echo check-fmt; } && { echo lint; } && { echo test; }"), + ninja_content.contains( + "{ eval 'echo check-fmt'; } && { eval 'echo lint'; } && { eval 'echo test'; }" + ), "expected the command list joined into a fail-fast chain:\n{ninja_content}" ); ensure!( @@ -162,7 +168,10 @@ fn multi_command_manifest_ninja_snapshot() -> Result<()> { #[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 index 2b115c82e..3d91fb696 100644 --- a/tests/snapshots/ninja/ninja_snapshot_tests__multi_command_manifest_ninja.snap +++ b/tests/snapshots/ninja/ninja_snapshot_tests__multi_command_manifest_ninja.snap @@ -3,9 +3,9 @@ source: tests/ninja_snapshot_tests.rs expression: ninja_content --- rule 2ba9145cfe6f2224b0cbcf527eea1371cf3cc742c8790e332ddfb7ba0c51e3da - command = { echo check-fmt; } && { echo lint; } && { echo test; } + command = { eval 'echo check-fmt'; } && { eval 'echo lint'; } && { eval 'echo test'; } description = Run the required checks sequentially build aggregate: 2ba9145cfe6f2224b0cbcf527eea1371cf3cc742c8790e332ddfb7ba0c51e3da -build done: 2ba9145cfe6f2224b0cbcf527eea1371cf3cc742c8790e332ddfb7ba0c51e3da \ No newline at end of file +build done: 2ba9145cfe6f2224b0cbcf527eea1371cf3cc742c8790e332ddfb7ba0c51e3da From ce631a746d4fff59eb84a287d3e37d2aecb2d29e Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 12 Aug 2026 04:24:02 +0200 Subject: [PATCH 07/14] Document command-list lowering --- docs/developers-guide.md | 39 ++++++++++++ docs/netsuke-design.md | 127 ++++++++++++++++++++------------------- docs/users-guide.md | 40 ++++++++++-- 3 files changed, 138 insertions(+), 68 deletions(-) diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 2bd610e15..b69be1e05 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -196,6 +196,45 @@ 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 `StringOrList::Empty` 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. +- `src/runner/process` forwards the command's output and recognises the + bounded `netsuke command-list failure: action N, entry M` marker. A failed + list therefore retains the original exit status while adding the generated + action index and one-based entry index to the Ninja failure error. + +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 7aa98e5e5..e3f3be572 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -246,22 +246,27 @@ Each entry in the `rules` list is a mapping that defines a reusable action. - `name`: A unique string identifier for the rule. - `command`: A command string, or a non-empty ordered list of command strings, - to be executed. Each entry 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 [`shell-quote`](https://docs.rs/shell-quote/latest/shell_quote/) - crate (Sh mode) before hashing the action. After interpolation, a scalar - command passes through unchanged, while a list is lowered to brace groups - that evaluate each entry and are joined by `&&` (for example, - `{ eval 'first'; } && { eval 'second'; }`). 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 while each entry - remains a separate shell unit. 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. 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. + 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. 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 `#!` @@ -332,7 +337,9 @@ rule: ``` - `command`: A command string or non-empty ordered list of command strings to - run directly for this target. + 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. @@ -793,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` @@ -1951,17 +1960,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: @@ -2005,9 +2016,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 @@ -2179,33 +2193,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 @@ -2215,10 +2217,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 a5bb8b741..b1fa56b51 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -298,11 +298,23 @@ 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. All entries run in one shell process, so working directory, -environment, and shell variables set by an earlier entry carry into later -entries, exactly as they do for `script`. Each later entry starts only when -the preceding entry exits with status zero. An empty command list is rejected -when the manifest is parsed. +`&&` 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 1, entry 2`. @@ -322,6 +334,19 @@ targets: 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 +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. @@ -1071,7 +1096,10 @@ Netsuke reduces some common quoting mistakes, but it is not a sandbox: 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. + halts the chain. The generated brace/eval boundary keeps comments and + trailing control operators inside an entry from changing the chain's + structure. Failure diagnostics include the action and entry positions when + Netsuke can attribute the failed list entry. - Literal shell dollar expressions currently require Ninja-aware escaping, such as `$$PATH`. From 2f435608e07a654bbf2862f4f88ca33bdd516c25 Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 12 Aug 2026 04:47:28 +0200 Subject: [PATCH 08/14] Harden command-list lowering (#550) Cover direct target command lists from rendering through real Ninja execution, and reject programmatic empty command recipes explicitly. Isolate and attribute list-entry failures without exposing command payloads, while preserving the byte-identical scalar output path. Reuse bindings per recipe and document the lowering contract. --- docs/developers-guide.md | 2 +- docs/users-guide.md | 4 + src/ir/cmd_interpolate.rs | 101 ++++++++++---- src/ir/from_manifest_support.rs | 21 ++- src/ir/from_manifest_support_tests.rs | 63 +++++++++ src/manifest/render.rs | 74 ++++++---- src/manifest/render_command_list_tests.rs | 35 +++++ src/ninja_gen.rs | 90 ++++++++++--- src/ninja_gen_property_tests.rs | 101 +++++++++++++- src/ninja_gen_tests.rs | 36 ++++- src/runner/process/child_exit.rs | 57 ++++++++ src/runner/process/failure_attribution.rs | 127 ++++++++++++++++++ src/runner/process/mod.rs | 108 +++++++-------- src/runner/process/tests.rs | 2 +- tests/command_env_ui_tests.rs | 15 +++ tests/documentation_examples_tests.rs | 2 + tests/logging_stderr/command_list_failure.rs | 110 +++++++++++++++ tests/logging_stderr_tests.rs | 3 + ...inja_gen_command_list_integration_tests.rs | 105 ++++++++++++++- tests/ninja_snapshot_tests.rs | 2 +- ...t_tests__multi_command_manifest_ninja.snap | 2 +- tests/ui/command_list_public_api_pass.rs | 25 ++++ 22 files changed, 939 insertions(+), 146 deletions(-) create mode 100644 src/ir/from_manifest_support_tests.rs create mode 100644 src/manifest/render_command_list_tests.rs create mode 100644 src/runner/process/child_exit.rs create mode 100644 src/runner/process/failure_attribution.rs create mode 100644 tests/logging_stderr/command_list_failure.rs create mode 100644 tests/ui/command_list_public_api_pass.rs diff --git a/docs/developers-guide.md b/docs/developers-guide.md index b69be1e05..b1c8aa790 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -225,7 +225,7 @@ The lowering stages have deliberately separate responsibilities: subshell, so directory changes, environment assignments, and shell variables can carry from one entry to the next. The `&&` chain remains fail-fast. -- `src/runner/process` forwards the command's output and recognises the +- `src/runner/process` forwards the command's output and recognizes the bounded `netsuke command-list failure: action N, entry M` marker. A failed list therefore retains the original exit status while adding the generated action index and one-based entry index to the Ninja failure error. diff --git a/docs/users-guide.md b/docs/users-guide.md index b1fa56b51..7c8260e1c 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -337,7 +337,11 @@ targets: 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: 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 f8840cace..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,20 +31,15 @@ pub(super) fn register_action( ) -> Result { let resolved_recipe = match recipe { Recipe::Command { command } => { + let command_bindings = CommandBindings::new(bindings.inputs, bindings.outputs); let interpolated = match command { - StringOrList::String(cmd) => StringOrList::String(interpolate_command( - &cmd, - bindings.inputs, - bindings.outputs, - )?), + 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( - &item, - bindings.inputs, - bindings.outputs, - )?); + rendered.push(interpolate_command_with_bindings(&item, &command_bindings)?); } StringOrList::List(rendered) } @@ -355,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/manifest/render.rs b/src/manifest/render.rs index 9b2d8fdd2..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 @@ -111,8 +114,9 @@ fn render_recipe_string_or_list( what: impl FnOnce() -> String, ) -> Result<()> { let label = what(); + let recipe_ctx = recipe_render_context(ctx); let render_entry = |entry: &mut String, position: Option| -> Result<()> { - *entry = render_recipe_str_with(env, entry, ctx, || { + *entry = render_str_with(env, entry, &recipe_ctx, || { position.map_or_else(|| label.clone(), |index| format!("{label} entry {index}")) })?; Ok(()) @@ -129,29 +133,13 @@ fn render_recipe_string_or_list( Ok(()) } -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) -} - -/// 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()) @@ -159,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)] @@ -353,3 +373,7 @@ mod tests { 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/ninja_gen.rs b/src/ninja_gen.rs index 68c812a4b..8a069f356 100644 --- a/src/ninja_gen.rs +++ b/src/ninja_gen.rs @@ -14,7 +14,6 @@ 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 { @@ -26,6 +25,12 @@ pub enum NinjaGenError { /// 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, + }, /// Formatting the Ninja output failed. #[error("{message}")] Format { @@ -45,7 +50,6 @@ impl From for NinjaGenError { } } } - macro_rules! write_kv { ($f:expr, $key:expr, $opt:expr) => { if let Some(val) = $opt { @@ -92,8 +96,9 @@ 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, or writing to the output +/// fails. pub fn generate(graph: &BuildGraph) -> Result { let mut out = String::new(); generate_into(graph, &mut out)?; @@ -131,12 +136,24 @@ 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, 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 { - write!(out, "{}", NamedAction { id, action })?; + 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, + action_index, + } + )?; } let mut edges: Vec<_> = graph.targets.values().collect(); @@ -210,22 +227,48 @@ fn escape_script(script: &str) -> String { /// 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(value: &str) -> String { - let escaped = value.replace('\'', r"'\\''"); + let escaped = value.replace('\'', r"'\''"); format!("'{escaped}'") } +/// Prefix used to carry bounded list-entry failure attribution through Ninja. +pub(crate) const COMMAND_LIST_FAILURE_PREFIX: &str = "netsuke command-list failure: action "; + +const fn validate_action_recipe( + action: &crate::ir::Action, + action_index: usize, +) -> Result<(), NinjaGenError> { + if matches!( + action.recipe, + Recipe::Command { + command: StringOrList::Empty + } + ) { + return Err(NinjaGenError::EmptyCommandRecipe { action_index }); + } + Ok(()) +} + /// Wrapper struct to display a rule with its identifier. struct NamedAction<'a> { id: &'a str, action: &'a crate::ir::Action, + action_index: usize, } impl NamedAction<'_> { fn write_recipe(&self, f: &mut Formatter<'_>) -> fmt::Result { match &self.action.recipe { - Recipe::Command { command } => { - let command_line = match command { - StringOrList::String(cmd) => cmd.clone(), + Recipe::Command { + command: StringOrList::String(scalar_command), + } => { + Self::assert_shell_command(scalar_command); + writeln!(f, " command = {scalar_command}") + } + Recipe::Command { + command: StringOrList::List(items), + } => { + let command_line = // 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 @@ -233,15 +276,18 @@ impl NamedAction<'_> { // directory, environment, and variables set by one entry // still carry into the next, and the `&&` chain stays // fail-fast. - StringOrList::List(items) => items - .iter() - .map(|item| format!("{{ eval {}; }}", shell_single_quote(item))) - .join(" && "), - StringOrList::Empty => return Self::reject_empty_command_recipe(), - }; + items.iter() + .enumerate() + .map(|(entry_index, item)| { + command_list_entry(item, self.action_index, entry_index + 1) + }) + .join(" && "); Self::assert_shell_command(&command_line); writeln!(f, " command = {command_line}") } + 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(), } @@ -303,6 +349,15 @@ impl NamedAction<'_> { } } +fn command_list_entry(command: &str, action_index: usize, entry_index: usize) -> String { + let context = format!("{COMMAND_LIST_FAILURE_PREFIX}{action_index}, entry {entry_index}"); + format!( + "{{ if eval {}; then :; else _netsuke_command_status=$$?; printf '%s\\n' '{}' >&2; exit \"$$_netsuke_command_status\"; fi; }}", + shell_single_quote(command), + context, + ) +} + impl Display for NamedAction<'_> { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { writeln!(f, "rule {}", self.id)?; @@ -310,7 +365,6 @@ impl Display for NamedAction<'_> { self.write_metadata(f) } } - /// Wrapper struct to display a build edge. struct DisplayEdge<'a> { edge: &'a BuildEdge, diff --git a/src/ninja_gen_property_tests.rs b/src/ninja_gen_property_tests.rs index fb3c2d8d7..69be4a040 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}; +use crate::{ + ast::{Recipe, StringOrList}, + ir::{Action, BuildEdge, BuildGraph}, +}; fn edge_strategy_with_ranges( input_range: std::ops::Range, @@ -67,6 +70,47 @@ fn bare_pipe_position(line: &str) -> Option { line.match_indices(" | ").map(|(index, _)| index).next() } +fn command_list_graph(entries: &[String]) -> BuildGraph { + let mut graph = BuildGraph::default(); + graph.actions.insert( + "action".into(), + Action { + recipe: Recipe::Command { + command: StringOrList::List( + entries + .iter() + .map(|entry| format!("echo {entry}")) + .collect(), + ), + }, + description: None, + depfile: None, + deps_format: None, + pool: None, + restat: false, + }, + ); + graph +} + +fn scalar_graph(command: String) -> BuildGraph { + let mut graph = BuildGraph::default(); + graph.actions.insert( + "action".into(), + Action { + recipe: Recipe::Command { + command: StringOrList::String(command), + }, + description: None, + depfile: None, + deps_format: None, + pool: None, + restat: false, + }, + ); + graph +} + proptest! { #[test] fn implicit_deps_separator_precedes_order_only_separator(edge in edge_strategy_with_ranges(1..5, 1..5, 1..5)) { @@ -100,4 +144,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("[a-z]{1,12}", 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 expected_entries: Vec = entries.iter().enumerate().map(|(index, entry)| { + format!( + "{{ if eval 'echo {entry}'; then :; else _netsuke_command_status=$$?; printf '%s\\n' 'netsuke command-list failure: action 1, entry {}' >&2; exit \"$$_netsuke_command_status\"; fi; }}", + index + 1, + ) + }).collect(); + + let mut previous = 0usize; + for expected_entry in &expected_entries { + let position = command_line + .get(previous..) + .and_then(|remaining| remaining.find(expected_entry)) + .expect("every entry should retain its independent shell boundary"); + previous += position + expected_entry.len(); + } + prop_assert_eq!(command_line.matches("{ if eval '").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("{ if eval '"); + prop_assert!(retains_scalar_form); + prop_assert!(!uses_list_boundary); + } + + #[test] + fn programmatic_empty_command_recipes_are_rejected(action_id in "[a-z]{1,12}") { + let mut graph = BuildGraph::default(); + graph.actions.insert( + action_id, + Action { + recipe: Recipe::Command { command: StringOrList::Empty }, + description: None, + depfile: None, + deps_format: None, + pool: None, + restat: false, + }, + ); + 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_tests.rs b/src/ninja_gen_tests.rs index 8d733122c..429389ba9 100644 --- a/src/ninja_gen_tests.rs +++ b/src/ninja_gen_tests.rs @@ -116,14 +116,44 @@ fn generate_command_list_ninja_joins_a_fail_fast_chain() -> Result<()> { let ninja = generate(&graph)?; ensure!( - ninja.contains( - "command = { eval 'echo one'; } && { eval 'echo two'; } && { eval 'echo three'; }" - ), + ninja.contains(concat!( + "command = { if eval 'echo one'; then :; else _netsuke_command_status=$$?; ", + "printf '%s\\n' 'netsuke command-list failure: action 1, entry 1' >&2; ", + "exit \"$$_netsuke_command_status\"; fi; } && ", + "{ if eval 'echo two'; then :; else _netsuke_command_status=$$?; ", + "printf '%s\\n' 'netsuke command-list failure: action 1, entry 2' >&2; ", + "exit \"$$_netsuke_command_status\"; fi; } && ", + "{ if eval 'echo three'; then :; else _netsuke_command_status=$$?; ", + "printf '%s\\n' 'netsuke command-list failure: action 1, entry 3' >&2; ", + "exit \"$$_netsuke_command_status\"; fi; }" + )), "command list entries should be isolated brace groups joined by &&:\n{ninja}" ); Ok(()) } +#[test] +fn programmatic_empty_command_recipe_returns_a_typed_generation_error() { + let action = Action { + recipe: Recipe::Command { + command: StringOrList::Empty, + }, + description: None, + depfile: None, + deps_format: None, + pool: None, + restat: false, + }; + 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 assert_shell_command_tolerates_complex_syntax() { let command = r#"/bin/sh -c "echo 'nested quotes' && echo \"double\" && (echo subshell)""#; diff --git a/src/runner/process/child_exit.rs b/src/runner/process/child_exit.rs new file mode 100644 index 000000000..59041c78e --- /dev/null +++ b/src/runner/process/child_exit.rs @@ -0,0 +1,57 @@ +//! Child-process shutdown and Ninja non-zero exit conversion helpers. + +use std::{ + io, + process::{Child, ExitStatus}, + thread, +}; + +use super::streaming::ForwardStats; + +/// 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<&str>, +) -> 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/failure_attribution.rs b/src/runner/process/failure_attribution.rs new file mode 100644 index 000000000..f8a48040b --- /dev/null +++ b/src/runner/process/failure_attribution.rs @@ -0,0 +1,127 @@ +//! Bounded extraction of command-list failure attribution from Ninja stderr. + +use crate::ninja_gen::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()) +} + +pub(super) struct FailureAttributionWriter { + inner: W, + pending: Vec, + failure: Option, +} + +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) { + let Ok(line) = std::str::from_utf8(&self.pending) else { + return; + }; + let Some((action, entry)) = line + .strip_prefix(COMMAND_LIST_FAILURE_PREFIX) + .and_then(|suffix| suffix.split_once(", entry ")) + .and_then(|(action, entry)| { + Some((action.parse::().ok()?, entry.parse::().ok()?)) + }) + else { + return; + }; + if action > 0 && entry > 0 { + self.failure = Some(format!( + "{COMMAND_LIST_FAILURE_PREFIX}{action}, entry {entry}" + )); + } + } +} + +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::*; + + #[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(b"ure: action 7, entry 3\n") + .expect("second chunk should write"); + + assert_eq!( + writer.into_failure().as_deref(), + Some("netsuke command-list failure: action 7, 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(b"netsuke command-list failure: action 7, entry 3\n") + .expect("valid marker after unbounded content should write"); + + assert!(writer.into_failure().is_none()); + } +} diff --git a/src/runner/process/mod.rs b/src/runner/process/mod.rs index 417ff36d6..e24f11693 100644 --- a/src/runner/process/mod.rs +++ b/src/runner/process/mod.rs @@ -9,9 +9,10 @@ use std::{ process::{Child, Command, ExitStatus}, thread, }; -use tracing::{debug, warn}; +mod child_exit; mod command_logging; +mod failure_attribution; mod file_io; mod ninja_program; mod ninja_status; @@ -21,10 +22,12 @@ mod streaming; #[cfg(test)] mod tests; +use child_exit::{finalize_streaming, ninja_exit_error, terminate_child}; use command_logging::{ CommandLogContext, command_span, log_command_execution, log_command_exit_failure, log_command_spawn_failure, }; +use failure_attribution::{FailureAttributionWriter, forward_stderr_with_attribution}; pub use file_io::*; pub use ninja_program::resolve_ninja_program; #[cfg(doctest)] @@ -48,7 +51,6 @@ use streaming::{ForwardStats, forward_child_output, forward_child_output_with_ni /// 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); - // Public helpers for doctests only. This exposes internal helpers as a stable // testing surface without exporting them in release builds. #[cfg(doctest)] @@ -69,18 +71,35 @@ pub mod doc { }; } +#[derive(Clone, Copy)] +struct ExitFailureContext<'a> { + operation: &'a str, + suppress_stderr: bool, + command_list_failure: Option<&'a str>, +} + fn check_exit_status_with_context( status: ExitStatus, context: &CommandLogContext, - operation: &str, - suppress_stderr: bool, + failure_context: ExitFailureContext<'_>, ) -> 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) + log_command_exit_failure( + context, + failure_context.operation, + failure_context.suppress_stderr, + status, + ); + if let Some(failure) = failure_context.command_list_failure { + tracing::warn!( + command_list_failure = failure, + "Ninja command-list entry failed" + ); + } + ninja_exit_error(status, failure_context.command_list_failure) } } @@ -99,8 +118,17 @@ fn run_command_and_stream_with_context( tracing::Span::current().record("failure_category", "spawn"); log_command_spawn_failure(&context, operation, 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, suppress_stderr)?; + check_exit_status_with_context( + status, + &context, + ExitFailureContext { + operation, + suppress_stderr, + command_list_failure: command_list_failure.as_deref(), + }, + ) } /// Invoke the Ninja executable with the provided CLI settings. @@ -292,41 +320,28 @@ pub(crate) fn run_ninja_tool_with_status( 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 { +) -> (ForwardStats, Option) { + let mut attribution_writer = FailureAttributionWriter::new(output); + let stats = match status_observer { Some(observer) => forward_child_output_with_ninja_status( BufReader::new(stdout), - output, + &mut attribution_writer, observer, "stdout", ), - None => forward_child_output(BufReader::new(stdout), output, "stdout"), - } + None => forward_child_output(BufReader::new(stdout), &mut attribution_writer, "stdout"), + }; + (stats, attribution_writer.into_failure()) } fn spawn_and_stream_output( mut child: Child, status_observer: Option>, suppress_stderr: bool, -) -> io::Result { +) -> 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")); @@ -342,16 +357,16 @@ fn spawn_and_stream_output( // 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") + forward_stderr_with_attribution(BufReader::new(stderr), io::sink()) } else { - forward_child_output(BufReader::new(stderr), io::stderr(), "stderr") + 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 = if suppress_stderr { + let (stdout_stats, stdout_failure) = if suppress_stderr { let mut output = io::sink(); forward_stdout(stdout, &mut output, status_observer) } else { @@ -363,31 +378,6 @@ fn spawn_and_stream_output( // 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}"))) + let (status, stderr_failure) = finalize_streaming(wait_result, stdout_stats, err_handle)?; + Ok((status, stderr_failure.or(stdout_failure))) } diff --git a/src/runner/process/tests.rs b/src/runner/process/tests.rs index 880ab3afa..6ff246d7f 100644 --- a/src/runner/process/tests.rs +++ b/src/runner/process/tests.rs @@ -111,7 +111,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")); diff --git a/tests/command_env_ui_tests.rs b/tests/command_env_ui_tests.rs index 07394aad3..6306090df 100644 --- a/tests/command_env_ui_tests.rs +++ b/tests/command_env_ui_tests.rs @@ -40,6 +40,21 @@ fn command_env_embedder_fixture_compiles() -> io::Result<()> { Ok(()) } +/// The public command-list constructors compile for an external embedder. +#[test] +fn command_list_public_api_fixture_compiles() -> io::Result<()> { + let rlib = NetsukeRlib::build()?; + let output = rlib.compile("tests/ui/command_list_public_api_pass.rs")?; + + if !output.status.success() { + return Err(io::Error::other(format!( + "the command-list public API fixture should compile:\n{}", + stderr(&output), + ))); + } + Ok(()) +} + /// The `netsuke` rlib and the deps directory holding its dependencies. struct NetsukeRlib { rlib: PathBuf, diff --git a/tests/documentation_examples_tests.rs b/tests/documentation_examples_tests.rs index dc81a3de3..0863a272f 100644 --- a/tests/documentation_examples_tests.rs +++ b/tests/documentation_examples_tests.rs @@ -23,6 +23,7 @@ const EXPECTED_EXAMPLE_IDS: &[&str] = &[ "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", @@ -157,6 +158,7 @@ fn every_documented_fence_has_a_known_unique_identifier() -> Result<()> { #[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/logging_stderr/command_list_failure.rs b/tests/logging_stderr/command_list_failure.rs new file mode 100644 index 000000000..2f94c597c --- /dev/null +++ b/tests/logging_stderr/command_list_failure.rs @@ -0,0 +1,110 @@ +//! 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_CONTEXT: &str = "netsuke command-list failure: action 1, entry 2"; + +fn failing_command_list_workspace() -> 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", + r#" +netsuke_version: "1.0.0" +targets: + - name: result.txt + command: + - "echo first > $out" + - "false" + - "echo unexpected >> $out" +"#, + )?; + 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()? 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!( + stderr.contains(FAILURE_CONTEXT), + "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()? 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!( + diagnostics.to_string().contains(FAILURE_CONTEXT), + "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()? 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") && stderr.contains(FAILURE_CONTEXT), + "tracing should record the bounded command-list failure context: {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/ninja_gen_command_list_integration_tests.rs b/tests/ninja_gen_command_list_integration_tests.rs index 4c9522927..385b82e17 100644 --- a/tests/ninja_gen_command_list_integration_tests.rs +++ b/tests/ninja_gen_command_list_integration_tests.rs @@ -7,8 +7,10 @@ use anyhow::{Context, Result, ensure}; use camino::Utf8PathBuf; use cap_std::{ambient_authority, fs_utf8::Dir}; -use netsuke::ast::{Recipe, StringOrList}; +use minijinja::Environment; +use netsuke::ast::{NetsukeManifest, Recipe, StringOrList}; use netsuke::ir::{Action, BuildEdge, BuildGraph}; +use netsuke::manifest::{self, render_manifest}; use netsuke::ninja_gen::generate; use std::process::Command; use tempfile::TempDir; @@ -101,3 +103,104 @@ fn command_list_entry_ending_in_background_operator_preserves_the_next_boundary( "second", ) } + +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 = Utf8PathBuf::from_path_buf(dir.path().to_path_buf()) + .map_err(|path| anyhow::anyhow!("temp dir path {path:?} is not UTF-8"))?; + + let handle = Dir::open_ambient_dir(&dir_path, ambient_authority()) + .with_context(|| format!("open ambient dir for temp workspace at {dir_path}"))?; + 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/ninja_snapshot_tests.rs b/tests/ninja_snapshot_tests.rs index 7b6c7e165..34b1541da 100644 --- a/tests/ninja_snapshot_tests.rs +++ b/tests/ninja_snapshot_tests.rs @@ -145,7 +145,7 @@ fn multi_command_manifest_ninja_snapshot() -> Result<()> { ensure!( ninja_content.contains( - "{ eval 'echo check-fmt'; } && { eval 'echo lint'; } && { eval 'echo test'; }" + "{ if eval 'echo check-fmt'; then :; else _netsuke_command_status=$$?; printf '%s\\n' 'netsuke command-list failure: action 1, entry 1' >&2; exit \"$$_netsuke_command_status\"; fi; } && { if eval 'echo lint'; then :; else _netsuke_command_status=$$?; printf '%s\\n' 'netsuke command-list failure: action 1, entry 2' >&2; exit \"$$_netsuke_command_status\"; fi; } && { if eval 'echo test'; then :; else _netsuke_command_status=$$?; printf '%s\\n' 'netsuke command-list failure: action 1, entry 3' >&2; exit \"$$_netsuke_command_status\"; fi; }" ), "expected the command list joined into a fail-fast chain:\n{ninja_content}" ); 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 index 3d91fb696..223487015 100644 --- a/tests/snapshots/ninja/ninja_snapshot_tests__multi_command_manifest_ninja.snap +++ b/tests/snapshots/ninja/ninja_snapshot_tests__multi_command_manifest_ninja.snap @@ -3,7 +3,7 @@ source: tests/ninja_snapshot_tests.rs expression: ninja_content --- rule 2ba9145cfe6f2224b0cbcf527eea1371cf3cc742c8790e332ddfb7ba0c51e3da - command = { eval 'echo check-fmt'; } && { eval 'echo lint'; } && { eval 'echo test'; } + command = { if eval 'echo check-fmt'; then :; else _netsuke_command_status=$$?; printf '%s\n' 'netsuke command-list failure: action 1, entry 1' >&2; exit "$$_netsuke_command_status"; fi; } && { if eval 'echo lint'; then :; else _netsuke_command_status=$$?; printf '%s\n' 'netsuke command-list failure: action 1, entry 2' >&2; exit "$$_netsuke_command_status"; fi; } && { if eval 'echo test'; then :; else _netsuke_command_status=$$?; printf '%s\n' 'netsuke command-list failure: action 1, entry 3' >&2; exit "$$_netsuke_command_status"; fi; } description = Run the required checks sequentially build aggregate: 2ba9145cfe6f2224b0cbcf527eea1371cf3cc742c8790e332ddfb7ba0c51e3da 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")); +} From 9fe7ef8ba07729a33961793a2267e0bb962959f9 Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 12 Aug 2026 12:29:24 +0200 Subject: [PATCH 09/14] Harden command-list failure boundaries (#550) Report `exit` and background-job failures with bounded action attribution, preserving fail-fast ordered execution and exit status. Reject programmatic empty command lists, add bounded failure telemetry, and document the opt-in migration path. --- docs/developers-guide.md | 20 +++- docs/v0-1-0-migration-guide.md | 9 ++ src/ninja_gen.rs | 48 ++------ src/ninja_gen_command_list.rs | 61 ++++++++++ src/ninja_gen_property_tests.rs | 29 ++--- src/ninja_gen_tests.rs | 50 ++++---- src/runner/process/child_exit.rs | 8 +- src/runner/process/command_list_telemetry.rs | 83 +++++++++++++ src/runner/process/failure_attribution.rs | 71 ++++++++--- src/runner/process/mod.rs | 22 ++-- tests/logging_stderr/command_list_failure.rs | 12 +- ...inja_gen_command_list_integration_tests.rs | 110 ++++++++++++++++++ tests/ninja_snapshot_tests.rs | 7 +- ...t_tests__multi_command_manifest_ninja.snap | 2 +- 14 files changed, 412 insertions(+), 120 deletions(-) create mode 100644 src/ninja_gen_command_list.rs create mode 100644 src/runner/process/command_list_telemetry.rs diff --git a/docs/developers-guide.md b/docs/developers-guide.md index b1c8aa790..b130cd236 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -202,8 +202,9 @@ 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 `StringOrList::Empty` during Ninja -generation rather than emitting an unusable rule. +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: @@ -226,9 +227,18 @@ The lowering stages have deliberately separate responsibilities: variables can carry from one entry to the next. The `&&` chain remains fail-fast. - `src/runner/process` forwards the command's output and recognizes the - bounded `netsuke command-list failure: action N, entry M` marker. A failed - list therefore retains the original exit status while adding the generated - action index and one-based entry index to the Ninja failure error. + 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. + +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`. 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 diff --git a/docs/v0-1-0-migration-guide.md b/docs/v0-1-0-migration-guide.md index cb4d9493e..e5c9718f1 100644 --- a/docs/v0-1-0-migration-guide.md +++ b/docs/v0-1-0-migration-guide.md @@ -23,6 +23,7 @@ Table: v0.1.0 child-environment API additions and their impact | Convenience wrappers | Unchanged. `run_ninja` and `run_ninja_tool` behave exactly as before, inheriting the process environment. | [Users' guide](users-guide.md) | | 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) | +| 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 @@ -30,6 +31,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/src/ninja_gen.rs b/src/ninja_gen.rs index 8a069f356..9cb4d874f 100644 --- a/src/ninja_gen.rs +++ b/src/ninja_gen.rs @@ -14,6 +14,11 @@ use itertools::Itertools; use std::collections::HashSet; use std::fmt::{self, Display, Formatter, Write}; use thiserror::Error; + +#[path = "ninja_gen_command_list.rs"] +pub(crate) mod ninja_gen_command_list; + +use ninja_gen_command_list::command_list_entry; /// Errors produced while rendering Ninja manifests. #[derive(Debug, Error)] pub enum NinjaGenError { @@ -145,15 +150,7 @@ pub fn generate_into(graph: &BuildGraph, out: &mut W) -> Result<(), Ni 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, - action_index, - } - )?; + write!(out, "{}", NamedAction { id, action })?; } let mut edges: Vec<_> = graph.targets.values().collect(); @@ -222,28 +219,13 @@ fn escape_script(script: &str) -> String { .replace('\n', "\\n") } -/// 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(value: &str) -> String { - let escaped = value.replace('\'', r"'\''"); - format!("'{escaped}'") -} - -/// Prefix used to carry bounded list-entry failure attribution through Ninja. -pub(crate) const COMMAND_LIST_FAILURE_PREFIX: &str = "netsuke command-list failure: action "; - const fn validate_action_recipe( action: &crate::ir::Action, action_index: usize, ) -> Result<(), NinjaGenError> { - if matches!( - action.recipe, - Recipe::Command { - command: StringOrList::Empty - } - ) { + if let Recipe::Command { command } = &action.recipe + && command.is_empty_content() + { return Err(NinjaGenError::EmptyCommandRecipe { action_index }); } Ok(()) @@ -253,7 +235,6 @@ const fn validate_action_recipe( struct NamedAction<'a> { id: &'a str, action: &'a crate::ir::Action, - action_index: usize, } impl NamedAction<'_> { @@ -279,7 +260,7 @@ impl NamedAction<'_> { items.iter() .enumerate() .map(|(entry_index, item)| { - command_list_entry(item, self.action_index, entry_index + 1) + command_list_entry(item, self.id, entry_index + 1) }) .join(" && "); Self::assert_shell_command(&command_line); @@ -349,15 +330,6 @@ impl NamedAction<'_> { } } -fn command_list_entry(command: &str, action_index: usize, entry_index: usize) -> String { - let context = format!("{COMMAND_LIST_FAILURE_PREFIX}{action_index}, entry {entry_index}"); - format!( - "{{ if eval {}; then :; else _netsuke_command_status=$$?; printf '%s\\n' '{}' >&2; exit \"$$_netsuke_command_status\"; fi; }}", - shell_single_quote(command), - context, - ) -} - impl Display for NamedAction<'_> { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { writeln!(f, "rule {}", self.id)?; diff --git a/src/ninja_gen_command_list.rs b/src/ninja_gen_command_list.rs new file mode 100644 index 000000000..10bf5800b --- /dev/null +++ b/src/ninja_gen_command_list.rs @@ -0,0 +1,61 @@ +//! Shell-safe rendering for ordered Ninja command-list entries. + +use sha2::{Digest, Sha256}; + +/// Prefix used to carry bounded list-entry failure attribution through Ninja. +pub(crate) const COMMAND_LIST_FAILURE_PREFIX: &str = "netsuke command-list failure: action "; + +/// Render one entry so it fails atomically without exposing command content. +pub(crate) fn command_list_entry(command: &str, action_id: &str, entry_index: usize) -> String { + let identity = action_identity(action_id); + let context = format!("{COMMAND_LIST_FAILURE_PREFIX}{identity}, entry {entry_index}"); + format!( + concat!( + "{{ _netsuke_background_before=$${{!:-}}; ", + "trap '_netsuke_command_status=$$?; printf \"%s\\n\" \"{}\" >&2; ", + "trap - EXIT; exit \"$$_netsuke_command_status\"' EXIT; ", + "if eval {}; 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 ", + "wait \"$$_netsuke_background_after\"; _netsuke_command_status=$$?; fi; ", + "if [ \"$$_netsuke_command_status\" -eq 0 ]; then trap - EXIT; :; ", + "else trap - EXIT; printf '%s\\n' '{}' >&2; ", + "exit \"$$_netsuke_command_status\"; fi; }}" + ), + context, + shell_single_quote(command), + context, + ) +} + +/// 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: &str) -> String { + let digest = Sha256::digest(action_id.as_bytes()); + let mut identity = String::with_capacity(digest.len() * 2); + for byte in digest { + identity.push(hex_digit(byte >> 4)); + identity.push(hex_digit(byte & 0x0f)); + } + identity +} + +const fn hex_digit(nibble: u8) -> char { + match nibble { + 0..=9 => (b'0' + nibble) as char, + _ => (b'a' + (nibble - 10)) as char, + } +} + +/// 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(value: &str) -> String { + let escaped = value.replace('\'', r"'\''"); + format!("'{escaped}'") +} diff --git a/src/ninja_gen_property_tests.rs b/src/ninja_gen_property_tests.rs index 69be4a040..51ebf9ac3 100644 --- a/src/ninja_gen_property_tests.rs +++ b/src/ninja_gen_property_tests.rs @@ -150,23 +150,17 @@ proptest! { 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 expected_entries: Vec = entries.iter().enumerate().map(|(index, entry)| { - format!( - "{{ if eval 'echo {entry}'; then :; else _netsuke_command_status=$$?; printf '%s\\n' 'netsuke command-list failure: action 1, entry {}' >&2; exit \"$$_netsuke_command_status\"; fi; }}", - index + 1, - ) - }).collect(); - let mut previous = 0usize; - for expected_entry in &expected_entries { + for entry in &entries { + let expected_entry = format!("if eval 'echo {entry}'"); let position = command_line .get(previous..) - .and_then(|remaining| remaining.find(expected_entry)) + .and_then(|remaining| remaining.find(&expected_entry)) .expect("every entry should retain its independent shell boundary"); previous += position + expected_entry.len(); } - prop_assert_eq!(command_line.matches("{ if eval '").count(), entries.len()); - prop_assert_eq!(command_line.matches(" && ").count(), entries.len() - 1); + prop_assert_eq!(command_line.matches("{ _netsuke_background_before=$${!:-};").count(), entries.len()); + prop_assert_eq!(command_line.matches("} && {").count(), entries.len() - 1); } #[test] @@ -180,12 +174,21 @@ proptest! { } #[test] - fn programmatic_empty_command_recipes_are_rejected(action_id in "[a-z]{1,12}") { + fn programmatic_empty_command_recipes_are_rejected( + action_id in "[a-z]{1,12}", + use_empty_list in any::(), + ) { let mut graph = BuildGraph::default(); graph.actions.insert( action_id, Action { - recipe: Recipe::Command { command: StringOrList::Empty }, + recipe: Recipe::Command { + command: if use_empty_list { + StringOrList::List(Vec::new()) + } else { + StringOrList::Empty + }, + }, description: None, depfile: None, deps_format: None, diff --git a/src/ninja_gen_tests.rs b/src/ninja_gen_tests.rs index 429389ba9..f00e738ea 100644 --- a/src/ninja_gen_tests.rs +++ b/src/ninja_gen_tests.rs @@ -116,17 +116,11 @@ fn generate_command_list_ninja_joins_a_fail_fast_chain() -> Result<()> { let ninja = generate(&graph)?; ensure!( - ninja.contains(concat!( - "command = { if eval 'echo one'; then :; else _netsuke_command_status=$$?; ", - "printf '%s\\n' 'netsuke command-list failure: action 1, entry 1' >&2; ", - "exit \"$$_netsuke_command_status\"; fi; } && ", - "{ if eval 'echo two'; then :; else _netsuke_command_status=$$?; ", - "printf '%s\\n' 'netsuke command-list failure: action 1, entry 2' >&2; ", - "exit \"$$_netsuke_command_status\"; fi; } && ", - "{ if eval 'echo three'; then :; else _netsuke_command_status=$$?; ", - "printf '%s\\n' 'netsuke command-list failure: action 1, entry 3' >&2; ", - "exit \"$$_netsuke_command_status\"; fi; }" - )), + ninja.contains("command = { _netsuke_background_before=$${!:-};") + && ninja.contains("if eval 'echo one'") + && ninja.contains("if eval 'echo two'") + && ninja.contains("if eval 'echo three'") + && ninja.matches("} && {").count() == 2, "command list entries should be isolated brace groups joined by &&:\n{ninja}" ); Ok(()) @@ -134,24 +128,24 @@ fn generate_command_list_ninja_joins_a_fail_fast_chain() -> Result<()> { #[test] fn programmatic_empty_command_recipe_returns_a_typed_generation_error() { - let action = Action { - recipe: Recipe::Command { - command: StringOrList::Empty, - }, - description: None, - depfile: None, - deps_format: None, - pool: None, - restat: false, - }; - let mut graph = BuildGraph::default(); - graph.actions.insert("empty".into(), action); + for command in [StringOrList::Empty, StringOrList::List(Vec::new())] { + let action = Action { + recipe: Recipe::Command { command }, + description: None, + depfile: None, + deps_format: None, + pool: None, + restat: false, + }; + 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:?}" - ); + 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] diff --git a/src/runner/process/child_exit.rs b/src/runner/process/child_exit.rs index 59041c78e..6a78c9fc6 100644 --- a/src/runner/process/child_exit.rs +++ b/src/runner/process/child_exit.rs @@ -6,7 +6,7 @@ use std::{ thread, }; -use super::streaming::ForwardStats; +use super::{failure_attribution::CommandListFailure, streaming::ForwardStats}; /// Terminate a partially configured child and reap it before returning an error. pub(super) fn terminate_child(child: &mut Child, context: &str) { @@ -21,7 +21,7 @@ pub(super) fn terminate_child(child: &mut Child, context: &str) { /// Convert a Ninja exit status into an error with optional bounded attribution. pub(super) fn ninja_exit_error( status: ExitStatus, - command_list_failure: Option<&str>, + command_list_failure: Option<&CommandListFailure>, ) -> io::Result<()> { let message = command_list_failure.map_or_else( || format!("ninja exited with {status}"), @@ -34,8 +34,8 @@ pub(super) fn ninja_exit_error( pub(super) fn finalize_streaming( wait_result: io::Result, stdout_stats: ForwardStats, - err_handle: thread::JoinHandle<(ForwardStats, Option)>, -) -> io::Result<(ExitStatus, Option)> { + 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)) => { diff --git a/src/runner/process/command_list_telemetry.rs b/src/runner/process/command_list_telemetry.rs new file mode 100644 index 000000000..2014105f1 --- /dev/null +++ b/src/runner/process/command_list_telemetry.rs @@ -0,0 +1,83 @@ +//! 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"; +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 index f8a48040b..b44efca4a 100644 --- a/src/runner/process/failure_attribution.rs +++ b/src/runner/process/failure_attribution.rs @@ -1,6 +1,6 @@ //! Bounded extraction of command-list failure attribution from Ninja stderr. -use crate::ninja_gen::COMMAND_LIST_FAILURE_PREFIX; +use crate::ninja_gen::ninja_gen_command_list::COMMAND_LIST_FAILURE_PREFIX; use std::io::{self, Write}; use super::streaming::{ForwardStats, forward_child_output}; @@ -9,7 +9,7 @@ use super::streaming::{ForwardStats, forward_child_output}; pub(super) fn forward_stderr_with_attribution( reader: R, output: W, -) -> (ForwardStats, Option) +) -> (ForwardStats, Option) where R: io::Read, W: Write, @@ -22,7 +22,36 @@ where pub(super) struct FailureAttributionWriter { inner: W, pending: Vec, - failure: Option, + 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 { @@ -36,7 +65,7 @@ impl FailureAttributionWriter { } } - pub(super) fn into_failure(self) -> Option { + pub(super) fn into_failure(self) -> Option { self.failure } @@ -58,20 +87,23 @@ impl FailureAttributionWriter { let Some((action, entry)) = line .strip_prefix(COMMAND_LIST_FAILURE_PREFIX) .and_then(|suffix| suffix.split_once(", entry ")) - .and_then(|(action, entry)| { - Some((action.parse::().ok()?, entry.parse::().ok()?)) - }) + .and_then(|(action, entry)| Some((action, entry.parse::().ok()?))) else { return; }; - if action > 0 && entry > 0 { - self.failure = Some(format!( - "{COMMAND_LIST_FAILURE_PREFIX}{action}, entry {entry}" - )); + if is_action_identity(action) && entry > 0 { + self.failure = Some(CommandListFailure { + action_identity: action.to_owned(), + entry_index: entry, + }); } } } +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)?; @@ -93,6 +125,9 @@ mod tests { use super::*; + const ACTION_IDENTITY: &str = + "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; + #[test] fn extracts_a_valid_marker_split_across_writes() { let mut writer = FailureAttributionWriter::new(Vec::new()); @@ -100,12 +135,15 @@ mod tests { .write_all(b"ninja output\nnetsuke command-list fail") .expect("first chunk should write"); writer - .write_all(b"ure: action 7, entry 3\n") + .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!( - writer.into_failure().as_deref(), - Some("netsuke command-list failure: action 7, entry 3") + failure.as_deref(), + Some( + "netsuke command-list failure: action 0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef, entry 3" + ) ); } @@ -119,7 +157,10 @@ mod tests { .write_all(&[b'x'; FailureAttributionWriter::>::MAX_LINE_BYTES + 1]) .expect("unbounded marker should write"); writer - .write_all(b"netsuke command-list failure: action 7, entry 3\n") + .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()); diff --git a/src/runner/process/mod.rs b/src/runner/process/mod.rs index e24f11693..2ec3dc2b5 100644 --- a/src/runner/process/mod.rs +++ b/src/runner/process/mod.rs @@ -8,9 +8,11 @@ use std::{ path::Path, process::{Child, Command, ExitStatus}, thread, + time::Instant, }; mod child_exit; +mod command_list_telemetry; mod command_logging; mod failure_attribution; mod file_io; @@ -27,7 +29,9 @@ use command_logging::{ CommandLogContext, command_span, log_command_execution, log_command_exit_failure, log_command_spawn_failure, }; -use failure_attribution::{FailureAttributionWriter, forward_stderr_with_attribution}; +use failure_attribution::{ + CommandListFailure, FailureAttributionWriter, forward_stderr_with_attribution, +}; pub use file_io::*; pub use ninja_program::resolve_ninja_program; #[cfg(doctest)] @@ -75,7 +79,8 @@ pub mod doc { struct ExitFailureContext<'a> { operation: &'a str, suppress_stderr: bool, - command_list_failure: Option<&'a str>, + command_list_failure: Option<&'a CommandListFailure>, + started: Instant, } fn check_exit_status_with_context( @@ -94,10 +99,7 @@ fn check_exit_status_with_context( status, ); if let Some(failure) = failure_context.command_list_failure { - tracing::warn!( - command_list_failure = failure, - "Ninja command-list entry failed" - ); + command_list_telemetry::record_failure(failure, failure_context.started.elapsed()); } ninja_exit_error(status, failure_context.command_list_failure) } @@ -114,6 +116,7 @@ fn run_command_and_stream_with_context( let _entered = span.enter(); log_command_execution(&context, operation, suppress_stderr); + let started = Instant::now(); let child = cmd.spawn().inspect_err(|err| { tracing::Span::current().record("failure_category", "spawn"); log_command_spawn_failure(&context, operation, suppress_stderr, err); @@ -126,7 +129,8 @@ fn run_command_and_stream_with_context( ExitFailureContext { operation, suppress_stderr, - command_list_failure: command_list_failure.as_deref(), + command_list_failure: command_list_failure.as_ref(), + started, }, ) } @@ -324,7 +328,7 @@ fn forward_stdout( stdout: impl io::Read, output: &mut impl io::Write, status_observer: Option>, -) -> (ForwardStats, Option) { +) -> (ForwardStats, Option) { let mut attribution_writer = FailureAttributionWriter::new(output); let stats = match status_observer { Some(observer) => forward_child_output_with_ninja_status( @@ -341,7 +345,7 @@ fn spawn_and_stream_output( mut child: Child, status_observer: Option>, suppress_stderr: bool, -) -> io::Result<(ExitStatus, Option)> { +) -> 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")); diff --git a/tests/logging_stderr/command_list_failure.rs b/tests/logging_stderr/command_list_failure.rs index 2f94c597c..b2ef74bf6 100644 --- a/tests/logging_stderr/command_list_failure.rs +++ b/tests/logging_stderr/command_list_failure.rs @@ -8,7 +8,11 @@ use serde_json::Value; use tempfile::TempDir; use test_support::ninja::ninja_integration_workspace; -const FAILURE_CONTEXT: &str = "netsuke command-list failure: action 1, entry 2"; +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() -> Result> { let temp = match ninja_integration_workspace() { @@ -55,7 +59,7 @@ fn failed_command_list_entry_is_attributed_in_human_output() -> Result<()> { ); let stderr = String::from_utf8(output.stderr).context("stderr should be valid UTF-8")?; ensure!( - stderr.contains(FAILURE_CONTEXT), + identifies_entry(&stderr, 2), "human diagnostics should name the bounded failing entry: {stderr}" ); let output_file = open_workspace(&temp)? @@ -81,7 +85,7 @@ fn failed_command_list_entry_is_attributed_in_json_diagnostics() -> Result<()> { 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!( - diagnostics.to_string().contains(FAILURE_CONTEXT), + identifies_entry(&diagnostics.to_string(), 2), "JSON diagnostics should retain bounded entry attribution: {diagnostics}" ); ensure!( @@ -103,7 +107,7 @@ fn failed_command_list_entry_is_attributed_in_tracing_output() -> Result<()> { ); let stderr = String::from_utf8(output.stderr).context("stderr should be valid UTF-8")?; ensure!( - stderr.contains("command_list_failure") && stderr.contains(FAILURE_CONTEXT), + stderr.contains("command_list_failure") && identifies_entry(&stderr, 2), "tracing should record the bounded command-list failure context: {stderr}" ); Ok(()) diff --git a/tests/ninja_gen_command_list_integration_tests.rs b/tests/ninja_gen_command_list_integration_tests.rs index 385b82e17..2e10f2698 100644 --- a/tests/ninja_gen_command_list_integration_tests.rs +++ b/tests/ninja_gen_command_list_integration_tests.rs @@ -104,6 +104,116 @@ fn command_list_entry_ending_in_background_operator_preserves_the_next_boundary( ) } +fn failing_command_list_command(entries: Vec) -> Result { + let action = Action { + recipe: Recipe::Command { + command: StringOrList::List(entries), + }, + description: None, + depfile: None, + deps_format: None, + pool: None, + restat: false, + }; + 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 open_temp_workspace(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"))?; + Dir::open_ambient_dir(&dir_path, ambient_authority()).context("open command-list workspace") +} + +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 = failing_command_list_command(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" + ); + Ok(()) +} + +#[test] +fn command_list_background_failure_waits_before_the_next_entry() -> Result<()> { + let Some(dir) = ninja_integration_setup() else { + return Ok(()); + }; + let command = failing_command_list_command(vec![ + "false &".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(()) +} + fn rendered_direct_target_manifest() -> Result { let manifest = manifest::from_str( r#" diff --git a/tests/ninja_snapshot_tests.rs b/tests/ninja_snapshot_tests.rs index 34b1541da..c477012b6 100644 --- a/tests/ninja_snapshot_tests.rs +++ b/tests/ninja_snapshot_tests.rs @@ -144,9 +144,10 @@ fn multi_command_manifest_ninja_snapshot() -> Result<()> { let ninja_content = ninja_gen::generate(&ir)?; ensure!( - ninja_content.contains( - "{ if eval 'echo check-fmt'; then :; else _netsuke_command_status=$$?; printf '%s\\n' 'netsuke command-list failure: action 1, entry 1' >&2; exit \"$$_netsuke_command_status\"; fi; } && { if eval 'echo lint'; then :; else _netsuke_command_status=$$?; printf '%s\\n' 'netsuke command-list failure: action 1, entry 2' >&2; exit \"$$_netsuke_command_status\"; fi; } && { if eval 'echo test'; then :; else _netsuke_command_status=$$?; printf '%s\\n' 'netsuke command-list failure: action 1, entry 3' >&2; exit \"$$_netsuke_command_status\"; fi; }" - ), + 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!( 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 index 223487015..bbe42d110 100644 --- a/tests/snapshots/ninja/ninja_snapshot_tests__multi_command_manifest_ninja.snap +++ b/tests/snapshots/ninja/ninja_snapshot_tests__multi_command_manifest_ninja.snap @@ -3,7 +3,7 @@ source: tests/ninja_snapshot_tests.rs expression: ninja_content --- rule 2ba9145cfe6f2224b0cbcf527eea1371cf3cc742c8790e332ddfb7ba0c51e3da - command = { if eval 'echo check-fmt'; then :; else _netsuke_command_status=$$?; printf '%s\n' 'netsuke command-list failure: action 1, entry 1' >&2; exit "$$_netsuke_command_status"; fi; } && { if eval 'echo lint'; then :; else _netsuke_command_status=$$?; printf '%s\n' 'netsuke command-list failure: action 1, entry 2' >&2; exit "$$_netsuke_command_status"; fi; } && { if eval 'echo test'; then :; else _netsuke_command_status=$$?; printf '%s\n' 'netsuke command-list failure: action 1, entry 3' >&2; exit "$$_netsuke_command_status"; fi; } + command = { _netsuke_background_before=$${!:-}; 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 wait "$$_netsuke_background_after"; _netsuke_command_status=$$?; fi; if [ "$$_netsuke_command_status" -eq 0 ]; then trap - EXIT; :; else trap - EXIT; printf '%s\n' 'netsuke command-list failure: action aad603e884fb9196bd1cfe656b4b72baa75d7b3eae7cb3f30a56e840ae134bc3, entry 1' >&2; exit "$$_netsuke_command_status"; fi; } && { _netsuke_background_before=$${!:-}; 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 wait "$$_netsuke_background_after"; _netsuke_command_status=$$?; fi; if [ "$$_netsuke_command_status" -eq 0 ]; then trap - EXIT; :; else trap - EXIT; printf '%s\n' 'netsuke command-list failure: action aad603e884fb9196bd1cfe656b4b72baa75d7b3eae7cb3f30a56e840ae134bc3, entry 2' >&2; exit "$$_netsuke_command_status"; fi; } && { _netsuke_background_before=$${!:-}; 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 wait "$$_netsuke_background_after"; _netsuke_command_status=$$?; fi; if [ "$$_netsuke_command_status" -eq 0 ]; then trap - EXIT; :; 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 From 9474467986b390cf05ec1f0594b3310a4420a0b8 Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 12 Aug 2026 18:52:13 +0200 Subject: [PATCH 10/14] Attribute direct exec command failures (#550) Emit the bounded list-entry marker before a direct `exec` can replace the shell, preserving fail-fast diagnostics for `exec false`. Document hashed action attribution in the users' guide and cover the behaviour with real Ninja. --- docs/users-guide.md | 6 ++-- src/ninja_gen_command_list.rs | 28 ++++++++++++++--- ...inja_gen_command_list_integration_tests.rs | 30 +++++++++++++++++++ 3 files changed, 57 insertions(+), 7 deletions(-) diff --git a/docs/users-guide.md b/docs/users-guide.md index 7c8260e1c..17f61699c 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -314,7 +314,7 @@ 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 1, entry 2`. +`netsuke command-list failure: action HASH, entry 2`. @@ -1102,8 +1102,8 @@ Netsuke reduces some common quoting mistakes, but it is not a sandbox: 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. Failure diagnostics include the action and entry positions when - Netsuke can attribute the failed list entry. + structure. 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/src/ninja_gen_command_list.rs b/src/ninja_gen_command_list.rs index 10bf5800b..e10798ac5 100644 --- a/src/ninja_gen_command_list.rs +++ b/src/ninja_gen_command_list.rs @@ -9,12 +9,13 @@ pub(crate) const COMMAND_LIST_FAILURE_PREFIX: &str = "netsuke command-list failu pub(crate) fn command_list_entry(command: &str, action_id: &str, 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, &context); format!( concat!( "{{ _netsuke_background_before=$${{!:-}}; ", "trap '_netsuke_command_status=$$?; printf \"%s\\n\" \"{}\" >&2; ", "trap - EXIT; exit \"$$_netsuke_command_status\"' EXIT; ", - "if eval {}; then _netsuke_command_status=0; ", + "if {}; then _netsuke_command_status=0; ", "else _netsuke_command_status=$$?; fi; ", "_netsuke_background_after=$${{!:-}}; ", "if [ -n \"$$_netsuke_background_after\" ] && ", @@ -24,12 +25,31 @@ pub(crate) fn command_list_entry(command: &str, action_id: &str, entry_index: us "else trap - EXIT; printf '%s\\n' '{}' >&2; ", "exit \"$$_netsuke_command_status\"; fi; }}" ), - context, - shell_single_quote(command), - context, + context, evaluator, context, ) } +/// Evaluate an entry while preserving attribution before a direct `exec`. +/// +/// `exec` replaces the current shell, preventing its EXIT trap and outer +/// failure branch from running. Emit the bounded marker first in that narrow +/// case, then retain normal process-replacement semantics. +fn command_evaluator(command: &str, context: &str) -> String { + let quoted = shell_single_quote(command); + if command_starts_with_exec(command) { + format!("printf '%s\\n' '{context}' >&2; eval {quoted}") + } else { + format!("eval {quoted}") + } +} + +/// Whether an entry's first shell word is the process-replacing `exec` builtin. +fn command_starts_with_exec(command: &str) -> bool { + shlex::split(command) + .and_then(|words| words.into_iter().next()) + .is_some_and(|word| word == "exec") +} + /// Return a fixed-width fingerprint for an action identifier. /// /// IR-generated identifiers are already hashes, but hashing again prevents a diff --git a/tests/ninja_gen_command_list_integration_tests.rs b/tests/ninja_gen_command_list_integration_tests.rs index 2e10f2698..79d234d9a 100644 --- a/tests/ninja_gen_command_list_integration_tests.rs +++ b/tests/ninja_gen_command_list_integration_tests.rs @@ -177,6 +177,36 @@ fn command_list_exit_entry_preserves_status_and_emits_attribution() -> Result<() Ok(()) } +#[test] +fn command_list_exec_failure_preserves_attribution_and_stops_the_chain() -> Result<()> { + let Some(dir) = ninja_integration_setup() else { + return Ok(()); + }; + let command = failing_command_list_command(vec![ + "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"), + "exec failure should emit the first-entry marker: {diagnostics}" + ); + let workspace = open_temp_workspace(&dir)?; + 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 { From c243435d7300f77cda85ab31d9aeea3aa4a00e09 Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 12 Aug 2026 18:55:21 +0200 Subject: [PATCH 11/14] Deduplicate public UI fixture compilation Compile each public API fixture through one helper while retaining its fixture-specific failure message and stderr diagnostics. --- tests/command_env_ui_tests.rs | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/tests/command_env_ui_tests.rs b/tests/command_env_ui_tests.rs index 6306090df..0dbe45517 100644 --- a/tests/command_env_ui_tests.rs +++ b/tests/command_env_ui_tests.rs @@ -28,28 +28,30 @@ use std::{ /// The embedder fixture type-checks against the public API. #[test] fn command_env_embedder_fixture_compiles() -> io::Result<()> { - let rlib = NetsukeRlib::build()?; - let output = rlib.compile("tests/ui/command_env_embedder_pass.rs")?; - - if !output.status.success() { - return Err(io::Error::other(format!( - "the embedder fixture should compile against the public API:\n{}", - stderr(&output), - ))); - } - Ok(()) + 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_list_public_api_pass.rs")?; + let output = rlib.compile(source)?; if !output.status.success() { return Err(io::Error::other(format!( - "the command-list public API fixture should compile:\n{}", - stderr(&output), + "{failure_message}:\n{}", + stderr(&output) ))); } Ok(()) From bc57c3806f824cc42e863c4ee12ebbc8dcad156d Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 12 Aug 2026 19:09:02 +0200 Subject: [PATCH 12/14] Wait for every command-list background job (#550) Track every job an entry starts and preserve the first failure before continuing the ordered chain. Cover multiple background jobs through real Ninja, so a failing job cannot silently allow the next entry to run. --- src/ninja_gen_command_list.rs | 14 ++++++--- src/ninja_gen_property_tests.rs | 7 ++++- src/ninja_gen_tests.rs | 3 +- ...inja_gen_command_list_integration_tests.rs | 31 +++++++++++++++++++ ...t_tests__multi_command_manifest_ninja.snap | 3 +- 5 files changed, 50 insertions(+), 8 deletions(-) diff --git a/src/ninja_gen_command_list.rs b/src/ninja_gen_command_list.rs index e10798ac5..815504b9f 100644 --- a/src/ninja_gen_command_list.rs +++ b/src/ninja_gen_command_list.rs @@ -12,15 +12,19 @@ pub(crate) fn command_list_entry(command: &str, action_id: &str, entry_index: us let evaluator = command_evaluator(command, &context); format!( concat!( - "{{ _netsuke_background_before=$${{!:-}}; ", + "{{ _netsuke_background_before=\"$$(jobs -p)\"; ", "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 ", - "wait \"$$_netsuke_background_after\"; _netsuke_command_status=$$?; fi; ", + "_netsuke_background_after=\"$$(jobs -p)\"; ", + "for _netsuke_background_job in $$_netsuke_background_after; do ", + "case \" $$_netsuke_background_before \" in ", + "*\" $$_netsuke_background_job \"*) ;; ", + "*) if wait \"$$_netsuke_background_job\"; then :; ", + "else _netsuke_background_status=$$?; ", + "if [ \"$$_netsuke_command_status\" -eq 0 ]; then ", + "_netsuke_command_status=$$_netsuke_background_status; fi; fi;; esac; done; ", "if [ \"$$_netsuke_command_status\" -eq 0 ]; then trap - EXIT; :; ", "else trap - EXIT; printf '%s\\n' '{}' >&2; ", "exit \"$$_netsuke_command_status\"; fi; }}" diff --git a/src/ninja_gen_property_tests.rs b/src/ninja_gen_property_tests.rs index 51ebf9ac3..369575287 100644 --- a/src/ninja_gen_property_tests.rs +++ b/src/ninja_gen_property_tests.rs @@ -159,7 +159,12 @@ proptest! { .expect("every entry should retain its independent shell boundary"); previous += position + expected_entry.len(); } - prop_assert_eq!(command_line.matches("{ _netsuke_background_before=$${!:-};").count(), entries.len()); + prop_assert_eq!( + command_line + .matches("{ _netsuke_background_before=\"$$(jobs -p)\";") + .count(), + entries.len() + ); prop_assert_eq!(command_line.matches("} && {").count(), entries.len() - 1); } diff --git a/src/ninja_gen_tests.rs b/src/ninja_gen_tests.rs index f00e738ea..cc4e04b01 100644 --- a/src/ninja_gen_tests.rs +++ b/src/ninja_gen_tests.rs @@ -116,10 +116,11 @@ fn generate_command_list_ninja_joins_a_fail_fast_chain() -> Result<()> { let ninja = generate(&graph)?; ensure!( - ninja.contains("command = { _netsuke_background_before=$${!:-};") + ninja.contains("command = { _netsuke_background_before=\"$$(jobs -p)\";") && ninja.contains("if eval 'echo one'") && ninja.contains("if eval 'echo two'") && ninja.contains("if eval 'echo three'") + && ninja.contains("for _netsuke_background_job in $$_netsuke_background_after; do") && ninja.matches("} && {").count() == 2, "command list entries should be isolated brace groups joined by &&:\n{ninja}" ); diff --git a/tests/ninja_gen_command_list_integration_tests.rs b/tests/ninja_gen_command_list_integration_tests.rs index 79d234d9a..c9c7a72b3 100644 --- a/tests/ninja_gen_command_list_integration_tests.rs +++ b/tests/ninja_gen_command_list_integration_tests.rs @@ -244,6 +244,37 @@ fn command_list_background_failure_waits_before_the_next_entry() -> Result<()> { Ok(()) } +#[test] +fn command_list_waits_for_every_background_job_before_the_next_entry() -> Result<()> { + let Some(dir) = ninja_integration_setup() else { + return Ok(()); + }; + let command = failing_command_list_command(vec![ + "false & true &".into(), + "echo unexpected > continued-after-multiple-background-jobs.txt".into(), + ])?; + let output = run_generated_command_with_ninja(&dir, &command)?; + ensure!( + !output.status.success(), + "a failing background job must fail the Ninja build" + ); + let diagnostics = format!( + "{}{}", + String::from_utf8(output.stdout).context("Ninja stdout should be UTF-8")?, + String::from_utf8(output.stderr).context("Ninja stderr should be UTF-8")?, + ); + ensure!( + diagnostics.contains(", entry 1"), + "the background failure should identify the first entry: {diagnostics}" + ); + let workspace = open_temp_workspace(&dir)?; + ensure!( + !workspace.exists("continued-after-multiple-background-jobs.txt"), + "a failed background job must prevent a later entry from running" + ); + Ok(()) +} + fn rendered_direct_target_manifest() -> Result { let manifest = manifest::from_str( r#" 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 index bbe42d110..fd9921d22 100644 --- a/tests/snapshots/ninja/ninja_snapshot_tests__multi_command_manifest_ninja.snap +++ b/tests/snapshots/ninja/ninja_snapshot_tests__multi_command_manifest_ninja.snap @@ -1,9 +1,10 @@ --- source: tests/ninja_snapshot_tests.rs +assertion_line: 164 expression: ninja_content --- rule 2ba9145cfe6f2224b0cbcf527eea1371cf3cc742c8790e332ddfb7ba0c51e3da - command = { _netsuke_background_before=$${!:-}; 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 wait "$$_netsuke_background_after"; _netsuke_command_status=$$?; fi; if [ "$$_netsuke_command_status" -eq 0 ]; then trap - EXIT; :; else trap - EXIT; printf '%s\n' 'netsuke command-list failure: action aad603e884fb9196bd1cfe656b4b72baa75d7b3eae7cb3f30a56e840ae134bc3, entry 1' >&2; exit "$$_netsuke_command_status"; fi; } && { _netsuke_background_before=$${!:-}; 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 wait "$$_netsuke_background_after"; _netsuke_command_status=$$?; fi; if [ "$$_netsuke_command_status" -eq 0 ]; then trap - EXIT; :; else trap - EXIT; printf '%s\n' 'netsuke command-list failure: action aad603e884fb9196bd1cfe656b4b72baa75d7b3eae7cb3f30a56e840ae134bc3, entry 2' >&2; exit "$$_netsuke_command_status"; fi; } && { _netsuke_background_before=$${!:-}; 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 wait "$$_netsuke_background_after"; _netsuke_command_status=$$?; fi; if [ "$$_netsuke_command_status" -eq 0 ]; then trap - EXIT; :; else trap - EXIT; printf '%s\n' 'netsuke command-list failure: action aad603e884fb9196bd1cfe656b4b72baa75d7b3eae7cb3f30a56e840ae134bc3, entry 3' >&2; exit "$$_netsuke_command_status"; fi; } + command = { _netsuke_background_before="$$(jobs -p)"; 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="$$(jobs -p)"; for _netsuke_background_job in $$_netsuke_background_after; do case " $$_netsuke_background_before " in *" $$_netsuke_background_job "*) ;; *) if wait "$$_netsuke_background_job"; then :; else _netsuke_background_status=$$?; if [ "$$_netsuke_command_status" -eq 0 ]; then _netsuke_command_status=$$_netsuke_background_status; fi; fi;; esac; done; if [ "$$_netsuke_command_status" -eq 0 ]; then trap - EXIT; :; else trap - EXIT; printf '%s\n' 'netsuke command-list failure: action aad603e884fb9196bd1cfe656b4b72baa75d7b3eae7cb3f30a56e840ae134bc3, entry 1' >&2; exit "$$_netsuke_command_status"; fi; } && { _netsuke_background_before="$$(jobs -p)"; 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="$$(jobs -p)"; for _netsuke_background_job in $$_netsuke_background_after; do case " $$_netsuke_background_before " in *" $$_netsuke_background_job "*) ;; *) if wait "$$_netsuke_background_job"; then :; else _netsuke_background_status=$$?; if [ "$$_netsuke_command_status" -eq 0 ]; then _netsuke_command_status=$$_netsuke_background_status; fi; fi;; esac; done; if [ "$$_netsuke_command_status" -eq 0 ]; then trap - EXIT; :; else trap - EXIT; printf '%s\n' 'netsuke command-list failure: action aad603e884fb9196bd1cfe656b4b72baa75d7b3eae7cb3f30a56e840ae134bc3, entry 2' >&2; exit "$$_netsuke_command_status"; fi; } && { _netsuke_background_before="$$(jobs -p)"; 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="$$(jobs -p)"; for _netsuke_background_job in $$_netsuke_background_after; do case " $$_netsuke_background_before " in *" $$_netsuke_background_job "*) ;; *) if wait "$$_netsuke_background_job"; then :; else _netsuke_background_status=$$?; if [ "$$_netsuke_command_status" -eq 0 ]; then _netsuke_command_status=$$_netsuke_background_status; fi; fi;; esac; done; if [ "$$_netsuke_command_status" -eq 0 ]; then trap - EXIT; :; 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 From 0bf6e1f2e12f8c602a2b1752154ba6d11439f3eb Mon Sep 17 00:00:00 2001 From: leynos Date: Thu, 13 Aug 2026 01:33:53 +0200 Subject: [PATCH 13/14] Document command-list shell boundaries --- docs/developers-guide.md | 9 ++++++++- docs/users-guide.md | 11 +++++++++-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/docs/developers-guide.md b/docs/developers-guide.md index b130cd236..46c1d1c55 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -225,7 +225,14 @@ The lowering stages have deliberately separate responsibilities: 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. + 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. 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 diff --git a/docs/users-guide.md b/docs/users-guide.md index 17f61699c..89548e936 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -1102,8 +1102,15 @@ Netsuke reduces some common quoting mistakes, but it is not a sandbox: 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. Failure diagnostics include the action fingerprint and one-based - entry position when Netsuke can attribute the failed list entry. + 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. 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`. From ef1e732f80d2f24406d9d28e9620314374a80834 Mon Sep 17 00:00:00 2001 From: leynos Date: Thu, 13 Aug 2026 01:51:14 +0200 Subject: [PATCH 14/14] Harden command-list shell boundaries (#550) Preserve failure attribution for supported direct `exec` entries without reporting successful replacements as failures. Reject list entries with multiple background jobs or structured `exec` forms when their execution cannot be attributed reliably. --- src/ninja_gen.rs | 37 ++- src/ninja_gen_command_list.rs | 212 +++++++++++++++--- src/ninja_gen_property_tests.rs | 2 +- src/ninja_gen_tests.rs | 32 ++- src/ninja_gen_validation.rs | 41 ++++ ...inja_gen_command_list_integration_tests.rs | 84 ++++--- ...t_tests__multi_command_manifest_ninja.snap | 3 +- 7 files changed, 338 insertions(+), 73 deletions(-) create mode 100644 src/ninja_gen_validation.rs diff --git a/src/ninja_gen.rs b/src/ninja_gen.rs index 9cb4d874f..c60026b22 100644 --- a/src/ninja_gen.rs +++ b/src/ninja_gen.rs @@ -17,8 +17,11 @@ use thiserror::Error; #[path = "ninja_gen_command_list.rs"] pub(crate) mod ninja_gen_command_list; +#[path = "ninja_gen_validation.rs"] +mod ninja_gen_validation; use ninja_gen_command_list::command_list_entry; +use ninja_gen_validation::validate_action_recipe; /// Errors produced while rendering Ninja manifests. #[derive(Debug, Error)] pub enum NinjaGenError { @@ -36,6 +39,28 @@ pub enum NinjaGenError { /// One-based stable position in generated action order. action_index: usize, }, + /// A list entry starts more than one background job, which cannot be + /// attributed reliably by a shared POSIX shell. + #[error( + "command-list action {action_index}, entry {entry_index} starts multiple 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, + }, /// Formatting the Ninja output failed. #[error("{message}")] Format { @@ -219,18 +244,6 @@ fn escape_script(script: &str) -> String { .replace('\n', "\\n") } -const 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 }); - } - Ok(()) -} - /// Wrapper struct to display a rule with its identifier. struct NamedAction<'a> { id: &'a str, diff --git a/src/ninja_gen_command_list.rs b/src/ninja_gen_command_list.rs index 815504b9f..7f27600ed 100644 --- a/src/ninja_gen_command_list.rs +++ b/src/ninja_gen_command_list.rs @@ -5,53 +5,215 @@ use sha2::{Digest, Sha256}; /// 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 more than one background job. + MultipleBackgroundJobs, + /// An `exec` occurs in a shell structure the list wrapper cannot supervise. + UnsupportedExec, +} + +/// Return the unsupported boundary, if any, for one command-list entry. +pub(crate) fn command_list_entry_error(command: &str) -> Option { + if background_operator_count(command) > 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. pub(crate) fn command_list_entry(command: &str, action_id: &str, 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, &context); + let (evaluator, exec_succeeded) = command_evaluator(command); format!( concat!( - "{{ _netsuke_background_before=\"$$(jobs -p)\"; ", + "{{ _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=\"$$(jobs -p)\"; ", - "for _netsuke_background_job in $$_netsuke_background_after; do ", - "case \" $$_netsuke_background_before \" in ", - "*\" $$_netsuke_background_job \"*) ;; ", - "*) if wait \"$$_netsuke_background_job\"; then :; ", + "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;; esac; done; ", - "if [ \"$$_netsuke_command_status\" -eq 0 ]; then trap - EXIT; :; ", + "_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, context, + context, evaluator, exec_succeeded, context, ) } -/// Evaluate an entry while preserving attribution before a direct `exec`. +/// Evaluate a supported direct `exec` in a retaining subshell. /// -/// `exec` replaces the current shell, preventing its EXIT trap and outer -/// failure branch from running. Emit the bounded marker first in that narrow -/// case, then retain normal process-replacement semantics. -fn command_evaluator(command: &str, context: &str) -> String { +/// 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. +fn command_evaluator(command: &str) -> (String, &'static str) { let quoted = shell_single_quote(command); - if command_starts_with_exec(command) { - format!("printf '%s\\n' '{context}' >&2; eval {quoted}") + if exec_boundary(command) == ExecBoundary::Direct { + (format!("(eval {quoted})"), " _netsuke_exec_succeeded=1;") + } else { + (format!("eval {quoted}"), "") + } +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum ExecBoundary { + None, + Direct, + Unsupported, +} + +/// Classify `exec` only when it begins a simple command after assignments. +fn exec_boundary(command: &str) -> ExecBoundary { + let Some(words) = shlex::split(command) else { + return ExecBoundary::None; + }; + let Some(first_non_assignment) = words.iter().find(|word| !is_assignment(word)) else { + return ExecBoundary::None; + }; + if first_non_assignment == "exec" { + ExecBoundary::Direct + } else if is_unsupported_exec_structure(first_non_assignment, &words) { + ExecBoundary::Unsupported } else { - format!("eval {quoted}") + ExecBoundary::None + } +} + +/// Whether a shell structure can replace the wrapper before it reports failure. +fn is_unsupported_exec_structure(first_word: &str, words: &[String]) -> bool { + is_exec_wrapper(first_word) && words.iter().any(|word| word == "exec") +} + +/// Whether `word` can invoke `exec` outside the direct supported boundary. +fn is_exec_wrapper(word: &str) -> bool { + matches!(word, "if" | "command") +} + +/// Whether `word` is a valid POSIX shell assignment word. +fn is_assignment(word: &str) -> bool { + let Some((name, _)) = word.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()) +} + +/// Count unquoted background operators without mistaking `&&` for one. +fn background_operator_count(command: &str) -> usize { + let mut state = ShellScanState::new(); + let mut count = 0; + let mut characters = command.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, } -/// Whether an entry's first shell word is the process-replacing `exec` builtin. -fn command_starts_with_exec(command: &str) -> bool { - shlex::split(command) - .and_then(|words| words.into_iter().next()) - .is_some_and(|word| word == "exec") +impl ShellScanState { + const fn new() -> Self { + Self { + quote: None, + escaped: false, + word_boundary: true, + } + } + + 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.word_boundary = true; + 0 + } + '&' => { + self.word_boundary = true; + 1 + } + ';' | '|' | '(' | ')' => { + self.word_boundary = true; + 0 + } + whitespace if whitespace.is_whitespace() => { + self.word_boundary = true; + 0 + } + _ => { + self.word_boundary = false; + 0 + } + } + } } /// Return a fixed-width fingerprint for an action identifier. diff --git a/src/ninja_gen_property_tests.rs b/src/ninja_gen_property_tests.rs index 369575287..f44f6978c 100644 --- a/src/ninja_gen_property_tests.rs +++ b/src/ninja_gen_property_tests.rs @@ -161,7 +161,7 @@ proptest! { } prop_assert_eq!( command_line - .matches("{ _netsuke_background_before=\"$$(jobs -p)\";") + .matches("{ _netsuke_background_before=$${!:-};") .count(), entries.len() ); diff --git a/src/ninja_gen_tests.rs b/src/ninja_gen_tests.rs index cc4e04b01..addb86a4d 100644 --- a/src/ninja_gen_tests.rs +++ b/src/ninja_gen_tests.rs @@ -116,11 +116,11 @@ fn generate_command_list_ninja_joins_a_fail_fast_chain() -> Result<()> { let ninja = generate(&graph)?; ensure!( - ninja.contains("command = { _netsuke_background_before=\"$$(jobs -p)\";") + ninja.contains("command = { _netsuke_background_before=$${!:-};") && ninja.contains("if eval 'echo one'") && ninja.contains("if eval 'echo two'") && ninja.contains("if eval 'echo three'") - && ninja.contains("for _netsuke_background_job in $$_netsuke_background_after; do") + && ninja.contains("if wait \"$$_netsuke_background_after\"; then :;") && ninja.matches("} && {").count() == 2, "command list entries should be isolated brace groups joined by &&:\n{ninja}" ); @@ -149,6 +149,34 @@ fn programmatic_empty_command_recipe_returns_a_typed_generation_error() { } } +#[test] +fn nested_command_list_exec_returns_a_typed_generation_error() { + let action = Action { + recipe: Recipe::Command { + command: StringOrList::List(vec!["if true; then exec false; fi".into()]), + }, + description: None, + depfile: None, + deps_format: None, + pool: None, + restat: false, + }; + 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:?}" + ); +} + #[test] fn assert_shell_command_tolerates_complex_syntax() { let command = r#"/bin/sh -c "echo 'nested quotes' && echo \"double\" && (echo subshell)""#; diff --git a/src/ninja_gen_validation.rs b/src/ninja_gen_validation.rs new file mode 100644 index 000000000..4e4138c35 --- /dev/null +++ b/src/ninja_gen_validation.rs @@ -0,0 +1,41 @@ +//! Validation for command-list boundaries before Ninja rendering. + +use super::NinjaGenError; +use super::ninja_gen_command_list::{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(entry) { + Some(CommandListEntryError::MultipleBackgroundJobs) => { + return Err(NinjaGenError::MultipleBackgroundJobs { + action_index, + entry_index, + }); + } + Some(CommandListEntryError::UnsupportedExec) => { + return Err(NinjaGenError::UnsupportedCommandListExec { + action_index, + entry_index, + }); + } + None => {} + } + } + } + Ok(()) +} diff --git a/tests/ninja_gen_command_list_integration_tests.rs b/tests/ninja_gen_command_list_integration_tests.rs index c9c7a72b3..d00d86f73 100644 --- a/tests/ninja_gen_command_list_integration_tests.rs +++ b/tests/ninja_gen_command_list_integration_tests.rs @@ -11,7 +11,7 @@ use minijinja::Environment; use netsuke::ast::{NetsukeManifest, Recipe, StringOrList}; use netsuke::ir::{Action, BuildEdge, BuildGraph}; use netsuke::manifest::{self, render_manifest}; -use netsuke::ninja_gen::generate; +use netsuke::ninja_gen::{NinjaGenError, generate}; use std::process::Command; use tempfile::TempDir; use test_support::ninja_gen::ninja_integration_setup; @@ -98,10 +98,19 @@ fn command_list_entry_ending_in_background_operator_preserves_the_next_boundary( }; run_command_list( &dir, - vec!["true &".into(), "echo second > after-background.txt".into()], + vec![ + "(sleep 0.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(()) } fn failing_command_list_command(entries: Vec) -> Result { @@ -178,12 +187,37 @@ fn command_list_exit_entry_preserves_status_and_emits_attribution() -> Result<() } #[test] -fn command_list_exec_failure_preserves_attribution_and_stops_the_chain() -> Result<()> { +fn command_list_exec_entries_preserve_attribution_and_success() -> Result<()> { let Some(dir) = ninja_integration_setup() else { return Ok(()); }; + let successful_command = failing_command_list_command(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 = failing_command_list_command(vec![ - "exec false".into(), + "FOO=1 exec false".into(), "echo unexpected > continued-after-exec.txt".into(), ])?; let output = run_generated_command_with_ninja(&dir, &command)?; @@ -197,9 +231,8 @@ fn command_list_exec_failure_preserves_attribution_and_stops_the_chain() -> Resu ensure!( diagnostics.contains("netsuke command-list failure: action ") && diagnostics.contains(", entry 1"), - "exec failure should emit the first-entry marker: {diagnostics}" + "assignment-prefixed exec failure should emit the first-entry marker: {diagnostics}" ); - let workspace = open_temp_workspace(&dir)?; ensure!( !workspace.exists("continued-after-exec.txt"), "an exec failure must not run a later entry" @@ -213,7 +246,7 @@ fn command_list_background_failure_waits_before_the_next_entry() -> Result<()> { return Ok(()); }; let command = failing_command_list_command(vec![ - "false &".into(), + "sh -c 'sleep 0.1; exit 1' &".into(), "echo unexpected > continued-after-background.txt".into(), ])?; let shell_command = command.replace("$$", "$"); @@ -245,32 +278,21 @@ fn command_list_background_failure_waits_before_the_next_entry() -> Result<()> { } #[test] -fn command_list_waits_for_every_background_job_before_the_next_entry() -> Result<()> { - let Some(dir) = ninja_integration_setup() else { - return Ok(()); - }; - let command = failing_command_list_command(vec![ +fn command_list_rejects_multiple_background_jobs() -> Result<()> { + let error = failing_command_list_command(vec![ "false & true &".into(), "echo unexpected > continued-after-multiple-background-jobs.txt".into(), - ])?; - let output = run_generated_command_with_ninja(&dir, &command)?; - ensure!( - !output.status.success(), - "a failing background job must fail the Ninja build" - ); - let diagnostics = format!( - "{}{}", - String::from_utf8(output.stdout).context("Ninja stdout should be UTF-8")?, - String::from_utf8(output.stderr).context("Ninja stderr should be UTF-8")?, - ); - ensure!( - diagnostics.contains(", entry 1"), - "the background failure should identify the first entry: {diagnostics}" - ); - let workspace = open_temp_workspace(&dir)?; + ]) + .expect_err("multiple background jobs should be rejected before Ninja runs"); ensure!( - !workspace.exists("continued-after-multiple-background-jobs.txt"), - "a failed background job must prevent a later entry from running" + matches!( + error.downcast_ref::(), + Some(NinjaGenError::MultipleBackgroundJobs { + action_index: 1, + entry_index: 1, + }) + ), + "multiple background jobs should return a stable typed error: {error:?}" ); Ok(()) } 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 index fd9921d22..34ebff72d 100644 --- a/tests/snapshots/ninja/ninja_snapshot_tests__multi_command_manifest_ninja.snap +++ b/tests/snapshots/ninja/ninja_snapshot_tests__multi_command_manifest_ninja.snap @@ -1,10 +1,9 @@ --- source: tests/ninja_snapshot_tests.rs -assertion_line: 164 expression: ninja_content --- rule 2ba9145cfe6f2224b0cbcf527eea1371cf3cc742c8790e332ddfb7ba0c51e3da - command = { _netsuke_background_before="$$(jobs -p)"; 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="$$(jobs -p)"; for _netsuke_background_job in $$_netsuke_background_after; do case " $$_netsuke_background_before " in *" $$_netsuke_background_job "*) ;; *) if wait "$$_netsuke_background_job"; then :; else _netsuke_background_status=$$?; if [ "$$_netsuke_command_status" -eq 0 ]; then _netsuke_command_status=$$_netsuke_background_status; fi; fi;; esac; done; if [ "$$_netsuke_command_status" -eq 0 ]; then trap - EXIT; :; else trap - EXIT; printf '%s\n' 'netsuke command-list failure: action aad603e884fb9196bd1cfe656b4b72baa75d7b3eae7cb3f30a56e840ae134bc3, entry 1' >&2; exit "$$_netsuke_command_status"; fi; } && { _netsuke_background_before="$$(jobs -p)"; 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="$$(jobs -p)"; for _netsuke_background_job in $$_netsuke_background_after; do case " $$_netsuke_background_before " in *" $$_netsuke_background_job "*) ;; *) if wait "$$_netsuke_background_job"; then :; else _netsuke_background_status=$$?; if [ "$$_netsuke_command_status" -eq 0 ]; then _netsuke_command_status=$$_netsuke_background_status; fi; fi;; esac; done; if [ "$$_netsuke_command_status" -eq 0 ]; then trap - EXIT; :; else trap - EXIT; printf '%s\n' 'netsuke command-list failure: action aad603e884fb9196bd1cfe656b4b72baa75d7b3eae7cb3f30a56e840ae134bc3, entry 2' >&2; exit "$$_netsuke_command_status"; fi; } && { _netsuke_background_before="$$(jobs -p)"; 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="$$(jobs -p)"; for _netsuke_background_job in $$_netsuke_background_after; do case " $$_netsuke_background_before " in *" $$_netsuke_background_job "*) ;; *) if wait "$$_netsuke_background_job"; then :; else _netsuke_background_status=$$?; if [ "$$_netsuke_command_status" -eq 0 ]; then _netsuke_command_status=$$_netsuke_background_status; fi; fi;; esac; done; if [ "$$_netsuke_command_status" -eq 0 ]; then trap - EXIT; :; else trap - EXIT; printf '%s\n' 'netsuke command-list failure: action aad603e884fb9196bd1cfe656b4b72baa75d7b3eae7cb3f30a56e840ae134bc3, entry 3' >&2; exit "$$_netsuke_command_status"; fi; } + 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