Merge pull request #5352 from dra27/build-env

Expand variable interpolations in `build-env`

master_changes.md

@@ -167,6 +167,7 @@ users)
   * Add `with-tools` variable for recommended tools [#5016 @rjbou]
   * Add `x-locked` extension fields for overlay internal use, it stores if the files originate from a locked file, if so its extension [#5080 @rjbou]
   * Set `depext-bypass` parsing with depth 1, no needed brakcet if single package [#5154 @rjbou]
+  * [BUG] Variables are now expanded in build-env (as for setenv) [#5352 @dra27]
 
 ## External dependencies
   * Support MSYS2 on Windows for depexts [#5348 @jonahbeckford]
@@ -502,6 +503,7 @@ users)
   * `OpamArg`: externalise `post`, `dev`, `doc_flag`, `test`, and `devsetup` package selection flags, to avoid redefining them [#5299 @rjbou]
   * `OpamConfigCommand.global_allowed_fields`: add `archive-mirrors` (`dl_cache`) to allowed modifiable fields, extendable [#5321 @hannesm @rjbou]
   * `OpamClient.update_with_init_config`: Fix passing the `dl_cache` from `InitConfig` to `Config` [#5315 @hannesm]
+  * `OpamAction`: in `build_package`, `install_package`, and `remove_package` expand `build-env` variables content added to the environment [#5352 @dra27]
 
 ## opam-repository
   * `OpamRepositoryConfig`: add in config record `repo_tarring` field and as an argument to config functions, and a new constructor `REPOSITORYTARRING` in `E` environment module and its access function [#5015 @rjbou]
@@ -534,6 +536,7 @@ users)
   * Add `OpamPinned.version_opt` [#5325 @kit-ty-kate]
   * Add optional argument `?env:(variable_contents option Lazy.t * string) OpamVariable.Map.t` to `OpamSysPoll` and `OpamSysInteract` functions. It is used to get syspolling variables from the environment first. [#4892 @rjbou]
   * `OpamSwitchState`: move and reimplement `opam-solver` `dependencies` and `reverse_dependencies` [#5337 @rjbou]
+  * `OpamEnv`: add `env_expansion` [#5352 @dra27]
 
 ## opam-solver
   * `OpamCudf`: Change type of `conflict_case.Conflict_cycle` (`string list list` to `Cudf.package action list list`) and `cycle_conflict`, `string_of_explanations`, `conflict_explanations_raw` types accordingly [#4039 @gasche]

src/client/opamAction.ml

@@ -520,6 +520,9 @@ let prepare_package_source st nv dir =
 
 let compilation_env t opam =
   let open OpamParserTypes in
+  let build_env =
+    List.map (OpamEnv.env_expansion ~opam t) (OpamFile.OPAM.build_env opam)
+  in
   let scrub = OpamClientConfig.(!r.scrubbed_environment_variables) in
   OpamEnv.get_full ~scrub ~set_opamroot:true ~set_opamswitch:true
     ~force_path:true t ~updates:([
@@ -534,7 +537,7 @@ let compilation_env t opam =
       Some "build environment definition";
       "OPAMCLI", Eq, "2.0", Some "opam CLI version";
     ] @
-      OpamFile.OPAM.build_env opam)
+      build_env)
 
 let installed_opam_opt st nv =
   OpamStd.Option.Op.(

src/state/opamEnv.ml

@@ -185,18 +185,22 @@ let add (env: env) (updates: env_update list) =
   in
   env @ expand updates
 
+let env_expansion ?opam st (name, op, str, cmt) =
+  let fenv v =
+    try OpamPackageVar.resolve st ?opam v
+    with Not_found ->
+      log "Undefined variable: %s" (OpamVariable.Full.to_string v);
+      None
+  in
+  let s = OpamFilter.expand_string ~default:(fun _ -> "") fenv str in
+  name, op, s, cmt
+
 let compute_updates ?(force_path=false) st =
   (* Todo: put these back into their packages!
   let perl5 = OpamPackage.Name.of_string "perl5" in
   let add_to_perl5lib =  OpamPath.Switch.lib t.root t.switch t.switch_config perl5 in
   let new_perl5lib = "PERL5LIB", "+=", OpamFilename.Dir.to_string add_to_perl5lib in
 *)
-  let fenv ?opam v =
-    try OpamPackageVar.resolve st ?opam v
-    with Not_found ->
-      log "Undefined variable: %s" (OpamVariable.Full.to_string v);
-      None
-  in
   let bindir =
     OpamPath.Switch.bin st.switch_global.root st.switch st.switch_config
   in
@@ -218,21 +222,17 @@ let compute_updates ?(force_path=false) st =
             st.switch_global.root st.switch st.switch_config),
       Some "Current opam switch man dir"]
   in
-  let env_expansion ?opam (name,op,str,cmt) =
-    let s = OpamFilter.expand_string ~default:(fun _ -> "") (fenv ?opam) str in
-    name, op, s, cmt
-  in
   let switch_env =
     ("OPAM_SWITCH_PREFIX", Eq,
      OpamFilename.Dir.to_string
        (OpamPath.Switch.root st.switch_global.root st.switch),
      Some "Prefix of the current opam switch") ::
-    List.map env_expansion st.switch_config.OpamFile.Switch_config.env
+    List.map (env_expansion st) st.switch_config.OpamFile.Switch_config.env
   in
   let pkg_env = (* XXX: Does this need a (costly) topological sort? *)
     OpamPackage.Set.fold (fun nv acc ->
         match OpamPackage.Map.find_opt nv st.opams with
-        | Some opam -> List.map (env_expansion ~opam) (OpamFile.OPAM.env opam) @ acc
+        | Some opam -> List.map (env_expansion ~opam st) (OpamFile.OPAM.env opam) @ acc
         | None -> acc)
       st.installed []
   in

src/state/opamEnv.mli

@@ -94,6 +94,9 @@ val path: force_path:bool -> dirname -> switch -> string
 val full_with_path:
   force_path:bool -> ?updates:env_update list -> dirname -> switch -> env
 
+(** Performs variable expansion on the strings in an environment update *)
+val env_expansion: ?opam:OpamFile.OPAM.t -> 'a switch_state -> env_update -> env_update
+
 (** {2 Shell and initialisation support} *)
 
 (** Sets the opam configuration in the user shell, after detailing the process

tests/reftests/env.test

@@ -53,6 +53,23 @@ OPAM_SWITCH_PREFIX=${BASEDIR}/OPAM/conffile
 ### opam exec --no-switch -- env | grep "^NV_VARS|^OPAM_SWITCH_PREFIX|${OPAM}"
 NV_VARS=/another/path
 OPAM=${OPAM}
+### : Buil environment variable expansion :
+### <pkg:bde.1.2.3>
+opam-version: "2.0"
+build: [ "sh" "-c" "echo V$BDE_VERSION > pkg.version" ]
+install: [ "cp" "pkg.version" "%{doc}%/pkg.version" ]
+build-env: [ BDE_VERSION = "%{version}%" ]
+### opam switch create build-env --empty
+### opam install bde
+The following actions will be performed:
+=== install 1 package
+  - install bde 1.2.3
+
+<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
+-> installed bde.1.2.3
+Done.
+### cat $OPAMROOT/build-env/doc/pkg.version
+V1.2.3
 ### : root and switch with spaces :
 ### RT="$BASEDIR/root 2"
 ### SW="switch w spaces"