Merge pull request ocaml#5352 from dra27/build-env

Expand variable interpolations in `build-env`

master_changes.md

@@ -42,6 +42,11 @@ New option/command/subcommand are prefixed with ◈.
 
 ## Opamfile
   *
+  * Fix substring errors in preserved_format [#4941 @rjbou - fix #4936]
+  * 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
 
@@ -95,10 +100,57 @@ New option/command/subcommand are prefixed with ◈.
 
 # API updates
 ## opam-client
+  * `OpamStd.ABSTRACT`: add `compare` and `equal`, that added those functions to `OpamCLIVersion` [#4918 @rjbou]
+  * `OpamConfigCommand`: add a labelled argument `no_switch` to `exec` [#4957 @kit-ty-kate]
+  * `OpamClient`: fix `update_with_init_config`, when ``jobs` was set in `init_config`, it dropped rest of `config` update [#5056 @rjbou]
+  * Add an optional argument to `OpamArg.mk_subdoc` for extra default elements: `?extra_defaults:(validity * string * string) list` [#4910 @kit-ty-kate]
+  * Add `OpamSwitchCommand.previous_switch` [#4910 @kit-ty-kate]
+  * `OpamClient`: `requested` argument moved from `name_package_set` to `package_set`, to precise installed packages with `--best-effort` [#4796 @LasseBlaauwbroek]
+  * `OpamConfigCommand`: `set_opt_switch`, `set_var_switch`, `options_list_switch`, and `var_list_switch` now raise configuration error exception (50) if no switch is found [#5027 @rjbou]
+  * `OpamArgs`, `OpamArgTools`: add `experimental` optional argument to `cli_from` and replace `default` by `option:['experimental | 'ëefault]` for `cli_between`, to handle experimental features [#5099 @rjbou]
+  * OpamAction: `prepare_package_source` can now take any switch state (previously required `rw`) [#4850 @rjbou]
+  * `OpamClient`: handle formula on several functions, adding a `formula` labelled or optional argument  (`upgrade_t`, `compute_upgrade_t`, `upgrade`, `fixup`, `install_t`, `install`, `remove_t`, and `remove`) [#4975 @AltGr]
+  * `OpamSolution`: add `print_requested` to print actions reasons [#4975 @AltGr]
+  * `OpamSolution.apply`: take an optional argument `skip`, to avoid filtering solution beforehand [#4975 @AltGr]
+  * `OpamAction`: add `?tools` filtering argument in `build_package`, `install_package` [#5016 @rjbou]
+  * `OpamListCommand`: add `?tools` filtering argument in `dependency_toggles` [#5016 @rjbou]
+  * `OpamPinCommand`, `OpamClient`, `OpamAuxCommands`: use `OpamStateTypes` pin record types [#5080 @rjbou]
+  `OpamPinCommand.fetch_all_pins`: return the list of well fetched pins instead of fetched urls [#5080 @rjbou]
+  * `OpamAuxCommand`: add `?locked` (and handle lock file then) argument to `name_and_dir_of_opam_file`, `opams_of_dir`, `opams_of_dir_w_target`, `resolve_locals`, `autopin`, and `simulate_autopin` [#5080 @rjbou]
+  * `OpamClient.PIN`: change `?locked:bool` argument into `string`, to have lock extension name [#5080 @rjbou]
+  * `OpamClient.Pin.post_pin_action`: no more updates depexts information, moved to `OpamSwitchState.update_pin` [#5047 @rjbou]
+  * `OpamArgTools`: all flag definition takes now a section as a labelled argument [#5275 @rjbou]
+  * `OpamArg`: all flag definition takes now a section as an optional argument, default is set to `Manpage.s_options` [#5275 @rjbou]
+  * Add `OpamTreeCommand` [#5171 @cannorin]
+  * `OpamSolution`: add `dry_run` to simulate the new switch state after applying a solution [#5171 @cannorin]
+  * `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
 
 ## opam-state
+  * `OpamSwitchState.universe`: `requested` argument moved from `name_package_set` to `package_set`, to precise installed packages with `--best-effort` [#4796 @LasseBlaauwbroek]
+  * `OpamSwitchState.universe`: add a chrono for universe loading [#4975 @AltGr]
+  * `OpamSwitchState.universe`: set to false unresolved variables used in constraint, and warn [#5141 @rjbou - fix #5139]
+  * `OpamStateConfig`: add with-tools support ; i.e. add `E.withtools`, add `with_tools` in config record [#5016 @rjbou]
+  * `OpamPackageVar`: add `?tools` filtering argument in `filter_depends_formula`, `all_depends` [#5016 @rjbou]
+  * `OpamSwitchState`: add `?tools` filtering argument in `universe` [#5016 @rjbou]
+  * `OpamStateTypes`: Add record types for to pin and pinned packages informations (in order to avoid n-uplet with `n` growing) ; `name_and_file`, `name_and_file_w_url`, `nameopt_and_file`, `nameopt_and_file_w_url`, and `pinned_opam` [#5080 @rjbou]
+  * `OpamPinned`: use pin record types [#5080 @rjbou]
+  * `OpamPinned`: add `?locked:string` (and handle lock file then) argument to `files_in_source`, and `name_of_opam_filename` [#5080 @rjbou]
+  * `OpamPinned`: when looking at opam files, keep (and return) information about its locked origin [#5080 @rjbou]
+  * `OpamUpdate.pinned_package`: use locked information to automatically update from locked file if present, if `?autolock` is given to true [#5080 @rjbou]
+  * `OpamSwitchState.update_pin`: updates depexts state informations (`sys_packages`, `avalaible_packages`) [#5047 @rjbou]
+  * `OpamSysInteract`: add `package_manager_name` [#5268 @rjbou]
+  * `OpamSysInteract.install_packages_command`: change return type to `(['AsAdmin of string | 'AsUser of string] * string list) list
+` [#5268 @kit-ty-kate]
+  * `OpamUpdate`: change `repository` output to update function option, to not write cache and new repo config if nothing changed in `repositories` [#5146 @rjbou]
+  * 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
 

src/client/opamAction.ml

@@ -479,6 +479,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:([
@@ -493,7 +496,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

@@ -50,3 +50,41 @@ NV_VARS=''; export NV_VARS;
 NV_VARS=hej!!
 OPAM=${OPAM}
 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"
+### OPAMNOENVNOTICE=0
+### opam init -na --bare --bypass-check --disable-sandbox --root "$RT" defaut ./REPO
+No configuration file found, using built-in defaults.
+
+<><> Fetching repository information ><><><><><><><><><><><><><><><><><><><><><>
+[defaut] Initialised
+### opam switch create "./$SW" nv --root "$RT"
+
+<><> Installing new switch packages <><><><><><><><><><><><><><><><><><><><><><>
+Switch invariant: ["nv"]
+
+<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
+-> installed nv.1
+Done.
+# Run eval $(opam env '--root=${BASEDIR}/root 2' '--switch=${BASEDIR}/switch w spaces') to update the current shell environment