diff --git a/master_changes.md b/master_changes.md index acd65036b2b..83a7bc7d75f 100644 --- a/master_changes.md +++ b/master_changes.md @@ -29,6 +29,7 @@ users) * 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] + * Redirect the opam root to C:\opamroot when the opam root contains spaces on Windows [#5457 @rjbou] ## Config report diff --git a/src/client/opamClient.ml b/src/client/opamClient.ml index 0fa35bc1d17..53df62cc682 100644 --- a/src/client/opamClient.ml +++ b/src/client/opamClient.ml @@ -1645,6 +1645,114 @@ let reinit ?(init_config=OpamInitDefaults.init_config()) ~interactive in OpamRepositoryState.drop rt +let has_space s = OpamStd.String.contains_char s ' ' + +let default_redirect_root = OpamFilename.Dir.of_string "C:\\opamroot" + +let setup_redirection target = + let {contents = {OpamStateConfig.original_root_dir = root; _}} = + OpamStateConfig.r + in + let target = + match target with + | Some target -> target + | None -> + OpamFilename.mkdir default_redirect_root; + let readme = OpamFilename.Op.(default_redirect_root // "ReadMe.txt") in + if not (OpamFilename.exists readme) then + OpamFilename.write readme + "This directory is used to contain redirected opam roots.\n\n\ + The contents may be shared with other users on this system."; + OpamSystem.mk_unique_dir ~dir:(OpamFilename.Dir.to_string default_redirect_root) () + in + let root_dir = OpamFilename.Dir.of_string target in + OpamFilename.write (OpamPath.redirected root) target; + OpamStateConfig.update ~root_dir (); + root_dir + +let get_redirected_root () = + let {contents = {OpamStateConfig.original_root_dir = root; root_from; _}} = + OpamStateConfig.r + in + let r = OpamConsole.colorise `bold (OpamFilename.Dir.to_string root) in + let collision = + let collision = OpamConsole.utf8_symbol OpamConsole.Symbols.collision "" in + if collision = "" then + "" + else + " " ^ collision + in + let options = [ + `Redirect, Printf.sprintf + "Redirect files to a directory in %s" + (OpamConsole.colorise `bold (OpamFilename.Dir.to_string default_redirect_root)); + `Ask, "Redirect files to an alternate directory"; + `Endure, Printf.sprintf + "Do not redirect anything and stick with %s%s" r collision; + `Quit, "Abort initialisation" + ] in + let default, explanation = + match root_from with + | `Command_line -> + (* The user has been explicit with --root; nemo salvet modo... *) + `Endure, + "You have specified a root directory for opam containing a space." + | `Env -> + (* The user has perhaps carelessly set an environment variable *) + `Redirect, + "Your OPAMROOT environment variable contains a space." + | `Default -> + (* The user has fallen victim to the defaults of Windows Setup and has a + space in their user name *) + `Redirect, + Printf.sprintf + "By default, opam would store its data in:\n\ + %s\n\ + however, this directory contains a space." r + in + let rec ask () = + let check r = + if Filename.is_relative r then begin + OpamConsole.msg + "That path is relative!\n\ + Please enter an absolute path without spaces.\n"; + ask () + end else if has_space r then begin + OpamConsole.msg + "That path contains contains a space!\n\ + Please enter an absolute path without spaces.\n"; + ask () + end else + Some (Some r) + in + OpamStd.Option.replace check (OpamConsole.read "Root directory for opam: ") + in + let rec menu () = + match OpamConsole.menu "Where should opam store files?" ~default ~options + ~no:default with + | `Redirect -> + Some None + | `Endure -> + None + | `Ask -> + let r = ask () in + if r = None then + menu () + else + r + | `Quit -> + OpamStd.Sys.exit_because `Aborted + in + OpamConsole.header_msg "opam root file store"; + OpamConsole.msg + "\n\ + %s\n\ + \n\ + Many parts of the OCaml ecosystem do not presently work correctly\n\ + when installed to directories containing spaces. You have been warned!%s\n\ + \n" explanation collision; + Option.map setup_redirection (menu ()) + let init ~init_config ~interactive ?repo ?(bypass_checks=false) @@ -1654,10 +1762,34 @@ let init shell = log "INIT %a" (slog @@ OpamStd.Option.to_string OpamRepositoryBackend.to_string) repo; + let original_root = OpamStateConfig.(!r.original_root_dir) in + let root_empty = + not (OpamFilename.exists_dir original_root) + || OpamFilename.dir_is_empty original_root in let root = OpamStateConfig.(!r.root_dir) in + let root, remove_root = + let ignore_non_fatal f x = + try f x + with e -> OpamStd.Exn.fatal e + in + let new_root = + if root_empty && + Sys.win32 && + has_space (OpamFilename.Dir.to_string root) then + get_redirected_root () + else + None + in + match new_root with + | None -> + root, (fun () -> ignore_non_fatal OpamFilename.rmdir root) + | Some root -> + root, (fun () -> + ignore_non_fatal OpamFilename.rmdir root; + ignore_non_fatal OpamFilename.rmdir original_root + ) + in let config_f = OpamPath.config root in - let root_empty = - not (OpamFilename.exists_dir root) || OpamFilename.dir_is_empty root in let gt, rt, default_compiler = if OpamFile.exists config_f then ( @@ -1671,7 +1803,7 @@ let init ) else ( if not root_empty then ( OpamConsole.warning "%s exists and is not empty" - (OpamFilename.Dir.to_string root); + (OpamFilename.Dir.to_string original_root); if not (OpamConsole.confirm "Proceed?") then OpamStd.Sys.exit_because `Aborted); try @@ -1743,7 +1875,7 @@ let init in if failed <> [] then (if root_empty then - (try OpamFilename.rmdir root with _ -> ()); + remove_root (); OpamConsole.error_and_exit `Sync_error "Initial download of repository failed."); let default_compiler = @@ -1778,7 +1910,7 @@ let init OpamStd.Exn.finalise e @@ fun () -> if not (OpamConsole.debug ()) && root_empty then begin OpamSystem.release_all_locks (); - OpamFilename.rmdir root + remove_root () end) in OpamEnv.setup root ~interactive diff --git a/src/core/opamConsole.ml b/src/core/opamConsole.ml index 58143463779..4188918f024 100644 --- a/src/core/opamConsole.ml +++ b/src/core/opamConsole.ml @@ -105,6 +105,7 @@ module Symbols = struct let downwards_double_arrow = Uchar.of_int 0x21d3 let black_down_pointing_triangle = Uchar.of_int 0x25bc let downwards_black_arrow = Uchar.of_int 0x2b07 + let collision = Uchar.of_int 0x1f4a5 end type win32_glyph_checker = { diff --git a/src/core/opamConsole.mli b/src/core/opamConsole.mli index 869d9447ce4..ce2cb279361 100644 --- a/src/core/opamConsole.mli +++ b/src/core/opamConsole.mli @@ -66,6 +66,7 @@ module Symbols : sig val downwards_double_arrow : Uchar.t val downwards_black_arrow : Uchar.t val black_down_pointing_triangle : Uchar.t + val collision : Uchar.t end val utf8_symbol: diff --git a/src/core/opamSystem.ml b/src/core/opamSystem.ml index f595e3ddaf4..03c9c30487d 100644 --- a/src/core/opamSystem.ml +++ b/src/core/opamSystem.ml @@ -88,6 +88,13 @@ let rec mk_temp_dir ?(prefix="opam") () = else real_path s +let rec mk_unique_dir ~dir ?(prefix="opam") () = + let s = dir / Printf.sprintf "%s-%06x" prefix (Random.int 0xFFFFFF) in + if Sys.file_exists s then + mk_unique_dir ~dir ~prefix () + else + real_path s + let safe_mkdir dir = try log "mkdir %s" dir; diff --git a/src/core/opamSystem.mli b/src/core/opamSystem.mli index 25dc6c1e043..f736089dff8 100644 --- a/src/core/opamSystem.mli +++ b/src/core/opamSystem.mli @@ -51,6 +51,10 @@ val verbose_for_base_commands: unit -> bool (if [prefix] is not set), pid, and random number. *) val mk_temp_dir: ?prefix:string -> unit -> string +(** Returns a directory name, in the [~dir], composed by {i opam} + (if [prefix] is not set), and a random number. *) +val mk_unique_dir: dir:string -> ?prefix:string -> unit -> string + (** [copy_file src dst] copies [src] to [dst]. Remove [dst] before the copy if it is a link. *) val copy_file: string -> string -> unit diff --git a/src/state/opamStateConfig.ml b/src/state/opamStateConfig.ml index 1314c1154b9..4aebea8fb5c 100644 --- a/src/state/opamStateConfig.ml +++ b/src/state/opamStateConfig.ml @@ -71,6 +71,12 @@ 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%. *) @@ -88,7 +94,7 @@ let default_root = concat_and_resolve local_appdata "opam" let default = { - root_dir = default_root; + root_dir = win_space_redirection default_root; original_root_dir = default_root; root_from = `Default; current_switch = None; @@ -185,7 +191,7 @@ let initk k = | None -> None, None, None | Some root -> let root = OpamFilename.Dir.of_string root in - Some root, Some root, Some `Env + Some (win_space_redirection root), Some root, Some `Env in let current_switch, switch_from = match E.switch () with @@ -219,11 +225,11 @@ let init ?noop:_ = initk (fun () -> ()) let opamroot ?root_dir () = match root_dir with - | Some root -> `Command_line, root + | Some root -> `Command_line, win_space_redirection root | None -> match OpamStd.Env.getopt "OPAMROOT" with | Some root -> - `Env, OpamFilename.Dir.of_string root + `Env, win_space_redirection (OpamFilename.Dir.of_string root) | None -> `Default, default.root_dir diff --git a/tests/reftests/env.test b/tests/reftests/env.test index f4ac8d1dafa..484703ec25b 100644 --- a/tests/reftests/env.test +++ b/tests/reftests/env.test @@ -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 : ### opam-version: "2.0" diff --git a/tests/reftests/env.unix.test b/tests/reftests/env.unix.test index eaa9395e32c..be7fdf29e00 100644 --- a/tests/reftests/env.unix.test +++ b/tests/reftests/env.unix.test @@ -1,4 +1,30 @@ N0REP0 +### : root and switches with spaces : +### +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 ### ::::::::::::::::::