ocaml / opam

opam is a source-based package manager. It supports multiple simultaneous compiler installations, flexible package constraints, and a Git-friendly development workflow.
https://opam.ocaml.org
Other
1.21k stars 348 forks source link

Rename and slightly repurpose `OpamClient.git_for_windows_check` #5997

Closed dra27 closed 1 month ago

dra27 commented 1 month ago

The diff for this PR is horrific in the GH interface, but with --ignore-space-change, it is just:

diff --git a/src/client/opamClient.ml b/src/client/opamClient.ml
index 879b2a770..e768ca879 100644
--- a/src/client/opamClient.ml
+++ b/src/client/opamClient.ml
@@ -654,9 +654,7 @@ let is_git_for_windows git =
     end
   | _ -> false

-let git_for_windows_check =
-  if not Sys.win32 then fun ?git_location:_ () -> None else
-  fun ?git_location () ->
+let git_for_windows ?git_location () =
   let header () = OpamConsole.header_msg "Git" in
   let contains_git p =
     OpamSystem.resolve_command ~env:[||] (Filename.concat p "git.exe")
@@ -693,7 +691,8 @@ let git_for_windows_check =
            warning suggesting that the machine be reconfigured to enable them
            in PATH, but also gives the opportunity to use the git-location
            mechanism to select it for opam's internal use. *)
-          let test_for_installation ((gits, gfw_message, abort_action) as acc) (hive, key) =
+        let test_for_installation ((gits, gfw_message, abort_action) as acc)
+                                  (hive, key) =
           let process root =
             let git_location = Filename.concat root "cmd" in
             let git = Filename.concat git_location "git.exe" in
@@ -764,7 +763,9 @@ let git_for_windows_check =
       let options =
         (`Default, "Use default Cygwin Git")
         :: (List.filter_map (fun (git, bash) ->
-              if bash then None else
+            if bash then
+              None
+            else
               let bin = Filename.dirname git in
               Some (`Location bin, "Use found git in "^bin))
             gits)
@@ -851,7 +852,12 @@ let windows_checks ?cygwin_setup ?git_location config =
         (OpamFilename.Dir.to_string gl_cli) ;
       Some (Left gl_cli)
   in
-  let git_location = git_for_windows_check ?git_location () in
+  let git_location =
+    if Sys.win32 then
+      git_for_windows ?git_location ()
+    else
+      None
+  in
   OpamCoreConfig.update ?git_location ();
   let config =
     match git_location with

The idea is to make the diff of the large opam init PR which is coming a bit easier to handle. The semantic change is instead of having a no-op function is to make it the responsibility of the caller not to call this function on "not Windows".

kit-ty-kate commented 1 month ago

Thanks!