Merge pull request #5843 from rjbou/init-msys2

kit-ty-kate · web-flow · commit 314500ff597f · 2024-03-04T13:05:24.000Z
Opam 2.2 init with MSYS2
diff --git a/master_changes.md b/master_changes.md
@@ -23,6 +23,7 @@ users)
   * Disable ACL in Cygwin internal install to avoid permission mismatch errors [#5796 @kit-ty-kate - fix #5781]
   * Add `sys-pkg-manager-cmd` as an accepted field in opamrc files [#5847 @rjbou - fix #5844]
   * Fix `git-location` handling in init config file [#5848 @rjbou - fix #5845]
+  * Fix MSYS2 support [#5843 @rjbou - fix #5683]
 
 ## Config report
 
@@ -131,15 +132,22 @@ users)
 
 # API updates
 ## opam-client
+  * `OpamClient.windows_checks`: On existing cygwin install, permit to detect msys2 and store `os-distribution=msys2` in `global-variables` config file field [#5843 @rjbou]
+  * `OpamClient.windows_checks`: When updating config file for msys2, resolve `pacman` path and store it in `sys-pkg-manager-cmd` for msys2 [#5843 @rjbou]
+  * `OpamArg.apply_global_options`: load MSYS2 Cygwin binary path too [#5843 @rjbou]
 
 ## opam-repository
 
 ## opam-state
   * `OpamEnv.env_expansion`: Fix detection of out-of-date environment variables, a filter predicate was inverted [#5837 @dra27]
+  * `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]
 
 ## opam-solver
 
 ## opam-format
   * `OpamFile.InitConfig`: add `sys-pkg-manager-cmd` field [#5847 @rjbou]
 
 ## 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]
diff --git a/src/client/opamArg.ml b/src/client/opamArg.ml
@@ -600,6 +600,21 @@ let apply_global_options cli o =
             | _, element::elements -> aux (Some element) elements
           in
           aux None elements
+        | { pelem = Variable ({ pelem = "global-variables"; _},
+                              {pelem = List { pelem = elements; _}; _}); _} ->
+          let rec aux last elements =
+            match last, elements with
+            | _, [] -> ()
+            | Some { pelem = Ident "os-distribution"; _},
+              { pelem = String "msys2"; _}::_  ->
+              let cygbin =
+                OpamStd.Option.map Filename.dirname
+                  (OpamSystem.resolve_command "cygcheck")
+              in
+              OpamCoreConfig.update ?cygbin ()
+            | _, element::elements -> aux (Some element) elements
+          in
+          aux None elements
         | { pelem = Variable ({ pelem = "git-location"; _},
                               {pelem = String git_location; _}); _} ->
           OpamCoreConfig.update ~git_location ()
diff --git a/src/client/opamClient.ml b/src/client/opamClient.ml
@@ -775,18 +775,29 @@ let windows_checks ?cygwin_setup ?git_location config =
     | None -> config
   in
   (* Cygwin handling *)
+  let is_cygwin cygcheck =
+      OpamStd.Sys.is_cygwin_cygcheck
+        ~cygbin:(Some OpamFilename.(Dir.to_string (dirname cygcheck)))
+  in
+  let is_variant cygcheck =
+      OpamStd.Sys.is_cygwin_variant_cygcheck
+        ~cygbin:(Some OpamFilename.(Dir.to_string (dirname cygcheck)))
+  in
+  let is_msys2 cygcheck = is_variant cygcheck && not (is_cygwin cygcheck) in
   let success cygcheck =
+    let cygbin = OpamFilename.dirname cygcheck in
+    let distrib = if is_cygwin cygcheck then "cygwin" else "msys2" in
     let config =
       let os_distribution = OpamVariable.of_string "os-distribution" in
       let update vars =
         OpamFile.Config.with_global_variables
-          ((os_distribution, S "cygwin", "Set by opam init")::vars)
+          ((os_distribution, S distrib, "Set by opam init")::vars)
           config
       in
       match OpamStd.List.pick (fun (v,_,_) ->
           OpamVariable.equal v os_distribution)
           vars with
-      | Some (_, S "cygwin", _), _ -> config
+      | Some (_, S d, _), _ when String.equal d distrib -> config
       | None, vars ->  update vars
       | Some (_, vc, _), vars ->
         OpamConsole.warning
@@ -802,17 +813,41 @@ let windows_checks ?cygwin_setup ?git_location config =
           OpamStd.Sys.exit_because `Aborted
     in
     let config =
-      OpamFile.Config.with_sys_pkg_manager_cmd
-        (OpamStd.String.Map.add "cygwin" cygcheck
-           (OpamFile.Config.sys_pkg_manager_cmd config))
-        config
+      if is_msys2 cygcheck then
+        let env =
+          OpamStd.Env.cyg_env ~cygbin:(OpamFilename.Dir.to_string cygbin)
+            ~git_location:None
+        in
+        match OpamSystem.resolve_command ~env "pacman.exe" with
+        | Some pacman ->
+          if OpamConsole.confirm
+              "Found package manager pacman binary at %s.\n\
+               Do you want to use it for depexts?"
+              pacman then
+            OpamFile.Config.with_sys_pkg_manager_cmd
+              (OpamStd.String.Map.add distrib (OpamFilename.of_string pacman)
+                 (OpamFile.Config.sys_pkg_manager_cmd config))
+              config
+          else config
+        | None -> config
+      else
+        OpamFile.Config.with_sys_pkg_manager_cmd
+          (OpamStd.String.Map.add distrib cygcheck
+             (OpamFile.Config.sys_pkg_manager_cmd config))
+          config
     in
     OpamConsole.note "Configured with %s for depexts"
-      (if OpamSysInteract.Cygwin.is_internal config then
-         "internal Cygwin install"
+      (if is_cygwin cygcheck then
+         if OpamSysInteract.Cygwin.is_internal config then
+           "internal Cygwin install"
+         else
+           (* cygcheck is in CYGWINROOT/bin *)
+           Printf.sprintf "Cygwin at %s"
+             OpamFilename.(Dir.to_string (dirname_dir cygbin))
        else
-         Printf.sprintf "Cygwin at %s"
-           OpamFilename.(Dir.to_string (dirname_dir (dirname cygcheck))));
+         (* cygcheck is in MSYS2ROOT/usr/bin *)
+         Printf.sprintf "MSYS2 at %s"
+           OpamFilename.(Dir.to_string (dirname_dir (dirname_dir cygbin))));
     config
   in
   let install_cygwin_tools () =
@@ -826,12 +861,10 @@ let windows_checks ?cygwin_setup ?git_location config =
     OpamSysInteract.Cygwin.install ~packages
   in
   let header () = OpamConsole.header_msg "Unix support infrastructure" in
+
   let get_cygwin = function
-    | Some cygcheck
-      when OpamFilename.exists cygcheck
-        && OpamStd.Sys.is_cygwin_cygcheck
-             ~cygbin:(Some OpamFilename.(Dir.to_string (dirname cygcheck))) ->
-      success cygcheck
+    | Some cygcheck when OpamFilename.exists cygcheck && is_variant cygcheck ->
+        success cygcheck
     | Some _ | None ->
       let rec menu () =
         let enter_paths () =
@@ -862,7 +895,8 @@ let windows_checks ?cygwin_setup ?git_location config =
           in
           (* Check for default cygwin installation path *)
           let default =
-            match OpamSysInteract.Cygwin.(check_install default_cygroot) with
+            match OpamSysInteract.Cygwin.(check_install
+                                            ~variant:true default_cygroot) with
             | Ok cygcheck ->
               let prompt_cygroot () =
                 let options = [
@@ -894,7 +928,7 @@ let windows_checks ?cygwin_setup ?git_location config =
               | None -> None
               | Some entry ->
                 let cygcheck =
-                  OpamSysInteract.Cygwin.check_install entry
+                  OpamSysInteract.Cygwin.check_install ~variant:true entry
                 in
                 match cygcheck with
                 | Ok cygcheck -> Some cygcheck
@@ -903,7 +937,8 @@ let windows_checks ?cygwin_setup ?git_location config =
           (* And finally ask for setup.exe *)
           match cygcheck with
           | Some cygcheck ->
-            OpamSysInteract.Cygwin.check_setup (enter_setup ());
+            if is_cygwin cygcheck then
+              OpamSysInteract.Cygwin.check_setup (enter_setup ());
             Some (success cygcheck)
           | None -> None
         in
@@ -967,31 +1002,34 @@ let windows_checks ?cygwin_setup ?git_location config =
                    | `default_location -> OpamSysInteract.Cygwin.default_cygroot
                    | `location dir -> OpamFilename.Dir.to_string dir
                  in
-                 (match OpamSysInteract.Cygwin.check_install cygroot with
-                  | Ok cygcheck -> cygcheck
-                  | Error msg ->
-                    OpamConsole.error_and_exit `Not_found
-                      "Error while checking %sCygwin install (%s): %s"
-                      (match setup with
-                       | `default_location -> " default"
-                       | `location _ -> "")
-                      (OpamSysInteract.Cygwin.default_cygroot) msg)
+                 (match OpamSysInteract.Cygwin.check_install ~variant:true
+                          cygroot with
+                 | Ok cygcheck -> cygcheck
+                 | Error msg ->
+                   OpamConsole.error_and_exit `Not_found
+                     "Error while checking %sCygwin install (%s): %s"
+                     (match setup with
+                      | `default_location -> " default"
+                      | `location _ -> "")
+                     (OpamSysInteract.Cygwin.default_cygroot) msg)
              in
-             OpamSysInteract.Cygwin.check_setup None;
+             if is_cygwin cygcheck then
+               OpamSysInteract.Cygwin.check_setup None;
              success cygcheck)
-        | Some "cygwin" ->
+        | Some "cygwin" | Some "msys2" ->
           (* We check that current install is good *)
           (match OpamSysInteract.Cygwin.cygroot_opt config with
            | Some cygroot ->
-             (match OpamSysInteract.Cygwin.check_install
+             (match OpamSysInteract.Cygwin.check_install ~variant:true
                       (OpamFilename.Dir.to_string cygroot) with
              | Ok cygcheck ->
                OpamSysInteract.Cygwin.check_setup None;
                success cygcheck
              | Error err -> OpamConsole.error "%s" err; get_cygwin None)
            | None ->
-             (* Cygwin is detected from environment (path), we check the install
-                in that case and stores it in config *)
+             (* A Cygwin install (Cygwin or MSYS2) is detected from environment
+               (path), we check the install in that case and stores it in
+               config *)
              OpamSystem.resolve_command "cygcheck"
              |> OpamStd.Option.map OpamFilename.of_string
              |> get_cygwin
@@ -1000,9 +1038,18 @@ let windows_checks ?cygwin_setup ?git_location config =
       else
         config
   in
-  let cygbin = OpamStd.Option.Op.(
-      OpamSysInteract.Cygwin.cygbin_opt config
-      >>| OpamFilename.Dir.to_string)
+  let cygbin =
+    match OpamSysInteract.Cygwin.cygbin_opt config with
+    | Some cygbin -> Some (OpamFilename.Dir.to_string cygbin)
+    | None ->
+      if List.exists (function
+          | (v, S "msys2", _) ->
+            String.equal (OpamVariable.to_string v) "os-distribution"
+          | _ -> false) (OpamFile.Config.global_variables config)
+      then
+        OpamStd.Option.map Filename.dirname
+          (OpamSystem.resolve_command "cygcheck")
+      else None
   in
   OpamCoreConfig.update ?cygbin ();
   config
diff --git a/src/core/opamCoreConfig.ml b/src/core/opamCoreConfig.ml
@@ -63,6 +63,7 @@ type t = {
   errlog_length: int;
   merged_output: bool;
   precise_tracking: bool;
+  (* Updated in OpamGlobalState.load_config and OpamArg.opam_init *)
   cygbin: string option;
   git_location: string option;
   set: bool;
diff --git a/src/core/opamCoreConfig.mli b/src/core/opamCoreConfig.mli
@@ -72,7 +72,10 @@ type t = private {
   (** If set, will take full md5 of all files when checking diffs (to track
       installations), rather than rely on just file size and mtime *)
   cygbin: string option;
+  (** Windows specific: the path of binary directory (bin/) of currently used
+      Cygwin install: internal or external Cygwin, or MSYS2. *)
   git_location: string option;
+  (** Windows specific: the full path of the git binary to use on Windows. *)
   set : bool;
   (** Options have not yet been initialised (i.e. defaults are active) *)
 }
diff --git a/src/core/opamStd.ml b/src/core/opamStd.ml
@@ -1254,14 +1254,6 @@ module OpamSys = struct
     else
     fun ~cygbin:_ _ -> `Native
 
-  let is_cygwin_cygcheck ~cygbin =
-    match cygbin with
-    | Some cygbin ->
-      let cygpath = Filename.concat cygbin "cygpath.exe" in
-      Sys.file_exists cygpath
-      && (get_windows_executable_variant ~cygbin:(Some cygbin) cygpath = `Cygwin)
-    | None -> false
-
   let get_cygwin_variant ~cygbin cmd =
     (* Treat MSYS2's variant of `cygwin1.dll` called `msys-2.0.dll` equivalently.
        Confer https://www.msys2.org/wiki/How-does-MSYS2-differ-from-Cygwin/ *)
@@ -1274,6 +1266,20 @@ module OpamSys = struct
   let is_cygwin_variant ~cygbin cmd =
     get_cygwin_variant ~cygbin cmd = `Cygwin
 
+  let is_cygwin_cygcheck_t ~variant ~cygbin =
+    match cygbin with
+    | Some cygbin ->
+      let cygpath = Filename.concat cygbin "cygpath.exe" in
+      Sys.file_exists cygpath
+      && (variant ~cygbin:(Some cygbin) cygpath = `Cygwin)
+    | None -> false
+
+  let is_cygwin_variant_cygcheck ~cygbin =
+    is_cygwin_cygcheck_t ~variant:get_cygwin_variant ~cygbin
+
+  let is_cygwin_cygcheck ~cygbin =
+    is_cygwin_cygcheck_t ~variant:get_windows_executable_variant ~cygbin
+
   exception Exit of int
   exception Exec of string * string array * string array
 
diff --git a/src/core/opamStd.mli b/src/core/opamStd.mli
@@ -545,10 +545,14 @@ module Sys : sig
     string -> [ `Native | `Cygwin | `Tainted of [ `Msys2 | `Cygwin] | `Msys2 ]
 
   (** Determines if cygcheck in given cygwin binary directory comes from a
-      Cygwin or MSYS2 installation. Determined by analysing the cygpath command
+      Cygwin installation. Determined by analysing the cygpath command
       found with it. *)
   val is_cygwin_cygcheck : cygbin:string option -> bool
 
+  (** As [is_cygwin_cygcheck], but returns true if it is a Cygwin variant
+      (Cygwin, Msys2). *)
+  val is_cygwin_variant_cygcheck : cygbin:string option -> bool
+
   (** For native Windows builds, returns [`Cygwin] if the command is a Cygwin-
       or Msys2- compiled executable, and [`CygLinked] if the command links to a
       library which is itself Cygwin/Msys2-compiled, or [`Native] otherwise.
diff --git a/src/state/opamGlobalState.ml b/src/state/opamGlobalState.ml
@@ -38,10 +38,22 @@ let load_config lock_kind global_lock root =
   let config =
     OpamFormatUpgrade.as_necessary lock_kind global_lock root config
   in
-  OpamStd.Option.iter
-    (fun cygbin ->
-       OpamCoreConfig.update ~cygbin:(OpamFilename.Dir.to_string cygbin) ())
-    (OpamSysInteract.Cygwin.cygbin_opt (fst config));
+  (* Update Cygwin variants cygbin *)
+  let cygbin =
+    let config = fst config in
+    match OpamSysInteract.Cygwin.cygbin_opt config with
+    | Some cygbin -> Some (OpamFilename.Dir.to_string cygbin)
+    | None ->
+      if List.exists (function
+          | (v, S "msys2", _) ->
+            String.equal (OpamVariable.to_string v) "os-distribution"
+          | _ -> false) (OpamFile.Config.global_variables config)
+      then
+        OpamStd.Option.map Filename.dirname
+          (OpamSystem.resolve_command "cygcheck")
+      else None
+  in
+  OpamCoreConfig.update ?cygbin ();
   config
 
 let inferred_from_system = "Inferred from system"
diff --git a/src/state/opamSysInteract.ml b/src/state/opamSysInteract.ml
diff --git a/src/state/opamSysInteract.mli b/src/state/opamSysInteract.mli
