From 41b1180cd381435d1c0f54f37a28dbe4910110bc Mon Sep 17 00:00:00 2001 From: David Allsopp Date: Thu, 21 Jul 2022 17:12:42 +0100 Subject: [PATCH] Don't generate .profile for cmd/powershell The PowerShell implementation isn't yet correct and the Command Processor will never have one. Refactor the code to skip .profile files for those. --- src/client/opamCommands.ml | 13 +++-- src/core/opamStd.ml | 56 +++++++++----------- src/core/opamStd.mli | 5 +- src/state/opamEnv.ml | 103 ++++++++++++++++++++++++------------- 4 files changed, 101 insertions(+), 76 deletions(-) diff --git a/src/client/opamCommands.ml b/src/client/opamCommands.ml index 3885c95d703..276f9afbe06 100644 --- a/src/client/opamCommands.ml +++ b/src/client/opamCommands.ml @@ -359,10 +359,9 @@ let init cli = | Some s -> s | None -> OpamStd.Sys.guess_shell_compat () in - let dot_profile = match dot_profile_o with - | Some n -> n - | None -> - OpamFilename.of_string (OpamStd.Sys.guess_dot_profile shell) + let dot_profile = + OpamStd.Option.Op.(dot_profile_o >>+ fun () -> + OpamStd.Sys.guess_dot_profile shell >>| OpamFilename.of_string) in if already_init then if reinit then @@ -371,7 +370,7 @@ let init cli = ~no_default_config_file:no_config_file ~add_config_file:config_file in let reinit conf = - OpamClient.reinit ~init_config ~interactive ~dot_profile + OpamClient.reinit ~init_config ~interactive ?dot_profile ?update_config ?env_hook ?completion ~inplace ~bypass_checks ~check_sandbox:(not no_sandboxing) conf shell @@ -394,7 +393,7 @@ let init cli = "Opam was already initialised. If you want to set it up again, \ use `--interactive', `--reinit', or choose a different \ `--root'.\n"; - OpamEnv.setup root ~interactive ~dot_profile ?update_config ?env_hook + OpamEnv.setup root ~interactive ?dot_profile ?update_config ?env_hook ?completion ~inplace shell) else let init_config = @@ -410,7 +409,7 @@ let init cli = let gt, rt, default_compiler = OpamClient.init ~init_config ~interactive - ?repo ~bypass_checks ~dot_profile + ?repo ~bypass_checks ?dot_profile ?update_config ?env_hook ?completion ~check_sandbox:(not no_sandboxing) shell diff --git a/src/core/opamStd.ml b/src/core/opamStd.ml index f419c35d155..be94f7402fa 100644 --- a/src/core/opamStd.ml +++ b/src/core/opamStd.ml @@ -1141,48 +1141,42 @@ module OpamSys = struct Option.default unix_default_shell shell let guess_dot_profile shell = - let win_my_powershell f = - let p = Filename.concat (home ()) "Documents" in - if Sys.file_exists p then Filename.concat (Filename.concat p "PowerShell") f - else let p = Filename.concat (home ()) "My Documents" in - if Sys.file_exists p then Filename.concat (Filename.concat p "PowerShell") f - else f - in let home f = try Filename.concat (home ()) f with Not_found -> f in match shell with | SH_fish -> - List.fold_left Filename.concat (home ".config") ["fish"; "config.fish"] - | SH_zsh -> home ".zshrc" + Some (List.fold_left Filename.concat (home ".config") ["fish"; "config.fish"]) + | SH_zsh -> Some (home ".zshrc") | SH_bash -> - (try - List.find Sys.file_exists [ - (* Bash looks up these 3 files in order and only loads the first, - for LOGIN shells *) - home ".bash_profile"; - home ".bash_login"; - home ".profile"; - (* Bash loads .bashrc INSTEAD, for interactive NON login shells only; - but it's often included from the above. - We may include our variables in both to be sure ; for now we rely - on non-login shells inheriting their env from a login shell - somewhere... *) - ] - with Not_found -> - (* iff none of the above exist, creating this should be safe *) - home ".bash_profile") + let shell = + (try + List.find Sys.file_exists [ + (* Bash looks up these 3 files in order and only loads the first, + for LOGIN shells *) + home ".bash_profile"; + home ".bash_login"; + home ".profile"; + (* Bash loads .bashrc INSTEAD, for interactive NON login shells only; + but it's often included from the above. + We may include our variables in both to be sure ; for now we rely + on non-login shells inheriting their env from a login shell + somewhere... *) + ] + with Not_found -> + (* iff none of the above exist, creating this should be safe *) + home ".bash_profile") + in + Some shell | SH_csh -> let cshrc = home ".cshrc" in let tcshrc = home ".tcshrc" in - if Sys.file_exists cshrc then cshrc else tcshrc + Some (if Sys.file_exists cshrc then cshrc else tcshrc) | SH_pwsh _ -> - if Sys.win32 then win_my_powershell "Microsoft.Powershell_profile.ps1" else - List.fold_left Filename.concat (home ".config") ["powershell"; "Microsoft.Powershell_profile.ps1"] - | SH_sh -> home ".profile" + None + | SH_sh -> Some (home ".profile") | SH_cmd -> - (* cmd.exe does not have a concept of profiles *) - home ".profile" + None let registered_at_exit = ref [] diff --git a/src/core/opamStd.mli b/src/core/opamStd.mli index 9080dbbe44a..9d337a2d2f4 100644 --- a/src/core/opamStd.mli +++ b/src/core/opamStd.mli @@ -507,8 +507,9 @@ module Sys : sig (** Guess the shell compat-mode *) val guess_shell_compat: unit -> shell - (** Guess the location of .profile *) - val guess_dot_profile: shell -> string + (** Guess the location of .profile. Returns None if the shell doesn't + support the concept of a .profile file. *) + val guess_dot_profile: shell -> string option (** The separator character used in the PATH variable (varies depending on OS) *) diff --git a/src/state/opamEnv.ml b/src/state/opamEnv.ml index 1fa71593058..65365761d9a 100644 --- a/src/state/opamEnv.ml +++ b/src/state/opamEnv.ml @@ -885,10 +885,9 @@ let setup let shell, update_dot_profile, env_hook = match update_config, dot_profile, interactive with | Some false, _, _ -> shell, None, env_hook - | _, None, _ -> invalid_arg "OpamEnv.setup" - | Some true, Some dot_profile, _ -> shell, Some dot_profile, env_hook + | Some true, dot_profile, _ -> shell, dot_profile, env_hook | None, _, false -> shell, None, env_hook - | None, Some dot_profile, true -> + | None, dot_profile, true -> OpamConsole.header_msg "Required setup - please read"; OpamConsole.msg @@ -896,49 +895,78 @@ let setup \ In normal operation, opam only alters files within %s.\n\ \n\ \ However, to best integrate with your system, some environment variables\n\ - \ should be set. If you allow it to, this initialisation step will update\n\ - \ your %s configuration by adding the following line to %s:\n\ + \ should be set. " + opam_root_msg; + begin match dot_profile with + | Some dot_profile -> + OpamConsole.msg + "If you allow it to, this initialisation step will update\n\ + \ your %s configuration by adding the following line to %s:\n\ + \n\ + \ %s\ + \n\ + \ Otherwise, every time" + (OpamConsole.colorise `bold (string_of_shell shell)) + (OpamConsole.colorise `cyan @@ OpamFilename.prettify dot_profile) + (OpamConsole.colorise `bold @@ source root shell (init_file shell)); + | None -> + OpamConsole.msg "When" + end; + OpamConsole.msg + " you want to access your opam installation, you will\n\ + \ need to run:\n\ \n\ - \ %s\ + \ %s\n\ \n\ \ You can always re-run this setup with 'opam init' later.\n\n" - opam_root_msg - (OpamConsole.colorise `bold @@ string_of_shell shell) - (OpamConsole.colorise `cyan @@ OpamFilename.prettify dot_profile) - (OpamConsole.colorise `bold @@ source root shell (init_file shell)); + (OpamConsole.colorise `bold @@ shell_eval_invocation shell (opam_env_invocation shell)); if OpamCoreConfig.answer_is_yes () then begin - OpamConsole.warning "Shell not updated in non-interactive mode: use --shell-setup"; + if dot_profile <> None then + OpamConsole.warning "Shell not updated in non-interactive mode: use --shell-setup"; shell, None, env_hook end else let rec menu shell dot_profile default = + let colorised_shell = OpamConsole.colorise `bold (string_of_shell shell) in let opam_env_inv = OpamConsole.colorise `bold @@ shell_eval_invocation shell (opam_env_invocation shell) in - match - OpamConsole.menu "Do you want opam to configure %s?" - (OpamConsole.colorise `bold (string_of_shell shell)) - ~default ~no:`No ~options:[ - `Yes, Printf.sprintf "Yes, update %s" - (OpamConsole.colorise `cyan (OpamFilename.prettify dot_profile)); - `No_hooks, Printf.sprintf "Yes, but don't setup any hooks. You'll \ - have to run %s whenever you change \ - your current 'opam switch'" - opam_env_inv; - `Change_shell, "Select a different shell"; - `Change_file, "Specify another config file to update instead"; - `No, Printf.sprintf "No, I'll remember to run %s when I need opam" - opam_env_inv; - ] - with + let prompt () = + match dot_profile with + | Some dot_profile -> + let options = [ + `Yes, Printf.sprintf "Yes, update %s" + (OpamConsole.colorise `cyan (OpamFilename.prettify dot_profile)); + `No_hooks, Printf.sprintf "Yes, but don't setup any hooks. You'll \ + have to run %s whenever you change \ + your current 'opam switch'" + opam_env_inv; + `Change_shell, "Select a different shell"; + `Change_file, "Specify another config file to update instead"; + `No, Printf.sprintf "No, I'll remember to run %s when I need opam" + opam_env_inv + ] in + OpamConsole.menu "Do you want opam to configure %s?" colorised_shell + ~default ~no:`No ~options + | None -> + if OpamConsole.confirm ~default:false + "opam doesn't have any configuration options for %s; you will have to run %s \ + whenever you change you current 'opam switch' or start a new terminal session. \ + Alternatively, would you like to select a different shell?" colorised_shell opam_env_inv then + `Change_shell + else + `No + in + match prompt () with | `No -> shell, None, env_hook - | `Yes -> shell, Some dot_profile, Some true - | `No_hooks -> shell, Some dot_profile, Some false + | `Yes -> shell, dot_profile, Some true + | `No_hooks -> shell, dot_profile, Some false | `Change_shell -> - let shell = OpamConsole.menu ~default:shell ~no:shell + let shell = + OpamConsole.menu ~default:shell ~no:shell "Please select a shell to configure" ~options: (List.map (fun s -> s, string_of_shell s) OpamStd.Sys.all_shells) in - menu shell (OpamFilename.of_string (OpamStd.Sys.guess_dot_profile shell)) + menu shell (OpamStd.Option.map OpamFilename.of_string (OpamStd.Sys.guess_dot_profile shell)) default | `Change_file -> let open OpamStd.Option.Op in @@ -948,14 +976,17 @@ let setup if Filename.is_implicit f then Filename.concat (OpamStd.Sys.home ()) f else f) >>| OpamFilename.of_string) - +! dot_profile in menu shell dot_profile `Yes in - let default = match env_hook with - | Some true -> `Yes - | Some false -> `No_hooks - | None -> `No + let default = + if dot_profile = None then + `No + else + match env_hook with + | Some true -> `Yes + | Some false -> `No_hooks + | None -> `No in menu shell dot_profile default in