From fbf6d227decd52ffd914a700744de9b2d33a2a77 Mon Sep 17 00:00:00 2001 From: Ali Caglayan Date: Wed, 8 Jul 2026 15:32:34 +0100 Subject: [PATCH] fix(promote): exit 0 for auto-promote-only failures Signed-off-by: Ali Caglayan --- src/dune_engine/build_system.ml | 44 ++++++++++++++--- src/dune_engine/diff_action.ml | 49 ++++++++++++------- src/dune_engine/print_diff.ml | 4 +- src/dune_engine/print_diff.mli | 2 +- .../test-cases/cinaps/runtime-deps.t | 1 - .../blackbox-tests/test-cases/cinaps/simple.t | 1 - .../cram/cram-command-syntax-error-gh4230.t | 2 - .../test-cases/cram/custom-build-dir.t | 1 - .../test-cases/cram/git-access.t | 1 - .../test-cases/cram/hg-access.t | 1 - .../test-cases/formatting/dune-fmt-fail.t | 27 ++++++++++ .../test-cases/formatting/fmt-after-revert.t | 4 -- .../test-cases/formatting/fmt-no-promote.t | 1 - .../formatting/format-alternative.t | 1 - .../clicolor-force-inline-tests-github6607.t | 2 - .../test-cases/menhir/opam-menhir-dep.t | 6 --- .../fmt-lockdir-behavior-gh11037.t | 1 - ...amlformat-avoid-taking-from-project-deps.t | 1 - .../pkg/ocamlformat/ocamlformat-ignore.t | 1 - .../ocamlformat-patch-extra-files.t | 1 - .../ocamlformat-relaxed-version-constraints.t | 2 - ...amlformat-rerun-on-source-change-gh10991.t | 2 - .../promote/auto-promote-exit-code.t | 45 ++++++++++++++--- .../test-cases/promote/old-tests.t/run.t | 1 - .../stanzas/mdx-stanza/env-variables.t | 2 - .../mdx-prelude-runtime-deps-github7077.t | 1 - .../dune_rpc_e2e/dune_rpc_diagnostics.ml | 8 +-- 27 files changed, 136 insertions(+), 76 deletions(-) create mode 100644 test/blackbox-tests/test-cases/formatting/dune-fmt-fail.t diff --git a/src/dune_engine/build_system.ml b/src/dune_engine/build_system.ml index f674e10cda0..0b2d12ed834 100644 --- a/src/dune_engine/build_system.ml +++ b/src/dune_engine/build_system.ml @@ -1144,6 +1144,21 @@ let handle_final_exns exns = List.iter exns ~f:Dune_util.Report_error.report) ;; +let auto_promote_enabled () = + match !Clflags.promote with + | Some Automatically -> true + | Some Never | None -> false +;; + +let error_has_promotion error = Option.is_some (Error.promotion error) +let exn_has_promotion exn = Error.of_exn exn |> List.for_all ~f:error_has_promotion + +let all_errors_are_promotions exns = + match exns with + | [] -> false + | _ :: _ -> List.for_all exns ~f:exn_has_promotion +;; + let run_with_error_collection ?restart_started_at ~build_started_at ~build collect_errors = let build = match build with @@ -1190,15 +1205,26 @@ let run_with_error_collection ?restart_started_at ~build_started_at ~build colle State.set Build_succeeded__now_waiting_for_changes; Fiber.return (Ok res) | Error exns -> + let auto_promoted_errors = + auto_promote_enabled () + && Diff_promotion.has_pending () + && all_errors_are_promotions exns + in handle_final_exns exns; finalize_diff_promotion (); - let final_status = - if List.exists exns ~f:caused_by_cancellation - then State.Restarting_current_build - else Build_failed__now_waiting_for_changes - in - State.set final_status; - Fiber.return (Error `Already_reported) + if auto_promoted_errors + then ( + let+ () = State.reset_errors () in + State.set Build_succeeded__now_waiting_for_changes; + Ok ()) + else ( + let final_status = + if List.exists exns ~f:caused_by_cancellation + then State.Restarting_current_build + else Build_failed__now_waiting_for_changes + in + State.set final_status; + Fiber.return (Error `Already_reported)) in Metrics.reset (); let+ () = Scheduler.flush_file_watcher () in @@ -1340,7 +1366,9 @@ let run ?restart_started_at ?build f = | Ok () -> (match !result with | Some result -> Ok result - | None -> Code_error.raise "build request did not produce a result" []) + (* Promotion-only failures can make [run_build_requests] return success + even though this result-producing memo did not complete. *) + | None -> Error `Already_reported) ;; let run_exn f = diff --git a/src/dune_engine/diff_action.ml b/src/dune_engine/diff_action.ml index e7b80d95fd6..d4333501572 100644 --- a/src/dune_engine/diff_action.ml +++ b/src/dune_engine/diff_action.ml @@ -17,9 +17,14 @@ type file_diff = ; file2 : Path.t } +type message = + { source_file : Path.Source.t + ; messages : User_message.Style.t Pp.t list + } + type change = | File_diff of file_diff - | Message of User_message.Style.t Pp.t list + | Message of message type promotion = | Promote_file of @@ -73,20 +78,20 @@ let promotion source_file = } ;; -let run_change loc ~patch_back (mode : Diff.Mode.t) = function - | Message messages -> User_error.raise ~loc messages +let run_change loc ~patch_back ~will_promote (mode : Diff.Mode.t) = function + | Message { source_file; messages } -> + User_error.raise + ?promotion:(Option.some_if will_promote (promotion source_file)) + ~loc + messages | File_diff { source_file; file1; file2 } -> + let promotion = Option.some_if will_promote (promotion source_file) in (match mode with | Text -> - Print_diff.print - ~patch_back - (promotion source_file) - file1 - file2 - ~skip_trailing_cr:Sys.win32 + Print_diff.print ~patch_back promotion file1 file2 ~skip_trailing_cr:Sys.win32 | Binary -> User_error.raise - ~promotion:(promotion source_file) + ?promotion ~loc [ Pp.textf "Files %s and %s differ." @@ -153,7 +158,9 @@ let plan_tree_diff ({ mode; source_root; _ } as t) = let kind_of_target rel = kind_of_path ~loc:t.loc (target_path rel) in let list_target_directory rel = list_directory ~loc:t.loc (target_path rel) in let path_name rel = Path.Source.to_string_maybe_quoted (source_file rel) in - let add_message plan message = add_change plan (Message [ message ]) in + let add_message plan rel message = + add_change plan (Message { source_file = source_file rel; messages = [ message ] }) + in let add_promote_file plan rel = add_promotion plan @@ -177,12 +184,12 @@ let plan_tree_diff ({ mode; source_root; _ } as t) = | `File -> "File" | `Directory -> "Directory" in - add_message plan (Pp.textf "%s %s should be deleted" what_text (path_name rel)) + add_message plan rel (Pp.textf "%s %s should be deleted" what_text (path_name rel)) in let add_create_directory ~announce plan rel = let plan = add_promotion plan (Create_directory (source_file rel)) in if announce - then add_message plan (Pp.textf "Directory %s should be created" (path_name rel)) + then add_message plan rel (Pp.textf "Directory %s should be created" (path_name rel)) else plan in let rec collect_target_only rel target_kind plan = @@ -208,6 +215,7 @@ let plan_tree_diff ({ mode; source_root; _ } as t) = let plan = add_promote_file plan rel in add_message plan + rel (Pp.textf "Directory %s should be replaced with a file" (path_name rel)) | File, Missing -> add_delete plan `File rel | Directory, Missing -> add_delete plan `Directory rel @@ -227,6 +235,7 @@ let plan_tree_diff ({ mode; source_root; _ } as t) = (let plan = add_create_directory ~announce:false plan rel in add_message plan + rel (Pp.textf "File %s should be replaced with a directory" (path_name rel))) ~f:(fun plan name -> let rel = Path.Local.relative_fname rel name in @@ -307,15 +316,19 @@ let exec_plan let target_is_copied_from_source_tree = is_copied_from_source_tree (Path.build file2) in + let will_promote = + match optional with + | false -> in_source_or_target && not target_is_copied_from_source_tree + | true -> in_source_or_target + in Fiber.finalize - (fun () -> Fiber.parallel_iter changes ~f:(run_change loc ~patch_back mode)) + (fun () -> + Fiber.parallel_iter changes ~f:(run_change loc ~patch_back ~will_promote mode)) ~finally:(fun () -> (match optional with - | false -> - if in_source_or_target && not target_is_copied_from_source_tree - then register_promotions `Copy promotions + | false -> if will_promote then register_promotions `Copy promotions | true -> - if in_source_or_target + if will_promote then register_promotions `Move promotions else remove_intermediate_target file2); Fiber.return ()) diff --git a/src/dune_engine/print_diff.ml b/src/dune_engine/print_diff.ml index 5502532db2f..74c2a504b5f 100644 --- a/src/dune_engine/print_diff.ml +++ b/src/dune_engine/print_diff.ml @@ -255,7 +255,7 @@ let prepare ~skip_trailing_cr promotion path1 path2 = let print ~skip_trailing_cr ~patch_back promotion path1 path2 = let p = match patch_back with - | None -> prepare ~skip_trailing_cr (Some promotion) path1 path2 + | None -> prepare ~skip_trailing_cr promotion path1 path2 | Some dir -> let path1 = Path.source (Path.drop_optional_build_context_src_exn path1) in let loc = Loc.in_file path1 in @@ -265,7 +265,7 @@ let print ~skip_trailing_cr ~patch_back promotion path1 path2 = ~skip_trailing_cr ~dir loc - (Some promotion) + promotion (label1, path1) (label2, path2) in diff --git a/src/dune_engine/print_diff.mli b/src/dune_engine/print_diff.mli index 249eda2715f..8ad04001413 100644 --- a/src/dune_engine/print_diff.mli +++ b/src/dune_engine/print_diff.mli @@ -4,7 +4,7 @@ open Import val print : skip_trailing_cr:bool -> patch_back:Path.t option - -> User_message.Diff_annot.t + -> User_message.Diff_annot.t option -> Path.t -> Path.t -> _ Fiber.t diff --git a/test/blackbox-tests/test-cases/cinaps/runtime-deps.t b/test/blackbox-tests/test-cases/cinaps/runtime-deps.t index a8be98f1ca0..1543bcf4102 100644 --- a/test/blackbox-tests/test-cases/cinaps/runtime-deps.t +++ b/test/blackbox-tests/test-cases/cinaps/runtime-deps.t @@ -26,6 +26,5 @@ Runtime dependencies for running cinaps -(*) +(*$ let f = open_in "foo" in print_endline (input_line f); close_in f *)hello world Promoting _build/default/test.ml.cinaps-corrected to test.ml. - [1] $ cat test.ml (*$ let f = open_in "foo" in print_endline (input_line f); close_in f *)hello world diff --git a/test/blackbox-tests/test-cases/cinaps/simple.t b/test/blackbox-tests/test-cases/cinaps/simple.t index 7fc2b14c0a1..10abea08d44 100644 --- a/test/blackbox-tests/test-cases/cinaps/simple.t +++ b/test/blackbox-tests/test-cases/cinaps/simple.t @@ -40,7 +40,6 @@ The cinaps stanza offers a promotion workflow: (*$*) let x = 1 Promoting _build/default/test.ml.cinaps-corrected to test.ml. - [1] $ cat test.ml (*$ print_endline "\nhello" *) diff --git a/test/blackbox-tests/test-cases/cram/cram-command-syntax-error-gh4230.t b/test/blackbox-tests/test-cases/cram/cram-command-syntax-error-gh4230.t index d90332925e6..a42b1189bba 100644 --- a/test/blackbox-tests/test-cases/cram/cram-command-syntax-error-gh4230.t +++ b/test/blackbox-tests/test-cases/cram/cram-command-syntax-error-gh4230.t @@ -10,7 +10,6 @@ Syntax error inside a cram command File "t1.t", line 1, characters 0-0: Error: Files _build/default/t1.t and _build/default/t1.t.corrected differ. Promoting _build/default/t1.t.corrected to t1.t. - [1] $ cat >t1.t < $ exit 1 @@ -26,7 +25,6 @@ Syntax error inside a cram command $ echo foobar + ***** UNREACHABLE ***** Promoting _build/default/t1.t.corrected to t1.t. - [1] $ cat t1.t $ exit 1 ***** UNREACHABLE ***** diff --git a/test/blackbox-tests/test-cases/cram/custom-build-dir.t b/test/blackbox-tests/test-cases/cram/custom-build-dir.t index dcd14dbf705..8c152550d11 100644 --- a/test/blackbox-tests/test-cases/cram/custom-build-dir.t +++ b/test/blackbox-tests/test-cases/cram/custom-build-dir.t @@ -35,7 +35,6 @@ path Promoting $TESTCASE_ROOT/tmp/default/foo.t.corrected to foo.t. - [1] $ cat foo.t $ echo " $ echo bar" >bar.t $ if [ -e "$DUNE_BUILD_DIR/.rpc" ]; then diff --git a/test/blackbox-tests/test-cases/cram/git-access.t b/test/blackbox-tests/test-cases/cram/git-access.t index d40ff1fffa9..8a952417407 100644 --- a/test/blackbox-tests/test-cases/cram/git-access.t +++ b/test/blackbox-tests/test-cases/cram/git-access.t @@ -17,7 +17,6 @@ Check that actions don't have access to the outer git repository. + fatal: invalid gitfile format: $TESTCASE_ROOT/git/_build/.sandbox/.git + [128] Promoting _build/default/test.t.corrected to test.t. - [1] The inner call to git shouldn't be able to access the outer git repo: diff --git a/test/blackbox-tests/test-cases/cram/hg-access.t b/test/blackbox-tests/test-cases/cram/hg-access.t index 7dd15b282e3..584fc045682 100644 --- a/test/blackbox-tests/test-cases/cram/hg-access.t +++ b/test/blackbox-tests/test-cases/cram/hg-access.t @@ -25,7 +25,6 @@ enough for a few hg commands such as "hg root" to succeed: + abort: repository requires features unknown to this Mercurial: Escaping the Dune sandbox + (see https://mercurial-scm.org/wiki/MissingRequirement for more information) Promoting _build/default/test.t.corrected to test.t. - [1] The inner call to hg shouldn't be able to access the outer hg repo: diff --git a/test/blackbox-tests/test-cases/formatting/dune-fmt-fail.t b/test/blackbox-tests/test-cases/formatting/dune-fmt-fail.t new file mode 100644 index 00000000000..d332b4834ca --- /dev/null +++ b/test/blackbox-tests/test-cases/formatting/dune-fmt-fail.t @@ -0,0 +1,27 @@ +`dune fmt` should still fail when the formatter action itself fails. This +must not be classified as a promotion-only failure. + + $ make_dune_project 3.25 + $ cat > dune < (library + > (name foo)) + > EOF + $ touch .ocamlformat + $ cat > foo.ml < let = + > EOF + +Create a formatter that behaves like ocamlformat failing to parse the source. + + $ mkdir bin + $ cat > bin/ocamlformat < #!/bin/sh + > echo "ocamlformat: failed to parse foo.ml" >&2 + > exit 1 + > EOF + $ chmod +x bin/ocamlformat + + $ PATH="$PWD/bin:$PATH" dune fmt + File "foo.ml", line 1, characters 0-0: + ocamlformat: failed to parse foo.ml + [1] diff --git a/test/blackbox-tests/test-cases/formatting/fmt-after-revert.t b/test/blackbox-tests/test-cases/formatting/fmt-after-revert.t index 1b5eab6e1a2..d2d3485036b 100644 --- a/test/blackbox-tests/test-cases/formatting/fmt-after-revert.t +++ b/test/blackbox-tests/test-cases/formatting/fmt-after-revert.t @@ -23,7 +23,6 @@ formatted version. +(rule + (write-file a b)) Promoting _build/default/dune.corrected to dune. - [1] $ cat dune (rule @@ -37,8 +36,6 @@ reported and the file is reformatted. > (rule (write-file a b)) > EOF -CR-soon Alizter: dune fmt is not re-running and this is a bug. - $ dune fmt File "dune", line 1, characters 0-0: --- dune @@ -48,7 +45,6 @@ CR-soon Alizter: dune fmt is not re-running and this is a bug. +(rule + (write-file a b)) Promoting _build/default/dune.corrected to dune. - [1] $ cat dune (rule (write-file a b)) diff --git a/test/blackbox-tests/test-cases/formatting/fmt-no-promote.t b/test/blackbox-tests/test-cases/formatting/fmt-no-promote.t index 123da7ebb1e..6c510097fbe 100644 --- a/test/blackbox-tests/test-cases/formatting/fmt-no-promote.t +++ b/test/blackbox-tests/test-cases/formatting/fmt-no-promote.t @@ -39,7 +39,6 @@ Actually format the file +(rule + (write-file a b)) Promoting _build/default/dune.corrected to dune. - [1] Now the output of `dune fmt --preview is empty`. $ dune fmt --preview diff --git a/test/blackbox-tests/test-cases/formatting/format-alternative.t b/test/blackbox-tests/test-cases/formatting/format-alternative.t index eaaedaf0dd0..0a4eedb5dc3 100644 --- a/test/blackbox-tests/test-cases/formatting/format-alternative.t +++ b/test/blackbox-tests/test-cases/formatting/format-alternative.t @@ -21,7 +21,6 @@ Dune files names dune-file should be formatted + foo + (echo bar))) Promoting _build/default/dune-file.corrected to dune-file. - [1] $ cat dune-file (rule diff --git a/test/blackbox-tests/test-cases/inline-tests/clicolor-force-inline-tests-github6607.t b/test/blackbox-tests/test-cases/inline-tests/clicolor-force-inline-tests-github6607.t index c6ae7273188..eb359dfd881 100644 --- a/test/blackbox-tests/test-cases/inline-tests/clicolor-force-inline-tests-github6607.t +++ b/test/blackbox-tests/test-cases/inline-tests/clicolor-force-inline-tests-github6607.t @@ -42,7 +42,6 @@ file. - [%expect] + [%expect {| Error: |}] Promoting _build/default/l.ml.corrected to l.ml. - [1] $ < l.ml tr '\033' '?' open[@ocaml.alert "-unstable"] Stdune @@ -61,7 +60,6 @@ file. - [%expect {| Error: |}] + [%expect {| Error: |}] Promoting _build/default/l.ml.corrected to l.ml. - [1] $ < l.ml tr '\033' '?' open[@ocaml.alert "-unstable"] Stdune diff --git a/test/blackbox-tests/test-cases/menhir/opam-menhir-dep.t b/test/blackbox-tests/test-cases/menhir/opam-menhir-dep.t index 11d9b96b6ec..09468630230 100644 --- a/test/blackbox-tests/test-cases/menhir/opam-menhir-dep.t +++ b/test/blackbox-tests/test-cases/menhir/opam-menhir-dep.t @@ -15,7 +15,6 @@ Case 0: package does not declare menhir. Dune does not add it. > EOF $ dune build @opam --auto-promote > /dev/null 2>&1 - [1] $ grep menhir foo.opam [1] @@ -32,7 +31,6 @@ Case 1: bare [(depends menhir)]. Dune fills in the lower bound. > EOF $ dune build @opam --auto-promote > /dev/null 2>&1 - [1] $ grep menhir foo.opam "menhir" {>= "20180523"} @@ -49,7 +47,6 @@ Case 2: user-written version bound is preserved verbatim. > EOF $ dune build @opam --auto-promote > /dev/null 2>&1 - [1] $ grep menhir foo.opam "menhir" {>= "20211128"} @@ -68,7 +65,6 @@ for an unrelated reason (e.g. runtime). > EOF $ dune build @opam --auto-promote > /dev/null 2>&1 - [1] $ grep menhir foo.opam "menhir" @@ -89,7 +85,6 @@ bound; [bar.opam] has no [menhir] line at all. > EOF $ dune build @opam --auto-promote > /dev/null 2>&1 - [1] $ grep menhir foo.opam "menhir" {>= "20180523"} $ test -f bar.opam && grep -c '^opam-version' bar.opam @@ -117,6 +112,5 @@ stanza for the existing opam file. > EOF $ dune build @opam --auto-promote > /dev/null 2>&1 - [1] $ grep menhir foo.opam "menhir" {with-test} diff --git a/test/blackbox-tests/test-cases/pkg/ocamlformat/fmt-lockdir-behavior-gh11037.t b/test/blackbox-tests/test-cases/pkg/ocamlformat/fmt-lockdir-behavior-gh11037.t index 81dd249400d..7db1376ba38 100644 --- a/test/blackbox-tests/test-cases/pkg/ocamlformat/fmt-lockdir-behavior-gh11037.t +++ b/test/blackbox-tests/test-cases/pkg/ocamlformat/fmt-lockdir-behavior-gh11037.t @@ -48,7 +48,6 @@ attempt to build the package "foo". let () = print_endline "Hello, world" +(* formatted with fake ocamlformat *) Promoting _build/default/foo.ml.corrected to foo.ml. - [1] $ cat foo.ml let () = print_endline "Hello, world" (* formatted with fake ocamlformat *) diff --git a/test/blackbox-tests/test-cases/pkg/ocamlformat/ocamlformat-avoid-taking-from-project-deps.t b/test/blackbox-tests/test-cases/pkg/ocamlformat/ocamlformat-avoid-taking-from-project-deps.t index 2dc171565fa..29828bd796a 100644 --- a/test/blackbox-tests/test-cases/pkg/ocamlformat/ocamlformat-avoid-taking-from-project-deps.t +++ b/test/blackbox-tests/test-cases/pkg/ocamlformat/ocamlformat-avoid-taking-from-project-deps.t @@ -53,7 +53,6 @@ dev-tool (0.26.3). -let () = print_endline "Hello, world" +formatted with version 0.26.3 Promoting _build/default/foo.ml.corrected to foo.ml. - [1] $ cat foo.ml formatted with version 0.26.3 diff --git a/test/blackbox-tests/test-cases/pkg/ocamlformat/ocamlformat-ignore.t b/test/blackbox-tests/test-cases/pkg/ocamlformat/ocamlformat-ignore.t index 89fb724f20c..bc34aa8b730 100644 --- a/test/blackbox-tests/test-cases/pkg/ocamlformat/ocamlformat-ignore.t +++ b/test/blackbox-tests/test-cases/pkg/ocamlformat/ocamlformat-ignore.t @@ -55,7 +55,6 @@ Check without the feature when ".ocamlformat-ignore" file exists. +ignoring some files +fake ocamlformat from PATH Promoting _build/default/foo.ml.corrected to foo.ml. - [1] $ ls _build/default/.ocamlformat-ignore _build/default/.ocamlformat-ignore $ cat foo.ml diff --git a/test/blackbox-tests/test-cases/pkg/ocamlformat/ocamlformat-patch-extra-files.t b/test/blackbox-tests/test-cases/pkg/ocamlformat/ocamlformat-patch-extra-files.t index 63be50093b2..513f4e5094a 100644 --- a/test/blackbox-tests/test-cases/pkg/ocamlformat/ocamlformat-patch-extra-files.t +++ b/test/blackbox-tests/test-cases/pkg/ocamlformat/ocamlformat-patch-extra-files.t @@ -68,7 +68,6 @@ First run of 'dune fmt' is supposed to format the fail. -let () = print_endline "Hello, world" +formatted with version 0.26.2 Promoting _build/default/foo.ml.corrected to foo.ml. - [1] The foo.ml file is now formatted with the patched version of ocamlformat. $ cat foo.ml diff --git a/test/blackbox-tests/test-cases/pkg/ocamlformat/ocamlformat-relaxed-version-constraints.t b/test/blackbox-tests/test-cases/pkg/ocamlformat/ocamlformat-relaxed-version-constraints.t index 6325a21644d..0ac1021c51d 100644 --- a/test/blackbox-tests/test-cases/pkg/ocamlformat/ocamlformat-relaxed-version-constraints.t +++ b/test/blackbox-tests/test-cases/pkg/ocamlformat/ocamlformat-relaxed-version-constraints.t @@ -37,7 +37,6 @@ This should choose the 0.24+foo version: let () = print_endline "Hello, world" +(* formatted with fake ocamlformat 0.24+foo *) Promoting _build/default/foo.ml.corrected to foo.ml. - [1] $ cat foo.ml let () = print_endline "Hello, world" (* formatted with fake ocamlformat 0.24+foo *) @@ -56,7 +55,6 @@ This should choose the 0.24+bar version: (* formatted with fake ocamlformat 0.24+foo *) +(* formatted with fake ocamlformat 0.25+bar *) Promoting _build/default/foo.ml.corrected to foo.ml. - [1] $ cat foo.ml let () = print_endline "Hello, world" (* formatted with fake ocamlformat 0.24+foo *) diff --git a/test/blackbox-tests/test-cases/pkg/ocamlformat/ocamlformat-rerun-on-source-change-gh10991.t b/test/blackbox-tests/test-cases/pkg/ocamlformat/ocamlformat-rerun-on-source-change-gh10991.t index 305dfe292db..cb952fa09a6 100644 --- a/test/blackbox-tests/test-cases/pkg/ocamlformat/ocamlformat-rerun-on-source-change-gh10991.t +++ b/test/blackbox-tests/test-cases/pkg/ocamlformat/ocamlformat-rerun-on-source-change-gh10991.t @@ -27,7 +27,6 @@ Initial file: let () = print_endline "Hello, world" +(* formatted with fake ocamlformat *) Promoting _build/default/foo.ml.corrected to foo.ml. - [1] After formatting the fake ocamlformat has added a suffix: $ cat foo.ml @@ -47,7 +46,6 @@ Update the file: let () = print_endline "Hello, ocaml!" +(* formatted with fake ocamlformat *) Promoting _build/default/foo.ml.corrected to foo.ml. - [1] The update to the file persists after formatting it a second time: $ cat foo.ml diff --git a/test/blackbox-tests/test-cases/promote/auto-promote-exit-code.t b/test/blackbox-tests/test-cases/promote/auto-promote-exit-code.t index 9ae35ccc35a..865b0d8ca4f 100644 --- a/test/blackbox-tests/test-cases/promote/auto-promote-exit-code.t +++ b/test/blackbox-tests/test-cases/promote/auto-promote-exit-code.t @@ -1,9 +1,7 @@ Regression test for https://github.com/ocaml/dune/issues/785. -`dune build --auto-promote` applies the promotion but currently still exits -with a non-zero code, forcing callers to wrap the command in `|| true`. This -test captures that behaviour; once #785 is fixed the build should exit 0 once -the promotion has been applied (a fixpoint is reached). +`dune build --auto-promote` applies the promotion and exits with code 0, +instead of forcing callers to wrap the command in `|| true`. $ make_dune_project 3.25 @@ -17,8 +15,7 @@ the promotion has been applied (a fixpoint is reached). $ printf titi > x -The diff is shown and the file is promoted, but the build still exits with -code 1 -- this is the bug tracked by #785: +The diff is shown and the file is promoted, and the build exits 0: $ dune build @blah --auto-promote File "x", line 1, characters 0-0: @@ -30,7 +27,6 @@ code 1 -- this is the bug tracked by #785: +toto \ No newline at end of file Promoting _build/default/x.gen to x. - [1] $ cat x toto @@ -64,3 +60,38 @@ mask: +bbb \ No newline at end of file [1] + +Result-producing commands must not treat an auto-promoted diff as success for +the current run. The executable is not linked until the command is re-run after +promotion reaches a fixpoint. + + $ cat > dune < (executable + > (name foo) + > (link_deps (alias blah))) + > (rule + > (with-stdout-to x.gen (echo "toto"))) + > (rule + > (alias blah) + > (action (diff x x.gen))) + > EOF + + $ printf titi > x + $ cat > foo.ml < let () = print_endline "foo" + > EOF + + $ dune exec --auto-promote ./foo.exe + File "x", line 1, characters 0-0: + --- x + +++ x.gen + @@ -1 +1 @@ + -titi + \ No newline at end of file + +toto + \ No newline at end of file + Promoting _build/default/x.gen to x. + [1] + + $ dune exec --auto-promote ./foo.exe + foo diff --git a/test/blackbox-tests/test-cases/promote/old-tests.t/run.t b/test/blackbox-tests/test-cases/promote/old-tests.t/run.t index f59894db34d..3710e4a12b6 100644 --- a/test/blackbox-tests/test-cases/promote/old-tests.t/run.t +++ b/test/blackbox-tests/test-cases/promote/old-tests.t/run.t @@ -65,7 +65,6 @@ Otherwise this test fails on OSX +toto \ No newline at end of file Promoting _build/default/x.gen to x. - [1] $ cat x toto $ dune build @blah diff --git a/test/blackbox-tests/test-cases/stanzas/mdx-stanza/env-variables.t b/test/blackbox-tests/test-cases/stanzas/mdx-stanza/env-variables.t index 83cff48b20b..4e047927f4b 100644 --- a/test/blackbox-tests/test-cases/stanzas/mdx-stanza/env-variables.t +++ b/test/blackbox-tests/test-cases/stanzas/mdx-stanza/env-variables.t @@ -30,7 +30,6 @@ variable. This is only the case since mdx 2.1.0. ```ocaml non-deterministic Promoting _build/default/.mdx/README.md.corrected to README.md. - [1] $ cat README.md ```ocaml @@ -55,7 +54,6 @@ variable. This is only the case since mdx 2.1.0. +- : string = "b" ``` Promoting _build/default/.mdx/README.md.corrected to README.md. - [1] $ cat README.md ```ocaml diff --git a/test/blackbox-tests/test-cases/stanzas/mdx-stanza/mdx-prelude-runtime-deps-github7077.t b/test/blackbox-tests/test-cases/stanzas/mdx-stanza/mdx-prelude-runtime-deps-github7077.t index bd887960f9e..738edf9daab 100644 --- a/test/blackbox-tests/test-cases/stanzas/mdx-stanza/mdx-prelude-runtime-deps-github7077.t +++ b/test/blackbox-tests/test-cases/stanzas/mdx-stanza/mdx-prelude-runtime-deps-github7077.t @@ -33,7 +33,6 @@ run time. This test makes sure that this is recorded as a dependency. +- : int = 2 ``` Promoting _build/default/.mdx/README.md.corrected to README.md. - [1] $ cat README.md ```ocaml diff --git a/test/expect-tests/dune_rpc_e2e/dune_rpc_diagnostics.ml b/test/expect-tests/dune_rpc_e2e/dune_rpc_diagnostics.ml index c964e3e6478..d5f38521029 100644 --- a/test/expect-tests/dune_rpc_e2e/dune_rpc_diagnostics.ml +++ b/test/expect-tests/dune_rpc_e2e/dune_rpc_diagnostics.ml @@ -375,7 +375,6 @@ let%expect_test "optional promotion" = {| Building (alias foo) Build (alias foo) failed - FAILURE: promotion file $CWD/_build/.promotion-staging/output.expected does not exist [ "Add" ; [ [ "directory"; "$CWD" ] ; [ "id"; "0" ] @@ -408,12 +407,7 @@ let%expect_test "optional promotion" = " ] ] - ; [ "promotion" - ; [ [ [ "in_build"; "$CWD/_build/.promotion-staging/output.expected" ] - ; [ "in_source"; "$CWD/output.expected" ] - ] - ] - ] + ; [ "promotion"; [] ] ; [ "related"; [] ] ; [ "severity"; "error" ] ; [ "targets"; [] ]