Skip to content

Commit

Permalink
Redirect opam on Windows if path contains a space.
Browse files Browse the repository at this point in the history
It is needed for Cygwin installation, that doesn't handle paths with
space.
At init, detection and redirection are done, afterwards opam always load
redirected opam root.
Original root directory is stored in
`OpamStateConfig.!r.original_root_dir`.
  • Loading branch information
rjbou authored and kit-ty-kate committed Jun 7, 2024
1 parent e8e6564 commit 1883aac
Show file tree
Hide file tree
Showing 7 changed files with 107 additions and 77 deletions.
1 change: 1 addition & 0 deletions master_changes.md
Original file line number Diff line number Diff line change
Expand Up @@ -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]
* Redirect the opam root to C:\opamroot when the opam root contains spaces on Windows [#5457 @rjbou]

## Config report

Expand Down
37 changes: 36 additions & 1 deletion src/client/opamClient.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1235,7 +1235,42 @@ let init
shell =
log "INIT %a"
(slog @@ OpamStd.Option.to_string OpamRepositoryBackend.to_string) repo;
let root = OpamStateConfig.(!r.root_dir) in
let root =
let root = OpamStateConfig.(!r.root_dir) in
let has_space s = OpamStd.String.contains_char s ' ' in
if Sys.win32 &&
has_space (OpamFilename.Dir.to_string root) then
(let default = "C:\\opamroot" in
let rec ask () =
match OpamConsole.read "Opam root: " with
| Some r ->
if has_space r then
(OpamConsole.msg
"Given path '%s' contains space, please choose another one.\n"
(OpamConsole.colorise `bold r);
ask ())
else r
| None -> default
in
let new_root_f =
if OpamConsole.confirm ~default:false
"Your opam root path '%s' contains a space, we'll redirect to \
'%s'.\nDo you want to choose and enter another spaceless folder?"
(OpamFilename.Dir.to_string root) default then
ask ()
else default
in
let new_root = OpamFilename.Dir.of_string new_root_f in
OpamFilename.write (OpamPath.redirected root) new_root_f;
(* Add the readme file in C:\opamroot as redirected *)
OpamFilename.write
OpamFilename.Op.(root // "readme.txt")
(Printf.sprintf "Opam root redirected from %s"
(OpamFilename.Dir.to_string OpamStateConfig.(!r.root_dir)));
OpamStateConfig.update ~root_dir:new_root ();
new_root)
else root
in
let config_f = OpamPath.config root in
let root_empty =
not (OpamFilename.exists_dir root) || OpamFilename.dir_is_empty root in
Expand Down
9 changes: 9 additions & 0 deletions src/client/opamClientConfig.ml
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,15 @@ let opam_init ?root_dir ?strict ?solver =

(* (i) get root dir *)
let root = OpamStateConfig.opamroot ?root_dir () in
if Sys.win32
(* if default, redirection will be handled by opam init, or should have
been handled *)
&& (root_dir <> None || OpamStateConfig.E.root () <> None)
&& OpamStd.String.contains_char (OpamFilename.Dir.to_string root) ' ' then
OpamConsole.error "You opam root directory contains a space, this may lead \
to several malfunction... bzzz.... nooo%s"
(* NOTE: UTF-8 Collision emoji *)
(if OpamConsole.color () then "\xF0\x9F\x92\xA5" else "");

(* (ii) load conf file and set defaults *)
(* the init for OpamFormat is done in advance since (a) it has an effect on
Expand Down
44 changes: 27 additions & 17 deletions src/state/opamStateConfig.ml
Original file line number Diff line number Diff line change
Expand Up @@ -70,23 +70,30 @@ type t = {
no_depexts: bool;
}

let win_space_redirection root =
let redirected = OpamPath.redirected root in
if OpamFilename.exists redirected then
OpamFilename.Dir.of_string (OpamFilename.read redirected)
else root

let default_root () =
(* On Windows, if a .opam directory is found in %HOME% or %USERPROFILE% then
then we'll use it. Otherwise, we use %LOCALAPPDATA%. *)
let home_location =
let open OpamFilename in
concat_and_resolve (Dir.of_string (OpamStd.Sys.home ())) ".opam"
in
if not Sys.win32 || OpamFilename.exists_dir home_location then
home_location
else
let open OpamFilename in
let local_appdata =
Dir.of_string (OpamStubs.getPathToLocalAppData ())
in
concat_and_resolve local_appdata "opam"

let default = {
root_dir = (
(* On Windows, if a .opam directory is found in %HOME% or %USERPROFILE% then
then we'll use it. Otherwise, we use %LOCALAPPDATA%. *)
let home_location =
let open OpamFilename in
concat_and_resolve (Dir.of_string (OpamStd.Sys.home ())) ".opam"
in
if not Sys.win32 || OpamFilename.exists_dir home_location then
home_location
else
let open OpamFilename in
let local_appdata =
Dir.of_string (OpamStubs.getPathToLocalAppData ())
in
concat_and_resolve local_appdata "opam"
);
root_dir = default_root () |> win_space_redirection;
original_root_dir = default_root ();
current_switch = None;
switch_from = `Default;
Expand Down Expand Up @@ -180,7 +187,9 @@ let initk k =
| Some s -> Some (OpamSwitch.of_string s), Some `Env
in
setk (setk (fun c -> r := c; k)) !r
?root_dir:(E.root () >>| OpamFilename.Dir.of_string)
?root_dir:(E.root ()
>>| OpamFilename.Dir.of_string
>>| win_space_redirection)
?original_root_dir:(E.root () >>| OpamFilename.Dir.of_string)
?current_switch
?switch_from
Expand Down Expand Up @@ -208,6 +217,7 @@ let opamroot ?root_dir () =
(root_dir >>+ fun () ->
OpamStd.Env.getopt "OPAMROOT" >>| OpamFilename.Dir.of_string)
+! default.root_dir
|> win_space_redirection

let is_newer_raw = function
| Some v ->
Expand Down
54 changes: 0 additions & 54 deletions tests/reftests/env.test
Original file line number Diff line number Diff line change
Expand Up @@ -439,60 +439,6 @@ The following actions will be performed:
Done.
### opam exec -- sh -c "eval $(opam env | tr -d '\\r'); opam remove foo; opam env; eval $(opam env | tr -d '\\r'); opam env" | grep "FOO"
FOO=''; export FOO;
### : root and switch with spaces :
### RT="$BASEDIR/root 2"
### SW="switch w spaces"
### OPAMNOENVNOTICE=0
### opam init -na --bare --bypass-check --disable-sandbox --root "$RT" defaut ./REPO | grep -v Cygwin
No configuration file found, using built-in defaults.

<><> Fetching repository information ><><><><><><><><><><><><><><><><><><><><><>
[defaut] Initialised
### opam switch create "./$SW" nv --root "$RT"

<><> Installing new switch packages <><><><><><><><><><><><><><><><><><><><><><>
Switch invariant: ["nv"]

<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
-> installed nv.1
Done.
# Run eval $(opam env '--root=${BASEDIR}/root 2' '--switch=${BASEDIR}/switch w spaces') to update the current shell environment
### opam env --root "$RT" --switch "./$SW" | grep "NV_VARS" | ';' -> ':'
NV_VARS3='foo:/yet/another/different/path': export NV_VARS3:
NV_VARS4='': export NV_VARS4:
NV_VARS_5925_1='foo': export NV_VARS_5925_1:
NV_VARS_5925_2='foo': export NV_VARS_5925_2:
NV_VARS_5925_3='foo': export NV_VARS_5925_3:
NV_VARS_5925_4='foo': export NV_VARS_5925_4:
NV_VARS_5925_5='foo:': export NV_VARS_5925_5:
NV_VARS_5925_6='foo:': export NV_VARS_5925_6:
NV_VARS_5925_7=':foo': export NV_VARS_5925_7:
NV_VARS_5925_8=':foo': export NV_VARS_5925_8:
NV_VARS_5926_L_1='b::a': export NV_VARS_5926_L_1:
NV_VARS_5926_L_2='b::a': export NV_VARS_5926_L_2:
NV_VARS_5926_L_3=':a:b': export NV_VARS_5926_L_3:
NV_VARS_5926_L_4=':a:b': export NV_VARS_5926_L_4:
NV_VARS_5926_L_5='b::a': export NV_VARS_5926_L_5:
NV_VARS_5926_L_6='b::a': export NV_VARS_5926_L_6:
NV_VARS_5926_L_7=':a:b': export NV_VARS_5926_L_7:
NV_VARS_5926_L_8=':a:b': export NV_VARS_5926_L_8:
NV_VARS_5926_M_1='b:a1::a2': export NV_VARS_5926_M_1:
NV_VARS_5926_M_2='a1::a2:b': export NV_VARS_5926_M_2:
NV_VARS_5926_M_3='b:a1::a2': export NV_VARS_5926_M_3:
NV_VARS_5926_M_4='a1::a2:b': export NV_VARS_5926_M_4:
NV_VARS_5926_S_1='a:': export NV_VARS_5926_S_1:
NV_VARS_5926_S_2=':a': export NV_VARS_5926_S_2:
NV_VARS_5926_S_3='a:': export NV_VARS_5926_S_3:
NV_VARS_5926_S_4=':a': export NV_VARS_5926_S_4:
NV_VARS_5926_T_1='b:a:': export NV_VARS_5926_T_1:
NV_VARS_5926_T_2='b:a:': export NV_VARS_5926_T_2:
NV_VARS_5926_T_3='a::b': export NV_VARS_5926_T_3:
NV_VARS_5926_T_4='a::b': export NV_VARS_5926_T_4:
NV_VARS_5926_T_5='b:a:': export NV_VARS_5926_T_5:
NV_VARS_5926_T_6='b:a:': export NV_VARS_5926_T_6:
NV_VARS_5926_T_7='a::b': export NV_VARS_5926_T_7:
NV_VARS_5926_T_8='a::b': export NV_VARS_5926_T_8:
### OPAMNOENVNOTICE=1
### : Env hooks :
### <pkg:av.1>
opam-version: "2.0"
Expand Down
26 changes: 26 additions & 0 deletions tests/reftests/env.unix.test
Original file line number Diff line number Diff line change
@@ -1,4 +1,30 @@
N0REP0
### : root and switches with spaces :
### <pkg:nv.1>
opam-version: "2.0"
flags: compiler
### RT="$BASEDIR/root 2"
### SW="switch w spaces"
### OPAMNOENVNOTICE=0
### opam init -na --bare --bypass-check --disable-sandbox --root "$RT" defaut ./REPO
No configuration file found, using built-in defaults.

<><> Fetching repository information ><><><><><><><><><><><><><><><><><><><><><>
[defaut] Initialised
### opam switch create "./$SW" nv --root "$RT"

<><> Installing new switch packages <><><><><><><><><><><><><><><><><><><><><><>
Switch invariant: ["nv"]

<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
-> installed nv.1
Done.
# Run eval $(opam env '--root=${BASEDIR}/root 2' '--switch=${BASEDIR}/switch w spaces') to update the current shell environment
### opam env --root "$RT" --switch "./$SW" | grep "PREFIX" | ';' -> ':'
OPAM_SWITCH_PREFIX='${BASEDIR}/switch w spaces/_opam': export OPAM_SWITCH_PREFIX:
### opam var root --root "$RT"
${BASEDIR}/root 2
### OPAMNOENVNOTICE=1
### : setenv & build env rewriting :
### opam switch create rewriting --empty
### ::::::::::::::::::
Expand Down
13 changes: 8 additions & 5 deletions tests/reftests/init.win32.test
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,26 @@ N0REP0
### rm -rf $OPAMROOT
### OPAMROOT="roots/with path"
### opam init --no-setup --bare --bypass-checks default REPO/ | grep -v Cygwin
[ERROR] You opam root directory contains a space, this may lead to several malfunction... bzzz.... nooo
No configuration file found, using built-in defaults.
Your opam root path '${BASEDIR}/roots/with path' contains a space, we'll redirect to 'C:\opamroot'.
Do you want to choose and enter another spaceless folder? [y/n] n

<><> Fetching repository information ><><><><><><><><><><><><><><><><><><><><><>
[default] Initialised
### opam var root --debug --debug-level=-1
CLI Parsing CLI version 2.2
GSTATE LOAD-GLOBAL-STATE @ ${BASEDIR}/roots/with path
${BASEDIR}/roots/with path
GSTATE LOAD-GLOBAL-STATE @ C:\opamroot
C:\opamroot
### cat 'roots/with path/redirected-opamroot'
/usr/bin/cat: 'roots/with path/redirected-opamroot': No such file or directory
# Return code 1 #
C:\opamroot
### opam switch create --empty test
### opam switch --debug --debug-level=-1
CLI Parsing CLI version 2.2
GSTATE LOAD-GLOBAL-STATE @ ${BASEDIR}/roots/with path
GSTATE LOAD-GLOBAL-STATE @ C:\opamroot
SWITCH list
# switch compiler description
-> test test
### echo $OPAMROOT
roots/with path
### rm -rf 'C:\opamroot'

0 comments on commit 1883aac

Please sign in to comment.