From 9265e2c584b8c9e9e3333da83a5c37a162dd8d28 Mon Sep 17 00:00:00 2001
From: Stephen Sherratt <stephen@sherra.tt>
Date: Thu, 28 Mar 2024 11:20:50 +1100
Subject: [PATCH] pkg: Set OCAMLFIND_DESTDIR for actions (#10267)

If a package uses ocamlfind to install its files then ocamlfind will
consult its config file to determine where in the filesystem the
installed files ought to go. In dune, each package is built in an
isolated sandbox, and so by default ocamlfind won't install files to the
correct location. The OCAMLFIND_DESTDIR environment variable can be used
to override ocamlfind's default behaviour for determining where to
install files.

This change sets the OCAMLFIND_DESTDIR variable to the "target/lib"
directory within a package's build sandbox when executing a package's
build and install commands.

Signed-off-by: Stephen Sherratt <stephen@sherra.tt>
---
 src/dune_rules/pkg_rules.ml                   | 41 +++++++++++++++----
 .../test-cases/pkg/set-ocamlfind-destdir.t    | 14 +++++++
 2 files changed, 46 insertions(+), 9 deletions(-)
 create mode 100644 test/blackbox-tests/test-cases/pkg/set-ocamlfind-destdir.t

diff --git a/src/dune_rules/pkg_rules.ml b/src/dune_rules/pkg_rules.ml
index 066558d3c0c..8607b703f5a 100644
--- a/src/dune_rules/pkg_rules.ml
+++ b/src/dune_rules/pkg_rules.ml
@@ -456,6 +456,7 @@ module Run_with_path = struct
     type ('path, 'target) t =
       { prog : Action.Prog.t
       ; args : 'path arg Array.Immutable.t
+      ; ocamlfind_destdir : 'path
       }
 
     let name = "run-with-path"
@@ -467,10 +468,16 @@ module Run_with_path = struct
         | Path p -> Path (f p))
     ;;
 
-    let bimap t f _g = { t with args = Array.Immutable.map t.args ~f:(map_arg ~f) }
+    let bimap t f _g =
+      { t with
+        args = Array.Immutable.map t.args ~f:(map_arg ~f)
+      ; ocamlfind_destdir = f t.ocamlfind_destdir
+      }
+    ;;
+
     let is_useful_to ~memoize:_ = true
 
-    let encode { prog; args } path _ : Dune_lang.t =
+    let encode { prog; args; ocamlfind_destdir } path _ : Dune_lang.t =
       let prog =
         Dune_lang.atom_or_quoted_string
         @@
@@ -485,10 +492,17 @@ module Run_with_path = struct
               | String s -> Dune_lang.atom_or_quoted_string s
               | Path p -> path p)))
       in
-      List ([ Dune_lang.atom_or_quoted_string name; prog ] @ args)
+      List
+        [ List ([ Dune_lang.atom_or_quoted_string name; prog ] @ args)
+        ; path ocamlfind_destdir
+        ]
     ;;
 
-    let action { prog; args } ~(ectx : Action.Ext.context) ~(eenv : Action.Ext.env) =
+    let action
+      { prog; args; ocamlfind_destdir }
+      ~(ectx : Action.Ext.context)
+      ~(eenv : Action.Ext.env)
+      =
       let open Fiber.O in
       match prog with
       | Error e -> Action.Prog.Not_found.raise e
@@ -501,6 +515,12 @@ module Run_with_path = struct
             |> String.concat ~sep:"")
         in
         let metadata = Process.create_metadata ~purpose:ectx.purpose () in
+        let env =
+          Env.add
+            eenv.env
+            ~var:"OCAMLFIND_DESTDIR"
+            ~value:(Path.to_absolute_filename ocamlfind_destdir)
+        in
         Process.run
           (Accept eenv.exit_codes)
           prog
@@ -511,21 +531,21 @@ module Run_with_path = struct
           ~stderr_to:eenv.stderr_to
           ~stdin_from:eenv.stdin_from
           ~dir:eenv.working_dir
-          ~env:eenv.env
+          ~env
         >>= (function
          | Error _ -> Fiber.return ()
          | Ok () -> Fiber.return ())
     ;;
   end
 
-  let action prog args =
+  let action prog args ~ocamlfind_destdir =
     let module M = struct
       type path = Path.t
       type target = Path.Build.t
 
       module Spec = Spec
 
-      let v = { Spec.prog; args }
+      let v = { Spec.prog; args; ocamlfind_destdir }
     end
     in
     Action.Extension (module M)
@@ -786,7 +806,10 @@ module Action_expander = struct
                | String s -> Run_with_path.Spec.String s
                | Path p | Dir p -> Path p))
          in
-         Run_with_path.action exe args)
+         let ocamlfind_destdir =
+           (Lazy.force expander.paths.install_roots).lib_root |> Path.build
+         in
+         Run_with_path.action exe args ~ocamlfind_destdir)
     | Progn t ->
       let+ args = Memo.parallel_map t ~f:(expand ~expander) in
       Action.Progn args
@@ -958,7 +981,7 @@ module Action_expander = struct
   ;;
 
   let install_command context (pkg : Pkg.t) =
-    Option.map pkg.install_command ~f:(expand context pkg)
+    Option.map pkg.install_command ~f:(fun action -> expand context pkg action)
   ;;
 
   let exported_env (expander : Expander.t) (env : _ Env_update.t) =
diff --git a/test/blackbox-tests/test-cases/pkg/set-ocamlfind-destdir.t b/test/blackbox-tests/test-cases/pkg/set-ocamlfind-destdir.t
new file mode 100644
index 00000000000..0bff0491672
--- /dev/null
+++ b/test/blackbox-tests/test-cases/pkg/set-ocamlfind-destdir.t
@@ -0,0 +1,14 @@
+Test that the OCAMLFIND_DESTDIR environment variable is set when running
+install and build commands.
+  $ . ./helpers.sh
+
+  $ make_lockdir
+  $ cat >dune.lock/test.pkg <<'EOF'
+  > (version 0.0.1)
+  > (build (run sh -c "echo [build] OCAMLFIND_DESTDIR=$OCAMLFIND_DESTDIR"))
+  > (install (run sh -c "echo [install] OCAMLFIND_DESTDIR=$OCAMLFIND_DESTDIR"))
+  > EOF
+
+  $ build_pkg test 2>&1 | sed "s#$(pwd)#PWD#" | sed 's#\.sandbox/.*/_private#\.sandbox/SANDBOX/_private#'
+  [build] OCAMLFIND_DESTDIR=PWD/_build/.sandbox/SANDBOX/_private/default/.pkg/test/target/lib
+  [install] OCAMLFIND_DESTDIR=PWD/_build/.sandbox/SANDBOX/_private/default/.pkg/test/target/lib