From 26bb033d9f68ccf234e5a02a5d06ac8746f68136 Mon Sep 17 00:00:00 2001 From: Alpha DIALLO Date: Mon, 23 Sep 2024 12:04:56 +0200 Subject: [PATCH] fix(pkg): `dune fmt` missing extra files from the locks. Signed-off-by: Alpha DIALLO --- bin/build_cmd.ml | 54 ++++++++++------ .../pkg/ocamlformat/patch-extra-files.t | 61 +++++++++++++++++++ 2 files changed, 97 insertions(+), 18 deletions(-) create mode 100644 test/blackbox-tests/test-cases/pkg/ocamlformat/patch-extra-files.t diff --git a/bin/build_cmd.ml b/bin/build_cmd.ml index fec6191252a..f817525c7b2 100644 --- a/bin/build_cmd.ml +++ b/bin/build_cmd.ml @@ -82,12 +82,18 @@ let run_build_system ~common ~request = Fiber.return ()) ;; -let run_build_command_poll_eager ~(common : Common.t) ~config ~request : unit = +let run_build_command_poll_eager ~pre_request ~(common : Common.t) ~config ~request : unit + = + let open Fiber.O in Scheduler.go_with_rpc_server_and_console_status_reporting ~common ~config (fun () -> - Scheduler.Run.poll (run_build_system ~common ~request)) + Scheduler.Run.poll + (let* () = pre_request () in + run_build_system ~common ~request)) ;; -let run_build_command_poll_passive ~(common : Common.t) ~config ~request:_ : unit = +let run_build_command_poll_passive ~pre_request:_ ~(common : Common.t) ~config ~request:_ + : unit + = (* CR-someday aalekseyev: It would've been better to complain if [request] is non-empty, but we can't check that here because [request] is a function.*) let open Fiber.O in @@ -106,9 +112,10 @@ let run_build_command_poll_passive ~(common : Common.t) ~config ~request:_ : uni run_build_system ~common ~request, ivar)) ;; -let run_build_command_once ~(common : Common.t) ~config ~request = +let run_build_command_once ~pre_request ~(common : Common.t) ~config ~request = let open Fiber.O in let once () = + let* () = pre_request () in let+ res = run_build_system ~common ~request in match res with | Error `Already_reported -> raise Dune_util.Report_error.Already_reported @@ -122,6 +129,30 @@ let run_build_command ~(common : Common.t) ~config ~request = | Yes Eager -> run_build_command_poll_eager | Yes Passive -> run_build_command_poll_passive | No -> run_build_command_once) + ~pre_request:(fun () -> Fiber.return ()) + ~common + ~config + ~request +;; + +let run_build_command_fmt ~(common : Common.t) ~config ~request = + let lock_ocamlformat () = + if Lazy.force Lock_dev_tool.is_enabled + then + (* Note that generating the ocamlformat lockdir here means + that it will be created when a user runs `dune fmt` but not + when a user runs `dune build @fmt`. It's important that + this logic remain outside of `dune build`, as `dune + build` is intended to only build targets, and generating + a lockdir is not building a target. *) + Lock_dev_tool.lock_ocamlformat () |> Memo.run + else Fiber.return () + in + (match Common.watch common with + | Yes Eager -> run_build_command_poll_eager + | Yes Passive -> run_build_command_poll_passive + | No -> run_build_command_once) + ~pre_request:lock_ocamlformat ~common ~config ~request @@ -231,24 +262,11 @@ let fmt = in let common, config = Common.init builder in let request (setup : Import.Main.build_system) = - let open Action_builder.O in - let* () = - if Lazy.force Lock_dev_tool.is_enabled - then - (* Note that generating the ocamlformat lockdir here means - that it will be created when a user runs `dune fmt` but not - when a user runs `dune build @fmt`. It's important that - this logic remain outside of `dune build`, as `dune - build` is intended to only build targets, and generating - a lockdir is not building a target. *) - Action_builder.of_memo (Lock_dev_tool.lock_ocamlformat ()) - else Action_builder.return () - in let dir = Path.(relative root) (Common.prefix_target common ".") in Alias.in_dir ~name:Dune_rules.Alias.fmt ~recursive:true ~contexts:setup.contexts dir |> Alias.request in - run_build_command ~common ~config ~request + run_build_command_fmt ~common ~config ~request in Cmd.v (Cmd.info "fmt" ~doc ~man ~envs:Common.envs) term ;; diff --git a/test/blackbox-tests/test-cases/pkg/ocamlformat/patch-extra-files.t b/test/blackbox-tests/test-cases/pkg/ocamlformat/patch-extra-files.t new file mode 100644 index 00000000000..9811625c18e --- /dev/null +++ b/test/blackbox-tests/test-cases/pkg/ocamlformat/patch-extra-files.t @@ -0,0 +1,61 @@ +'dune fmt' could miss the extra-files from "dev-tools.locks" directory for the first run, if the +auto-locking comes before the setup of the dune project meaning the loading of dune source tree. + + $ . ./helpers.sh + $ mkrepo + +Make a fake ocamlformat: + $ make_fake_ocamlformat "0.1" + +A patch that changes the version from "0.1" to "0.26.2": + $ cat > patch-for-ocamlformat.patch < diff a/ocamlformat.ml b/ocamlformat.ml + > --- a/ocamlformat.ml + > +++ b/ocamlformat.ml + > @@ -1,6 +1,6 @@ + > -let version = "0.1" + > +let version = "0.26.2" + > let () = + > if Sys.file_exists ".ocamlformat-ignore" then + > print_endline "ignoring some files" + > ;; + > let () = print_endline ("formatted with version "^version) + > EOF + +Make the ocamlformat opam package which uses a patch: + $ mkpkg ocamlformat "0.26.2" < build: [ + > [ + > "dune" + > "build" + > "-p" + > name + > "-j" + > jobs + > ] + > ] + > extra-files: ["patch-for-ocamlformat.patch" "md5=$(md5sum patch-for-ocamlformat.patch | cut -f1 -d' ')"] + > patches: ["patch-for-ocamlformat.patch"] + > url { + > src:"file://$PWD/ocamlformat-0.1.tar.gz" + > checksum: [ + > "md5=$(md5sum ocamlformat-0.1.tar.gz | cut -f1 -d' ')" + > ] + > } + > EOF + $ mkdir -p mock-opam-repository/packages/ocamlformat/ocamlformat.0.26.2/files/ + $ cp patch-for-ocamlformat.patch mock-opam-repository/packages/ocamlformat/ocamlformat.0.26.2/files/ + +Make a project that uses the fake ocamlformat: + $ make_project_with_dev_tool_lockdir + +First time run + $ DUNE_CONFIG__LOCK_DEV_TOOL=enabled dune fmt 2>&1 | sed -E 's#.*.sandbox/[^/]+#.sandbox/$SANDBOX#g' + Solution for dev-tools.locks/ocamlformat: + - ocamlformat.0.26.2 + 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. + $ cat foo.ml + formatted with version 0.26.2