Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3.15] Backport #10327 #10332

Merged
merged 3 commits into from
Mar 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
3.15.0~alpha2 (unreleased)
--------------------------

- Backport #10327: fix a regression in `dune install` not performing artifact
substitution.

3.15.0~alpha1 (2024-03-26)
--------------------------

Expand Down
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
40 changes: 40 additions & 0 deletions otherlibs/dune-site/test/install-subst.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
This checks that generated sites files correctly substituted with `dune install`.

See #10317.

$ cat > dune-project << EOF
> (lang dune 3.0)
> (using dune_site 0.1)
> (package
> (name a)
> (sites (share data)))
> EOF

$ cat > dune << EOF
> (library
> (public_name a)
> (libraries dune-site))
> (generate_sites_module
> (module s)
> (sites a))
> EOF

$ dune build

$ check_placeholder() {
> name=$1;
> if grep -q DUNE_PLACEHOLDER "$name"; then
> echo placeholder found in $name
> fi
> }

The generated module has placeholders.

$ check_placeholder _build/default/S.ml
placeholder found in _build/default/S.ml

$ dune install a --prefix out/ --datadir /nonexistent/data

We expect no placeholders to be present after installation.

$ find out -type f | sort | while read f ; do check_placeholder $f ; done
Loading