Skip to content

Commit

Permalink
fix(codesign): run hook for promoted executables
Browse files Browse the repository at this point in the history
Fixes ocaml#9272

The fix in ocaml#8361 relies on a `~executable` argument that was not passed
in the promotion case. Instead, we call `stat` in `Conf.run_sign_hook`.

Signed-off-by: Etienne Millon <me@emillon.org>
  • Loading branch information
emillon committed Mar 13, 2024
1 parent ffa6e1f commit d82161c
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 21 deletions.
2 changes: 1 addition & 1 deletion bin/install_uninstall.ml
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ module File_ops_real (W : sig
=
let chmod = if executable then fun _ -> 0o755 else fun _ -> 0o644 in
match (special_file : Special_file.t option) with
| None -> Artifact_substitution.copy_file ~conf ~executable ~src ~dst ~chmod ()
| None -> Artifact_substitution.copy_file ~conf ~src ~dst ~chmod ()
| Some sf ->
(* CR-rgrinberg: slow copying *)
let ic, oc = Io.setup_copy ~chmod ~src ~dst () in
Expand Down
32 changes: 14 additions & 18 deletions src/dune_rules/artifact_substitution.ml
Original file line number Diff line number Diff line change
Expand Up @@ -175,10 +175,18 @@ module Conf = struct
match has_subst with
| No_substitution -> Fiber.return ()
| Some_substitution ->
Memo.run t.sign_hook
|> Fiber.bind ~f:(function
| Some hook -> hook file
| None -> Fiber.return ())
let executable =
match Path.Untracked.stat file with
| Error _ -> false
| Ok { st_perm; _ } -> Path.Permissions.test Path.Permissions.execute st_perm
in
if executable
then
Memo.run t.sign_hook
|> Fiber.bind ~f:(function
| Some hook -> hook file
| None -> Fiber.return ())
else Fiber.return ()
;;
end

Expand Down Expand Up @@ -675,15 +683,7 @@ let replace_if_different ~delete_dst_if_it_is_a_directory ~src ~dst =
if not up_to_date then Path.rename src dst
;;

let copy_file
~conf
?(executable = false)
?chmod
?(delete_dst_if_it_is_a_directory = false)
~src
~dst
()
=
let copy_file ~conf ?chmod ?(delete_dst_if_it_is_a_directory = false) ~src ~dst () =
(* We create a temporary file in the same directory to ensure it's on the same
partition as [dst] (otherwise, [Path.rename temp_file dst] won't work). The
prefix ".#" is used because Dune ignores such files and so creating this
Expand All @@ -698,11 +698,7 @@ let copy_file
let open Fiber.O in
Path.parent dst |> Option.iter ~f:Path.mkdir_p;
let* has_subst = copy_file_non_atomic ~conf ?chmod ~src ~dst:temp_file () in
let+ () =
if executable
then Conf.run_sign_hook conf ~has_subst temp_file
else Fiber.return ()
in
let+ () = Conf.run_sign_hook conf ~has_subst temp_file in
replace_if_different ~delete_dst_if_it_is_a_directory ~src:temp_file ~dst)
~finally:(fun () ->
Path.unlink_no_err temp_file;
Expand Down
1 change: 0 additions & 1 deletion src/dune_rules/artifact_substitution.mli
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ val decode : string -> t option
and then atomically renamed to [dst]. *)
val copy_file
: conf:Conf.t
-> ?executable:bool
-> ?chmod:(int -> int)
-> ?delete_dst_if_it_is_a_directory:bool
-> src:Path.t
Expand Down
2 changes: 1 addition & 1 deletion test/blackbox-tests/test-cases/github9272.t
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,4 @@ can be enabled for all systems.
> EOF

$ ocaml -I +unix unix.cma exec.ml
WSIGNALED -7
WEXITED 0

0 comments on commit d82161c

Please sign in to comment.