From cb3489de05b31b9e9be90a415a52eb3550349bdb Mon Sep 17 00:00:00 2001
From: Etienne Millon <me@emillon.org>
Date: Mon, 24 Jul 2023 16:24:15 +0200
Subject: [PATCH] fix(sendfile): unlink dst before fallback (#8234)

Fixes #8210

If the `sendfile` branch failed, it leaves an empty target file, so
retrying will fail. Instead we reuse the fd directly.

Signed-off-by: Etienne Millon <me@emillon.org>
---
 CHANGES.md                                  |  3 +++
 otherlibs/stdune/src/io.ml                  | 14 +++++++-------
 test/blackbox-tests/test-cases/github8041.t |  6 ------
 3 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/CHANGES.md b/CHANGES.md
index 7eb3cc85891..94ee3264728 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -21,6 +21,9 @@ Unreleased
 - Improve `dune describe external-lib-deps` by adding the internal dependencies
   for more information. (#7478, @moyodiallo)
 
+- Fix permission errors when `sendfile` is not available (#8234, fixes #8120,
+  @emillon)
+
 3.9.1 (2023-07-06)
 ------------------
 
diff --git a/otherlibs/stdune/src/io.ml b/otherlibs/stdune/src/io.ml
index b0341b0cf0a..48cbc3e09db 100644
--- a/otherlibs/stdune/src/io.ml
+++ b/otherlibs/stdune/src/io.ml
@@ -101,7 +101,7 @@ module Copyfile = struct
     | Linux -> `Sendfile
     | Windows | Other -> `Nothing
 
-  let sendfile =
+  let sendfile_with_fallback =
     let setup_copy ?(chmod = Fun.id) ~src ~dst () =
       match Unix.openfile src [ O_RDONLY ] 0 with
       | exception Unix.Unix_error (Unix.ENOENT, _, _) -> Error `Src_missing
@@ -147,7 +147,12 @@ module Copyfile = struct
         raise (Sys_error message)
       | Ok (src, dst, src_size) ->
         Exn.protect
-          ~f:(fun () -> sendfile ~src ~dst src_size)
+          ~f:(fun () ->
+            try sendfile ~src ~dst src_size
+            with Unix.Unix_error (EINVAL, "sendfile", _) ->
+              let ic = Unix.in_channel_of_descr src in
+              let oc = Unix.out_channel_of_descr dst in
+              copy_channels ic oc)
           ~finally:(fun () ->
             Unix.close src;
             Unix.close dst)
@@ -176,11 +181,6 @@ module Copyfile = struct
     Exn.protectx (setup_copy ?chmod ~src ~dst ()) ~finally:close_both
       ~f:(fun (ic, oc) -> copy_channels ic oc)
 
-  let sendfile_with_fallback ?chmod ~src ~dst () =
-    try sendfile ?chmod ~src ~dst ()
-    with Unix.Unix_error (EINVAL, "sendfile", _) ->
-      copy_file_portable ?chmod ~src ~dst ()
-
   let copy_file_best =
     match available with
     | `Sendfile -> sendfile_with_fallback
diff --git a/test/blackbox-tests/test-cases/github8041.t b/test/blackbox-tests/test-cases/github8041.t
index 8c5a39835f4..e26f450dbab 100644
--- a/test/blackbox-tests/test-cases/github8041.t
+++ b/test/blackbox-tests/test-cases/github8041.t
@@ -17,12 +17,6 @@
 If sendfile fails, we should fallback to the portable implementation.
 
   $ strace -e inject=sendfile:error=EINVAL -o /dev/null dune build @install
-  Error: _build/default/data2.txt: Permission denied
-  -> required by _build/default/data2.txt
-  -> required by _build/install/default/share/p/data2.txt
-  -> required by _build/default/p.install
-  -> required by alias install
-  [1]
 
 #8210: data2.txt is copied from readonly-file data.txt (#3092), so it needs to
 be adequately unlinked before starting the fallback.