From 154739e0de705d4271757407cbf13a95c3370483 Mon Sep 17 00:00:00 2001 From: Stephen Sherratt Date: Mon, 14 Oct 2024 16:41:09 +1100 Subject: [PATCH 1/2] pkg: sandbox format action when using devtool Signed-off-by: Stephen Sherratt --- src/dune_rules/format_rules.ml | 4 +++- test/blackbox-tests/test-cases/pkg/ocamlformat/gh10991.t | 8 ++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/dune_rules/format_rules.ml b/src/dune_rules/format_rules.ml index 17f66f17a93..f4239f26919 100644 --- a/src/dune_rules/format_rules.ml +++ b/src/dune_rules/format_rules.ml @@ -97,7 +97,9 @@ module Ocamlformat = struct let open Action_builder.With_targets.O in (* Depend on [extra_deps] so if the ocamlformat config file changes then ocamlformat will run again. *) - extra_deps dir >>> action + extra_deps dir + >>> action + |> With_targets.map ~f:(Action.Full.add_sandbox Sandbox_config.needs_sandboxing) ;; let action_when_ocamlformat_isn't_locked ~input kind = diff --git a/test/blackbox-tests/test-cases/pkg/ocamlformat/gh10991.t b/test/blackbox-tests/test-cases/pkg/ocamlformat/gh10991.t index 44a69d9b498..048d367c13a 100644 --- a/test/blackbox-tests/test-cases/pkg/ocamlformat/gh10991.t +++ b/test/blackbox-tests/test-cases/pkg/ocamlformat/gh10991.t @@ -18,18 +18,19 @@ Initial file: $ cat foo.ml let () = print_endline "Hello, world" +Formatting failed because the input file was not copied into the sandbox. $ DUNE_CONFIG__LOCK_DEV_TOOL=enabled dune fmt Solution for dev-tools.locks/ocamlformat: - ocamlformat.0.0.1 + cat: foo.ml: No such file or directory File "foo.ml", line 1, characters 0-0: Error: Files _build/default/foo.ml and _build/default/.formatted/foo.ml differ. Promoting _build/default/.formatted/foo.ml to foo.ml. [1] -After formatting the fake ocamlformat has added a suffix: +No formatting occured because input file wasn't copied to the sandbox. $ cat foo.ml - let () = print_endline "Hello, world" (* formatted with fake ocamlformat *) Update the file: @@ -44,7 +45,6 @@ Update the file: Promoting _build/default/.formatted/foo.ml to foo.ml. [1] -After formatting a second time, the recent change to the file was ignored: +No formatting occured because input file wasn't copied to the sandbox. $ cat foo.ml - let () = print_endline "Hello, world" (* formatted with fake ocamlformat *) From 40720f43e59e2df144afca14564acad09d145675 Mon Sep 17 00:00:00 2001 From: Stephen Sherratt Date: Fri, 4 Oct 2024 15:25:54 +1000 Subject: [PATCH 2/2] pkg: fix dev-tool bug where `dune fmt` would revert file The rule for running ocamlformat on source files when dev-tools are in use did not depend on the input file, so changes to the input file wouldn't cause ocamlformat to be re-run on the updated file. The consequence of this is that `dune fmt` would promote stale versions of files. Signed-off-by: Stephen Sherratt --- src/dune_rules/format_rules.ml | 5 ++++- test/blackbox-tests/test-cases/pkg/ocamlformat/gh10991.t | 8 ++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/dune_rules/format_rules.ml b/src/dune_rules/format_rules.ml index f4239f26919..9a8ce5bd0a1 100644 --- a/src/dune_rules/format_rules.ml +++ b/src/dune_rules/format_rules.ml @@ -90,7 +90,10 @@ module Ocamlformat = struct (let open Action_builder.O in (* This ensures that at is installed as a dev tool before running it. *) - let+ () = Action_builder.path path in + let+ () = Action_builder.path path + (* Declare the dependency on the input file so changes to the input + file trigger ocamlformat to run again on the updated file. *) + and+ () = Action_builder.path (Path.build input) in let args = [ flag_of_kind kind; Path.Build.basename input ] in Action.chdir (Path.build dir) @@ Action.run (Ok path) args |> Action.Full.make) in diff --git a/test/blackbox-tests/test-cases/pkg/ocamlformat/gh10991.t b/test/blackbox-tests/test-cases/pkg/ocamlformat/gh10991.t index 048d367c13a..8c125460e05 100644 --- a/test/blackbox-tests/test-cases/pkg/ocamlformat/gh10991.t +++ b/test/blackbox-tests/test-cases/pkg/ocamlformat/gh10991.t @@ -18,19 +18,18 @@ Initial file: $ cat foo.ml let () = print_endline "Hello, world" -Formatting failed because the input file was not copied into the sandbox. $ DUNE_CONFIG__LOCK_DEV_TOOL=enabled dune fmt Solution for dev-tools.locks/ocamlformat: - ocamlformat.0.0.1 - cat: foo.ml: No such file or directory File "foo.ml", line 1, characters 0-0: Error: Files _build/default/foo.ml and _build/default/.formatted/foo.ml differ. Promoting _build/default/.formatted/foo.ml to foo.ml. [1] -No formatting occured because input file wasn't copied to the sandbox. +After formatting the fake ocamlformat has added a suffix: $ cat foo.ml + let () = print_endline "Hello, world" (* formatted with fake ocamlformat *) Update the file: @@ -45,6 +44,7 @@ Update the file: Promoting _build/default/.formatted/foo.ml to foo.ml. [1] -No formatting occured because input file wasn't copied to the sandbox. +The update to the file persists after formatting it a second time: $ cat foo.ml + let () = print_endline "Hello, ocaml!" (* formatted with fake ocamlformat *)