From 74304ab810883c7c033566af020550cd339c8eb2 Mon Sep 17 00:00:00 2001 From: Etienne Millon Date: Thu, 14 Dec 2023 10:50:58 +0100 Subject: [PATCH 01/13] fix(engine): remove old target even if it is a dir Fixes #6575 Signed-off-by: Etienne Millon --- otherlibs/stdune/src/path.ml | 1 + otherlibs/stdune/src/path.mli | 1 + src/dune_engine/build_system.ml | 13 +++++++++++-- test/blackbox-tests/test-cases/github6575.t | 10 +--------- 4 files changed, 14 insertions(+), 11 deletions(-) diff --git a/otherlibs/stdune/src/path.ml b/otherlibs/stdune/src/path.ml index f3e1db8be9d..03dfd9f99cb 100644 --- a/otherlibs/stdune/src/path.ml +++ b/otherlibs/stdune/src/path.ml @@ -734,6 +734,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 diff --git a/otherlibs/stdune/src/path.mli b/otherlibs/stdune/src/path.mli index f2a7839c25c..aaaa80bc88b 100644 --- a/otherlibs/stdune/src/path.mli +++ b/otherlibs/stdune/src/path.mli @@ -195,6 +195,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 diff --git a/src/dune_engine/build_system.ml b/src/dune_engine/build_system.ml index df2c8f13fa7..f4d192d3f1d 100644 --- a/src/dune_engine/build_system.ml +++ b/src/dune_engine/build_system.ml @@ -567,10 +567,19 @@ end = struct in let* () = maybe_async_rule_file_op (fun () -> + 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 Targets.Validated.iter targets - ~file:Path.Build.unlink_no_err - ~dir:(fun dir -> Path.rm_rf (Path.build dir))) + ~file:remove_target_file + ~dir:remove_target_dir) in let* produced_targets, dynamic_deps_stages = (* Step III. Try to restore artifacts from the shared cache. *) diff --git a/test/blackbox-tests/test-cases/github6575.t b/test/blackbox-tests/test-cases/github6575.t index 242801e2521..ac1f69d66c7 100644 --- a/test/blackbox-tests/test-cases/github6575.t +++ b/test/blackbox-tests/test-cases/github6575.t @@ -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 From b06cdcf72889b17103ad26b8bfab48c101f3c2d1 Mon Sep 17 00:00:00 2001 From: Etienne Millon Date: Thu, 30 Nov 2023 11:09:09 +0100 Subject: [PATCH 02/13] Add changelog entry Signed-off-by: Etienne Millon --- doc/changes/9327.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 doc/changes/9327.md diff --git a/doc/changes/9327.md b/doc/changes/9327.md new file mode 100644 index 00000000000..3062f862cbd --- /dev/null +++ b/doc/changes/9327.md @@ -0,0 +1,2 @@ +- When a directory is changed to a file, correctly remove it in subsequent + `dune build` runs. (#9327, fix #6575, @emillon) From 517d07a122e331b850ef07faf167afd09408f093 Mon Sep 17 00:00:00 2001 From: Etienne Millon Date: Thu, 30 Nov 2023 13:30:55 +0100 Subject: [PATCH 03/13] Ignore EPERM on darwin Signed-off-by: Etienne Millon --- src/dune_engine/build_system.ml | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/dune_engine/build_system.ml b/src/dune_engine/build_system.ml index f4d192d3f1d..ee17fcd7cfb 100644 --- a/src/dune_engine/build_system.ml +++ b/src/dune_engine/build_system.ml @@ -570,10 +570,12 @@ end = struct 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 + (* If target changed from a directory to a file, delete + in anyway. *) + | Unix.Unix_error (error, _, _) -> + (match error, Platform.OS.value with + | EISDIR, _ | EPERM, Darwin -> remove_target_dir path + | _ -> ()) | _ -> () in Targets.Validated.iter From e6dfb2901458b4c364c943368719923287167dfc Mon Sep 17 00:00:00 2001 From: Etienne Millon Date: Mon, 4 Dec 2023 16:57:05 +0100 Subject: [PATCH 04/13] Extract Fpath.unlink_status Signed-off-by: Etienne Millon --- otherlibs/stdune/src/fpath.ml | 15 +++++++++++++++ otherlibs/stdune/src/fpath.mli | 9 +++++++++ otherlibs/stdune/src/path.ml | 2 +- otherlibs/stdune/src/path.mli | 2 +- src/dune_engine/build_system.ml | 15 +++++++-------- 5 files changed, 33 insertions(+), 10 deletions(-) diff --git a/otherlibs/stdune/src/fpath.ml b/otherlibs/stdune/src/fpath.ml index 03b3623f80a..2e77ea73005 100644 --- a/otherlibs/stdune/src/fpath.ml +++ b/otherlibs/stdune/src/fpath.ml @@ -115,6 +115,21 @@ let win32_unlink fn = let unlink = if Stdlib.Sys.win32 then win32_unlink else Unix.unlink +type unlink_status = + | Success + | Is_a_directory + | Other_error + +let unlink_status t = + match unlink t with + | () -> Success + | exception Unix.Unix_error (error, _, _) -> + (match error, Platform.OS.value with + | EISDIR, _ | EPERM, Darwin -> Is_a_directory + | _ -> Other_error) + | exception _ -> Other_error +;; + let unlink_no_err t = try unlink t with | _ -> () diff --git a/otherlibs/stdune/src/fpath.mli b/otherlibs/stdune/src/fpath.mli index 9e1586b4d24..879329dd17b 100644 --- a/otherlibs/stdune/src/fpath.mli +++ b/otherlibs/stdune/src/fpath.mli @@ -29,6 +29,15 @@ val follow_symlinks : string -> string option val unlink : string -> unit val unlink_no_err : string -> unit + +type unlink_status = + | Success + | Is_a_directory + | Other_error + +(** Unlink and return error, if any. *) +val unlink_status : string -> unlink_status + val initial_cwd : string type clear_dir_result = diff --git a/otherlibs/stdune/src/path.ml b/otherlibs/stdune/src/path.ml index 03dfd9f99cb..554d68de92c 100644 --- a/otherlibs/stdune/src/path.ml +++ b/otherlibs/stdune/src/path.ml @@ -734,7 +734,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_status t = Fpath.unlink_status (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 diff --git a/otherlibs/stdune/src/path.mli b/otherlibs/stdune/src/path.mli index aaaa80bc88b..f4dac2c09db 100644 --- a/otherlibs/stdune/src/path.mli +++ b/otherlibs/stdune/src/path.mli @@ -195,7 +195,7 @@ module Build : sig val chmod : t -> mode:int -> unit val lstat : t -> Unix.stats - val unlink : t -> unit + val unlink_status : t -> Fpath.unlink_status val unlink_no_err : t -> unit module Table : Hashtbl.S with type key = t diff --git a/src/dune_engine/build_system.ml b/src/dune_engine/build_system.ml index ee17fcd7cfb..f43eabe39b3 100644 --- a/src/dune_engine/build_system.ml +++ b/src/dune_engine/build_system.ml @@ -569,14 +569,13 @@ end = struct maybe_async_rule_file_op (fun () -> let remove_target_dir dir = Path.rm_rf (Path.build dir) in let remove_target_file path = - try Path.Build.unlink path with - (* If target changed from a directory to a file, delete - in anyway. *) - | Unix.Unix_error (error, _, _) -> - (match error, Platform.OS.value with - | EISDIR, _ | EPERM, Darwin -> remove_target_dir path - | _ -> ()) - | _ -> () + match Path.Build.unlink_status path with + | Success -> () + | Is_a_directory -> + (* If target changed from a directory to a file, delete + in anyway. *) + remove_target_dir path + | Other_error -> () in Targets.Validated.iter targets From fc582010dedc2f0458e650fdb7ddfbd559f27dff Mon Sep 17 00:00:00 2001 From: Etienne Millon Date: Mon, 4 Dec 2023 16:59:49 +0100 Subject: [PATCH 05/13] Change file to directory Signed-off-by: Etienne Millon --- test/blackbox-tests/test-cases/github6575.t | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/blackbox-tests/test-cases/github6575.t b/test/blackbox-tests/test-cases/github6575.t index ac1f69d66c7..095e8970d89 100644 --- a/test/blackbox-tests/test-cases/github6575.t +++ b/test/blackbox-tests/test-cases/github6575.t @@ -14,3 +14,12 @@ We turn it into a single-file test: Dune detects the change: $ dune build @sometest + +The other way. Turn the file test into a directory test: + + $ mv sometest.t sometest.t.bak + $ mkdir sometest.t + $ mv sometest.t.bak sometest.t/run.t + +And this works: + $ dune build @sometest From f688b2d61f275ff8370a78b1feae3cb8e2de8d3d Mon Sep 17 00:00:00 2001 From: Etienne Millon Date: Fri, 22 Dec 2023 15:01:29 +0100 Subject: [PATCH 06/13] Keep exceptions Signed-off-by: Etienne Millon --- otherlibs/stdune/src/fpath.ml | 9 +++++---- otherlibs/stdune/src/fpath.mli | 3 ++- src/dune_engine/build_system.ml | 2 +- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/otherlibs/stdune/src/fpath.ml b/otherlibs/stdune/src/fpath.ml index 2e77ea73005..7468c255680 100644 --- a/otherlibs/stdune/src/fpath.ml +++ b/otherlibs/stdune/src/fpath.ml @@ -118,16 +118,17 @@ let unlink = if Stdlib.Sys.win32 then win32_unlink else Unix.unlink type unlink_status = | Success | Is_a_directory - | Other_error + | Unix_error of Dune_filesystem_stubs.Unix_error.Detailed.t + | Other_error of exn let unlink_status t = match unlink t with | () -> Success - | exception Unix.Unix_error (error, _, _) -> + | exception Unix.Unix_error (error, syscall, arg) -> (match error, Platform.OS.value with | EISDIR, _ | EPERM, Darwin -> Is_a_directory - | _ -> Other_error) - | exception _ -> Other_error + | _ -> Unix_error (error, syscall, arg)) + | exception exn -> Other_error exn ;; let unlink_no_err t = diff --git a/otherlibs/stdune/src/fpath.mli b/otherlibs/stdune/src/fpath.mli index 879329dd17b..c2ad5717822 100644 --- a/otherlibs/stdune/src/fpath.mli +++ b/otherlibs/stdune/src/fpath.mli @@ -33,7 +33,8 @@ val unlink_no_err : string -> unit type unlink_status = | Success | Is_a_directory - | Other_error + | Unix_error of Dune_filesystem_stubs.Unix_error.Detailed.t + | Other_error of exn (** Unlink and return error, if any. *) val unlink_status : string -> unlink_status diff --git a/src/dune_engine/build_system.ml b/src/dune_engine/build_system.ml index f43eabe39b3..b91116cae28 100644 --- a/src/dune_engine/build_system.ml +++ b/src/dune_engine/build_system.ml @@ -575,7 +575,7 @@ end = struct (* If target changed from a directory to a file, delete in anyway. *) remove_target_dir path - | Other_error -> () + | Unix_error _ | Other_error _ -> () in Targets.Validated.iter targets From 4254813f87023a42020f52c9b10a6c6186396156 Mon Sep 17 00:00:00 2001 From: Etienne Millon Date: Fri, 22 Dec 2023 15:03:03 +0100 Subject: [PATCH 07/13] Use a single constructor for exn Signed-off-by: Etienne Millon --- otherlibs/stdune/src/fpath.ml | 15 ++++++++------- otherlibs/stdune/src/fpath.mli | 3 +-- src/dune_engine/build_system.ml | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/otherlibs/stdune/src/fpath.ml b/otherlibs/stdune/src/fpath.ml index 7468c255680..5690e98c7d8 100644 --- a/otherlibs/stdune/src/fpath.ml +++ b/otherlibs/stdune/src/fpath.ml @@ -118,17 +118,18 @@ let unlink = if Stdlib.Sys.win32 then win32_unlink else Unix.unlink type unlink_status = | Success | Is_a_directory - | Unix_error of Dune_filesystem_stubs.Unix_error.Detailed.t - | Other_error of exn + | Error of exn let unlink_status t = match unlink t with | () -> Success - | exception Unix.Unix_error (error, syscall, arg) -> - (match error, Platform.OS.value with - | EISDIR, _ | EPERM, Darwin -> Is_a_directory - | _ -> Unix_error (error, syscall, arg)) - | exception exn -> Other_error exn + | exception exn -> + (match exn with + | Unix.Unix_error (error, _, _) -> + (match error, Platform.OS.value with + | EISDIR, _ | EPERM, Darwin -> Is_a_directory + | _ -> Error exn) + | _ -> Error exn) ;; let unlink_no_err t = diff --git a/otherlibs/stdune/src/fpath.mli b/otherlibs/stdune/src/fpath.mli index c2ad5717822..3236de4a381 100644 --- a/otherlibs/stdune/src/fpath.mli +++ b/otherlibs/stdune/src/fpath.mli @@ -33,8 +33,7 @@ val unlink_no_err : string -> unit type unlink_status = | Success | Is_a_directory - | Unix_error of Dune_filesystem_stubs.Unix_error.Detailed.t - | Other_error of exn + | Error of exn (** Unlink and return error, if any. *) val unlink_status : string -> unlink_status diff --git a/src/dune_engine/build_system.ml b/src/dune_engine/build_system.ml index b91116cae28..b173b96d6c1 100644 --- a/src/dune_engine/build_system.ml +++ b/src/dune_engine/build_system.ml @@ -575,7 +575,7 @@ end = struct (* If target changed from a directory to a file, delete in anyway. *) remove_target_dir path - | Unix_error _ | Other_error _ -> () + | Error _ -> () in Targets.Validated.iter targets From eb7402cde0c07183095410f75f5d083ef763b969 Mon Sep 17 00:00:00 2001 From: Etienne Millon Date: Fri, 22 Dec 2023 15:07:30 +0100 Subject: [PATCH 08/13] Log error Signed-off-by: Etienne Millon --- src/dune_engine/build_system.ml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/dune_engine/build_system.ml b/src/dune_engine/build_system.ml index b173b96d6c1..3a78eaf0426 100644 --- a/src/dune_engine/build_system.ml +++ b/src/dune_engine/build_system.ml @@ -575,7 +575,13 @@ end = struct (* If target changed from a directory to a file, delete in anyway. *) remove_target_dir path - | Error _ -> () + | Error exn -> + Log.info + [ Pp.textf + "Error while removing target %s: %s" + (Path.Build.to_string path) + (Printexc.to_string exn) + ] in Targets.Validated.iter targets From 7aa34ed60ebbef33ee3e44ef65c4951ed0748e5e Mon Sep 17 00:00:00 2001 From: Etienne Millon Date: Fri, 22 Dec 2023 15:15:07 +0100 Subject: [PATCH 09/13] rename Fpath.unlink to Fpath.unlink_exn Signed-off-by: Etienne Millon --- otherlibs/stdune/src/fpath.ml | 8 ++++---- otherlibs/stdune/src/fpath.mli | 2 +- otherlibs/stdune/src/path.ml | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/otherlibs/stdune/src/fpath.ml b/otherlibs/stdune/src/fpath.ml index 5690e98c7d8..ebcc9f480ab 100644 --- a/otherlibs/stdune/src/fpath.ml +++ b/otherlibs/stdune/src/fpath.ml @@ -113,7 +113,7 @@ let win32_unlink fn = | _ -> raise e) ;; -let unlink = if Stdlib.Sys.win32 then win32_unlink else Unix.unlink +let unlink_exn = if Stdlib.Sys.win32 then win32_unlink else Unix.unlink type unlink_status = | Success @@ -121,7 +121,7 @@ type unlink_status = | Error of exn let unlink_status t = - match unlink t with + match unlink_exn t with | () -> Success | exception exn -> (match exn with @@ -133,7 +133,7 @@ let unlink_status t = ;; let unlink_no_err t = - try unlink t with + try unlink_exn t with | _ -> () ;; @@ -174,7 +174,7 @@ let rm_rf fn = match Unix.lstat fn with | exception Unix.Unix_error (ENOENT, _, _) -> () | { Unix.st_kind = S_DIR; _ } -> rm_rf_dir fn - | _ -> unlink fn + | _ -> unlink_exn fn ;; let traverse_files = diff --git a/otherlibs/stdune/src/fpath.mli b/otherlibs/stdune/src/fpath.mli index 3236de4a381..47bad945637 100644 --- a/otherlibs/stdune/src/fpath.mli +++ b/otherlibs/stdune/src/fpath.mli @@ -27,7 +27,7 @@ val follow_symlink : string -> (string, follow_symlink_error) result [Error Max_depth_exceeded] on some intermediate path). *) val follow_symlinks : string -> string option -val unlink : string -> unit +val unlink_exn : string -> unit val unlink_no_err : string -> unit type unlink_status = diff --git a/otherlibs/stdune/src/path.ml b/otherlibs/stdune/src/path.ml index 554d68de92c..7d5fe6c5a86 100644 --- a/otherlibs/stdune/src/path.ml +++ b/otherlibs/stdune/src/path.ml @@ -1135,7 +1135,7 @@ let is_directory t = ;; let rmdir t = Unix.rmdir (to_string t) -let unlink t = Fpath.unlink (to_string t) +let unlink t = Fpath.unlink_exn (to_string t) let link src dst = match Unix.link (to_string src) (to_string dst) with From 0de31ef281ad8f3c50b0368e795c2c56df37ffc6 Mon Sep 17 00:00:00 2001 From: Etienne Millon Date: Fri, 22 Dec 2023 15:19:40 +0100 Subject: [PATCH 10/13] rename Path.unlink to Path.unlink_exn Signed-off-by: Etienne Millon --- bin/install_uninstall.ml | 2 +- otherlibs/stdune/src/io.ml | 2 +- otherlibs/stdune/src/path.ml | 2 +- otherlibs/stdune/src/path.mli | 2 +- src/dune_engine/action_exec.ml | 2 +- src/dune_engine/load_rules.ml | 2 +- src/dune_engine/process.ml | 2 +- src/dune_rules/pkg_rules.ml | 2 +- src/upgrader/dune_upgrader.ml | 2 +- 9 files changed, 9 insertions(+), 9 deletions(-) diff --git a/bin/install_uninstall.ml b/bin/install_uninstall.ml index 8a57fdb7749..ece8ebb9f2c 100644 --- a/bin/install_uninstall.ml +++ b/bin/install_uninstall.ml @@ -363,7 +363,7 @@ module File_ops_real (W : sig if Path.exists dst then ( print_line "Deleting %s" (Path.to_string_maybe_quoted dst); - print_unix_error (fun () -> Path.unlink dst)) + print_unix_error (fun () -> Path.unlink_exn dst)) ;; let remove_dir_if_exists ~if_non_empty dir = diff --git a/otherlibs/stdune/src/io.ml b/otherlibs/stdune/src/io.ml index 135b32f7cbe..576021ffb5b 100644 --- a/otherlibs/stdune/src/io.ml +++ b/otherlibs/stdune/src/io.ml @@ -500,7 +500,7 @@ let portable_hardlink ~src ~dst = destination (we also do this in the symlink case above). Perhaps, the list of dependencies may have duplicates? If yes, it may be better to filter out the duplicates first. *) - Path.unlink dst; + Path.unlink_exn dst; Path.link src dst | Unix.Unix_error (Unix.EMLINK, _, _) -> (* If we can't make a new hard link because we reached the limit on the diff --git a/otherlibs/stdune/src/path.ml b/otherlibs/stdune/src/path.ml index 7d5fe6c5a86..024ff8e4742 100644 --- a/otherlibs/stdune/src/path.ml +++ b/otherlibs/stdune/src/path.ml @@ -1135,7 +1135,7 @@ let is_directory t = ;; let rmdir t = Unix.rmdir (to_string t) -let unlink t = Fpath.unlink_exn (to_string t) +let unlink_exn t = Fpath.unlink_exn (to_string t) let link src dst = match Unix.link (to_string src) (to_string dst) with diff --git a/otherlibs/stdune/src/path.mli b/otherlibs/stdune/src/path.mli index f4dac2c09db..748cfd8c0c7 100644 --- a/otherlibs/stdune/src/path.mli +++ b/otherlibs/stdune/src/path.mli @@ -357,7 +357,7 @@ val is_dir_sep : char -> bool val is_directory : t -> bool val rmdir : t -> unit -val unlink : t -> unit +val unlink_exn : t -> unit val unlink_no_err : t -> unit val link : t -> t -> unit diff --git a/src/dune_engine/action_exec.ml b/src/dune_engine/action_exec.ml index 7a186b9885f..eee33a8bc0a 100644 --- a/src/dune_engine/action_exec.ml +++ b/src/dune_engine/action_exec.ml @@ -445,7 +445,7 @@ let rec exec t ~display ~ectx ~eenv : done_or_more_deps Produce.t = let remove_intermediate_file () = if optional then ( - try Path.unlink (Path.build file2) with + try Path.unlink_exn (Path.build file2) with | Unix.Unix_error (ENOENT, _, _) -> ()) in if diff_eq_files diff diff --git a/src/dune_engine/load_rules.ml b/src/dune_engine/load_rules.ml index bcadbded990..22791a82a94 100644 --- a/src/dune_engine/load_rules.ml +++ b/src/dune_engine/load_rules.ml @@ -205,7 +205,7 @@ let remove_old_artifacts match kind with | Unix.S_DIR -> if not (Subdir_set.mem subdirs_to_keep fn) then Path.rm_rf (Path.build path) - | _ -> Path.unlink (Path.build path))) + | _ -> Path.unlink_exn (Path.build path))) ;; (* We don't remove files in there as we don't know upfront if they are stale or diff --git a/src/dune_engine/process.ml b/src/dune_engine/process.ml index 258a0c465f4..cdff93c369f 100644 --- a/src/dune_engine/process.ml +++ b/src/dune_engine/process.ml @@ -853,7 +853,7 @@ let await { response_file; pid; _ } = let+ process_info, termination_reason = Scheduler.wait_for_build_process pid ~is_process_group_leader:true in - Option.iter response_file ~f:Path.unlink; + Option.iter response_file ~f:Path.unlink_exn; process_info, termination_reason ;; diff --git a/src/dune_rules/pkg_rules.ml b/src/dune_rules/pkg_rules.ml index 84ab03654cd..d0b78067106 100644 --- a/src/dune_rules/pkg_rules.ml +++ b/src/dune_rules/pkg_rules.ml @@ -1330,7 +1330,7 @@ module Install_action = struct section, file) |> Section.Map.of_list_multi in - let+ () = Async.async (fun () -> Path.unlink install_file) in + let+ () = Async.async (fun () -> Path.unlink_exn install_file) in map in (* TODO we should make sure that overwrites aren't allowed *) diff --git a/src/upgrader/dune_upgrader.ml b/src/upgrader/dune_upgrader.ml index 38f6dda1d9d..00011b93293 100644 --- a/src/upgrader/dune_upgrader.ml +++ b/src/upgrader/dune_upgrader.ml @@ -396,7 +396,7 @@ let upgrade () = (Path.Source.to_string_maybe_quoted new_file) ]; List.iter (original_file :: extra_files_to_delete) ~f:(fun p -> - Path.unlink (Path.source p)); + Path.unlink_exn (Path.source p)); Io.write_file (Path.source new_file) contents ~binary:true); if !v1_updates && not last then ( From abbf691a35b8c574e6b4e327452fefa0a61acf4e Mon Sep 17 00:00:00 2001 From: Etienne Millon Date: Fri, 22 Dec 2023 15:22:09 +0100 Subject: [PATCH 11/13] rename Fpath.unlink_status to Fpath.unlink Signed-off-by: Etienne Millon --- otherlibs/stdune/src/fpath.ml | 2 +- otherlibs/stdune/src/fpath.mli | 2 +- otherlibs/stdune/src/path.ml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/otherlibs/stdune/src/fpath.ml b/otherlibs/stdune/src/fpath.ml index ebcc9f480ab..f7cafa2221a 100644 --- a/otherlibs/stdune/src/fpath.ml +++ b/otherlibs/stdune/src/fpath.ml @@ -120,7 +120,7 @@ type unlink_status = | Is_a_directory | Error of exn -let unlink_status t = +let unlink t = match unlink_exn t with | () -> Success | exception exn -> diff --git a/otherlibs/stdune/src/fpath.mli b/otherlibs/stdune/src/fpath.mli index 47bad945637..990f59a3b3c 100644 --- a/otherlibs/stdune/src/fpath.mli +++ b/otherlibs/stdune/src/fpath.mli @@ -36,7 +36,7 @@ type unlink_status = | Error of exn (** Unlink and return error, if any. *) -val unlink_status : string -> unlink_status +val unlink : string -> unlink_status val initial_cwd : string diff --git a/otherlibs/stdune/src/path.ml b/otherlibs/stdune/src/path.ml index 024ff8e4742..3f9ace78077 100644 --- a/otherlibs/stdune/src/path.ml +++ b/otherlibs/stdune/src/path.ml @@ -734,7 +734,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_status t = Fpath.unlink_status (to_string t) + let unlink_status 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 From e9e05f171f5ade7e8a687a6fd947cdcda5955063 Mon Sep 17 00:00:00 2001 From: Etienne Millon Date: Fri, 22 Dec 2023 15:27:43 +0100 Subject: [PATCH 12/13] rename Path.unlink_status to Path.unlink Signed-off-by: Etienne Millon --- otherlibs/stdune/src/path.ml | 2 +- otherlibs/stdune/src/path.mli | 2 +- src/dune_engine/build_system.ml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/otherlibs/stdune/src/path.ml b/otherlibs/stdune/src/path.ml index 3f9ace78077..d181867d4aa 100644 --- a/otherlibs/stdune/src/path.ml +++ b/otherlibs/stdune/src/path.ml @@ -734,7 +734,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_status t = Fpath.unlink (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 diff --git a/otherlibs/stdune/src/path.mli b/otherlibs/stdune/src/path.mli index 748cfd8c0c7..e1647148b2b 100644 --- a/otherlibs/stdune/src/path.mli +++ b/otherlibs/stdune/src/path.mli @@ -195,7 +195,7 @@ module Build : sig val chmod : t -> mode:int -> unit val lstat : t -> Unix.stats - val unlink_status : t -> Fpath.unlink_status + val unlink : t -> Fpath.unlink_status val unlink_no_err : t -> unit module Table : Hashtbl.S with type key = t diff --git a/src/dune_engine/build_system.ml b/src/dune_engine/build_system.ml index 3a78eaf0426..7056ee10191 100644 --- a/src/dune_engine/build_system.ml +++ b/src/dune_engine/build_system.ml @@ -569,7 +569,7 @@ end = struct maybe_async_rule_file_op (fun () -> let remove_target_dir dir = Path.rm_rf (Path.build dir) in let remove_target_file path = - match Path.Build.unlink_status path with + match Path.Build.unlink path with | Success -> () | Is_a_directory -> (* If target changed from a directory to a file, delete From 1f41c742c2453700b0e1b4c34e04e5b41a42c83d Mon Sep 17 00:00:00 2001 From: Etienne Millon Date: Thu, 8 Feb 2024 15:52:36 +0100 Subject: [PATCH 13/13] do not log error if file does not exist Signed-off-by: Etienne Millon --- otherlibs/stdune/src/fpath.ml | 2 ++ otherlibs/stdune/src/fpath.mli | 1 + src/dune_engine/build_system.ml | 1 + 3 files changed, 4 insertions(+) diff --git a/otherlibs/stdune/src/fpath.ml b/otherlibs/stdune/src/fpath.ml index f7cafa2221a..23b57493ba6 100644 --- a/otherlibs/stdune/src/fpath.ml +++ b/otherlibs/stdune/src/fpath.ml @@ -117,6 +117,7 @@ let unlink_exn = if Stdlib.Sys.win32 then win32_unlink else Unix.unlink type unlink_status = | Success + | Does_not_exist | Is_a_directory | Error of exn @@ -125,6 +126,7 @@ let unlink t = | () -> Success | exception exn -> (match exn with + | Unix.Unix_error (ENOENT, _, _) -> Does_not_exist | Unix.Unix_error (error, _, _) -> (match error, Platform.OS.value with | EISDIR, _ | EPERM, Darwin -> Is_a_directory diff --git a/otherlibs/stdune/src/fpath.mli b/otherlibs/stdune/src/fpath.mli index 990f59a3b3c..3a9039e7780 100644 --- a/otherlibs/stdune/src/fpath.mli +++ b/otherlibs/stdune/src/fpath.mli @@ -32,6 +32,7 @@ val unlink_no_err : string -> unit type unlink_status = | Success + | Does_not_exist | Is_a_directory | Error of exn diff --git a/src/dune_engine/build_system.ml b/src/dune_engine/build_system.ml index 7056ee10191..98ce102ac73 100644 --- a/src/dune_engine/build_system.ml +++ b/src/dune_engine/build_system.ml @@ -571,6 +571,7 @@ end = struct let remove_target_file path = match Path.Build.unlink path with | Success -> () + | Does_not_exist -> () | Is_a_directory -> (* If target changed from a directory to a file, delete in anyway. *)