From 08683f15179e32e67015a2863056b88409d636b6 Mon Sep 17 00:00:00 2001 From: David Allsopp Date: Sat, 4 May 2024 09:45:12 +0100 Subject: [PATCH 01/15] Harmonise the warning overriding git-location --- src/client/opamClient.ml | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/client/opamClient.ml b/src/client/opamClient.ml index e768ca879fc..55b4aefeb0b 100644 --- a/src/client/opamClient.ml +++ b/src/client/opamClient.ml @@ -840,16 +840,14 @@ let windows_checks ?cygwin_setup ?git_location config = | Some (Right ()), None -> Some (Right ()) | Some (Right ()), Some _ -> OpamConsole.note - "As '--no-git-location' is given in argument, ignoring field \ - 'git-location' in opamrc"; + "'--no-git-location' specified; field 'git-location' in opamrc \ + has been ignored"; Some (Right ()) | Some (Left gl), None | None, Some gl -> Some (Left gl) - | Some (Left gl_cli), Some gl_config -> + | Some (Left gl_cli), Some _ -> OpamConsole.note - "Git location defined in opamrc '%s' and via CLI \ - ('--git-location' option, %s). Keeping last one." - (OpamFilename.Dir.to_string gl_config) - (OpamFilename.Dir.to_string gl_cli) ; + "'--git-location' specified; field 'git-location' in opamrc \ + has been ignored"; Some (Left gl_cli) in let git_location = From a562b1aac842bf37727acc33a9ea673e981f6b20 Mon Sep 17 00:00:00 2001 From: David Allsopp Date: Sat, 4 May 2024 21:00:21 +0100 Subject: [PATCH 02/15] Expose OpamEnv.cygwin_non_shadowed_programs --- master_changes.md | 1 + src/state/opamEnv.ml | 6 ++++-- src/state/opamEnv.mli | 6 ++++++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/master_changes.md b/master_changes.md index 58aed6e4b49..83b36f2c7c3 100644 --- a/master_changes.md +++ b/master_changes.md @@ -177,6 +177,7 @@ users) * `OpamDownload.download_command`: separate output from stdout and stderr [#5984 @kit-ty-kate] ## opam-state + * `OpamEnv.cygwin_non_shadowed_programs`: exposes the list of executables (not including git) which should always come from Cygwin [#6000 @dra27] ## opam-solver diff --git a/src/state/opamEnv.ml b/src/state/opamEnv.ml index 8a407fc9fbf..9b6e0990387 100644 --- a/src/state/opamEnv.ml +++ b/src/state/opamEnv.ml @@ -306,6 +306,9 @@ let rezip ?insert (l1, l2) = let rezip_to_string ?insert z = join_var (rezip ?insert z) +let cygwin_non_shadowed_programs = + ["bash.exe"; "make.exe"; "sort.exe"; "tar.exe"] + let apply_op_zip ~sepfmt var op arg (rl1,l2 as zip) = let arg = transform_format ~sepfmt var arg in let empty_tr = { tr_entry = ""; tr_raw = ""; tr_sep = arg.tr_sep } in @@ -314,8 +317,7 @@ let apply_op_zip ~sepfmt var op arg (rl1,l2 as zip) = Sys.file_exists (Filename.concat dir item) in let shadow_list = - List.filter (contains_in arg) - ["bash.exe"; "make.exe"; "sort.exe"; "tar.exe"; "git.exe"] + List.filter (contains_in arg) ("git.exe" :: cygwin_non_shadowed_programs) in let rec loop acc = function | [] -> acc, [arg] diff --git a/src/state/opamEnv.mli b/src/state/opamEnv.mli index 6ac0bc82986..98e0bcd3ee5 100644 --- a/src/state/opamEnv.mli +++ b/src/state/opamEnv.mli @@ -61,6 +61,12 @@ val hash_env_updates: ('a, euok_writeable) env_update list -> string and optionally the given updates *) val get_pure: ?updates:(spf_resolved, euok_internal) env_update list -> unit -> env +(** The list of executables from Cygwin which must not be allowed to be shadowed + by other directories of PATH. This list must not contain git.exe - it is + added only if Cygwin's git is installed. The list is used to determine the + furthest point in PATH that Cygwin's bin directory can be placed. *) +val cygwin_non_shadowed_programs : string list + (** Update an environment, including reverting opam changes that could have been previously applied (therefore, don't apply to an already updated env as returned by e.g. [get_full]!) *) From d96fa34e9af259574bcf8a3f66fa0237f55b9083 Mon Sep 17 00:00:00 2001 From: David Allsopp Date: Sun, 19 May 2024 18:20:09 +0100 Subject: [PATCH 03/15] Abort if --git-location doesn't contain Git Previously, --git-location was treated as a default answer for the Git menu, so it would display an error if Git wasn't found, but then simply exit and fallback to adding Git to the Cygwin installation. Fix this and validate either --git-location argument or the git-location opamrc field as referring to a directory which contains Git. --- master_changes.md | 1 + src/client/opamClient.ml | 38 ++++++++++++++++++++++++++------------ 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/master_changes.md b/master_changes.md index 83b36f2c7c3..66a110d54bb 100644 --- a/master_changes.md +++ b/master_changes.md @@ -28,6 +28,7 @@ users) * Skip Git-for-Windows menu if the Git binary resolved in PATH is Git-for-Windows [#5963 @dra27 - fix #5835] * Enhance the Git menu by warning if the user appears to need to restart the shell to pick up PATH changes [#5963 @dra27] * Include Git for Windows installations in the list of possibilities where the user instructed Git-for-Windows setup not to update PATH [#5963 @dra27] + * [BUG] Fail if `--git-location` points to a directory not containing git [#6000 @dra27] ## Config report diff --git a/src/client/opamClient.ml b/src/client/opamClient.ml index 55b4aefeb0b..c23713d14a1 100644 --- a/src/client/opamClient.ml +++ b/src/client/opamClient.ml @@ -817,6 +817,15 @@ let git_for_windows ?git_location () = git_location; git_location +let check_git_location_or_exit git_location source = + let git = + Filename.concat (OpamFilename.Dir.to_string git_location) "git.exe" + in + if OpamSystem.resolve_command ~env:[||] git = None then + OpamConsole.error_and_exit `Not_found + "The location specified with %s does not appear to contain a Git \ + executable!" source + let windows_checks ?cygwin_setup ?git_location config = if (not (Unix.has_symlink ())) then begin OpamConsole.header_msg "Windows Developer Mode"; @@ -837,18 +846,23 @@ let windows_checks ?cygwin_setup ?git_location config = let git_location = match git_location, OpamFile.Config.git_location config with | None, None -> None - | Some (Right ()), None -> Some (Right ()) - | Some (Right ()), Some _ -> - OpamConsole.note - "'--no-git-location' specified; field 'git-location' in opamrc \ - has been ignored"; - Some (Right ()) - | Some (Left gl), None | None, Some gl -> Some (Left gl) - | Some (Left gl_cli), Some _ -> - OpamConsole.note - "'--git-location' specified; field 'git-location' in opamrc \ - has been ignored"; - Some (Left gl_cli) + | Some (Right ()), git_location_opamrc -> + if git_location_opamrc <> None then + OpamConsole.note + "'--no-git-location' specified; field 'git-location' in opamrc has \ + been ignored"; + git_location + | None, Some git_location -> + check_git_location_or_exit git_location + "the 'git-location' field in opamrc"; + Some (Left git_location) + | (Some (Left git_location)) as result, git_location_opamrc -> + if git_location_opamrc <> None then + OpamConsole.note + "'--git-location' specified; field 'git-location' in opamrc has been \ + ignored"; + check_git_location_or_exit git_location "--git-location"; + result in let git_location = if Sys.win32 then From 55ef6727e529b58afe325a14483311fe011a34c9 Mon Sep 17 00:00:00 2001 From: David Allsopp Date: Wed, 29 May 2024 22:18:14 +0100 Subject: [PATCH 04/15] Add a function to enumerate Registry values --- master_changes.md | 1 + src/core/opamStubs.dummy.ml | 1 + src/core/opamStubs.mli | 6 ++ src/stubs/win32/opamWin32Stubs.ml | 1 + src/stubs/win32/opamWindows.c | 105 ++++++++++++++++++++++++++++++ 5 files changed, 114 insertions(+) diff --git a/master_changes.md b/master_changes.md index 66a110d54bb..3d6a6880cd1 100644 --- a/master_changes.md +++ b/master_changes.md @@ -199,3 +199,4 @@ users) * `OpamStd.Sys.resolve_in_path`: split the logic of `OpamStd.Sys.resolve_command` to allow searching for an arbitrary file in the search path [#5991 @dra27] * `OpamProcess.run_background`: name the stderr output file have the .err extension when cmd_stdout is given [#5984 @kit-ty-kate] * [BUG]: fix incorrect recursion case in `OpamSystem.mk_temp_dir` [#6005 @dra27] + * `OpamStubs.enumRegistry`: on Windows, retrieves all the values of a given type from a registry key, with their names [#6000 @dra27] diff --git a/src/core/opamStubs.dummy.ml b/src/core/opamStubs.dummy.ml index 44474257d54..a4aff833433 100644 --- a/src/core/opamStubs.dummy.ml +++ b/src/core/opamStubs.dummy.ml @@ -24,6 +24,7 @@ let getWindowsVersion = that's_a_no_no let getArchitecture = that's_a_no_no let waitpids _ = that's_a_no_no let readRegistry _ _ _ = that's_a_no_no +let enumRegistry _ _ = that's_a_no_no let writeRegistry _ _ _ = that's_a_no_no let getConsoleOutputCP = that's_a_no_no let getCurrentConsoleFontEx _ = that's_a_no_no diff --git a/src/core/opamStubs.mli b/src/core/opamStubs.mli index 9f51a528f1c..726baf12226 100644 --- a/src/core/opamStubs.mli +++ b/src/core/opamStubs.mli @@ -70,6 +70,12 @@ val readRegistry : registry_root -> string -> string -> 'a registry_value -> 'a @raise Failure If the value in the registry does not have [value_type] *) +val enumRegistry : registry_root -> string -> 'a registry_value -> (string * 'a) list + (** Windows only. [enumRegistry root key value_type] reads all the values + from registry key [key] of [root] which have type [value_type]. + + Returns [[]] if the key is not found. *) + val writeRegistry : registry_root -> string -> string -> 'a registry_value -> 'a -> unit (** Windows only. [writeRegistry root key name value_type value] sets the diff --git a/src/stubs/win32/opamWin32Stubs.ml b/src/stubs/win32/opamWin32Stubs.ml index d556b1f1063..dc4c2f7ef34 100644 --- a/src/stubs/win32/opamWin32Stubs.ml +++ b/src/stubs/win32/opamWin32Stubs.ml @@ -23,6 +23,7 @@ external getWindowsVersion : unit -> int * int * int * int = "OPAMW_GetWindowsVe external getArchitecture : unit -> 'a = "OPAMW_GetArchitecture" external waitpids : int list -> int -> int * Unix.process_status = "OPAMW_waitpids" external readRegistry : 'a -> string -> string -> 'b -> 'c option = "OPAMW_ReadRegistry" +external enumRegistry : 'a -> string -> 'b -> (string * 'c) list = "OPAMW_RegEnumValue" external writeRegistry : 'a -> string -> string -> 'b -> 'c -> unit = "OPAMW_WriteRegistry" external getConsoleOutputCP : unit -> int = "OPAMW_GetConsoleOutputCP" external getCurrentConsoleFontEx : 'a -> bool -> 'b = "OPAMW_GetCurrentConsoleFontEx" diff --git a/src/stubs/win32/opamWindows.c b/src/stubs/win32/opamWindows.c index e30f2ed8413..2621a11dde5 100644 --- a/src/stubs/win32/opamWindows.c +++ b/src/stubs/win32/opamWindows.c @@ -423,6 +423,111 @@ CAMLprim value OPAMW_ReadRegistry(value hKey, value sub_key, CAMLreturn(result); } +CAMLprim value OPAMW_RegEnumValue(value hKey, value sub_key, value value_type) +{ + CAMLparam0(); + CAMLlocal5(result, tail, v, v_name, v_data); + value cell; + + LPWSTR lpEnvironment; + + result = caml_alloc_small(2, 0); + Field(result, 0) = Val_int(0); /* Unused */ + Field(result, 1) = Val_emptylist; /* The actual result */ + tail = result; + + HKEY key; + DWORD type; + LSTATUS ret; + DWORD index = 0; + LPWSTR lpValueName = NULL; + DWORD cbValueName; + LPBYTE lpData = NULL; + DWORD cbData; + LPWSTR lpSubKey; + + if (!caml_string_is_c_safe(sub_key)) + caml_invalid_argument("OPAMW_RegEnumValue"); + + if (!(lpSubKey = caml_stat_strdup_to_utf16(String_val(sub_key)))) { + caml_raise_out_of_memory(); + } + + ret = RegOpenKey(roots[Int_val(hKey)], lpSubKey, &key); + if (ret == ERROR_SUCCESS) + ret = RegQueryInfoKey(key, NULL, NULL, NULL, NULL, NULL, NULL, NULL, &cbValueName, &cbData, NULL, NULL); + + if (ret == ERROR_SUCCESS) { + caml_stat_free(lpSubKey); + /* Cases match OpamStubsTypes.registry_value */ + switch (Int_val(value_type)) { + case 0: + type = REG_SZ; + break; + default: + RegCloseKey(key); + caml_failwith("OPAMW_RegEnumValue: value not implemented"); + break; + } + + if (ret == ERROR_SUCCESS) { + cbValueName++; + if ((lpData = malloc(cbData)) == NULL) { + caml_raise_out_of_memory(); + } else if ((lpValueName = malloc(cbValueName * sizeof(WCHAR))) == NULL) { + free(lpData); + caml_raise_out_of_memory(); + } + + DWORD valuename_len = cbValueName; + DWORD value_len = cbData; + DWORD value_type; + + while (ret == ERROR_SUCCESS) { + valuename_len = cbValueName; + value_len = cbData; + ret = RegEnumValue(key, index++, lpValueName, &valuename_len, NULL, &value_type, lpData, &value_len); + if (ret == ERROR_SUCCESS) { + if (type == value_type) { + value_len /= 2; /* bytes -> characters */ + if (((wchar_t *)lpData)[value_len - 1] == 0) + value_len--; /* remove NULL terminator */ + int len = win_wide_char_to_multi_byte((wchar_t *)lpData, value_len, NULL, 0); + v_data = caml_alloc_string(len); + win_wide_char_to_multi_byte((wchar_t *)lpData, value_len, (char *)String_val(v_data), len); + len = win_wide_char_to_multi_byte(lpValueName, valuename_len, NULL, 0); + v_name = caml_alloc_string(len); + win_wide_char_to_multi_byte(lpValueName, valuename_len, (char *)String_val(v_name), len); + v = caml_alloc_small(2, 0); + Field(v, 0) = v_name; + Field(v, 1) = v_data; + cell = caml_alloc_small(2, 0); + Field(cell, 0) = v; + Field(cell, 1) = Val_emptylist; + Store_field(tail, 1, cell); + tail = Field(tail, 1); + } + } + } + if (ret == ERROR_NO_MORE_ITEMS) + ret = ERROR_SUCCESS; + + free(lpData); + free(lpValueName); + } + + RegCloseKey(key); + } else { + caml_stat_free(lpSubKey); + } + + if (ret != ERROR_SUCCESS && ret != ERROR_FILE_NOT_FOUND) { + caml_failwith("OPAMW_RegEnumValue"); + } + + CAMLreturn(Field(result, 1)); +} + CAMLprim value OPAMW_WriteRegistry(value hKey, value sub_key, value value_name, From 75282c7e3f9510c9fcb582aba8d0f8808f4aa4a6 Mon Sep 17 00:00:00 2001 From: David Allsopp Date: Tue, 4 Jun 2024 11:49:37 +0100 Subject: [PATCH 05/15] Simplify cygbin check in OpamAction --- src/client/opamAction.ml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/client/opamAction.ml b/src/client/opamAction.ml index 7b309d29cf4..da544b35502 100644 --- a/src/client/opamAction.ml +++ b/src/client/opamAction.ml @@ -528,9 +528,8 @@ let compilation_env t opam = (OpamFile.OPAM.build_env opam) in let cygwin_env = - match OpamSysInteract.Cygwin.cygbin_opt t.switch_global.config with + match OpamCoreConfig.(!r.cygbin) with | Some cygbin -> - let cygbin = OpamFilename.Dir.to_string cygbin in [ OpamTypesBase.env_update_resolved "PATH" Cygwin cygbin ~comment:"Cygwin path" ] @ (match OpamCoreConfig.(!r.git_location) with From 2a03664550723470fa26161f56a96028fe9916c9 Mon Sep 17 00:00:00 2001 From: David Allsopp Date: Tue, 4 Jun 2024 12:01:31 +0100 Subject: [PATCH 06/15] Change the calculation for installing git Rather than filtering git out of the package list, instead add it when required. --- master_changes.md | 1 + src/client/opamClient.ml | 10 +++++----- src/client/opamInitDefaults.ml | 1 - 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/master_changes.md b/master_changes.md index 3d6a6880cd1..dbee760ab44 100644 --- a/master_changes.md +++ b/master_changes.md @@ -173,6 +173,7 @@ users) * `OpamClient.init` and `OpamClient.reinit`: now can have additional cygwin packages to install [#5930 @moyodiallo] * Expose `OpamSolution.print_depext_msg` [#5994 @dra27] * Extracted `OpamSolution.install_sys_packages` from `OpamSolution.install_depexts` [#5994 @dra27] + * `OpamInitDefaults.required_packages_for_cygwin`: no longer includes git; as the need to add that is computed in `OpamClient` [#6000 @dra27] ## opam-repository * `OpamDownload.download_command`: separate output from stdout and stderr [#5984 @kit-ty-kate] diff --git a/src/client/opamClient.ml b/src/client/opamClient.ml index c23713d14a1..5ed8990d898 100644 --- a/src/client/opamClient.ml +++ b/src/client/opamClient.ml @@ -955,12 +955,12 @@ let windows_checks ?cygwin_setup ?git_location config = config in let install_cygwin_tools packages = + let default_packages = OpamInitDefaults.required_packages_for_cygwin in let default_packages = - match OpamSystem.resolve_command "git" with - | None -> OpamInitDefaults.required_packages_for_cygwin - | Some _ -> - List.filter (fun c -> not OpamSysPkg.(equal (of_string "git") c)) - OpamInitDefaults.required_packages_for_cygwin + if OpamSystem.resolve_command "git" = None then + OpamSysPkg.of_string "git" :: default_packages + else + default_packages in (* packages comes last so that the user can override any potential version constraints in default_packages (although, with the current version of diff --git a/src/client/opamInitDefaults.ml b/src/client/opamInitDefaults.ml index 608ac078fb7..6b5a7017fbf 100644 --- a/src/client/opamInitDefaults.ml +++ b/src/client/opamInitDefaults.ml @@ -149,7 +149,6 @@ let required_tools ~sandboxing () = let required_packages_for_cygwin = [ "diffutils"; - "git"; (* XXX hg & mercurial ? *) "make"; "patch"; "tar"; From d4083aebd3488836142e724f3304a2249cf8c514 Mon Sep 17 00:00:00 2001 From: David Allsopp Date: Tue, 4 Jun 2024 12:03:53 +0100 Subject: [PATCH 07/15] Add OpamCompat.Seq.find_map --- master_changes.md | 1 + src/core/opamCompat.ml | 18 ++++++++++++++++++ src/core/opamCompat.mli | 5 +++++ 3 files changed, 24 insertions(+) diff --git a/master_changes.md b/master_changes.md index dbee760ab44..06e8a1e8904 100644 --- a/master_changes.md +++ b/master_changes.md @@ -201,3 +201,4 @@ users) * `OpamProcess.run_background`: name the stderr output file have the .err extension when cmd_stdout is given [#5984 @kit-ty-kate] * [BUG]: fix incorrect recursion case in `OpamSystem.mk_temp_dir` [#6005 @dra27] * `OpamStubs.enumRegistry`: on Windows, retrieves all the values of a given type from a registry key, with their names [#6000 @dra27] + * `OpamCompat`: add `Seq.find_map` from OCaml 4.14 [#6000 @dra27] diff --git a/src/core/opamCompat.ml b/src/core/opamCompat.ml index 216376563aa..70a668691dc 100644 --- a/src/core/opamCompat.ml +++ b/src/core/opamCompat.ml @@ -23,6 +23,24 @@ module String = struct include Stdlib.String end +module Seq = struct + [@@@warning "-32"] + + (** NOTE: OCaml >= 4.14 *) + let rec find_map f xs = + match xs() with + | Seq.Nil -> + None + | Seq.Cons (x, xs) -> + match f x with + | None -> + find_map f xs + | Some _ as result -> + result + + include Seq +end + module Either = struct (** NOTE: OCaml >= 4.12 *) type ('a, 'b) t = diff --git a/src/core/opamCompat.mli b/src/core/opamCompat.mli index 9596aedbb82..e22c4cad784 100644 --- a/src/core/opamCompat.mli +++ b/src/core/opamCompat.mli @@ -13,6 +13,11 @@ module String : sig val exists: (char -> bool) -> string -> bool end +module Seq : sig + (* NOTE: OCaml >= 4.14 *) + val find_map: ('a -> 'b option) -> 'a Seq.t -> 'b option +end + module Either : sig (* NOTE: OCaml >= 4.12 *) type ('a, 'b) t = From 610b66adf5341ec7f1459b4460292231c47e6c0b Mon Sep 17 00:00:00 2001 From: David Allsopp Date: Tue, 4 Jun 2024 12:33:54 +0100 Subject: [PATCH 08/15] Overhaul OpamStd.Sys Cygwin functions Make the ~cygbin parameter to the various executable identification functions optional. The parameter is renamed to search_in_first with a slightly revised semantics - if cygcheck.exe is not found in the directory, then PATH is still searched. This has a key benefit early on in opam init, as it allows cygbin to be set for an internal Cygwin installation which has not yet happened, but still permits curl (et al) to be used from PATH (e.g. when running opam init from an MSYS2 shell). Where before ~cygbin implied that cygcheck _must_ be used from the directory, the new parameter effectively prepends an additional directory to PATH. --- master_changes.md | 1 + src/core/opamProcess.ml | 2 +- src/core/opamStd.ml | 52 +++++++++++++++++++++++++++-------------- src/core/opamStd.mli | 29 ++++++++++++----------- src/core/opamSystem.ml | 4 ++-- 5 files changed, 54 insertions(+), 34 deletions(-) diff --git a/master_changes.md b/master_changes.md index 06e8a1e8904..5d9b2c64c01 100644 --- a/master_changes.md +++ b/master_changes.md @@ -202,3 +202,4 @@ users) * [BUG]: fix incorrect recursion case in `OpamSystem.mk_temp_dir` [#6005 @dra27] * `OpamStubs.enumRegistry`: on Windows, retrieves all the values of a given type from a registry key, with their names [#6000 @dra27] * `OpamCompat`: add `Seq.find_map` from OCaml 4.14 [#6000 @dra27] + * `OpamStd.Sys.{get_windows_executable_variant,get_cygwin_variant,is_cygwin_variant}`: renamed `~cygbin` to `?search_in_path` with a change in semantics so that it acts as though the directory was simply the first entry in PATH [#6000 @dra27] diff --git a/src/core/opamProcess.ml b/src/core/opamProcess.ml index a13a641c2f0..2d810728ed2 100644 --- a/src/core/opamProcess.ml +++ b/src/core/opamProcess.ml @@ -313,7 +313,7 @@ let create_process_env = fun cmd -> if OpamStd.Option.map_default (OpamStd.Sys.is_cygwin_variant - ~cygbin:(OpamCoreConfig.(!r.cygbin))) + ?search_in_first:(OpamCoreConfig.(!r.cygbin))) false (resolve_command cmd) then cygwin_create_process_env cmd diff --git a/src/core/opamStd.ml b/src/core/opamStd.ml index 97d6e257f11..899f63f876a 100644 --- a/src/core/opamStd.ml +++ b/src/core/opamStd.ml @@ -1358,40 +1358,56 @@ module OpamSys = struct in check_dll `Native in - fun ~cygbin name -> - match cygbin with - | Some cygbin -> - (let cygcheck = Filename.concat cygbin "cygcheck.exe" in - if Filename.is_relative name then - requires_cygwin cygcheck name - else - try Hashtbl.find results (cygcheck, name) - with Not_found -> - let result = requires_cygwin cygcheck name in - Hashtbl.add results (cygcheck, name) result; - result) + fun ?search_in_first name -> + let cygcheck = + let open Option.Op in + let contains_cygcheck dir = + let cygcheck = Filename.concat dir "cygcheck.exe" in + if Sys.file_exists cygcheck then + Some cygcheck + else + None + in + search_in_first >>= contains_cygcheck + >>+ fun () -> + (* ~search_in_first not supplied, or cygcheck.exe not found in it; + now try general PATH-resolution. *) + match resolve_command "cygcheck.exe" with + | `Cmd cmd -> Some cmd + | `Not_found | `Denied -> None + in + match cygcheck with | None -> `Native + | Some cygcheck -> + if Filename.is_relative name then + requires_cygwin cygcheck name + else + try Hashtbl.find results (cygcheck, name) + with Not_found -> + let result = requires_cygwin cygcheck name in + Hashtbl.add results (cygcheck, name) result; + result else - fun ~cygbin:_ _ -> `Native + fun ?search_in_first:_ _ -> `Native - let get_cygwin_variant ~cygbin cmd = + let get_cygwin_variant ?search_in_first 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/ *) - match get_windows_executable_variant ~cygbin cmd with + match get_windows_executable_variant ?search_in_first cmd with | `Native -> `Native | `Cygwin | `Msys2 -> `Cygwin | `Tainted _ -> `CygLinked - let is_cygwin_variant ~cygbin cmd = - get_cygwin_variant ~cygbin cmd = `Cygwin + let is_cygwin_variant ?search_in_first cmd = + get_cygwin_variant ?search_in_first 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) + && (variant ?search_in_first:(Some cygbin) cygpath = `Cygwin) | None -> false let is_cygwin_variant_cygcheck ~cygbin = diff --git a/src/core/opamStd.mli b/src/core/opamStd.mli index 960bd4fb114..c0111260411 100644 --- a/src/core/opamStd.mli +++ b/src/core/opamStd.mli @@ -565,11 +565,15 @@ module Sys : sig to a library which is itself Cygwin- or MSYS2-compiled, or [`Native] otherwise. - Note that this returns [`Native] on a Cygwin-build of opam! + If supplied, [~search_in_first] specifies a directory which should be + searched for cygcheck prior to searching the current PATH. - Both cygcheck and an unqualified command will be resolved if necessary - using the current PATH. *) - val get_windows_executable_variant: cygbin:string option -> + If the command given is not an absolute path, it too is resolved in the + current PATH. + + If cygcheck cannot be resolved in PATH, or when running the Cygwin build + of opam, the function returns `Native. *) + val get_windows_executable_variant: ?search_in_first:string -> string -> [ `Native | `Cygwin | `Tainted of [ `Msys2 | `Cygwin] | `Msys2 ] (** Determines if cygcheck in given cygwin binary directory comes from a @@ -581,18 +585,17 @@ module Sys : sig (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. + (** Behaviour is largely as {!get_windows_executable_variant} but where MSYS2 + and Cygwin are seen as equivalent. - Note that this returns [`Native] on a Cygwin-build of opam! - - Both cygcheck and an unqualified command will be resolved using the - current PATH. *) - val get_cygwin_variant: cygbin:string option -> string -> [ `Native | `Cygwin | `CygLinked ] + 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. *) + val get_cygwin_variant: + ?search_in_first:string -> string -> [ `Native | `Cygwin | `CygLinked ] (** Returns true if [get_cygwin_variant] is [`Cygwin] *) - val is_cygwin_variant: cygbin:string option -> string -> bool + val is_cygwin_variant: ?search_in_first:string -> string -> bool (** {3 Exit handling} *) diff --git a/src/core/opamSystem.ml b/src/core/opamSystem.ml index 888520b21fc..f595e3ddaf4 100644 --- a/src/core/opamSystem.ml +++ b/src/core/opamSystem.ml @@ -444,7 +444,7 @@ let get_cygpath_function = lazy ( if OpamStd.Option.map_default (OpamStd.Sys.is_cygwin_variant - ~cygbin:(OpamCoreConfig.(!r.cygbin))) + ?search_in_first:(OpamCoreConfig.(!r.cygbin))) false (resolve_command command) then apply_cygpath @@ -794,7 +794,7 @@ let install ?(warning=default_install_warning) ?exec src dst = copy_file_aux ~src ~dst (); if cygcheck then match OpamStd.Sys.get_windows_executable_variant - ~cygbin:OpamCoreConfig.(!r.cygbin) dst with + ?search_in_first:OpamCoreConfig.(!r.cygbin) dst with | `Native -> () | (`Cygwin | `Msys2 | `Tainted _) as code -> warning dst code end else From 857cae8f24e340029219b13d603b5379f52caf6b Mon Sep 17 00:00:00 2001 From: David Allsopp Date: Wed, 5 Jun 2024 08:42:29 +0100 Subject: [PATCH 09/15] Detect both Cygwin and MSYS2 os-distribution Previously, MSYS2 required os-distribution to be overridden in global-variables. Now, as with Cygwin, it is inferred in the same way from cygcheck. In this commit, opam init still sets os-distribution, however, if it is manually removed from the root config file, previously `opam var os-distribution` would return `win32` but now returns `msys2`. Additionally, sys_pkg_manager_cmd is handled in the same way for MSYS2 as for Cygwin, allowing MSYS2 to be automatically added in the same way as Cygwin. --- master_changes.md | 1 + src/client/opamClient.ml | 5 ++++- src/state/opamGlobalState.ml | 19 ++++++++----------- src/state/opamSysInteract.ml | 28 ++++++++++++---------------- src/state/opamSysInteract.mli | 5 ++--- src/state/opamSysPoll.ml | 15 ++++++++------- 6 files changed, 35 insertions(+), 38 deletions(-) diff --git a/master_changes.md b/master_changes.md index 5d9b2c64c01..61cccd4d2f4 100644 --- a/master_changes.md +++ b/master_changes.md @@ -97,6 +97,7 @@ users) * Pass --no-version-check to Cygwin setup (suppresses a message box if setup needs updating) [#5830 @dra27] * Pass --quiet-mode noinput to stop the user interrupting the setup GUI [#5830 @dra27] * Always pass --no-write-registry to the Cygwin installer, not just on first installation [#5995 @dra27] + * os-distribution is now by default calculated from cygpath for Cygwin and MSYS2, instead of needing to be set by opam init [#6000 @dra27] ## Format upgrade * Handle init OCaml `sys-ocaml-*` eval variables during format upgrade from 2.0 -> 2.1 -> 2.2 [#5829 @dra27] diff --git a/src/client/opamClient.ml b/src/client/opamClient.ml index 5ed8990d898..d560abb4cf0 100644 --- a/src/client/opamClient.ml +++ b/src/client/opamClient.ml @@ -1096,7 +1096,10 @@ let windows_checks ?cygwin_setup ?git_location config = because there are commands opam requires which are only provided using it (patch, etc.). MSYS2 avoids this by requiring os-distribution to be set. *) - let cygcheck = OpamSysInteract.Cygwin.cygcheck_opt config in + let cygcheck = + OpamStd.String.Map.find_opt "cygwin" + (OpamFile.Config.sys_pkg_manager_cmd config) + in (match cygwin_setup with | None -> get_cygwin cygcheck | Some setup -> diff --git a/src/state/opamGlobalState.ml b/src/state/opamGlobalState.ml index 02edf6cd924..b647416cdfa 100644 --- a/src/state/opamGlobalState.ml +++ b/src/state/opamGlobalState.ml @@ -41,17 +41,14 @@ let load_config lock_kind global_lock root = (* 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 + let cygwin = OpamSysInteract.Cygwin.cygbin_opt config in + let cygbin = + if cygwin = None then + OpamSysInteract.Cygwin.msys2bin_opt config + else + cygwin + in + Option.map OpamFilename.Dir.to_string cygbin in OpamCoreConfig.update ?cygbin (); config diff --git a/src/state/opamSysInteract.ml b/src/state/opamSysInteract.ml index 0a780db827f..7fcf5c72c47 100644 --- a/src/state/opamSysInteract.ml +++ b/src/state/opamSysInteract.ml @@ -101,22 +101,16 @@ module Commands = struct OpamStd.String.Map.find_opt family (OpamFile.Config.sys_pkg_manager_cmd config) - let get_cmd config family = - match get_cmd_opt config family with - | Some cmd -> cmd - | None -> - let field = "sys-pkg-manager-cmd" in - Printf.ksprintf failwith - "Config field '%s' must be set for '%s'. \ - Use opam option --global '%s+=[\"%s\" \"\"]'" - field family field family family - - let msys2 config = OpamFilename.to_string (get_cmd config "msys2") - let cygwin_t = "cygwin" - let cygcheck_opt config = get_cmd_opt config cygwin_t - let cygcheck config = OpamFilename.to_string (get_cmd config cygwin_t) + let msys2_t = "msys2" + let msys2 config = + let override = get_cmd_opt config msys2_t in + OpamStd.Option.map_default OpamFilename.to_string "pacman.exe" override + + let cygcheck config = + let override = get_cmd_opt config cygwin_t in + OpamStd.Option.map_default OpamFilename.to_string "cygcheck.exe" override end (* Please keep this alphabetically ordered, in the type definition, and in @@ -228,10 +222,12 @@ module Cygwin = struct let setupexe = "setup-x86_64.exe" let cygcheckexe = "cygcheck.exe" - let cygcheck_opt = Commands.cygcheck_opt open OpamStd.Option.Op let cygbin_opt config = - cygcheck_opt config + Commands.(get_cmd_opt config cygwin_t) + >>| OpamFilename.dirname + let msys2bin_opt config = + Commands.(get_cmd_opt config msys2_t) >>| OpamFilename.dirname let cygroot_opt config = cygbin_opt config diff --git a/src/state/opamSysInteract.mli b/src/state/opamSysInteract.mli index 754e975a49e..a697e98f6ff 100644 --- a/src/state/opamSysInteract.mli +++ b/src/state/opamSysInteract.mli @@ -69,10 +69,9 @@ module Cygwin : sig (* Return Cygwin binary path *) val cygbin_opt: OpamFile.Config.t -> OpamFilename.Dir.t option - (* Return Cygwin cygcheck.exe path *) - val cygcheck_opt: OpamFile.Config.t -> OpamFilename.t option - (* Return Cygwin installation prefix *) val cygroot_opt: OpamFile.Config.t -> OpamFilename.Dir.t option + (* Return MSYS2 binary path *) + val msys2bin_opt: OpamFile.Config.t -> OpamFilename.Dir.t option end diff --git a/src/state/opamSysPoll.ml b/src/state/opamSysPoll.ml index 2930d973d77..ebcfb2ffcd5 100644 --- a/src/state/opamSysPoll.ml +++ b/src/state/opamSysPoll.ml @@ -114,14 +114,15 @@ let poll_os_distribution () = try Scanf.sscanf s " %s " norm with Scanf.Scan_failure _ -> linux) | Some "win32" -> - (* If the user provides a Cygwin installation in PATH, by default we'll use - it. Note that this is _not_ done for MSYS2. *) - let cygwin = - OpamSystem.resolve_command "cygcheck" - >>| Filename.dirname - |> (fun cygbin -> OpamStd.Sys.is_cygwin_cygcheck ~cygbin) + let kind = + OpamStd.Sys.get_windows_executable_variant + ?search_in_first:(OpamCoreConfig.(!r.cygbin)) "cygpath.exe" in - if cygwin then Some "cygwin" else os + begin match kind with + | `Msys2 -> Some "msys2" + | `Cygwin -> Some "cygwin" + | `Native | `Tainted _ -> os + end | os -> os let os_distribution = Lazy.from_fun poll_os_distribution From 4f1ca994313e1063a9a9e41676433c840fa9bc06 Mon Sep 17 00:00:00 2001 From: David Allsopp Date: Wed, 5 Jun 2024 10:55:28 +0100 Subject: [PATCH 10/15] Ensure Cygwin setup is downloaded and up-to-date --- master_changes.md | 1 + src/client/opamSolution.ml | 4 ++ src/state/opamSysInteract.ml | 76 +++++++++++++++++++++++++++++------- 3 files changed, 66 insertions(+), 15 deletions(-) diff --git a/master_changes.md b/master_changes.md index 61cccd4d2f4..d1384cca8a5 100644 --- a/master_changes.md +++ b/master_changes.md @@ -98,6 +98,7 @@ users) * Pass --quiet-mode noinput to stop the user interrupting the setup GUI [#5830 @dra27] * Always pass --no-write-registry to the Cygwin installer, not just on first installation [#5995 @dra27] * os-distribution is now by default calculated from cygpath for Cygwin and MSYS2, instead of needing to be set by opam init [#6000 @dra27] + * Cygwin setup is now always downloaded and updated both for internal and external Cygwin installations [#6000 @dra27] ## Format upgrade * Handle init OCaml `sys-ocaml-*` eval variables during format upgrade from 2.0 -> 2.1 -> 2.2 [#5829 @dra27] diff --git a/src/client/opamSolution.ml b/src/client/opamSolution.ml index cdbee8b0d66..b0adf8673ea 100644 --- a/src/client/opamSolution.ml +++ b/src/client/opamSolution.ml @@ -1189,6 +1189,8 @@ let install_sys_packages ~map_sysmap ~confirm env config sys_packages t = | `Ignore -> bypass t | `Quit -> give_up_msg (); OpamStd.Sys.exit_because `Aborted and print_command sys_packages = + if OpamSysPoll.os_distribution env = Some "cygwin" then + OpamSysInteract.Cygwin.check_setup None; let commands = OpamSysInteract.install_packages_commands ~env config sys_packages |> List.map (fun ((`AsAdmin c | `AsUser c), a) -> c::a) @@ -1219,6 +1221,8 @@ let install_sys_packages ~map_sysmap ~confirm env config sys_packages t = | `Quit -> give_up () and auto_install t sys_packages = try + if OpamSysPoll.os_distribution env = Some "cygwin" then + OpamSysInteract.Cygwin.check_setup None; OpamSysInteract.install ~env config sys_packages; (* handles dry_run *) map_sysmap (fun _ -> OpamSysPkg.Set.empty) t with Failure msg -> diff --git a/src/state/opamSysInteract.ml b/src/state/opamSysInteract.ml index 7fcf5c72c47..129bec2f4e5 100644 --- a/src/state/opamSysInteract.ml +++ b/src/state/opamSysInteract.ml @@ -252,7 +252,20 @@ module Cygwin = struct let download_setupexe dst = let overwrite = true in + let checksum_file = OpamFilename.add_extension dst "sha512" in + let current_checksum = + if OpamFilename.exists checksum_file then + Some (OpamHash.sha512 (OpamFilename.read checksum_file)) + else + None + in let open OpamProcess.Job.Op in + log "Downloading Cygwin setup checksums"; + if OpamConsole.disp_status_line () then + if OpamFilename.exists dst then + OpamConsole.status_line "Checking if Cygwin setup is up-to-date" + else + OpamConsole.status_line "Downloading Cygwin setup from cygwin.com"; OpamFilename.with_tmp_dir_job @@ fun dir -> OpamDownload.download ~overwrite url_setupexe_sha512 dir @@+ fun file -> let checksum = @@ -272,7 +285,31 @@ module Cygwin = struct try Some (OpamHash.sha512 Re.(Group.get (exec re content) 1)) with Not_found -> None in - OpamDownload.download_as ~overwrite ?checksum url_setupexe dst + let kind = `SHA512 in + if OpamStd.Option.equal OpamHash.equal current_checksum checksum && + OpamFilename.exists dst && + OpamStd.Option.equal OpamHash.equal current_checksum + (Some (OpamHash.compute ~kind (OpamFilename.to_string dst))) then begin + log "Up-to-date"; + OpamConsole.clear_status (); + Done () + end else begin + log "Downloading setup-x86_64.exe"; + if OpamConsole.disp_status_line () then + OpamConsole.status_line "Downloading Cygwin setup from cygwin.com"; + OpamDownload.download_as ~overwrite ?checksum url_setupexe dst @@+ + fun () -> + OpamFilename.remove checksum_file; + let checksum = + match checksum with + | None -> OpamHash.compute ~kind (OpamFilename.to_string dst) + | Some sha512 -> sha512 + in + OpamFilename.with_open_out_bin checksum_file (fun c -> + output_string c (OpamHash.contents checksum)); + OpamConsole.clear_status (); + Done () + end let set_fstab_noacl = let orig = "binary," in @@ -389,16 +426,21 @@ module Cygwin = struct (* Set setup.exe in the good place, ie in .opam/.cygwin/ *) let check_setup setup = let dst = cygsetup () in - if OpamFilename.exists dst then () else - (match setup with - | Some setup -> - log "Copying %s into %s" - (OpamFilename.to_string setup) - (OpamFilename.to_string dst); - OpamFilename.copy ~src:setup ~dst - | None -> - log "Donwloading setup exe"; - OpamProcess.Job.run @@ download_setupexe dst) + match setup with + | Some setup -> + log "Copying %s into %s" + (OpamFilename.to_string setup) + (OpamFilename.to_string dst); + let sha512 = + OpamHash.compute ~kind:`SHA512 (OpamFilename.to_string setup) + in + OpamFilename.copy ~src:setup ~dst; + let checksum_file = OpamFilename.add_extension dst "sha512" in + OpamFilename.remove checksum_file; + OpamFilename.with_open_out_bin checksum_file @@ fun c -> + output_string c (OpamHash.contents sha512) + | None -> + OpamProcess.Job.run @@ download_setupexe dst end let yum_cmd = lazy begin @@ -1066,8 +1108,9 @@ let install ?env config packages = commands let update ?(env=OpamVariable.Map.empty) config = + let family = family ~env () in let cmd = - match family ~env () with + match family with | Alpine -> Some (`AsAdmin "apk", ["update"]) | Arch -> Some (`AsAdmin "pacman", ["-Sy"]) | Centos -> Some (`AsAdmin (Lazy.force yum_cmd), ["makecache"]) @@ -1086,9 +1129,12 @@ let update ?(env=OpamVariable.Map.empty) config = in match cmd with | None -> - OpamConsole.warning - "Unknown update command for %s, skipping system update" - OpamStd.Option.Op.(OpamSysPoll.os_family env +! "unknown") + if family = Cygwin then + Cygwin.check_setup None + else + OpamConsole.warning + "Unknown update command for %s, skipping system update" + OpamStd.Option.Op.(OpamSysPoll.os_family env +! "unknown") | Some (cmd, args) -> try sudo_run_command ~env cmd args with Failure msg -> failwith ("System package update " ^ msg) From 499c9e7199971cfbf49b2dc17cb5ca706728267a Mon Sep 17 00:00:00 2001 From: David Allsopp Date: Wed, 5 Jun 2024 11:00:07 +0100 Subject: [PATCH 11/15] De-label OpamSysInteract.Cygwin.install Single argument function with a non-base type. --- master_changes.md | 1 + src/client/opamClient.ml | 2 +- src/state/opamSysInteract.ml | 2 +- src/state/opamSysInteract.mli | 2 +- 4 files changed, 4 insertions(+), 3 deletions(-) diff --git a/master_changes.md b/master_changes.md index d1384cca8a5..b2e1951c1eb 100644 --- a/master_changes.md +++ b/master_changes.md @@ -182,6 +182,7 @@ users) ## opam-state * `OpamEnv.cygwin_non_shadowed_programs`: exposes the list of executables (not including git) which should always come from Cygwin [#6000 @dra27] + * `opamSysInteract.Cygwin.install`: de-label `packages` argument [#6000 @dra27] ## opam-solver diff --git a/src/client/opamClient.ml b/src/client/opamClient.ml index d560abb4cf0..7e6bd6be08f 100644 --- a/src/client/opamClient.ml +++ b/src/client/opamClient.ml @@ -966,7 +966,7 @@ let windows_checks ?cygwin_setup ?git_location config = constraints in default_packages (although, with the current version of setup, and with the list of default_packages in OpamInitDefaults, this at present doesn't matter too much). *) - OpamSysInteract.Cygwin.install ~packages:(default_packages @ packages) + OpamSysInteract.Cygwin.install (default_packages @ packages) in let header () = OpamConsole.header_msg "Unix support infrastructure" in diff --git a/src/state/opamSysInteract.ml b/src/state/opamSysInteract.ml index 129bec2f4e5..feadb89107c 100644 --- a/src/state/opamSysInteract.ml +++ b/src/state/opamSysInteract.ml @@ -320,7 +320,7 @@ module Cygwin = struct OpamFilename.with_open_out_bin_atomic fstab (fun oc -> Stdlib.output_string oc content) - let install ~packages = + let install packages = let open OpamProcess.Job.Op in let cygwin_root = internal_cygroot () in let cygwin_bin = cygwin_root / "bin" in diff --git a/src/state/opamSysInteract.mli b/src/state/opamSysInteract.mli index a697e98f6ff..239e530aa24 100644 --- a/src/state/opamSysInteract.mli +++ b/src/state/opamSysInteract.mli @@ -47,7 +47,7 @@ module Cygwin : sig val default_cygroot: string (* Install an internal Cygwin install, in /.cygwin *) - val install: packages:OpamSysPkg.t list -> OpamFilename.t + val install: OpamSysPkg.t list -> OpamFilename.t (* [check_install ~variant path] checks a Cygwin install at [path]. It checks that 'path\cygcheck.exe', 'path\bin\cygcheck.exe', or From 2337441b64cea8e91749b6cb486fdc2a51b2c5b7 Mon Sep 17 00:00:00 2001 From: David Allsopp Date: Wed, 5 Jun 2024 17:23:13 +0100 Subject: [PATCH 12/15] Expand Cygwin.check_install to analyse_install Includes more checks and also returns the kind of installation which was found. --- master_changes.md | 1 + src/client/opamClient.ml | 18 ++-- src/state/opamSysInteract.ml | 154 ++++++++++++++++++++-------------- src/state/opamSysInteract.mli | 26 ++++-- 4 files changed, 118 insertions(+), 81 deletions(-) diff --git a/master_changes.md b/master_changes.md index b2e1951c1eb..42053d2ba74 100644 --- a/master_changes.md +++ b/master_changes.md @@ -183,6 +183,7 @@ users) ## opam-state * `OpamEnv.cygwin_non_shadowed_programs`: exposes the list of executables (not including git) which should always come from Cygwin [#6000 @dra27] * `opamSysInteract.Cygwin.install`: de-label `packages` argument [#6000 @dra27] + * `OpamSysInteract.Cygwin.check_install` renamed to `analyse_install` which now also returns whether the installation found was MSYS2 or Cygwin [#6000 @dra27] ## opam-solver diff --git a/src/client/opamClient.ml b/src/client/opamClient.ml index 7e6bd6be08f..6159b7ecfd4 100644 --- a/src/client/opamClient.ml +++ b/src/client/opamClient.ml @@ -1003,9 +1003,8 @@ let windows_checks ?cygwin_setup ?git_location config = in (* Check for default cygwin installation path *) let default = - match OpamSysInteract.Cygwin.(check_install - ~variant:true default_cygroot) with - | Ok cygcheck -> + match OpamSysInteract.Cygwin.(analyse_install default_cygroot) with + | Ok (_, cygcheck) -> let prompt_cygroot () = let options = [ `manual, @@ -1036,10 +1035,10 @@ let windows_checks ?cygwin_setup ?git_location config = | None -> None | Some entry -> let cygcheck = - OpamSysInteract.Cygwin.check_install ~variant:true entry + OpamSysInteract.Cygwin.analyse_install entry in match cygcheck with - | Ok cygcheck -> Some cygcheck + | Ok (_, cygcheck) -> Some cygcheck | Error msg -> OpamConsole.error "%s" msg; None in (* And finally ask for setup.exe *) @@ -1113,9 +1112,8 @@ 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 ~variant:true - cygroot with - | Ok cygcheck -> cygcheck + (match OpamSysInteract.Cygwin.analyse_install cygroot with + | Ok (_, cygcheck) -> cygcheck | Error msg -> OpamConsole.error_and_exit `Not_found "Error while checking %sCygwin install (%s): %s" @@ -1131,9 +1129,9 @@ let windows_checks ?cygwin_setup ?git_location config = (* We check that current install is good *) (match OpamSysInteract.Cygwin.cygroot_opt config with | Some cygroot -> - (match OpamSysInteract.Cygwin.check_install ~variant:true + (match OpamSysInteract.Cygwin.analyse_install (OpamFilename.Dir.to_string cygroot) with - | Ok cygcheck -> + | Ok (_, cygcheck) -> OpamSysInteract.Cygwin.check_setup None; success cygcheck | Error err -> OpamConsole.error "%s" err; get_cygwin None) diff --git a/src/state/opamSysInteract.ml b/src/state/opamSysInteract.ml index feadb89107c..232d7e8520d 100644 --- a/src/state/opamSysInteract.ml +++ b/src/state/opamSysInteract.ml @@ -8,8 +8,6 @@ (* *) (**************************************************************************) -open OpamTypes - let log fmt = OpamConsole.log "XSYS" fmt (* Run commands *) @@ -232,10 +230,15 @@ module Cygwin = struct let cygroot_opt config = cygbin_opt config >>| OpamFilename.dirname_dir - let get_opt = function + let cygroot config = + match cygroot_opt config with | Some c -> c - | None -> failwith "Cygwin install not found" - let cygroot config = get_opt (cygroot_opt config) + | None -> + match OpamSystem.resolve_command "cygcheck.exe" with + | Some cygcheck -> + OpamFilename.dirname_dir (OpamFilename.Dir.of_string (Filename.dirname cygcheck)) + | None -> + failwith "Cygwin install not found" let internal_cygwin = let internal = @@ -252,10 +255,10 @@ module Cygwin = struct let download_setupexe dst = let overwrite = true in - let checksum_file = OpamFilename.add_extension dst "sha512" in + let kind = `SHA512 in let current_checksum = - if OpamFilename.exists checksum_file then - Some (OpamHash.sha512 (OpamFilename.read checksum_file)) + if OpamFilename.exists dst then + Some (OpamHash.compute ~kind (OpamFilename.to_string dst)) else None in @@ -285,7 +288,6 @@ module Cygwin = struct try Some (OpamHash.sha512 Re.(Group.get (exec re content) 1)) with Not_found -> None in - let kind = `SHA512 in if OpamStd.Option.equal OpamHash.equal current_checksum checksum && OpamFilename.exists dst && OpamStd.Option.equal OpamHash.equal current_checksum @@ -299,14 +301,6 @@ module Cygwin = struct OpamConsole.status_line "Downloading Cygwin setup from cygwin.com"; OpamDownload.download_as ~overwrite ?checksum url_setupexe dst @@+ fun () -> - OpamFilename.remove checksum_file; - let checksum = - match checksum with - | None -> OpamHash.compute ~kind (OpamFilename.to_string dst) - | Some sha512 -> sha512 - in - OpamFilename.with_open_out_bin checksum_file (fun c -> - output_string c (OpamHash.contents checksum)); OpamConsole.clear_status (); Done () end @@ -375,53 +369,89 @@ module Cygwin = struct let default_cygroot = "C:\\cygwin64" - let check_install ~variant path = - let is_cygwin = - if variant then OpamStd.Sys.is_cygwin_variant_cygcheck - else OpamStd.Sys.is_cygwin_cygcheck - in - if not (Sys.file_exists path) then - Error (Printf.sprintf "%s not found!" path) - else if Filename.basename path = "cygcheck.exe" then - (* We have cygcheck.exe path *) - let cygbin = Some (Filename.dirname path) in - if is_cygwin ~cygbin then - Ok (OpamFilename.of_string path) - else - Error - (Printf.sprintf - "%s found, but it is not from a Cygwin installation" - path) - else if not (Sys.is_directory path) then - Error (Printf.sprintf "%s is not a directory" path) - else - (* We have cygroot alike path *) - let bin = Filename.concat path "bin" in - let usr_bin = Filename.concat (Filename.concat path "usr") "bin" in - let check cygbin = - if Sys.file_exists cygbin then - if is_cygwin ~cygbin:(Some cygbin) then - Some (Left (OpamFilename.of_string - (Filename.concat cygbin "cygcheck.exe"))) - else - Some (Right cygbin) + let analysis_cache = Hashtbl.create 17 + + let analyse_install path = + let cygbin = + if not (Sys.file_exists path) then + Error (path ^ " not found!") + else if Filename.remove_extension (Filename.basename path) + = "cygcheck" then + (* path refers to cygcheck directly *) + Ok (Filename.dirname path) + else if not (Sys.is_directory path) then + Error (Printf.sprintf "%s neither a directory nor cygcheck.exe" path) else - None + (* path is a directory - search path, path\bin and path\usr\bin *) + let contains_cygcheck dir = + Sys.file_exists (Filename.concat dir "cygcheck.exe") + in + let tests = [ + path; (* e.g. C:\cygwin64\bin / C:\msys64\usr\bin *) + Filename.concat path "bin"; (* e.g. C:\cygwin64 *) + Filename.concat (Filename.concat path "usr") "bin" (* e.g. C:\msys64 *) + ] in + match List.filter contains_cygcheck tests with + | [] -> + Error (Printf.sprintf + "cygcheck.exe not found in %s, or subdirectories \ + bin and usr\\bin" path) + | _::_::_ -> + Error (Printf.sprintf + "cygcheck.exe found in multiple places in %s which suggests \ + it is not a Cygwin/MSYS2 installation" path) + | [path] -> + Ok path + in + let identify dir = + try Hashtbl.find analysis_cache dir + with Not_found -> + let result = + let cygpath = Filename.concat dir "cygpath.exe" in + if not (Sys.file_exists cygpath) then + Error (Printf.sprintf + "cygcheck.exe found in %s, but cygpath.exe was not" dir) + else + match OpamStd.Sys.get_windows_executable_variant + ~search_in_first:dir cygpath with + | `Native | `Tainted _ -> + Error (Printf.sprintf + "cygcheck.exe found in %s; but it does not appear \ + to be part of a Cygwin or MSYS2 installation" dir) + | (`Msys2 | `Cygwin) as kind -> + (* Check that pacman.exe is present with MSYS2: it is typically + not present with a Git-for-Windows Git Bash session, and as + these are basically unusable (they don't have all the required + tools, and we have no package manager with which to add them), + it's better to exclude them). *) + if kind = `Msys2 + && not (Sys.file_exists (Filename.concat dir "pacman.exe")) then + Error (Printf.sprintf + "cygcheck.exe found in %s, which appears to be from \ + an MSYS2 installation, but pacman.exe was not" dir) + else + let r = + OpamProcess.run + (OpamProcess.command ~name:(OpamSystem.temp_file "command") + ~allow_stdin:false cygpath ["-w"; "--"; "/"]) + in + OpamProcess.cleanup ~force:true r; + if OpamProcess.is_success r then + match r.OpamProcess.r_stdout with + | [] -> + Error ("Unexpected error translating \"/\" with " ^ cygpath) + | _::_ -> + let cygcheck = + OpamFilename.of_string (Filename.concat dir "cygpath.exe") + in + Ok (kind, cygcheck) + else + Error ("Could not determine the root for " ^ cygpath) + in + Hashtbl.add analysis_cache dir result; + result in - (* We need to keep that order, to have a better error message *) - match check bin, check usr_bin with - | Some (Left cygcheck), _ | _, Some (Left cygcheck) -> - Ok cygcheck - | Some (Right cygbin), _ | _, Some (Right cygbin) -> - Error - (Printf.sprintf - "%s found, but it does not appear to be a Cygwin installation" - cygbin) - | _, None -> - Error - (Printf.sprintf - "cygcheck.exe not found in %s subdirectories bin or usr\bin" - path) + Result.bind cygbin identify (* Set setup.exe in the good place, ie in .opam/.cygwin/ *) let check_setup setup = diff --git a/src/state/opamSysInteract.mli b/src/state/opamSysInteract.mli index 239e530aa24..8ce6eac17ee 100644 --- a/src/state/opamSysInteract.mli +++ b/src/state/opamSysInteract.mli @@ -49,21 +49,29 @@ module Cygwin : sig (* Install an internal Cygwin install, in /.cygwin *) val install: OpamSysPkg.t list -> OpamFilename.t - (* [check_install ~variant path] checks a Cygwin install at [path]. It checks - that 'path\cygcheck.exe', 'path\bin\cygcheck.exe', or - 'path\usr\bin\cygcheck.exe' exists. - If [~variant] is false, checks that it is strictly a Cygwin install, - otherwise a Cygwin-like install as MSYS2. *) - val check_install: - variant:bool -> string -> (OpamFilename.t, string) result + (* [analyse_install path] searches for and identifies Cygwin/MSYS2 + installations. [path] may be able the location of cygcheck.exe itself + (with or without the .exe) or just a directory. If [path] is just a + directory, then the function searches for 'path\cygcheck.exe', + 'path\bin\cygcheck.exe', or 'path\usr\bin\cygcheck.exe'. If exactly one + is found, and cygpath.exe is found with it, then cygpath is used both to + identify whether the installation is Cygwin or MSYS2 and to translate the + root directory [/] to its Windows path (i.e. to get the canonical root + directory of the installation). MSYS2 is additionally required to have + pacman.exe in the same directory as cygcheck.exe and cygpath.exe. + + On success, the result is the kind of installation (Cygwin/MSYS2) along + with the full path to cygcheck.exe, otherwise a description of the problem + encountered is returned. *) + val analyse_install: + string -> ([ `Cygwin | `Msys2 ] * OpamFilename.t, string) result (* Returns true if Cygwin install is internal *) val is_internal: OpamFile.Config.t -> bool (* [check_setup path] checks and store Cygwin setup executable. Is [path] is [None], it downloads it, otherwise it copies it to - /.cygwin/setup-x86_64.exe. If the file is already existent, it - is a no-op. *) + /.cygwin/setup-x86_64.exe. *) val check_setup: OpamFilename.t option -> unit (* Return Cygwin binary path *) From fa4d39f178253db70e1e33baf4ed48357bcc5560 Mon Sep 17 00:00:00 2001 From: David Allsopp Date: Sun, 2 Jun 2024 12:17:27 +0100 Subject: [PATCH 13/15] Supercalifragilisticexpialidocious --- master_changes.md | 1 + src/client/opamClient.ml | 1120 ++++++++++++++++++++++----------- src/core/opamStd.ml | 14 - src/core/opamStd.mli | 9 - src/state/opamSysInteract.ml | 18 +- src/state/opamSysInteract.mli | 20 +- 6 files changed, 782 insertions(+), 400 deletions(-) diff --git a/master_changes.md b/master_changes.md index 42053d2ba74..acd65036b2b 100644 --- a/master_changes.md +++ b/master_changes.md @@ -133,6 +133,7 @@ users) ## Client * Fix rounding error when displaying the timestamp in debug mode [#5912 @kit-ty-kate - fix #5910] + * Overhaul Windows `opam init` to determine Unix and Git configuration simultaneously, and to detect from Cygwin, Git and MSYS2 from all the known package managers and shells [#6000 @dra27] ## Shell diff --git a/src/client/opamClient.ml b/src/client/opamClient.ml index 6159b7ecfd4..c0257940735 100644 --- a/src/client/opamClient.ml +++ b/src/client/opamClient.ml @@ -635,7 +635,7 @@ let init_checks ?(hard_fail_exn=true) init_config = if hard_fail && hard_fail_exn then OpamStd.Sys.exit_because `Configuration_error else not (soft_fail || hard_fail) -let is_git_for_windows git = +let rec is_git_for_windows git = (* The resource file compiled for Git for Windows sets the ProductVersion string to M.m.r.windows.b where M.m.r is the git version and b is the revision number of Git for Windows. This differentiates it from very old @@ -652,21 +652,60 @@ let is_git_for_windows git = try Scanf.sscanf version "%u.%u.%u.windows.%u%!" (fun _ _ _ _ -> true) with Scanf.Scan_failure _ | Failure _ | End_of_file -> false end - | _ -> false + | _ -> + (* The Scoop package manager installs a shim git.exe (see + https://github.com/ScoopInstaller/Shim) which will fail our test of the + version information block, while actually being Git for Windows. + If git.shim and scoop.cmd are found with git.exe and we can parse the + path line from git.shim, then we test the executable pointed to instead. + *) + let dir = Filename.dirname git in + let git_shim = Filename.concat dir "git.shim" in + let scoop = Filename.concat dir "scoop.cmd" in + let find_path_value (key, value) = + if String.trim key = "path" then + Some (String.trim value) + else + None + in + let test_scoop_shim s = + let new_git = + let last = String.length s - 1 in + if last > 0 && s.[0] = '"' && s.[last] = '"' then + String.sub s 1 (last - 1) + else s + in + log "%s appears to be a Scoop shim; trying %s" git new_git; + is_git_for_windows new_git + in + if Sys.file_exists git_shim && Sys.file_exists scoop then + OpamSystem.read git_shim + |> String.split_on_char '\n' + |> List.filter_map (Fun.flip OpamStd.String.cut_at '=') + |> OpamStd.List.find_map_opt find_path_value + |> OpamStd.Option.map_default test_scoop_shim false + else + false + +let string_of_kind = function + | `Msys2 -> "MSYS2" + | `Cygwin -> "Cygwin" -let git_for_windows ?git_location () = - let header () = OpamConsole.header_msg "Git" in - let contains_git p = +let git_for_windows kind mechanism cygwin_is_tweakable = + let resolve_git_in p = OpamSystem.resolve_command ~env:[||] (Filename.concat p "git.exe") in let gits = OpamStd.Env.get "PATH" |> OpamStd.Sys.split_path_variable - |> OpamStd.List.filter_map (fun p -> - match contains_git p with - | Some git -> + |> OpamStd.List.fold_left_map (fun gits p -> + match resolve_git_in p with + | Some git when not (OpamStd.String.Set.mem git gits) -> + OpamStd.String.Set.add git gits, Some (git, OpamSystem.bin_contains_bash p) - | None -> None) + | _ -> gits, None) OpamStd.String.Set.empty + |> snd + |> List.filter_map Fun.id in let abort_action = "install Git for Windows." in let gits, gfw_message, abort_action = @@ -681,7 +720,7 @@ let git_for_windows ?git_location () = [], Some "It looks as though Git for Windows has been installed but \ the shell needs to be restarted. You may wish to abort and \ re-run opam init from a fresh session.", - "restart your shell." + "restart your shell." | _ -> (* Git is neither in the current nor the initial PATH. There is one further possibility: the user may have installed Git for Windows @@ -692,7 +731,7 @@ let git_for_windows ?git_location () = 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) = + (hive, key) = let process root = let git_location = Filename.concat root "cmd" in let git = Filename.concat git_location "git.exe" in @@ -726,7 +765,8 @@ let git_for_windows ?git_location () = (* User-specific installation *) (OpamStubsTypes.HKEY_CURRENT_USER, "Software"); ] in - List.fold_left test_for_installation (gits, None, abort_action) installations + List.fold_left test_for_installation + (gits, None, abort_action) installations else gits, None, abort_action in @@ -735,12 +775,15 @@ let git_for_windows ?git_location () = match git_location with | Some _ -> git_location | None -> - OpamConsole.read "Please enter the path containing git.exe (e.g. C:\\Program Files\\Git\\cmd):" + OpamConsole.read + "Please enter the path containing git.exe (e.g. \ + C:\\Program Files\\Git\\cmd):" in match bin with | None -> None | Some git_location -> - match contains_git git_location, OpamSystem.bin_contains_bash git_location with + match resolve_git_in git_location, + OpamSystem.bin_contains_bash git_location with | Some _, false -> OpamConsole.msg "Using Git from %s" git_location; Some git_location @@ -754,68 +797,118 @@ let git_for_windows ?git_location () = OpamConsole.error "No Git executable found in %s." git_location; None in + let options = + (List.filter_map (fun (git, bash) -> + if bash then + None + else + let bin = Filename.dirname git in + Some (`Location bin, "Use found git in "^bin)) + gits) + @ [ + `Specify, "Enter the location of your Git installation"; + `Abort, ("Abort initialisation to " ^ abort_action); + ] + in + let add_or_use_git root = + let bindir = OpamSysInteract.Cygwin.bindir_for_root kind root in + if resolve_git_in (OpamFilename.Dir.to_string bindir) = None then + "Add Git to" + else + "Use Git from" + in + let default, options = + match mechanism with + | `Internal -> + assert cygwin_is_tweakable; + let internal = + `Default, Printf.sprintf + "Install Git with along with %s internally" (string_of_kind kind) + in + `Default, internal::options + | `Root root -> + assert cygwin_is_tweakable; + let root = + `Default, Printf.sprintf + "%s the %s installation in %s" + (add_or_use_git root) + (string_of_kind kind) + (OpamFilename.Dir.to_string root) + in + `Default, root::options + | `Path root -> + match OpamSystem.resolve_command "git.exe" with + | Some git -> + let options = + if Filename.dirname git = root then + let option = + `Default, Printf.sprintf + "Use %s Git from the installation at %s in PATH" + (string_of_kind kind) root + in + option::options + else + (`Default, Printf.sprintf "Use Git from PATH")::options + in + `Default, options + | None -> + if cygwin_is_tweakable then + let option = + `Default, Printf.sprintf + "%s %s installation in %s (from PATH)" + (add_or_use_git (OpamFilename.Dir.of_string root)) + (string_of_kind kind) + root + in + `Default, option::options + else + (fst (List.hd options)), options + in let rec loop ?git_location () = match get_git_location ?git_location () with - | Some _ as git_location -> git_location + | Some _ as git_location -> git_location, false | None -> menu () and menu () = let prompt () = - let options = - (`Default, "Use default Cygwin Git") - :: (List.filter_map (fun (git, bash) -> - if bash then - None - else - let bin = Filename.dirname git in - Some (`Location bin, "Use found git in "^bin)) - gits) - @ [ - `Specify, "Enter the location of installed Git"; - `Abort, ("Abort initialisation to " ^ abort_action); - ] - in OpamStd.Option.iter (OpamConsole.warning "%s\n") gfw_message; OpamConsole.menu "Which Git should opam use?" - ~default:`Default ~no:`Default ~options + ~default ~no:default ~options in match prompt () with - | `Default -> None + | `Default -> None, cygwin_is_tweakable | `Specify -> loop () | `Location git_location -> loop ~git_location () | `Abort -> - OpamConsole.note "Once your choosen Git installed, open a new PowerShell or Command Prompt window, and relaunch opam init."; + OpamConsole.note + "Once your chosen Git is installed, open a new PowerShell or Command \ + Prompt window, and relaunch opam init."; OpamStd.Sys.exit_because `Aborted in - let git_location = - match git_location with - | Some (Right ()) -> None - | Some (Left git_location) -> - header (); - get_git_location ~git_location:(OpamFilename.Dir.to_string git_location) () - | None -> - let git_found = - match OpamSystem.resolve_command "git" with - | None -> false - | Some git -> is_git_for_windows git - in - if not git_found && OpamStd.Sys.tty_out then - (header (); - OpamConsole.msg - "Cygwin Git is functional but can have credentials issues for private repositories, \ - we recommend using:\n%s\n" - (OpamStd.Format.itemize (fun s -> s) - [ "Install via 'winget install Git.Git'"; - "Git for Windows can be downloaded and installed from https://gitforwindows.org" ]); - menu ()) - else - None + let git_location, use_cygwin = + let git_found = + match OpamSystem.resolve_command "git" with + | None -> false + | Some git -> is_git_for_windows git + in + if not git_found && OpamStd.Sys.tty_out then + (OpamConsole.header_msg "Git"; + OpamConsole.msg + "Cygwin Git is functional but can have credentials issues for private \ + repositories, we recommend using:\n%s\n" + (OpamStd.Format.itemize (fun s -> s) + [ "Install via 'winget install Git.Git'"; + "Git for Windows can be downloaded and installed from \ + https://gitforwindows.org" ]); + menu ()) + else + None, not git_found && cygwin_is_tweakable in OpamStd.Option.iter (fun _ -> OpamConsole.msg "You can change that later with \ 'opam option \"git-location=C:\\A\\Path\\bin\"'") git_location; - git_location + Option.map OpamFilename.Dir.of_string git_location, use_cygwin let check_git_location_or_exit git_location source = let git = @@ -826,23 +919,370 @@ let check_git_location_or_exit git_location source = "The location specified with %s does not appear to contain a Git \ executable!" source -let windows_checks ?cygwin_setup ?git_location config = +(* Default search mechanisms for Cygwin/MSYS2 *) +let cygwin_searches = [ + `Cygwins + (OpamStubsTypes.HKEY_LOCAL_MACHINE, "SOFTWARE\\Cygwin\\Installations"); + `Cygwins + (OpamStubsTypes.HKEY_CURRENT_USER, "Software\\Cygwin\\Installations"); + `Test "C:\\cygwin64"; + `Test "C:\\msys64"; + `Msys2_generic; + `ScoopMsys2; +] + +(* cygwin_searches is a sequence of `Path and `Test mechanisms based on the + cygwin_searches list above. If specified, the ~first parameter allows a + different first mechanism to be returned. *) +let cygwin_searches ?first () = + let cygwin_searches = + match first with + | Some first -> first::cygwin_searches + | None -> cygwin_searches + in + let rec seq searches () = + match searches with + | ((`Path | `Test _) as search)::searches -> + (* Return the next mechanism *) + Seq.Cons(search, seq searches) + | (`Cygwins (hive, key))::searches -> + (* Search the given registry hive key for Cygwin locations *) + let possibles = OpamStubs.enumRegistry hive key OpamStubsTypes.REG_SZ in + let map (_, path) = + let path = + if OpamStd.String.starts_with ~prefix:"\\??\\" path then + String.sub path 4 (String.length path - 4) + else + path + in + `Test path + in + seq (List.map map possibles @ searches) () + | `ScoopMsys2::searches -> + (* Scoop installs an msys2.cmd shim in PATH. If this is encountered, parse + it. *) + begin match OpamStd.Sys.resolve_in_path "msys2.cmd" with + | None -> + seq searches () + | Some msys2 -> + let re = + Re.(compile @@ seq [ + bos; + str "@\""; + group @@ rep @@ diff any (char '"'); + char '"'; + rep any; + str " -msys2"; + alt [char ' '; eos] + ]) + in + let parse_line s = + Stdlib.Option.bind (Re.exec_opt re s) (Fun.flip Re.Group.get_opt 1) + in + let msys2_shell = + OpamSystem.read msys2 + |> String.split_on_char '\n' + |> OpamStd.List.find_map_opt parse_line + in + match msys2_shell with + | None -> + seq searches () + | Some msys2_shell -> + Seq.Cons(`Test (Filename.dirname msys2_shell), seq searches) + end + | `Msys2_generic::searches -> + (* Some package managers put the root msys64 directory into PATH, in which + case there will be msys2.exe - if that can be resolved in PATH, try + that. *) + begin match OpamSystem.resolve_command "msys2.exe" with + | None -> + seq searches () + | Some msys2 -> + Seq.Cons(`Test (Filename.dirname msys2), seq searches) + end + | [] -> Seq.Nil + in + seq cygwin_searches + +let rec cygwin_menu header = + let start = Unix.gettimeofday () in + let test_mechanism (roots, count, mechanisms) search = + match test_mechanism header search with + | Some ((kind, `Root root) as mechanism) -> + if OpamFilename.Dir.Set.mem root roots then + roots, count, mechanisms + else + let roots = OpamFilename.Dir.Set.add root roots in + let mechanisms = + (`Chosen mechanism, + Printf.sprintf + "Use %s installation found in %s" + (string_of_kind kind) + (OpamFilename.Dir.to_string root))::mechanisms + in + let count = succ count in + if OpamConsole.disp_status_line () + && Unix.gettimeofday () -. start >= 0.5 then + OpamConsole.status_line + "Searching for Cygwin/MSYS2 installations: %d found so far" count; + roots, count, mechanisms + | _ -> roots, count, mechanisms + in + let detected = + let _, _, mechanisms = + Seq.fold_left test_mechanism + (OpamFilename.Dir.Set.empty, 0, []) (cygwin_searches ()) + in + List.rev mechanisms + in + OpamConsole.clear_status (); + let internal_option = `Chosen (`Cygwin, `Internal) in + let options = + (internal_option, + "Automatically create an internal Cygwin installation that will be \ + managed by opam (recommended)") :: + (detected @ + [`Specify, "Use an" ^ (if detected = [] then "" else "other") ^ + " existing Cygwin/MSYS2 installation"; + `Abort, "Abort initialisation"]) + in + let options, default, warn_path = + (* First of all see if cygcheck can be found in PATH *) + let cygcheck = + OpamSystem.resolve_command "cygcheck.exe" + |> Option.map OpamSysInteract.Cygwin.analyse_install + in + begin match cygcheck with + | Some (Error _) | None -> + (* cygcheck wasn't in PATH, so default to the internal installation *) + options, `Chosen (`Cygwin, `Internal), None + | Some (Ok (kind, root)) -> + let pacman = + OpamFilename.Op.(OpamSysInteract.Cygwin.bindir_for_root `Msys2 root // "pacman.exe") + |> OpamFilename.to_string + in + let root = OpamFilename.Dir.to_string root in + let path_option = `Chosen (kind, `Path root) in + let options = + (path_option, Printf.sprintf + "Use tools found in PATH (%s installation at %s)" + (string_of_kind kind) root)::options + in + (* Check whether cygcheck is still available in the initial environment. + This allows a warning to be displayed reminding the user to continue + running opam from a Cygwin/MSYS2 shell that has been manually started, + but is not displayed if they have permanently configured their PATH to + include Cygwin/MSYS2. *) + let env = OpamStubs.get_initial_environment () in + let cygcheck = + OpamSystem.resolve_command ~env:(Array.of_list env) "cygcheck.exe" + |> Option.map OpamSysInteract.Cygwin.analyse_install + in + begin match cygcheck with + | Some (Ok (kind2, root2)) -> + let root2 = OpamFilename.Dir.to_string root2 in + if (kind : [`Cygwin | `Msys2]) = kind2 && String.equal root root2 then + let default, warning = + if kind = `Msys2 && OpamSystem.resolve_command pacman = None then + internal_option, Some + (Printf.sprintf + "The current PATH gives an installation of MSYS2 at %s, \ + but it does not include the package manager, \ + pacman.exe (this is expected behaviour for the Git \ + Bash shell from Git for Windows). It's recommended \ + you use a full MSYS2 installation, rather than one \ + without its package manager." root) + else + path_option, None + in + options, default, warning + else + let warning = Printf.sprintf + "The current PATH gives an installation of %s at %s, but your \ + system appears to default to an installation of %s at %s for \ + new terminal sessions. You will need to ensure that the \ + correct installation is available in PATH when you run opam \ + in future." + (string_of_kind kind) root (string_of_kind kind2) root2 + in + options, internal_option, Some warning + | Some (Error _) -> + let warning = Printf.sprintf + "The current PATH gives an installation of %s at %s, but it \ + doesn't appear to be correctly available for new terminal \ + sessions. You will need to ensure that the correct \ + installation is available in PATH when you run opam in \ + future." (string_of_kind kind) root + in + options, internal_option, Some warning + | None -> + match OpamStd.Sys.guess_shell_compat () with + | SH_sh | SH_bash | SH_zsh | SH_csh | SH_fish -> + let default, warning = + if kind = `Msys2 && OpamSystem.resolve_command pacman = None then + internal_option, Printf.sprintf + "The current PATH gives an installation of MSYS2 at %s, \ + but it does not include the package manager, pacman.exe \ + (this is expected behaviour for the Git Bash shell from \ + Git for Windows). It's recommended you use a full MSYS2 \ + installation, rather than one without its package \ + manager.\n You will need to run opam from a terminal \ + session in future." + root + else + path_option, Printf.sprintf + "You will need to run opam from a terminal session for %s \ + in future." root + in + options, default, Some warning + | SH_pwsh _ | SH_cmd -> + let warning = Printf.sprintf + "You appear to have added %s to PATH for this session only. \ + You will need to do this again before running opam in future." + root + in + options, internal_option, Some warning + end + end + in + Lazy.force header; + OpamConsole.msg + "\n\ + opam and the OCaml ecosystem in general require various Unix tools in \ + order to operate correctly. At present, this requires the installation \ + of Cygwin to provide these tools.\n\n"; + match OpamConsole.menu "How should opam obtain Unix tools?" + ~default ~no:default ~options with + | `Chosen (kind, `Internal) -> + assert (kind = `Cygwin); + Some (kind, `Internal OpamInitDefaults.required_packages_for_cygwin) + | `Chosen (kind, ((`Root _) as mechanism)) -> + Some (kind, mechanism) + | `Chosen ((_, `Path _) as mechanism) -> + OpamStd.Option.iter (OpamConsole.warning "%s") warn_path; + Some mechanism + | `Specify -> + begin + match OpamConsole.read + "Enter the prefix of an existing Cygwin installation \ + (e.g. C:\\cygwin64)" with + | None -> None + | Some entry -> + match OpamSysInteract.Cygwin.analyse_install entry with + | Ok (kind, root) -> + Some (kind, `Root root) + | Error msg -> + OpamConsole.error "%s" msg; + cygwin_menu header + end + | `Abort -> OpamStd.Sys.exit_because `Aborted + +and test_mechanism header = function + | (`Internal _) as mechanism -> Some (`Cygwin, mechanism) + | `Path -> + let cygcheck = + OpamSystem.resolve_command "cygcheck.exe" + |> Option.map OpamSysInteract.Cygwin.analyse_install + in + begin match cygcheck with + | Some (Ok (kind, root)) -> + Some (kind, `Path (OpamFilename.Dir.to_string root)) + | Some (Error _) | None -> + None + end + | `Test dir -> + begin match OpamSysInteract.Cygwin.analyse_install dir with + | Ok (kind, root) -> Some (kind, `Root root) + | Error _ -> None + end + | `Location dir -> + begin match OpamSysInteract.Cygwin.analyse_install dir with + | Ok (kind, root) -> Some (kind, `Root root) + | Error msg -> + OpamConsole.error_and_exit `Not_found "%s" msg + end + | `Menu -> cygwin_menu header + +let string_of_cygwin_setup = function + | `internal pkgs -> + let pkgs = + if pkgs = [] then "" + else + " with " ^ String.concat ", " (List.map OpamSysPkg.to_string pkgs) + in + "Internal" ^ pkgs + | `default_location -> "Search" + | `location dir -> "External from " ^ OpamFilename.Dir.to_string dir + | `no -> "Path-only (and no tweaking)" + +let string_of_git_location_cli = function + | Left location -> "Using git-location=" ^ OpamFilename.Dir.to_string location + | Right () -> "git-location disabled via CLI" + +let initialise_msys2 root = + let bindir = OpamSysInteract.Cygwin.bindir_for_root `Msys2 root in + let pacman = OpamFilename.Op.(bindir // "pacman.exe") in + let gnupg_dir = OpamFilename.Op.(root / "etc" / "pacman.d" / "gnupg") in + if OpamFilename.exists pacman && not (OpamFilename.exists_dir gnupg_dir) then + let cmd = + OpamFilename.Op.(bindir // "bash.exe") + |> OpamFilename.to_string + in + let answer = + let cmd = OpamConsole.colorise `yellow (cmd ^ " -lc \"uname -a\"") in + OpamConsole.menu ~unsafe_yes:`Yes ~default:`Yes ~no:`Quit + "MSYS2 appears not to have been initialised. opam can:" + ~options:[ + `Yes, Printf.sprintf + "Run %s to initialise it" cmd; + `No, Printf.sprintf + "Wait while you %s manually (e.g. in another terminal)" cmd; + `Ignore, "Continue anyway (but note that external dependency \ + may not work correctly until MSYS2 is initialised)"; + `Quit, "Abort initialisation"; + ] + in + OpamConsole.msg "\n"; + match answer with + | `Yes -> + if OpamConsole.disp_status_line () then + OpamConsole.status_line "Initialising MSYS2 (this may take a minute)"; + let r = + OpamProcess.run + (OpamProcess.command ~name:(OpamSystem.temp_file "command") + ~allow_stdin:false cmd ["-lc"; "uname -a"]) + in + OpamProcess.cleanup ~force:true r; + OpamConsole.clear_status (); + if not (OpamProcess.is_success r) then + OpamConsole.error_and_exit `Aborted "MSYS2 failed to initialise" + | `No -> + OpamConsole.pause "Standing by, press enter to continue when done."; + OpamConsole.msg "\n" + | `Ignore -> + () + | `Quit -> + OpamStd.Sys.exit_because `Aborted + +let determine_windows_configuration ?cygwin_setup ?git_location config = + OpamStd.Option.iter + (log "Cygwin (from CLI): %a" (slog string_of_cygwin_setup)) cygwin_setup; + (* Check whether symlinks can be created. Developer Mode is not the only way + to do this, but it's the easiest. *) if (not (Unix.has_symlink ())) then begin OpamConsole.header_msg "Windows Developer Mode"; - OpamConsole.msg "opam does not require Developer Mode to be enabled on Windows, but it is\n\ - recommended, in particular because it enables support for symlinks without\n\ - requiring opam to be run elevated (which we do %s recommend doing).\n\ - \n\ - More information on enabling Developer Mode may be obtained from\n\ - https://learn.microsoft.com/en-gb/windows/apps/get-started/enable-your-device-for-development\n" - (OpamConsole.colorise `bold "not") + OpamConsole.msg + "opam does not require Developer Mode to be enabled on Windows, but it is\n\ + recommended, in particular because it enables support for symlinks without\n\ + requiring opam to be run elevated (which we do %s recommend doing).\n\ + \n\ + More information on enabling Developer Mode may be obtained from\n\ + https://learn.microsoft.com/en-gb/windows/apps/get-started/enable-your-device-for-development\n" + (OpamConsole.colorise `bold "not") end; - let vars = OpamFile.Config.global_variables config in - let env = - List.map (fun (v, c, s) -> v, (lazy (Some c), s)) vars - |> OpamVariable.Map.of_list - in - (* Git handling *) + + (* Augment ~git_location (from the CLI) with information from opamrc and + validate --git-location/git-location *) let git_location = match git_location, OpamFile.Config.git_location config with | None, None -> None @@ -864,304 +1304,230 @@ let windows_checks ?cygwin_setup ?git_location config = check_git_location_or_exit git_location "--git-location"; result in - let git_location = - if Sys.win32 then - git_for_windows ?git_location () - else - None + OpamStd.Option.iter (log "%a" (slog string_of_git_location_cli)) git_location; + + (* Checks and initialisation for both Cygwin/MSYS2 and Git (which is made + mandatory on Windows) + + The aim of this process is to determine four things: + - An optional directory containing git.exe but not shadowing any of + the executables in OpamEnv.cygwin_non_shadowed_programs. This is written + to git-location in ~/.opam/config and the resulting directory appears + as the first entry for Path on opam process calls + (see OpamStd.Env.cyg_env) + - Whether sys-pkg-manager-cmd should contain entries for either "cygwin" + or "msys2". The presence of one of those values also causes opam to add + the directory containing the package manager to Path + (see OpamCoreConfig.cygbin) + - Whether an internal installation of Cygwin is required, and if it needs + the git package + + The process is affected by various CLI options: + - --no-git-location causes git-location in opamrc to be ignored + - --git-location overrides git-location in opamrc and short-circuits + searching PATH for git.exe + - --no-cygwin-setup specifies that Cygwin/MSYS2 should be found in PATH + and no additional handling should be done + - --cygwin-internal-install specifies that opam should maintain its own + internal installation of Cygwin and make that fully available on Path + when building packages and executing commands internally. If + --git-location is not in use, and git.exe is not already installed, this + installation may include Cygwin's git package + - --cygwin-local-install specifies that opam should either search for + Cygwin/MSYS2 installations or, if --cygwin-location is specified, use + the Cygwin/MSYS2 installation specified. + *) + + let apply_git_location config git_location = + let config = OpamFile.Config.with_git_location git_location config in + let git_location = OpamFilename.Dir.to_string git_location in + OpamCoreConfig.update ~git_location (); + config in - OpamCoreConfig.update ?git_location (); - let config = + + (* If --git-location has been specified, apply it now *) + let config, git_location, git_determined, git_required_from_cygwin = match git_location with - | Some git_location -> - OpamFile.Config.with_git_location - (OpamFilename.Dir.of_string 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 distrib, "Set by opam init")::vars) - config - in - match OpamStd.List.pick (fun (v,_,_) -> - OpamVariable.equal v os_distribution) - vars with - | Some (_, S d, _), _ when String.equal d distrib -> config - | None, vars -> update vars - | Some (_, vc, _), vars -> - OpamConsole.warning - "'os-distribution' already set to another value %s" - (OpamVariable.string_of_variable_contents vc); - if OpamConsole.confirm ~default:false "Override?" then - (OpamConsole.msg - "You can revert this setting using \ - 'opam var --global os-distribution=%s'" - (OpamVariable.string_of_variable_contents vc); - update vars) - else - OpamStd.Sys.exit_because `Aborted + | Some (Left git_location) -> + apply_git_location config git_location, Some git_location, true, false + | Some (Right ()) -> + config, None, true, (OpamSystem.resolve_command "git.exe" = None) + | None -> + config, None, false, false + in + + (* Based on the supplied command line options, determine which mechanisms can + be tried to acquire a Unix environment. + mechanisms - list of things to try from: + `Path - search for cygcheck.exe in PATH and test from there + `Test - search given root directory for cygcheck.exe (either in bin + or usr\bin) + `Location - as `Test, but _must_ succeed (--cygwin-location) + `Internal - create a Cygwin internal with the given packages + `Menu - interactive mode permitted + tweakable - can pacman / Cygwin setup be used to adjust setup + (--no-cygwin-setup disables this) + *) + let mechanisms, cygwin_tweakable = + match cygwin_setup with + | Some (`internal packages) -> + (* git, if needed, will be added later *) + let packages = OpamInitDefaults.required_packages_for_cygwin @ packages in + Seq.return (`Internal packages), true + | Some `no -> + if git_required_from_cygwin then + OpamConsole.error_and_exit `Not_found + "Both --no-cygwin-setup and --no-git-location have been specified, \ + but Git was not found in PATH. opam requires Git - please either \ + install Git for Windows and make it available in PATH or re-run \ + opam init with less restrictive command line options." + else + Seq.return `Path, false + | Some `default_location -> + cygwin_searches ~first:`Path (), true + | Some (`location dir) -> + Seq.return (`Location (OpamFilename.Dir.to_string dir)), true + | None -> + Seq.return `Menu, true + in + + let header = lazy (OpamConsole.header_msg "Unix support infrastructure") in + + (* Reduce mechanisms to a single mechanism (which may therefore display a + menu). *) + let kind, mechanism = + match OpamCompat.Seq.find_map (test_mechanism header) mechanisms with + | Some result -> result + | None -> + Lazy.force header; + OpamConsole.error_and_exit `Not_found + "A solution for Unix infrastructure is required, but the options \ + given to opam have not yielded one!" + in + + (* If --git-location is in use, then there's no further checking required on + the Git executable. If not, then before cygbin is potentially applied + through --cygwin-location, determine if we need to check that Git for + Windows is not going to be shadowed. *) + let have_git_for_windows_in_path, git_in_path_dir = + if git_location = None then + match OpamSystem.resolve_command "git.exe" with + | Some git -> + is_git_for_windows git, Filename.dirname git + | None -> + false, "" + else + false, "" + in + + (* Apply cygbin, if necessary *) + let config, msys2_check_root = + let apply cygcheck = + let cygbin = OpamFilename.Dir.to_string (OpamFilename.dirname cygcheck) in + OpamCoreConfig.update ~cygbin (); + let family = match kind with `Msys2 -> "msys2" | `Cygwin -> "cygwin" in + OpamFile.Config.with_sys_pkg_manager_cmd + (OpamStd.String.Map.add family cygcheck + (OpamFile.Config.sys_pkg_manager_cmd config)) + config in - let config = - if is_msys2 cygcheck then - let env = - OpamStd.Env.cyg_env ~cygbin:(OpamFilename.Dir.to_string cygbin) - ~git_location:None ~env:(OpamStd.Env.raw_env ()) + let open OpamFilename.Op in + let config, msys2_check_root = + match mechanism with + | `Path root -> + let msys2_check_root = + if kind = `Msys2 then + Some (OpamFilename.Dir.of_string root) + else + 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 + (* For opam init --reinit, it may be necessary to remove + sys-pkg-manager-path *) OpamFile.Config.with_sys_pkg_manager_cmd - (OpamStd.String.Map.add distrib cygcheck - (OpamFile.Config.sys_pkg_manager_cmd config)) - config + OpamStd.String.Map.empty config, msys2_check_root + | `Internal _ -> + (* The directory gets applied, but obviously it's not yet been + installed *) + let cygcheck = + OpamSysInteract.Cygwin.internal_cygroot () / "bin" // "cygcheck.exe" + in + apply cygcheck, None + | `Root root -> + let bindir = OpamSysInteract.Cygwin.bindir_for_root kind root in + (* If the user has specified --no-git-location and Git for Windows was + in PATH and the given location occludes it, then this is our last + chance to warn about it. *) + if git_determined && have_git_for_windows_in_path && + OpamFilename.exists (bindir // "git.exe") then + OpamConsole.warning + "Git for Windows is in PATH (from %s), but it will be shadowed \ + when opam builds packages and executes commands internally. It is \ + recommended that only Git for Windows is used, and this could be \ + ensured by uninstalling the Git package from %s" + git_in_path_dir (OpamFilename.Dir.to_string bindir); + if kind = `Msys2 then + apply (bindir // "pacman.exe"), Some root + else + apply (bindir // "cygcheck.exe"), None in - OpamConsole.note "Configured with %s for depexts" - (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 - (* cygcheck is in MSYS2ROOT/usr/bin *) - Printf.sprintf "MSYS2 at %s" - OpamFilename.(Dir.to_string (dirname_dir (dirname_dir cygbin)))); - config + config, msys2_check_root in - let install_cygwin_tools packages = - let default_packages = OpamInitDefaults.required_packages_for_cygwin in - let default_packages = - if OpamSystem.resolve_command "git" = None then - OpamSysPkg.of_string "git" :: default_packages - else - default_packages - in - (* packages comes last so that the user can override any potential version - constraints in default_packages (although, with the current version of - setup, and with the list of default_packages in OpamInitDefaults, this at - present doesn't matter too much). *) - OpamSysInteract.Cygwin.install (default_packages @ packages) - in - let header () = OpamConsole.header_msg "Unix support infrastructure" in - - let get_cygwin = function - | Some cygcheck when OpamFilename.exists cygcheck && is_variant cygcheck -> - success cygcheck - | Some _ | None -> - let rec menu () = - let enter_paths () = - let prompt_setup () = - let options = [ - `download, "Let opam downloads it"; - `manual, "Manually enter its location on disk"; - `abort, "Abort initialisation"; - ] - in - OpamConsole.menu - "Opam needs Cygwin setup executable 'setup-x86_64.exe'" - ~default:`download ~no:`download ~options - in - let rec enter_setup () = - match prompt_setup () with - | `abort -> OpamStd.Sys.exit_because `Aborted - | `download -> None - | `manual -> - match OpamConsole.read "Enter path of Cygwin setup executable:" with - | None -> None - | Some setup -> - let setup = OpamFilename.of_string setup in - if OpamFilename.exists setup then Some setup else - (OpamConsole.msg "Cygwin setup executable doesn't exist at %s\n" - (OpamFilename.to_string setup); - enter_setup ()) - in - (* Check for default cygwin installation path *) - let default = - match OpamSysInteract.Cygwin.(analyse_install default_cygroot) with - | Ok (_, cygcheck) -> - let prompt_cygroot () = - let options = [ - `manual, - "Manually enter prefix of an existing Cygwin installation \ - (e.g. D:\\cygwin64)"; - `default, - (Printf.sprintf "Use default Cygwin installation at %s" - OpamSysInteract.Cygwin.default_cygroot); - `abort, "Abort initialisation"; - ] in - OpamConsole.menu "Cygwin location" - ~default:`default ~no:`default ~options - in - (match prompt_cygroot () with - | `abort -> OpamStd.Sys.exit_because `Aborted - | `manual -> None - | `default -> Some cygcheck) - | Error _ -> None - in - (* Otherwise, ask for prefix *) - let cygcheck = - match default with - | Some cygcheck -> Some cygcheck - | None -> - match OpamConsole.read - "Enter the prefix of an existing Cygwin installation \ - (e.g. C:\\cygwin64)" with - | None -> None - | Some entry -> - let cygcheck = - OpamSysInteract.Cygwin.analyse_install entry - in - match cygcheck with - | Ok (_, cygcheck) -> Some cygcheck - | Error msg -> OpamConsole.error "%s" msg; None - in - (* And finally ask for setup.exe *) - match cygcheck with - | Some cygcheck -> - if is_cygwin cygcheck then - OpamSysInteract.Cygwin.check_setup (enter_setup ()); - Some (success cygcheck) - | None -> None - in - let prompt () = - let options = [ - `Internal, - "Automatically create an internal Cygwin installation \ - that will be managed by opam (recommended)"; - `Specify, "Enter the location of an existing Cygwin installation"; - `Abort, "Abort initialisation"; - ] in - OpamConsole.menu "How should opam handle Cygwin?" - ~no:`Internal ~options + + (* Display the menu for Git configuration, if possible and required *) + let config, mechanism, cygwin_packages, git_location = + let mechanism, cygwin_packages = + match mechanism with + | `Internal pkgs -> + `Internal, pkgs + | (`Root _ | `Path _) as mechanism -> + let cygwin_packages = + if cygwin_tweakable && not OpamStateConfig.(!r.no_depexts) then + OpamInitDefaults.required_packages_for_cygwin + else + [] in - match prompt () with - | `Abort -> OpamStd.Sys.exit_because `Aborted - | `Internal -> - let cygcheck = install_cygwin_tools [] in - let config = success cygcheck in - config - | `Specify -> - match enter_paths () with - | Some config -> config - | None -> menu () + mechanism, cygwin_packages + in + if git_location = None && not git_determined + && not have_git_for_windows_in_path then + let git_location, from_cygwin = + git_for_windows kind mechanism cygwin_tweakable in - header (); - OpamConsole.msg - "\n\ - opam and the OCaml ecosystem in general require various Unix tools \ - in order to operate correctly. At present, this requires the \ - installation of Cygwin to provide these tools.\n\n"; - menu () - in - let config = - match cygwin_setup with - | Some `no -> config - | (Some (`internal _ | `default_location | `location _) | None) - as cygwin_setup -> - if OpamSysPoll.os env = Some "win32" then - match OpamSysPoll.os_distribution env with - | Some "win32" -> - (* If there's a "cygwin" entry in sys-pkg-manager-cmd, but - os-distribution hasn't (yet) been set to "cygwin", then that'll be - done here. Otherwise, the user must either allow opam to install - Cygwin or must provide the path to it. - Note that a depext solution is _mandatory_ on Windows for now, - because there are commands opam requires which are only provided - using it (patch, etc.). MSYS2 avoids this by requiring - os-distribution to be set. *) - let cygcheck = - OpamStd.String.Map.find_opt "cygwin" - (OpamFile.Config.sys_pkg_manager_cmd config) - in - (match cygwin_setup with - | None -> get_cygwin cygcheck - | Some setup -> - header (); - let cygcheck = - match setup with - | `internal pkgs -> install_cygwin_tools pkgs - | (`default_location | `location _ as setup) -> - let cygroot = - match setup with - | `default_location -> OpamSysInteract.Cygwin.default_cygroot - | `location dir -> OpamFilename.Dir.to_string dir - in - (match OpamSysInteract.Cygwin.analyse_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) - in - if is_cygwin cygcheck then - OpamSysInteract.Cygwin.check_setup None; - success cygcheck) - | Some "cygwin" | Some "msys2" -> - (* We check that current install is good *) - (match OpamSysInteract.Cygwin.cygroot_opt config with - | Some cygroot -> - (match OpamSysInteract.Cygwin.analyse_install - (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 -> - (* 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 - ) - | _ -> config - else - config + let config = + OpamStd.Option.map_default (apply_git_location config) + config git_location + in + let cygwin_packages = + if cygwin_tweakable && from_cygwin then + OpamSysPkg.of_string "git" :: cygwin_packages + else + cygwin_packages + in + config, mechanism, cygwin_packages, git_location + else + config, mechanism, cygwin_packages, git_location in - 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 + + log "Unix support mechanism: %s %s" (string_of_kind kind) + (match mechanism with + | `Path root -> Printf.sprintf "from PATH (%s)" root + | `Internal -> "internal installation" + | `Root root -> + "local installation at " ^ OpamFilename.Dir.to_string root); + if cygwin_packages <> [] then + log "Systems packages to check for: %s" + (String.concat ", " (List.map OpamSysPkg.to_string cygwin_packages)); + log "git-location %s" + (OpamStd.Option.map_default + (fun d -> Printf.sprintf "= %s" (OpamFilename.Dir.to_string d)) + "is not in use" git_location); + + let mechanism, cygwin_packages = + match mechanism with + | `Path _ | `Root _ -> None, cygwin_packages + | `Internal -> Some cygwin_packages, [] in - OpamCoreConfig.update ?cygbin (); - config + config, mechanism, cygwin_packages, msys2_check_root let update_with_init_config ?(overwrite=false) config init_config = let module I = OpamFile.InitConfig in @@ -1196,14 +1562,41 @@ let update_with_init_config ?(overwrite=false) config init_config = setifnew C.git_location C.with_git_location_opt (I.git_location init_config) +let check_for_sys_packages config system_packages = + if system_packages <> [] then + let ((missing, _) as set) = + OpamSysInteract.packages_status config + (OpamSysPkg.Set.of_list system_packages) + in + if not (OpamSysPkg.Set.is_empty missing) then + let vars = OpamFile.Config.global_variables config in + let env = + List.map (fun (v, c, s) -> v, (lazy (Some c), s)) vars + |> OpamVariable.Map.of_list + in + (*Lazy.force header;*) + OpamSolution.print_depext_msg set; + OpamSolution.install_sys_packages ~confirm:true env config missing () + let reinit ?(init_config=OpamInitDefaults.init_config()) ~interactive ?dot_profile ?update_config ?env_hook ?completion ?inplace ?(check_sandbox=true) ?(bypass_checks=false) ?cygwin_setup ?git_location config shell = + log "RE-INIT"; let root = OpamStateConfig.(!r.root_dir) in let config = update_with_init_config config init_config in - let config = windows_checks ?cygwin_setup ?git_location config in + let config, mechanism, system_packages, msys2_check_root = + if Sys.win32 then + determine_windows_configuration ?cygwin_setup ?git_location config + else + config, None, [], None + in + + OpamStd.Option.iter initialise_msys2 msys2_check_root; + OpamStd.Option.iter OpamSysInteract.Cygwin.install mechanism; + check_for_sys_packages config system_packages; + let _all_ok = if bypass_checks then false else init_checks ~hard_fail_exn:false init_config @@ -1280,7 +1673,16 @@ let init init_config |> OpamFile.Config.with_repositories (List.map fst repos) in - let config = windows_checks ?cygwin_setup ?git_location config in + let config, mechanism, system_packages, msys2_check_root = + if Sys.win32 then + determine_windows_configuration ?cygwin_setup ?git_location config + else + config, None, [], None + in + + OpamStd.Option.iter initialise_msys2 msys2_check_root; + OpamStd.Option.iter OpamSysInteract.Cygwin.install mechanism; + check_for_sys_packages config system_packages; let dontswitch = if bypass_checks then false else diff --git a/src/core/opamStd.ml b/src/core/opamStd.ml index 899f63f876a..ddcf8c3de41 100644 --- a/src/core/opamStd.ml +++ b/src/core/opamStd.ml @@ -1402,20 +1402,6 @@ module OpamSys = struct let is_cygwin_variant ?search_in_first cmd = get_cygwin_variant ?search_in_first 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 ?search_in_first:(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 index c0111260411..3ba4aa261f0 100644 --- a/src/core/opamStd.mli +++ b/src/core/opamStd.mli @@ -576,15 +576,6 @@ module Sys : sig val get_windows_executable_variant: ?search_in_first:string -> string -> [ `Native | `Cygwin | `Tainted of [ `Msys2 | `Cygwin] | `Msys2 ] - (** Determines if cygcheck in given cygwin binary directory comes from a - 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 - (** Behaviour is largely as {!get_windows_executable_variant} but where MSYS2 and Cygwin are seen as equivalent. diff --git a/src/state/opamSysInteract.ml b/src/state/opamSysInteract.ml index 232d7e8520d..dd0478e4f24 100644 --- a/src/state/opamSysInteract.ml +++ b/src/state/opamSysInteract.ml @@ -364,10 +364,7 @@ module Cygwin = struct args @@> fun r -> OpamSystem.raise_on_process_error r; set_fstab_noacl fstab; - Done ()); - cygcheck - - let default_cygroot = "C:\\cygwin64" + Done ()) let analysis_cache = Hashtbl.create 17 @@ -440,11 +437,8 @@ module Cygwin = struct match r.OpamProcess.r_stdout with | [] -> Error ("Unexpected error translating \"/\" with " ^ cygpath) - | _::_ -> - let cygcheck = - OpamFilename.of_string (Filename.concat dir "cygpath.exe") - in - Ok (kind, cygcheck) + | l::_ -> + Ok (kind, OpamFilename.Dir.of_string l) else Error ("Could not determine the root for " ^ cygpath) in @@ -453,6 +447,12 @@ module Cygwin = struct in Result.bind cygbin identify + let bindir_for_root kind root = + let open OpamFilename.Op in + match kind with + | `Msys2 -> root / "usr" / "bin" + | `Cygwin -> root / "bin" + (* Set setup.exe in the good place, ie in .opam/.cygwin/ *) let check_setup setup = let dst = cygsetup () in diff --git a/src/state/opamSysInteract.mli b/src/state/opamSysInteract.mli index 8ce6eac17ee..d9df5ba3b3b 100644 --- a/src/state/opamSysInteract.mli +++ b/src/state/opamSysInteract.mli @@ -43,11 +43,11 @@ val repo_enablers: ?env:gt_variables -> OpamFile.Config.t -> string option module Cygwin : sig - (* Default Cygwin installation prefix C:\cygwin64 *) - val default_cygroot: string + (* Location of the internal Cygwin installation *) + val internal_cygroot: unit -> OpamFilename.Dir.t (* Install an internal Cygwin install, in /.cygwin *) - val install: OpamSysPkg.t list -> OpamFilename.t + val install: OpamSysPkg.t list -> unit (* [analyse_install path] searches for and identifies Cygwin/MSYS2 installations. [path] may be able the location of cygcheck.exe itself @@ -61,10 +61,15 @@ module Cygwin : sig pacman.exe in the same directory as cygcheck.exe and cygpath.exe. On success, the result is the kind of installation (Cygwin/MSYS2) along - with the full path to cygcheck.exe, otherwise a description of the problem - encountered is returned. *) + with the root directory (e.g. {v C:\cygwin64 v} or {v C:\msys64 v}), + otherwise a description of the problem encountered is returned. *) val analyse_install: - string -> ([ `Cygwin | `Msys2 ] * OpamFilename.t, string) result + string -> ([ `Cygwin | `Msys2 ] * OpamFilename.Dir.t, string) result + + (* [bindir_for_root kind root] returns the bin directory for the given + installation root and [kind], as returned by {!analyse_install}. *) + val bindir_for_root: + [ `Cygwin | `Msys2 ] -> OpamFilename.Dir.t -> OpamFilename.Dir.t (* Returns true if Cygwin install is internal *) val is_internal: OpamFile.Config.t -> bool @@ -77,9 +82,6 @@ module Cygwin : sig (* Return Cygwin binary path *) val cygbin_opt: OpamFile.Config.t -> OpamFilename.Dir.t option - (* Return Cygwin installation prefix *) - val cygroot_opt: OpamFile.Config.t -> OpamFilename.Dir.t option - (* Return MSYS2 binary path *) val msys2bin_opt: OpamFile.Config.t -> OpamFilename.Dir.t option end From affdbabb26d779aa1e244d8302319f54f3e698c3 Mon Sep 17 00:00:00 2001 From: David Allsopp Date: Fri, 7 Jun 2024 11:36:44 +0100 Subject: [PATCH 14/15] Make --bypass-checks => --no-cygwin-setup --- src/client/opamClient.ml | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/src/client/opamClient.ml b/src/client/opamClient.ml index c0257940735..0fa35bc1d17 100644 --- a/src/client/opamClient.ml +++ b/src/client/opamClient.ml @@ -1004,10 +1004,10 @@ let cygwin_searches ?first () = in seq cygwin_searches -let rec cygwin_menu header = +let rec cygwin_menu ~bypass_checks header = let start = Unix.gettimeofday () in let test_mechanism (roots, count, mechanisms) search = - match test_mechanism header search with + match test_mechanism ~bypass_checks header search with | Some ((kind, `Root root) as mechanism) -> if OpamFilename.Dir.Set.mem root roots then roots, count, mechanisms @@ -1068,6 +1068,9 @@ let rec cygwin_menu header = "Use tools found in PATH (%s installation at %s)" (string_of_kind kind) root)::options in + if bypass_checks then + options, path_option, None + else (* Check whether cygcheck is still available in the initial environment. This allows a warning to be displayed reminding the user to continue running opam from a Cygwin/MSYS2 shell that has been manually started, @@ -1173,11 +1176,11 @@ let rec cygwin_menu header = Some (kind, `Root root) | Error msg -> OpamConsole.error "%s" msg; - cygwin_menu header + cygwin_menu ~bypass_checks header end | `Abort -> OpamStd.Sys.exit_because `Aborted -and test_mechanism header = function +and test_mechanism ~bypass_checks header = function | (`Internal _) as mechanism -> Some (`Cygwin, mechanism) | `Path -> let cygcheck = @@ -1201,7 +1204,7 @@ and test_mechanism header = function | Error msg -> OpamConsole.error_and_exit `Not_found "%s" msg end - | `Menu -> cygwin_menu header + | `Menu -> cygwin_menu ~bypass_checks header let string_of_cygwin_setup = function | `internal pkgs -> @@ -1264,7 +1267,8 @@ let initialise_msys2 root = | `Quit -> OpamStd.Sys.exit_because `Aborted -let determine_windows_configuration ?cygwin_setup ?git_location config = +let determine_windows_configuration ?cygwin_setup ?git_location + ~bypass_checks config = OpamStd.Option.iter (log "Cygwin (from CLI): %a" (slog string_of_cygwin_setup)) cygwin_setup; (* Check whether symlinks can be created. Developer Mode is not the only way @@ -1368,6 +1372,13 @@ let determine_windows_configuration ?cygwin_setup ?git_location config = tweakable - can pacman / Cygwin setup be used to adjust setup (--no-cygwin-setup disables this) *) + (* --bypass-checks => --no-cygwin-setup if nothing else was specified *) + let cygwin_setup = + if bypass_checks && cygwin_setup = None then + Some `no + else + cygwin_setup + in let mechanisms, cygwin_tweakable = match cygwin_setup with | Some (`internal packages) -> @@ -1396,7 +1407,8 @@ let determine_windows_configuration ?cygwin_setup ?git_location config = (* Reduce mechanisms to a single mechanism (which may therefore display a menu). *) let kind, mechanism = - match OpamCompat.Seq.find_map (test_mechanism header) mechanisms with + let test_mechanism = test_mechanism ~bypass_checks header in + match OpamCompat.Seq.find_map test_mechanism mechanisms with | Some result -> result | None -> Lazy.force header; @@ -1588,7 +1600,8 @@ let reinit ?(init_config=OpamInitDefaults.init_config()) ~interactive let config = update_with_init_config config init_config in let config, mechanism, system_packages, msys2_check_root = if Sys.win32 then - determine_windows_configuration ?cygwin_setup ?git_location config + determine_windows_configuration ?cygwin_setup ?git_location + ~bypass_checks config else config, None, [], None in @@ -1675,7 +1688,8 @@ let init in let config, mechanism, system_packages, msys2_check_root = if Sys.win32 then - determine_windows_configuration ?cygwin_setup ?git_location config + determine_windows_configuration ?cygwin_setup ?git_location + ~bypass_checks config else config, None, [], None in From c0cd8f466fdd596160704eca9cf63e8ae112f0f4 Mon Sep 17 00:00:00 2001 From: David Allsopp Date: Sat, 8 Jun 2024 10:36:42 +0100 Subject: [PATCH 15/15] Tweak OpamSysInteract.Cygwin.check_setup more The ability to copy setup-x86_64.exe is no longer needed - but restore the previous functionality which only downloaded setup if it didn't exist and use this when displaying a command to the user. In this mode, we only download setup-x86_64.exe so that the command we give to the user actually works. If we're actually going to run setup-x86_64.exe, then we download the latest version. --- src/client/opamSolution.ml | 6 ++++-- src/state/opamSysInteract.ml | 24 ++++++++---------------- src/state/opamSysInteract.mli | 9 +++++---- 3 files changed, 17 insertions(+), 22 deletions(-) diff --git a/src/client/opamSolution.ml b/src/client/opamSolution.ml index b0adf8673ea..905d53cd82d 100644 --- a/src/client/opamSolution.ml +++ b/src/client/opamSolution.ml @@ -1189,8 +1189,10 @@ let install_sys_packages ~map_sysmap ~confirm env config sys_packages t = | `Ignore -> bypass t | `Quit -> give_up_msg (); OpamStd.Sys.exit_because `Aborted and print_command sys_packages = + (* Ensure that setup-x86_64.exe exists, so that an invalid command is not + displayed to the user. *) if OpamSysPoll.os_distribution env = Some "cygwin" then - OpamSysInteract.Cygwin.check_setup None; + OpamSysInteract.Cygwin.check_setup ~update:false; let commands = OpamSysInteract.install_packages_commands ~env config sys_packages |> List.map (fun ((`AsAdmin c | `AsUser c), a) -> c::a) @@ -1222,7 +1224,7 @@ let install_sys_packages ~map_sysmap ~confirm env config sys_packages t = and auto_install t sys_packages = try if OpamSysPoll.os_distribution env = Some "cygwin" then - OpamSysInteract.Cygwin.check_setup None; + OpamSysInteract.Cygwin.check_setup ~update:true; OpamSysInteract.install ~env config sys_packages; (* handles dry_run *) map_sysmap (fun _ -> OpamSysPkg.Set.empty) t with Failure msg -> diff --git a/src/state/opamSysInteract.ml b/src/state/opamSysInteract.ml index dd0478e4f24..c14b8f1eb26 100644 --- a/src/state/opamSysInteract.ml +++ b/src/state/opamSysInteract.ml @@ -454,22 +454,9 @@ module Cygwin = struct | `Cygwin -> root / "bin" (* Set setup.exe in the good place, ie in .opam/.cygwin/ *) - let check_setup setup = + let check_setup ~update = let dst = cygsetup () in - match setup with - | Some setup -> - log "Copying %s into %s" - (OpamFilename.to_string setup) - (OpamFilename.to_string dst); - let sha512 = - OpamHash.compute ~kind:`SHA512 (OpamFilename.to_string setup) - in - OpamFilename.copy ~src:setup ~dst; - let checksum_file = OpamFilename.add_extension dst "sha512" in - OpamFilename.remove checksum_file; - OpamFilename.with_open_out_bin checksum_file @@ fun c -> - output_string c (OpamHash.contents sha512) - | None -> + if update || not (OpamFilename.exists dst) then OpamProcess.Job.run @@ download_setupexe dst end @@ -1159,8 +1146,13 @@ let update ?(env=OpamVariable.Map.empty) config = in match cmd with | None -> + (* Cygwin doesn't have an update database per se, but one is supposed to use + the most current setup program when downloading setup.ini (which is the + package database (cf. the --no-version-check option). + Also, when #5839 is addressed, we'll need to cache setup.ini, and that + will want to be updated here too. *) if family = Cygwin then - Cygwin.check_setup None + Cygwin.check_setup ~update:true else OpamConsole.warning "Unknown update command for %s, skipping system update" diff --git a/src/state/opamSysInteract.mli b/src/state/opamSysInteract.mli index d9df5ba3b3b..9254c9a00f6 100644 --- a/src/state/opamSysInteract.mli +++ b/src/state/opamSysInteract.mli @@ -74,10 +74,11 @@ module Cygwin : sig (* Returns true if Cygwin install is internal *) val is_internal: OpamFile.Config.t -> bool - (* [check_setup path] checks and store Cygwin setup executable. Is [path] is - [None], it downloads it, otherwise it copies it to - /.cygwin/setup-x86_64.exe. *) - val check_setup: OpamFilename.t option -> unit + (* [check_setup ~update] downloads and stores a Cygwin setup executable to + /.cygwin/setup-x86_64.exe. If [~update = false], this only + happens if the setup executable does not already exist, otherwise it is. + updated. *) + val check_setup: update:bool -> unit (* Return Cygwin binary path *) val cygbin_opt: OpamFile.Config.t -> OpamFilename.Dir.t option