Skip to content

Commit

Permalink
fix(pkg): dune fmt missing extra files from the locks.
Browse files Browse the repository at this point in the history
Signed-off-by: Alpha DIALLO <[email protected]>
  • Loading branch information
moyodiallo committed Sep 23, 2024
1 parent dedfb76 commit 26bb033
Show file tree
Hide file tree
Showing 2 changed files with 97 additions and 18 deletions.
54 changes: 36 additions & 18 deletions bin/build_cmd.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
;;
61 changes: 61 additions & 0 deletions test/blackbox-tests/test-cases/pkg/ocamlformat/patch-extra-files.t
Original file line number Diff line number Diff line change
@@ -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 <<EOF
> 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" <<EOF
> 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

0 comments on commit 26bb033

Please sign in to comment.