From a9df40927438d97802a6460e4953e4dfe3e88546 Mon Sep 17 00:00:00 2001 From: Roven Lamar Gabriel Date: Mon, 2 Mar 2026 12:32:46 +0000 Subject: [PATCH 1/6] fix: allow rename to overwrite read-only destination on Windows when deduplicating Dune cache In the Dune cache, when a temp artifact already exists in the cache, Dune attempts deduplication by deleting the temp artifact, linking it to the file already in the cache, and then moving the temp artifact to the build directory. The last step fails on Windows because Dune sets a read-only attribute on the destination file, causing Unix.rename to fail with EACCES in this case. Since the operation fails, Dune does not consider the file cached. Combined with the fact that this seems to happen quite often [1] on Windows, this effectively nullifies the cache speedup. This PR creates a helper function that removes the read-only attribute on the destination file before trying to overwrite it (similar to the existing `win32_unlink`) and use it in the deduplication logic of the cache, fixing the issue. Note: This issue does not exist on Unix. This discrepancy is also being addressed directly on OCaml's side: https://github.com/ocaml/ocaml/pull/14602. The test dedup.t already exhibits the issue on Windows. The `dune_cmd stat hardlinks _build/default/target` command returns 1 instead of the expected 3. This PR bumps the result to 2. To make the test completely pass, another issue around `Io.portable_hardlink` on Windows still needs to be addressed [1]. [1] It seems that the fact that on windows dune always copies files instead of trying to create a hardlink is linked to the fact that this deduplication happen more often on windows. Signed-off-by: Roven Lamar Gabriel --- otherlibs/stdune/src/fpath.ml | 15 +++++++++++++++ otherlibs/stdune/src/fpath.mli | 1 + src/dune_cache/local.ml | 2 +- 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/otherlibs/stdune/src/fpath.ml b/otherlibs/stdune/src/fpath.ml index 1ccb454d21d..9798f7a3cd5 100644 --- a/otherlibs/stdune/src/fpath.ml +++ b/otherlibs/stdune/src/fpath.ml @@ -204,6 +204,21 @@ let unlink_exn ~chmod dir fn = unlink_exn_ignore_missing fn ;; +let win32_rename_exn src dst = + match Unix.rename src dst with + | () -> () + | exception Unix.Unix_error (Unix.EACCES, _, _) -> + (* Try removing the read-only attribute. + Workaround a behavior discrepancy between Unix and Windows of the + [Unix.rename] function. It accepts readonly override on Unix but not + on Windows. This discrepancy will be fixed in OCaml 5.6 which will + include https://github.com/ocaml/ocaml/pull/14602 *) + Unix.chmod dst 0o666; + Unix.rename src dst +;; + +let rename_exn = if Stdlib.Sys.win32 then win32_rename_exn else Unix.rename + let rec clear_dir ?(chmod = false) dir = match match Readdir.read_directory_with_kinds dir with diff --git a/otherlibs/stdune/src/fpath.mli b/otherlibs/stdune/src/fpath.mli index a7b23251c49..47bd93e46b3 100644 --- a/otherlibs/stdune/src/fpath.mli +++ b/otherlibs/stdune/src/fpath.mli @@ -46,6 +46,7 @@ type unlink_status = (** Unlink and return error, if any. *) val unlink : string -> unlink_status +val rename_exn : string -> string -> unit val initial_cwd : string type clear_dir_result = diff --git a/src/dune_cache/local.ml b/src/dune_cache/local.ml index d7ff859bcfa..77d9a437411 100644 --- a/src/dune_cache/local.ml +++ b/src/dune_cache/local.ml @@ -191,7 +191,7 @@ module Artifacts = struct [rename] operation has a quirk where [path_in_temp_dir] can remain on disk. This is not a problem because we clean the temporary directory later. *) - Unix.rename + Fpath.rename_exn (Path.to_string path_in_temp_dir) (Path.to_string path_in_build_dir) with From f160e6357355bebe6c58ad7ba6de475106f1d8c7 Mon Sep 17 00:00:00 2001 From: Roven Lamar Gabriel Date: Mon, 2 Mar 2026 14:25:20 +0000 Subject: [PATCH 2/6] Add change file. Signed-off-by: Roven Lamar Gabriel --- doc/changes/fixed/13713.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 doc/changes/fixed/13713.md diff --git a/doc/changes/fixed/13713.md b/doc/changes/fixed/13713.md new file mode 100644 index 00000000000..d2d6d6510ca --- /dev/null +++ b/doc/changes/fixed/13713.md @@ -0,0 +1,2 @@ +- Fix dune cache deduplication logic on windows by allowing the rename used in + the logic to overwrite the readonly destination file. (#13713, @Nevor) From 6361b993f38d8189cba9596f5e04c049ecdd7f26 Mon Sep 17 00:00:00 2001 From: Roven Lamar Gabriel Date: Mon, 2 Mar 2026 14:46:31 +0000 Subject: [PATCH 3/6] Rephrase changes entry as suggested by @nojb. Signed-off-by: Roven Lamar Gabriel --- doc/changes/fixed/13713.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/doc/changes/fixed/13713.md b/doc/changes/fixed/13713.md index d2d6d6510ca..23812ed582e 100644 --- a/doc/changes/fixed/13713.md +++ b/doc/changes/fixed/13713.md @@ -1,2 +1,3 @@ -- Fix dune cache deduplication logic on windows by allowing the rename used in - the logic to overwrite the readonly destination file. (#13713, @Nevor) +- Fix the Dune cache on Windows by correctly handling renames onto read-only + files. Before this change, the Dune cache would be filled but the stored + artifacts would not generally be usable by Dune. (#13713, @Nevor) From 91ddbb41e0286a18c82600c8d97dfc04f4040ed5 Mon Sep 17 00:00:00 2001 From: Roven Lamar Gabriel Date: Mon, 2 Mar 2026 17:20:35 +0100 Subject: [PATCH 4/6] Update otherlibs/stdune/src/fpath.ml Co-authored-by: Ali Caglayan Signed-off-by: Roven Lamar Gabriel --- otherlibs/stdune/src/fpath.ml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/otherlibs/stdune/src/fpath.ml b/otherlibs/stdune/src/fpath.ml index 9798f7a3cd5..32c0af40592 100644 --- a/otherlibs/stdune/src/fpath.ml +++ b/otherlibs/stdune/src/fpath.ml @@ -217,7 +217,7 @@ let win32_rename_exn src dst = Unix.rename src dst ;; -let rename_exn = if Stdlib.Sys.win32 then win32_rename_exn else Unix.rename +let rename_exn = if Stdlib.Sys.win32 then fun x -> win32_rename_exn x else fun x -> Unix.rename x let rec clear_dir ?(chmod = false) dir = match From 12dcf29ff2842c6bae8c29e007540a92569bc835 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Ojeda=20B=C3=A4r?= Date: Mon, 2 Mar 2026 20:21:52 +0100 Subject: [PATCH 5/6] Apply suggestion from @nojb MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nicolás Ojeda Bär --- otherlibs/stdune/src/fpath.ml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/otherlibs/stdune/src/fpath.ml b/otherlibs/stdune/src/fpath.ml index 32c0af40592..8c6745fbe06 100644 --- a/otherlibs/stdune/src/fpath.ml +++ b/otherlibs/stdune/src/fpath.ml @@ -217,7 +217,8 @@ let win32_rename_exn src dst = Unix.rename src dst ;; -let rename_exn = if Stdlib.Sys.win32 then fun x -> win32_rename_exn x else fun x -> Unix.rename x +let rename_exn = + if Stdlib.Sys.win32 then fun x -> win32_rename_exn x else fun x -> Unix.rename x let rec clear_dir ?(chmod = false) dir = match From b7aee6d4c5097ab05354887dd47109e6f5ace1d3 Mon Sep 17 00:00:00 2001 From: Roven Lamar Gabriel Date: Mon, 2 Mar 2026 20:05:31 +0000 Subject: [PATCH 6/6] Expand on both arguments. Signed-off-by: Roven Lamar Gabriel --- otherlibs/stdune/src/fpath.ml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/otherlibs/stdune/src/fpath.ml b/otherlibs/stdune/src/fpath.ml index 8c6745fbe06..9f25579a587 100644 --- a/otherlibs/stdune/src/fpath.ml +++ b/otherlibs/stdune/src/fpath.ml @@ -218,7 +218,8 @@ let win32_rename_exn src dst = ;; let rename_exn = - if Stdlib.Sys.win32 then fun x -> win32_rename_exn x else fun x -> Unix.rename x + if Stdlib.Sys.win32 then fun x y -> win32_rename_exn x y else fun x y -> Unix.rename x y +;; let rec clear_dir ?(chmod = false) dir = match