Skip to content

Commit

Permalink
fix(install): do substitution for sites files (ocaml#10327)
Browse files Browse the repository at this point in the history
Fixes ocaml#10317

The mistake is that the logic is actually more complex that initially
thought: the "executable" status is used to determine is code signing
needs to happen; but artifact substitution should happen in more cases.

Signed-off-by: Etienne Millon <[email protected]>
  • Loading branch information
emillon authored and Philip White committed Apr 2, 2024
1 parent ec5fd93 commit 0e51624
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 7 deletions.
9 changes: 6 additions & 3 deletions bin/install_uninstall.ml
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,6 @@ module Special_file = struct
end

type copy_kind =
| Plain (** Just copy the file. Can use fast paths through [Io.copy_file] *)
| Substitute (** Use [Artifact_substitution.copy_file]. Will scan all bytes. *)
| Special of Special_file.t (** Hooks to add version numbers, replace sections, etc *)

Expand Down Expand Up @@ -348,7 +347,6 @@ module File_ops_real (W : sig
Fiber.return ()
in
match kind with
| Plain -> plain_copy ()
| Substitute -> Artifact_substitution.copy_file ~conf ~src ~dst ~chmod ()
| Special sf ->
let open Fiber.O in
Expand Down Expand Up @@ -515,7 +513,12 @@ let install_entry
let kind =
match special_file with
| Some special -> Special special
| None -> if executable then Substitute else Plain
| None ->
(* CR-emillon: for most cases we could use a fast copy here, but some
kinds of files do need artifact substitution(at least
executable files and artifacts built from generated sites
modules), but it's too late to know without reading the file. *)
Substitute
in
Ops.copy_file ~src:entry.src ~dst ~executable ~kind ~package ~conf
in
Expand Down
4 changes: 0 additions & 4 deletions otherlibs/dune-site/test/install-subst.t
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,3 @@ The generated module has placeholders.
We expect no placeholders to be present after installation.

$ find out -type f | sort | while read f ; do check_placeholder $f ; done
placeholder found in out/lib/a/S.ml
placeholder found in out/lib/a/a.a
placeholder found in out/lib/a/a.cma
placeholder found in out/lib/a/a__S.cmt

0 comments on commit 0e51624

Please sign in to comment.