Skip to content

Commit

Permalink
Merge pull request #5718 from rjbou/git-win
Browse files Browse the repository at this point in the history
Git on windows: check and advertise to use Git for Windows
  • Loading branch information
kit-ty-kate authored Jan 11, 2024
2 parents 5cd7e1b + 4952361 commit a358d94
Show file tree
Hide file tree
Showing 19 changed files with 264 additions and 38 deletions.
11 changes: 10 additions & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,15 @@ jobs:
- name: Test "static" binaries on Windows
if: endsWith(matrix.host, '-pc-cygwin') == false
run: ldd ./opam.exe | test "$(grep -v -F /cygdrive/c/Windows/)" = ''
- name: 'Upload opam binaries for Windows'
if: endsWith(matrix.host, '-pc-windows')
uses: actions/upload-artifact@v3
with:
name: opam-exe-${{ matrix.host }}-${{ matrix.ocamlv }}-${{ matrix.build }}
path: |
D:\Local\bin\opam.exe
D:\Local\bin\opam-installer.exe
D:\Local\bin\opam-putenv.exe
- name: Test (basic - Cygwin)
if: endsWith(matrix.host, '-pc-cygwin')
run: bash -exu .github/scripts/main/test.sh
Expand All @@ -219,7 +228,7 @@ jobs:
set Path=D:\Cache\ocaml-local\bin;%Path%
if "${{ matrix.host }}" equ "x86_64-pc-windows" call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Auxiliary\Build\vcvars64.bat"
if "${{ matrix.host }}" equ "i686-pc-windows" call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Auxiliary\Build\vcvars32.bat"
opam init --yes --bare default git+file://D:/opam-repository#${{ env.OPAM_TEST_REPO_SHA }} || exit /b 1
opam init --yes --bare default git+file://D:/opam-repository#${{ env.OPAM_TEST_REPO_SHA }} --no-git-location || exit /b 1
opam switch --yes create default ocaml-system || exit /b 1
opam env || exit /b 1
opam install --yes lwt || exit /b 1
Expand Down
3 changes: 3 additions & 0 deletions master_changes.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ users)
## Plugins

## Init
* Check and advertise to use Git for Windows [#5718 @rjbou - fix #5617]
* Add the `--git-location` and `--no-git-location` arguments [#5718 @rjbou]

## Config report

Expand All @@ -39,6 +41,7 @@ users)
## Show

## Var/Option
* Add a new git-location option on Windows [#5718 @rjbou]

## Update / Upgrade

Expand Down
11 changes: 8 additions & 3 deletions src/client/opamAction.ml
Original file line number Diff line number Diff line change
Expand Up @@ -531,10 +531,15 @@ let compilation_env t opam =
let cygwin_env =
match OpamSysInteract.Cygwin.cygbin_opt t.switch_global.config with
| Some cygbin ->
[ OpamTypesBase.env_update_resolved "PATH" EqPlus
(OpamFilename.Dir.to_string cygbin)
let cygbin = OpamFilename.Dir.to_string cygbin in
[ OpamTypesBase.env_update_resolved "PATH" EqPlus cygbin
~comment:"Cygwin path"
]
] @ (match OpamCoreConfig.(!r.git_location) with
| None -> []
| Some git_location ->
if String.equal cygbin git_location then [] else
[ OpamTypesBase.env_update_resolved "PATH" PlusEq
git_location ~comment:"Git binary path"])
| None -> []
in
let shell_sanitization = "shell env sanitization" in
Expand Down
6 changes: 5 additions & 1 deletion src/client/opamArg.ml
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,8 @@ let apply_global_options cli o =
(`A (List.map (fun s -> `String s) (Array.to_list Sys.argv)))
);
(* We need to retrieve very early cygwin root path to set up 'cygbin' config
field. It is retrieved from config file, and we use a low level reading of
field and git binary path.
It is retrieved from config file, and we use a low level reading of
that file instead of OpamStateConfig.safe_load to avoid multiple error
messages displayed if an error is detected in the config file. If there is
an error, or best effort notification, it will be highlighted after
Expand All @@ -596,6 +597,9 @@ let apply_global_options cli o =
{ pelem = String cygcheck; _}::_ ->
let cygbin = Filename.dirname cygcheck in
OpamCoreConfig.update ~cygbin ()
| Some { pelem = String "git-location"; _},
{ pelem = String git_location; _}::_ ->
OpamCoreConfig.update ~git_location ()
| _, element::elements -> aux (Some element) elements
in
aux None elements
Expand Down
145 changes: 129 additions & 16 deletions src/client/opamClient.ml
Original file line number Diff line number Diff line change
Expand Up @@ -635,12 +635,118 @@ 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 windows_checks ?cygwin_setup config =
let git_for_windows_check =
if not Sys.win32 && not Sys.cygwin then fun ?git_location:_ () -> None else
fun ?git_location () ->
let header () = OpamConsole.header_msg "Git" in
let contains_git 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 ->
Some (git, OpamSystem.bin_contains_bash p)
| None -> None)
in
let get_git_location ?git_location () =
let bin =
match git_location with
| Some _ -> git_location
| None ->
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
| Some _, false ->
OpamConsole.msg "Using Git from %s" git_location;
Some git_location
| Some _, true ->
OpamConsole.error
"A bash executable was found in %s, which will override \
Cygwin's bash. Please check your binary path."
git_location;
None
| None, _ ->
OpamConsole.error "No Git executable found in %s." git_location;
None
in
let rec loop ?git_location () =
match get_git_location ?git_location () with
| Some _ as git_location -> git_location
| 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 install recommended Git.";
]
in
OpamConsole.menu "Which Git should opam use?"
~default:`Default ~no:`Default ~options
in
match prompt () with
| `Default -> None
| `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.";
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 ->
if 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
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

