Skip to content

Commit

Permalink
fix(engine): remove old target even if it is a dir
Browse files Browse the repository at this point in the history
Fixes ocaml#6575

Signed-off-by: Etienne Millon <[email protected]>
  • Loading branch information
emillon committed Nov 30, 2023
1 parent 55143f3 commit 3a96dc7
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 11 deletions.
1 change: 1 addition & 0 deletions otherlibs/stdune/src/path.ml
Original file line number Diff line number Diff line change
Expand Up @@ -729,6 +729,7 @@ module Build = struct
let of_local t = t
let chmod t ~mode = Unix.chmod (to_string t) mode
let lstat t = Unix.lstat (to_string t)
let unlink t = Fpath.unlink (to_string t)
let unlink_no_err t = Fpath.unlink_no_err (to_string t)
let to_dyn s = Dyn.variant "In_build_dir" [ to_dyn s ]
end
Expand Down
1 change: 1 addition & 0 deletions otherlibs/stdune/src/path.mli
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ module Build : sig
val chmod : t -> mode:int -> unit

val lstat : t -> Unix.stats
val unlink : t -> unit
val unlink_no_err : t -> unit

module Table : Hashtbl.S with type key = t
Expand Down
13 changes: 11 additions & 2 deletions src/dune_engine/build_system.ml
Original file line number Diff line number Diff line change
Expand Up @@ -536,8 +536,17 @@ end = struct
in
let* () =
Targets.maybe_async (fun () ->
Path.Build.Set.iter targets.files ~f:Path.Build.unlink_no_err;
Path.Build.Set.iter targets.dirs ~f:(fun dir -> Path.rm_rf (Path.build dir)))
let remove_target_dir dir = Path.rm_rf (Path.build dir) in
let remove_target_file path =
try Path.Build.unlink path with
| Unix.Unix_error (EISDIR, _, _) ->
(* If target changed from a directory to a file, delete
in anyway. *)
remove_target_dir path
| _ -> ()
in
Path.Build.Set.iter targets.files ~f:remove_target_file;
Path.Build.Set.iter targets.dirs ~f:remove_target_dir)
in
let* produced_targets, dynamic_deps_stages =
(* Step III. Try to restore artifacts from the shared cache. *)
Expand Down
10 changes: 1 addition & 9 deletions test/blackbox-tests/test-cases/github6575.t
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,5 @@ We turn it into a single-file test:
$ mv sometest.t.bak/run.t sometest.t
$ rm -r sometest.t.bak

FIXME: Dune should detect the change:
$ dune build @sometest
Error: _build/default/sometest.t: Is a directory
-> required by _build/default/sometest.t
-> required by alias sometest
[1]

After a clean it works as expected:
$ dune clean
Dune detects the change:
$ dune build @sometest

0 comments on commit 3a96dc7

Please sign in to comment.