Merge pull request #5829 from dra27/eval-variables

Various `eval-variables` fixes and `sys-ocaml-system`

master_changes.md

@@ -27,6 +27,9 @@ users)
   * Test if file exists before sourcing in fish + powershell [#5864 @ElectreAAS]
   * Replace the dependency on GNU patch by a strict dependency on git [#5400 @kit-ty-kate - fix #3433 #3782 #3639]
   * Properly test if "we're in interactive mode" instead of "in a tty" in fish script [#5866 @ElectreAAS]
+  * Make the computation of the init default `sys-ocaml-*` eval variables on Windows faster, no more depending on Cygwin [#5829 @dra27 @rjbou]
+  * Simplify computation of OCaml init default `sys-ocaml-*` eval variables on Unix [#5829 @dra27]
+  * Add a init OCaml `sys-ocaml-system` eval variable [#5829 @dra27]
 
 ## Config report
 
@@ -76,6 +79,7 @@ users)
  * Add support for Wolfi OS, treat it like Apline family as it uses apk too [#5878 @xnox]
 
 ## Format upgrade
+  * Handle init OCaml `sys-ocaml-*` eval variables during format upgrade from 2.0 -> 2.1 -> 2.2 [#5829 @dra27]
 
 ## Sandbox
   * Mark the user temporary directory (as returned by `getconf DARWIN_USER_TEMP_DIR`) as writable when TMPDIR is not defined on macOS [#5780 @ElectreAAS]
@@ -130,6 +134,7 @@ users)
 ## Reftests
 ### Tests
   * Add init scripts tests [#5864 @rjbou]
+  * Add test for init OCaml predefined eval variables and their format upgrade [#5829 @rjbou]
 
 ### Engine
 
@@ -160,6 +165,7 @@ users)
   * `OpamSysInteract.Cygwin.check_install`: add `variant` argument to permit checking that it is an Cygwin-like install if it is set to true, keep checking that it is a strictly Cygwin install if false [#5843 @rjbou]
   * `OpamSysInteract.Cygwin.check_install`: look for `cygcheck.exe` in `usr/bin` also as MSYS2 doesn't have "bin" [#5843 @rjbou]
   * `OpamGlobalState.load_config`: load MSYS2 Cygwin binary path too at config file loading [#5843 @rjbou]
+  * `OpamEnv`: add `sys_ocaml_eval_variables` value, moved `OpamInitDefaults` as it is also needed in `OpamFormatUpgrade` too [#5829 @rjbou @kit-ty-kate]
 
 ## opam-solver
 
@@ -168,3 +174,4 @@ users)
 
 ## opam-core
   * `OpamStd.Sys`: add `is_cygwin_variant_cygcheck` that returns true if in path `cygcheck` is from a Cygwin or MSYS2 installation [#5843 @rjbou]
+  * `OpamStd.Env.cyg_env`: takes the environment to cygify, usually `OpamStd.Env.raw_env` [#5829 @dra27]

src/client/opamClient.ml

@@ -816,7 +816,7 @@ let windows_checks ?cygwin_setup ?git_location config =
       if is_msys2 cygcheck then
         let env =
           OpamStd.Env.cyg_env ~cygbin:(OpamFilename.Dir.to_string cygbin)
-            ~git_location:None
+            ~git_location:None ~env:(OpamStd.Env.raw_env ())
         in
         match OpamSystem.resolve_command ~env "pacman.exe" with
         | Some pacman ->

src/client/opamInitDefaults.ml

@@ -32,16 +32,12 @@ let default_invariant =
      OpamFormula.Atom
        (`Geq, OpamPackage.Version.of_string "4.05.0"))
 
-let eval_variables = [
+let sys_ocaml_version =
   OpamVariable.of_string "sys-ocaml-version", ["ocamlc"; "-vnum"],
-  "OCaml version present on your system independently of opam, if any";
-  OpamVariable.of_string "sys-ocaml-arch", ["sh"; "-c"; "ocamlc -config 2>/dev/null | tr -d '\\r' | grep '^architecture: ' | sed -e 's/.*: //' -e 's/i386/i686/' -e 's/amd64/x86_64/'"],
-  "Target architecture of the OCaml compiler present on your system";
-  OpamVariable.of_string "sys-ocaml-cc", ["sh"; "-c"; "ocamlc -config 2>/dev/null | tr -d '\\r' | grep '^ccomp_type: ' | sed -e 's/.*: //'"],
-  "Host C Compiler type of the OCaml compiler present on your system";
-  OpamVariable.of_string "sys-ocaml-libc", ["sh"; "-c"; "ocamlc -config 2>/dev/null | tr -d '\\r' | grep '^os_type: ' | sed -e 's/.*: //' -e 's/Win32/msvc/' -e '/^msvc$/!s/.*/libc/'"],
-  "Host C Runtime Library type of the OCaml compiler present on your system";
-]
+  "OCaml version present on your system independently of opam, if any"
+
+let eval_variables =
+  sys_ocaml_version :: OpamEnv.sys_ocaml_eval_variables
 
 let os_filter os =
   FOp (FIdent ([], OpamVariable.of_string "os", None), `Eq, FString os)

src/core/opamProcess.ml

@@ -14,9 +14,10 @@ let log ?level fmt =
 
 let default_env =
   let f () = lazy (
+    let env = OpamStd.Env.raw_env () in
     match OpamCoreConfig.(!r.cygbin) with
-    | Some cygbin -> OpamStd.Env.cyg_env ~cygbin ~git_location:OpamCoreConfig.(!r.git_location)
-    | None -> OpamStd.Env.raw_env ()
+    | Some cygbin -> OpamStd.Env.cyg_env ~env ~cygbin ~git_location:OpamCoreConfig.(!r.git_location)
+    | None -> env
   ) in
  fun () -> Lazy.force (f ())
 

src/core/opamStd.ml

@@ -837,8 +837,7 @@ module Env = struct
     let lazy_env = lazy (to_list (raw_env ())) in
     fun () -> Lazy.force lazy_env
 
-  let cyg_env ~cygbin ~git_location =
-    let env = raw_env () in
+  let cyg_env ~env ~cygbin ~git_location =
     let f v =
       match OpamString.cut_at v '=' with
       | Some (path, c) when Name.equal_string path "path" ->
@@ -1207,7 +1206,10 @@ module OpamSys = struct
     if Sys.win32 then
       let results = Hashtbl.create 17 in
       let requires_cygwin cygcheck name =
-        let env = Env.cyg_env ~cygbin:(Filename.dirname cygcheck) ~git_location:None in
+        let env =
+          Env.cyg_env ~env:(Env.raw_env ()) ~cygbin:(Filename.dirname cygcheck)
+            ~git_location:None
+        in
         let cmd = OpamCompat.Filename.quote_command cygcheck [name] in
         let ((c, _, _) as process) = Unix.open_process_full cmd env in
         let rec check_dll platform =

src/core/opamStd.mli

@@ -455,8 +455,15 @@ module Env : sig
   val getopt_full: Name.t -> Name.t * string option
 
   val list: unit -> (Name.t * string) list
-  val raw_env: unit -> string Array.t
-  val cyg_env: cygbin:string -> git_location:string option -> string Array.t
+  val raw_env: unit -> string array
+
+  (** [cyg_env ~env ~cygbin ~git_location] returns [env] environment with its
+      PATH variable updated with [git_location] and [cygbin] at the beginning,
+      ie PATH=<git_location>:<cygbin>:$PATH. Usual `env` argument is
+      `OpamStd.Env.raw_env`. *)
+  val cyg_env:
+    env:string array -> cygbin:string -> git_location:string option
+    -> string array
 end
 
 (** {2 System query and exit handling} *)

src/state/opamEnv.ml

@@ -1272,3 +1272,38 @@ let hook_env root =
   let hook_vnam = OpamVariable.of_string "hooks" in
   let hook_vval = Some (OpamVariable.dirname (OpamPath.hooks_dir root)) in
   OpamVariable.Map.singleton hook_vnam hook_vval
+
+
+(* -- Configuration eval variables -- *)
+(* They are used in [OpamFormatUpgrade] and [OpamInitDefaults] *)
+
+let sys_ocaml_eval_variables =
+  (* For Windows, these only return results for OCaml 4.08+ *)
+  List.map (fun (var, comment, unix, win32) ->
+      let var = OpamVariable.of_string var in
+      if Sys.win32 then var, win32, comment
+      else var, unix, comment)
+    [
+      "sys-ocaml-system",
+      "Target system of the OCaml compiler present on your system",
+      ["sh"; "-c";
+       "ocamlc -config 2>/dev/null | tr -d '\\r' | sed -n -e 's/system: //p'"],
+      ["cmd"; "/d"; "/c"; "ocamlc -config-var system 2>nul"];
+      "sys-ocaml-arch",
+      "Target architecture of the OCaml compiler present on your system",
+      ["sh"; "-c";
+       "ocamlc -config 2>/dev/null | tr -d '\\r' | sed -n -e 's/i386/i686/;s/amd64/x86_64/;s/^architecture: //p'"],
+      ["cmd"; "/d"; "/c";
+       "for /f %f in ('ocamlc -config-var architecture 2^>nul') do @if '%f' equ 'i386' (echo i686) else if '%f' equ 'amd64' (echo x86_64) else (echo %f)"];
+      "sys-ocaml-cc",
+      "Host C Compiler type of the OCaml compiler present on your system",
+      ["sh"; "-c";
+       "ocamlc -config 2>/dev/null | tr -d '\\r' | sed -n -e 's/^ccomp_type: //p'"],
+      ["cmd"; "/d"; "/c"; "ocamlc -config-var ccomp_type 2>nul"];
+      "sys-ocaml-libc",
+      "Host C Runtime Library type of the OCaml compiler present on your system",
+      ["sh"; "-c";
+       "ocamlc -config 2>/dev/null | tr -d '\\r' | sed -n -e 's/^os_type: Win32/msvc/p;s/^os_type: .*/libc/p'"],
+      ["cmd"; "/d"; "/c";
+       "for /f %f in ('ocamlc -config-var os_type 2^>nul') do @if '%f' equ 'Win32' (echo msvc) else (echo libc)"];
+    ]

src/state/opamEnv.mli

@@ -157,3 +157,8 @@ val check_and_print_env_warning: 'a switch_state -> unit
 (** Hook directory environment *)
 val hook_env:
   OpamPath.t -> OpamVariable.variable_contents option OpamVariable.Map.t
+
+(** {2 Misc} *)
+
+(** System OCaml eval variables for configuration. *)
+val sys_ocaml_eval_variables : (OpamVariable.t * string list * string) list

src/state/opamFormatUpgrade.ml

@@ -1080,6 +1080,36 @@ let from_2_1_alpha2_to_2_1_rc ~on_the_fly:_ root conf =
 
 let from_2_1_rc_to_2_1 ~on_the_fly:_ _ conf = conf, gtc_none
 
+let apply_eval_variables conf old_vars new_vars =
+  let current_eval_variables = OpamFile.Config.eval_variables conf in
+  let add map (name, cmd, docstring) = OpamVariable.Map.add name (cmd, docstring) map in
+  let old_vars = List.fold_left add OpamVariable.Map.empty old_vars in
+  let new_vars = List.fold_left add OpamVariable.Map.empty new_vars in
+  let update new_vars ((name, cmd, _) as var) =
+    match OpamVariable.Map.find_opt name old_vars with
+    | Some (cmd', _) when cmd = cmd' ->
+        let cmd, docstring = OpamVariable.Map.find name new_vars in
+        (OpamVariable.Map.remove name new_vars, (name, cmd, docstring))
+    | _ -> (new_vars, var) in
+  let (missing, eval_variables) = OpamStd.List.fold_left_map update new_vars current_eval_variables in
+  let eval_variables =
+    let add name (cmd, docstring) eval_variables =
+      (name, cmd, docstring)::eval_variables in
+    OpamVariable.Map.fold add missing eval_variables in
+  OpamFile.Config.with_eval_variables eval_variables conf
+
+let sys_config_variables_unix_2_1 = [
+  OpamVariable.of_string "sys-ocaml-arch",
+  ["sh"; "-c"; "ocamlc -config 2>/dev/null | tr -d '\\r' | grep '^architecture: ' | sed -e 's/.*: //' -e 's/i386/i686/' -e 's/amd64/x86_64/'"],
+  "Target architecture of the OCaml compiler present on your system";
+  OpamVariable.of_string "sys-ocaml-cc",
+  ["sh"; "-c"; "ocamlc -config 2>/dev/null | tr -d '\\r' | grep '^ccomp_type: ' | sed -e 's/.*: //'"],
+  "Host C Compiler type of the OCaml compiler present on your system";
+  OpamVariable.of_string "sys-ocaml-libc",
+  ["sh"; "-c"; "ocamlc -config 2>/dev/null | tr -d '\\r' | grep '^os_type: ' | sed -e 's/.*: //' -e 's/Win32/msvc/' -e '/^msvc$/!s/.*/libc/'"],
+  "Host C Runtime Library type of the OCaml compiler present on your system";
+]
+
 let from_2_0_to_2_1 ~on_the_fly _ conf =
   (* In opam < 2.1 "jobs" was set during initialisation
      This creates problems when upgrading from opam 2.0 as it
@@ -1100,11 +1130,14 @@ let from_2_0_to_2_1 ~on_the_fly _ conf =
    | Some prev_jobs when prev_jobs = max 1 (OpamSysPoll.cores () - 1) -> ()
    | Some prev_jobs -> info_jobs_changed ~prev_jobs
    | None -> info_jobs_changed ~prev_jobs:1);
+  let conf = apply_eval_variables conf [] sys_config_variables_unix_2_1 in
   OpamFile.Config.with_jobs_opt None conf, gtc_none
 
 let v2_2_alpha = OpamVersion.of_string "2.2~alpha"
 
-let from_2_1_to_2_2_alpha ~on_the_fly:_ _ conf = conf, gtc_none
+let from_2_1_to_2_2_alpha ~on_the_fly:_ _ conf =
+  apply_eval_variables conf sys_config_variables_unix_2_1
+    OpamEnv.sys_ocaml_eval_variables, gtc_none
 
 (* To add an upgrade layer
    * If it is a light upgrade, returns as second element if the repo or switch

src/state/opamGlobalState.ml

@@ -113,7 +113,13 @@ let load lock_kind =
   in
   let eval_variables = OpamFile.Config.eval_variables config in
   let global_variables =
-    let env = lazy (OpamEnv.get_pure () |> OpamTypesBase.env_array) in
+    let env = lazy (
+      let env = OpamEnv.get_pure () |> OpamTypesBase.env_array in
+      match OpamCoreConfig.(!r.cygbin) with
+      | Some cygbin ->
+        OpamStd.Env.cyg_env ~env ~cygbin
+          ~git_location:OpamCoreConfig.(!r.git_location)
+      | None -> env) in
     List.fold_left (fun acc (v, cmd, doc) ->
         OpamVariable.Map.update v
           (fun previous_value ->

tests/reftests/dune.inc

@@ -620,6 +620,48 @@
    %{targets}
    (run ./run.exe %{exe:../../src/client/opamMain.exe.exe} %{dep:extrasource.test} %{read-lines:testing-env}))))
 
+(rule
+ (alias reftest-init-ocaml-eval-variables.unix)
+ (enabled_if (= %{os_type} "Unix"))
+ (action
+  (diff init-ocaml-eval-variables.unix.test init-ocaml-eval-variables.unix.out)))
+
+(alias
+ (name reftest)
+ (enabled_if (= %{os_type} "Unix"))
+ (deps (alias reftest-init-ocaml-eval-variables.unix)))
+
+(rule
+ (targets init-ocaml-eval-variables.unix.out)
+ (deps root-N0REP0)
+ (enabled_if (= %{os_type} "Unix"))
+ (package opam)
+ (action
+  (with-stdout-to
+   %{targets}
+   (run ./run.exe %{exe:../../src/client/opamMain.exe.exe} %{dep:init-ocaml-eval-variables.unix.test} %{read-lines:testing-env}))))
+
+(rule
+ (alias reftest-init-ocaml-eval-variables.win32)
+ (enabled_if (= %{os_type} "Win32"))
+ (action
+  (diff init-ocaml-eval-variables.win32.test init-ocaml-eval-variables.win32.out)))
+
+(alias
+ (name reftest)
+ (enabled_if (= %{os_type} "Win32"))
+ (deps (alias reftest-init-ocaml-eval-variables.win32)))
+
+(rule
+ (targets init-ocaml-eval-variables.win32.out)
+ (deps root-N0REP0)
+ (enabled_if (= %{os_type} "Win32"))
+ (package opam)
+ (action
+  (with-stdout-to
+   %{targets}
+   (run ./run.exe %{exe:../../src/client/opamMain.exe.exe} %{dep:init-ocaml-eval-variables.win32.test} %{read-lines:testing-env}))))
+
 (rule
  (alias reftest-init-scripts.unix)
  (enabled_if (= %{os_type} "Unix"))