let windows_checks ?cygwin_setup ?git_location config =
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 *)
let git_location : string option = git_for_windows_check ?git_location () in
OpamCoreConfig.update ?git_location ();
let config =
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 success cygcheck =
let config =
let os_distribution = OpamVariable.of_string "os-distribution" in
Expand Down Expand Up @@ -681,6 +787,16 @@ let windows_checks ?cygwin_setup config =
OpamFilename.(Dir.to_string (dirname_dir (dirname cygcheck))));
config
in
let install_cygwin_tools () =
let 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
in
OpamSysInteract.Cygwin.install ~packages
in
let header () = OpamConsole.header_msg "Unix support infrastructure" in
let get_cygwin = function
| Some cygcheck
Expand Down Expand Up @@ -777,10 +893,7 @@ let windows_checks ?cygwin_setup config =
match prompt () with
| `Abort -> OpamStd.Sys.exit_because `Aborted
| `Internal ->
let cygcheck =
OpamSysInteract.Cygwin.install
~packages:OpamInitDefaults.required_packages_for_cygwin
in
let cygcheck = install_cygwin_tools () in
let config = success cygcheck in
config
| `Specify ->
Expand Down Expand Up @@ -819,9 +932,7 @@ let windows_checks ?cygwin_setup config =
header ();
let cygcheck =
match setup with
| `internal ->
OpamSysInteract.Cygwin.install
~packages:OpamInitDefaults.required_packages_for_cygwin
| `internal -> install_cygwin_tools ()
| (`default_location | `location _ as setup) ->
let cygroot =
match setup with
Expand Down Expand Up @@ -861,10 +972,11 @@ let windows_checks ?cygwin_setup config =
else
config
in
OpamCoreConfig.update
?cygbin:OpamStd.Option.Op.(
OpamSysInteract.Cygwin.cygbin_opt config
>>| OpamFilename.Dir.to_string) ();
let cygbin = OpamStd.Option.Op.(
OpamSysInteract.Cygwin.cygbin_opt config
>>| OpamFilename.Dir.to_string)
in
OpamCoreConfig.update ?cygbin ();
config

let update_with_init_config ?(overwrite=false) config init_config =
Expand Down Expand Up @@ -898,11 +1010,12 @@ let update_with_init_config ?(overwrite=false) config init_config =

let reinit ?(init_config=OpamInitDefaults.init_config()) ~interactive
?dot_profile ?update_config ?env_hook ?completion ?inplace
?(check_sandbox=true) ?(bypass_checks=false) ?cygwin_setup
?(check_sandbox=true) ?(bypass_checks=false)
?cygwin_setup ?git_location
config shell =
let root = OpamStateConfig.(!r.root_dir) in
let config = update_with_init_config config init_config in
let config = windows_checks ?cygwin_setup config in
let config = windows_checks ?cygwin_setup ?git_location config in
let _all_ok =
if bypass_checks then false else
init_checks ~hard_fail_exn:false init_config
Expand Down Expand Up @@ -943,7 +1056,7 @@ let init
?repo ?(bypass_checks=false)
?dot_profile ?update_config ?env_hook ?(completion=true)
?(check_sandbox=true)
?cygwin_setup
?cygwin_setup ?git_location
shell =
log "INIT %a"
(slog @@ OpamStd.Option.to_string OpamRepositoryBackend.to_string) repo;
Expand Down Expand Up @@ -979,7 +1092,7 @@ let init
init_config |>
OpamFile.Config.with_repositories (List.map fst repos)
in
let config = windows_checks ?cygwin_setup config in
let config = windows_checks ?cygwin_setup ?git_location config in

