From 218f0f05dde70190a30659555cb3921d1d67193b Mon Sep 17 00:00:00 2001 From: Jules Aguillon Date: Fri, 13 Dec 2024 10:40:22 +0100 Subject: [PATCH 1/2] test: Repro 'dune fmt' crash in presence of Nix result A 'ocamlformat' executable is added to the test to avoid depending on OCamlformat. Signed-off-by: Jules Aguillon --- .../read-only-symlink-target.t/.ocamlformat | 0 .../test-cases/read-only-symlink-target.t/dune | 2 ++ .../read-only-symlink-target.t/dune-project | 1 + .../read-only-symlink-target.t/ocamlformat.ml | 2 ++ .../test-cases/read-only-symlink-target.t/run.t | 16 ++++++++++++++++ 5 files changed, 21 insertions(+) create mode 100644 test/blackbox-tests/test-cases/read-only-symlink-target.t/.ocamlformat create mode 100644 test/blackbox-tests/test-cases/read-only-symlink-target.t/dune create mode 100644 test/blackbox-tests/test-cases/read-only-symlink-target.t/dune-project create mode 100644 test/blackbox-tests/test-cases/read-only-symlink-target.t/ocamlformat.ml create mode 100644 test/blackbox-tests/test-cases/read-only-symlink-target.t/run.t diff --git a/test/blackbox-tests/test-cases/read-only-symlink-target.t/.ocamlformat b/test/blackbox-tests/test-cases/read-only-symlink-target.t/.ocamlformat new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/blackbox-tests/test-cases/read-only-symlink-target.t/dune b/test/blackbox-tests/test-cases/read-only-symlink-target.t/dune new file mode 100644 index 00000000000..75e3ba2509e --- /dev/null +++ b/test/blackbox-tests/test-cases/read-only-symlink-target.t/dune @@ -0,0 +1,2 @@ +(executable + (name ocamlformat)) diff --git a/test/blackbox-tests/test-cases/read-only-symlink-target.t/dune-project b/test/blackbox-tests/test-cases/read-only-symlink-target.t/dune-project new file mode 100644 index 00000000000..c2e46604eed --- /dev/null +++ b/test/blackbox-tests/test-cases/read-only-symlink-target.t/dune-project @@ -0,0 +1 @@ +(lang dune 2.8) diff --git a/test/blackbox-tests/test-cases/read-only-symlink-target.t/ocamlformat.ml b/test/blackbox-tests/test-cases/read-only-symlink-target.t/ocamlformat.ml new file mode 100644 index 00000000000..4099c65085d --- /dev/null +++ b/test/blackbox-tests/test-cases/read-only-symlink-target.t/ocamlformat.ml @@ -0,0 +1,2 @@ +(* Avoid adding dependencies to this cram test *) +let () = print_endline "(* formatted *)" diff --git a/test/blackbox-tests/test-cases/read-only-symlink-target.t/run.t b/test/blackbox-tests/test-cases/read-only-symlink-target.t/run.t new file mode 100644 index 00000000000..43760753360 --- /dev/null +++ b/test/blackbox-tests/test-cases/read-only-symlink-target.t/run.t @@ -0,0 +1,16 @@ +Nix can leave a symlink to a store path in the tree, often called 'result'. +'dune fmt' crashes because of that. + + $ RESULT=`mktemp -d` + $ echo "let x = 2" > "$RESULT/foo.ml" + $ chmod -R a-w "$RESULT" + $ ln -s "$RESULT" result + +This command should succeed: + + $ dune fmt 2>/dev/null + [1] + +Allow Dune to remove temporary files (calling Dune crashes without this): + + $ chmod -R u+w "$RESULT" From 029d26ead14544c3a8dadba423848a48c53e91c9 Mon Sep 17 00:00:00 2001 From: Rudi Grinberg Date: Sat, 8 Feb 2025 22:20:44 +0000 Subject: [PATCH 2/2] fix: improve error messages Make user errors in hooks just regular errors Simplify the erorr for promotion failures Signed-off-by: Rudi Grinberg --- src/dune_engine/hooks.ml | 10 +++++++--- src/promote/diff_promotion.ml | 8 +++++++- .../test-cases/read-only-symlink-target.t/run.t | 8 +++++++- 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/dune_engine/hooks.ml b/src/dune_engine/hooks.ml index b9eb441e229..75aecb9ce4a 100644 --- a/src/dune_engine/hooks.ml +++ b/src/dune_engine/hooks.ml @@ -26,9 +26,13 @@ module Make () = struct match !exns with | [] -> () | exns -> - let exns = List.rev exns in - let open Dyn in - Code_error.raise "hooks failed" [ "exns", (list Exn_with_backtrace.to_dyn) exns ] + (match exns with + | [ { exn = User_error.E _ as e; backtrace = _ } ] -> raise e + | _ -> + let exns = List.rev exns in + Code_error.raise + "hooks failed" + [ "exns", (Dyn.list Exn_with_backtrace.to_dyn) exns ]) ;; end diff --git a/src/promote/diff_promotion.ml b/src/promote/diff_promotion.ml index d46b479a03e..b6e20c7c886 100644 --- a/src/promote/diff_promotion.ml +++ b/src/promote/diff_promotion.ml @@ -52,7 +52,13 @@ module File = struct let do_promote ~correction_file ~dst = Path.Source.unlink_no_err dst; let chmod = Path.Permissions.add Path.Permissions.write in - Io.copy_file ~chmod ~src:correction_file ~dst:(Path.source dst) () + match Io.copy_file ~chmod ~src:correction_file ~dst:(Path.source dst) () with + | () -> () + | exception Unix.Unix_error (e, _, _) -> + User_error.raise + [ Pp.textf "failed to promote %s" (Path.Source.to_string dst) + ; Pp.text (Unix.error_message e) + ] ;; let correction_file { src; staging; _ } = Path.build (Option.value staging ~default:src) diff --git a/test/blackbox-tests/test-cases/read-only-symlink-target.t/run.t b/test/blackbox-tests/test-cases/read-only-symlink-target.t/run.t index 43760753360..d3cf94e382c 100644 --- a/test/blackbox-tests/test-cases/read-only-symlink-target.t/run.t +++ b/test/blackbox-tests/test-cases/read-only-symlink-target.t/run.t @@ -8,7 +8,13 @@ Nix can leave a symlink to a store path in the tree, often called 'result'. This command should succeed: - $ dune fmt 2>/dev/null + $ dune fmt + File "result/foo.ml", line 1, characters 0-0: + Error: Files _build/default/result/foo.ml and + _build/default/result/.formatted/foo.ml differ. + Promoting _build/default/result/.formatted/foo.ml to result/foo.ml. + Error: failed to promote result/foo.ml + Permission denied [1] Allow Dune to remove temporary files (calling Dune crashes without this):