fix(codesign): run hook for promoted executables

bin/install_uninstall.ml

@@ -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

doc/changes/10263.md

@@ -0,0 +1,2 @@
+- regression fix: sign executables that are promoted into the source tree
+  (#10263, fixes #9272, @emillon)

src/dune_rules/artifact_substitution.ml

@@ -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
 
@@ -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
@@ -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;

src/dune_rules/artifact_substitution.mli

@@ -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

test/blackbox-tests/test-cases/dune

@@ -138,11 +138,3 @@
  (enabled_if
   (not %{arch_sixtyfour}))
  (deps %{bin:ocaml}))
-
-(cram
- (applies_to github9272)
- (enabled_if
-  (and
-   (= %{system} macosx)
-   (= %{architecture} arm64)))
- (deps %{bin:ocaml}))

test/blackbox-tests/test-cases/github9272.t

@@ -17,26 +17,4 @@ the source tree, the executable in the source tree segfaults.
 
   $ dune build
 
-Test peculiarity: we can not do ./hello.exe directly, because the shell itself
-(that runs the cram test) will display the pid of the crashing process.
-Instead, we start and wait for it in an OCaml program and only display the
-code.
-
-Once the bug is fixed, this can be replaced by just `./hello.exe` and the test
-can be enabled for all systems.
-
-  $ cat > exec.ml << EOF
-  > let () =
-  >   let pid =
-  >     Unix.create_process
-  >       "./hello.exe" [|"./hello.exe"|]
-  >       Unix.stdin Unix.stdout Unix.stderr
-  >   in
-  >   match Unix.waitpid [] pid with
-  >   | _, WEXITED n -> Printf.printf "WEXITED %d" n
-  >   | _, WSTOPPED n -> Printf.printf "WSTOPPED %d" n
-  >   | _, WSIGNALED n -> Printf.printf "WSIGNALED %d" n
-  > EOF
-
-  $ ocaml -I +unix unix.cma exec.ml
-  WSIGNALED -7
+  $ ./hello.exe