Skip to content

Commit

Permalink
fix(sendfile): unlink dst before fallback (#8234)
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
emillon authored Jul 24, 2023
1 parent 7489323 commit cb3489d
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 13 deletions.
3 changes: 3 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
------------------

Expand Down
14 changes: 7 additions & 7 deletions otherlibs/stdune/src/io.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
6 changes: 0 additions & 6 deletions test/blackbox-tests/test-cases/github8041.t
Original file line number Diff line number Diff line change
Expand Up @@ -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.

0 comments on commit cb3489d

Please sign in to comment.