let dontswitch =
if bypass_checks then false else
Expand Down
2 changes: 2 additions & 0 deletions src/client/opamClient.mli
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ val init:
?completion:bool ->
?check_sandbox:bool ->
?cygwin_setup: [ `internal | `default_location | `location of dirname | `no ] ->
?git_location:(dirname, unit) either ->
shell ->
rw global_state * unlocked repos_state * atom list

Expand All @@ -46,6 +47,7 @@ val reinit:
?update_config:bool -> ?env_hook:bool -> ?completion:bool -> ?inplace:bool ->
?check_sandbox:bool -> ?bypass_checks:bool ->
?cygwin_setup: [ `internal | `default_location | `location of dirname | `no ] ->
?git_location:(dirname, unit) either ->
OpamFile.Config.t -> shell -> unit

(** Install the given list of packages. [add_to_roots], if given, specifies that
Expand Down
1 change: 1 addition & 0 deletions src/client/opamClientConfig.mli
Original file line number Diff line number Diff line change
Expand Up @@ -165,4 +165,5 @@ val opam_init:
?merged_output:bool ->
?precise_tracking:bool ->
?cygbin:string ->
?git_location:string ->
unit -> unit
36 changes: 32 additions & 4 deletions src/client/opamCommands.ml
Original file line number Diff line number Diff line change
Expand Up @@ -327,12 +327,31 @@ let init cli =
else
Term.const None
in
let git_location =
if Sys.win32 then
mk_opt ~cli (cli_from ~experimental:true cli2_2)
["git-location"] "DIR"
"Specify git binary directory. \
Ensure that it doesn't contains bash in the same directory"
Arg.(some dirname) None
else
Term.const None
in
let no_git_location =
if Sys.win32 then
mk_flag ~cli (cli_from ~experimental:true cli2_2)
["no-git-location"]
"Don't specify nor ask to specify git binary directory."
else
Term.const false
in

let init global_options
build_options repo_kind repo_name repo_url
interactive update_config completion env_hook no_sandboxing shell
dot_profile_o compiler no_compiler config_file no_config_file reinit
show_opamrc bypass_checks
cygwin_internal cygwin_location
cygwin_internal cygwin_location git_location no_git_location
() =
apply_global_options cli global_options;
apply_build_options cli build_options;
Expand Down Expand Up @@ -401,6 +420,15 @@ let init cli =
| (`default_location | `none), Some dir -> Some (`location dir)
| (`internal | `default_location | `no) as setup, None -> Some setup
in
let git_location =
match git_location, no_git_location with
| Some _, true ->
OpamConsole.error_and_exit `Bad_arguments
"Options --no-git-location and --git-location are incompatible";
| None, false -> None
| Some d, false -> Some (Left d)
| None, true -> Some (Right ())
in
if already_init then
if reinit then
let init_config =
Expand All @@ -410,7 +438,7 @@ let init cli =
let reinit conf =
OpamClient.reinit ~init_config ~interactive ?dot_profile
?update_config ?env_hook ?completion ~inplace ~bypass_checks
~check_sandbox:(not no_sandboxing) ?cygwin_setup
~check_sandbox:(not no_sandboxing) ?cygwin_setup ?git_location
conf shell
in
let config =
Expand Down Expand Up @@ -450,7 +478,7 @@ let init cli =
?repo ~bypass_checks ?dot_profile
?update_config ?env_hook ?completion
~check_sandbox:(not no_sandboxing)
?cygwin_setup
?cygwin_setup ?git_location
shell
in
OpamStd.Exn.finally (fun () -> OpamRepositoryState.drop rt)
Expand Down Expand Up @@ -499,7 +527,7 @@ let init cli =
$setup_completion $env_hook $no_sandboxing $shell_opt cli
cli_original $dot_profile_flag cli cli_original $compiler
$no_compiler $config_file $no_config_file $reinit $show_default_opamrc
$bypass_checks $cygwin_internal $cygwin_location)
$bypass_checks $cygwin_internal $cygwin_location $git_location $no_git_location)

(* LIST *)
let list_doc = "Display the list of available packages."
Expand Down
Loading

0 comments on commit a358d94

Please sign in to comment.