fix: allow rename to overwrite read-only destination on Windows when deduplicating Dune cache (#13713)

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:
ocaml/ocaml#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. See #13714.

---------

Signed-off-by: Roven Lamar Gabriel <nevor@nevor.net>

Author: Nevor

doc/changes/fixed/13713.md

@@ -0,0 +1,3 @@
+- 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)

otherlibs/stdune/src/fpath.ml

@@ -204,6 +204,23 @@ 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 fun x y -> win32_rename_exn x y else fun x y -> Unix.rename x y
+;;
+
 let rec clear_dir ?(chmod = false) dir =
   match
     match Readdir.read_directory_with_kinds dir with

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 =

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