Fish integration: make sure we're not in login mode before sourcing files

master_changes.md

@@ -23,6 +23,8 @@ users)
   * Disable ACL in Cygwin internal install to avoid permission mismatch errors [#5796 @kit-ty-kate - fix #5781]
   * Add `sys-pkg-manager-cmd` as an accepted field in opamrc files [#5847 @rjbou - fix #5844]
   * Fix `git-location` handling in init config file [#5848 @rjbou - fix #5845]
+  * Test if file exists before sourcing in fish + powershell [#5802 @ElectreAAS]
+  * Make sure we're not in login mode before sourcing in fish [#5802 @ElectreAAS]
 
 ## Config report
 

src/state/opamEnv.ml

@@ -874,32 +874,32 @@ let env_hook_script shell =
     (env_hook_script_base shell)
 
 let source root shell f =
-  let fname = OpamFilename.to_string (OpamPath.init root // f) in
-  let unix_transform ?using_backslashes () =
-    let cygpath = Lazy.force OpamSystem.get_cygpath_path_transform in
-    cygpath ~pathlist:false fname
-    |> OpamStd.Env.escape_single_quotes ?using_backslashes
+  let if_exists_source: _ format = match shell with
+    | SH_csh -> "if ( -f '%s' ) source '%s'\n"
+    | SH_fish -> "test -r '%s' && source '%s'\n"
+    | SH_sh | SH_bash -> "test -r '%s' && . '%s'\n"
+    | SH_zsh -> "[[ ! -r '%s' ]] || source '%s'\n"
+    | SH_cmd -> "if exist \"%s\" call \"%s\"\n"
+    | SH_pwsh _ -> "if Test-Path \"%s\" { . \"%s\" }\n"
   in
+  let sanitized_fname =
+    let fname = OpamFilename.to_string (OpamPath.init root // f) in
+    let unix_transform ?using_backslashes () =
+      let cygpath = Lazy.force OpamSystem.get_cygpath_path_transform in
+      cygpath ~pathlist:false fname
+      |> OpamStd.Env.escape_single_quotes ?using_backslashes
+    in
+    match shell with
+    | SH_csh | SH_sh | SH_bash | SH_zsh -> unix_transform ()
+    | SH_fish -> unix_transform ~using_backslashes:true ()
+    | SH_cmd | SH_pwsh _ -> fname  
+  in
+  Printf.sprintf if_exists_source sanitized_fname sanitized_fname
+
+let if_not_login_script shell t =
   match shell with
-  | SH_csh ->
-    let fname = unix_transform () in
-    Printf.sprintf "if ( -f '%s' ) source '%s' >& /dev/null\n"
-      fname fname
-  | SH_fish ->
-    let fname = unix_transform ~using_backslashes:true () in
-    Printf.sprintf "source '%s' > /dev/null 2> /dev/null; or true\n" fname
-  | SH_sh | SH_bash ->
-    let fname = unix_transform () in
-    Printf.sprintf "test -r '%s' && . '%s' > /dev/null 2> /dev/null || true\n"
-      fname fname
-  | SH_zsh ->
-    let fname = unix_transform () in
-    Printf.sprintf "[[ ! -r '%s' ]] || source '%s' > /dev/null 2> /dev/null\n"
-      fname fname
-  | SH_cmd ->
-    Printf.sprintf "if exist \"%s\" call \"%s\" >NUL 2>NUL\n" fname fname
-  | SH_pwsh _ ->
-    Printf.sprintf ". \"%s\" *> $null\n" fname
+  | SH_fish -> Printf.sprintf "if not status is-login\n    %send\n" t
+  | _ -> t (* Seems this option is only needed for fish *)
 
 let if_interactive_script shell t e =
   let ielse else_opt = match else_opt with
@@ -922,7 +922,7 @@ let if_interactive_script shell t e =
   | SH_csh ->
     Printf.sprintf "if ( $?prompt ) then\n  %s%sendif\n" t @@ ielse e
   | SH_fish ->
-    Printf.sprintf "if isatty\n  %s%send\n" t @@ ielse e
+    Printf.sprintf "if status is-interactive\n  %s%send\n" t @@ ielse e
   | SH_cmd ->
     Printf.sprintf "echo %%cmdcmdline%% | find /i \"%%~0\" >nul\nif errorlevel 1 (\n%s%s)\n" t @@ ielse_cmd e
   | SH_pwsh _ ->
@@ -1124,7 +1124,7 @@ let update_dot_profile root dot_profile shell =
          # This section can be safely removed at any time if needed.\n\
          %s\
          # END opam configuration\n"
-        (source root shell init_file) in
+        (if_not_login_script shell (source root shell init_file)) in
     OpamFilename.write dot_profile (old_body ^ opam_section);
     OpamConsole.msg "  Added %d lines after line %d in %s.\n"
       (count_lines opam_section - 1) (count_lines old_body) pretty_dot_profile
@@ -1176,16 +1176,23 @@ let setup
         opam_root_msg;
       begin match dot_profile with
       | Some dot_profile ->
+        let re = Re.compile (Re.char '\n') in
+        let to_add = Re.replace_string re ~by:"\n    " @@
+          if_not_login_script shell @@
+          source root shell @@ init_file shell
+        in
+        let plural = if String.(contains (trim to_add) '\n') then "s" else "" in
         OpamConsole.msg
           "If you allow it to, this initialisation step will update\n\
-          \  your %s configuration by adding the following line to %s:\n\
+          \  your %s configuration by adding the following line%s to %s:\n\
            \n\
           \    %s\
            \n\
           \  Otherwise, every time"
         (OpamConsole.colorise `bold (string_of_shell shell))
+        plural
         (OpamConsole.colorise `cyan @@ OpamFilename.prettify dot_profile)
-        (OpamConsole.colorise `bold @@ source root shell (init_file shell));
+        (OpamConsole.colorise `bold to_add);
       | None ->
         OpamConsole.msg "When"
       end;

src/state/shellscripts/env_hook.fish

@@ -1,3 +1,3 @@
-function __opam_env_export_eval --on-event fish_prompt;
-    eval (opam env --shell=fish --readonly 2> /dev/null);
+function __opam_env_export_eval --on-event fish_prompt
+    eval (opam env --shell=fish --readonly 2> /dev/null)
 end

tests/reftests/dune.inc

@@ -620,6 +620,48 @@
    %{targets}
    (run ./run.exe %{exe:../../src/client/opamMain.exe.exe} %{dep:extrasource.test} %{read-lines:testing-env}))))
 
+(rule
+ (alias reftest-init-scripts.unix)
+ (enabled_if (= %{os_type} "Unix"))
+ (action
+  (diff init-scripts.unix.test init-scripts.unix.out)))
+
+(alias
+ (name reftest)
+ (enabled_if (= %{os_type} "Unix"))
+ (deps (alias reftest-init-scripts.unix)))
+
+(rule
+ (targets init-scripts.unix.out)
+ (deps root-N0REP0)
+ (enabled_if (= %{os_type} "Unix"))
+ (package opam)
+ (action
+  (with-stdout-to
+   %{targets}
+   (run ./run.exe %{exe:../../src/client/opamMain.exe.exe} %{dep:init-scripts.unix.test} %{read-lines:testing-env}))))
+
+(rule
+ (alias reftest-init-scripts.win32)
+ (enabled_if (= %{os_type} "Win32"))
+ (action
+  (diff init-scripts.win32.test init-scripts.win32.out)))
+
+(alias
+ (name reftest)
+ (enabled_if (= %{os_type} "Win32"))
+ (deps (alias reftest-init-scripts.win32)))
+
+(rule
+ (targets init-scripts.win32.out)
+ (deps root-N0REP0)
+ (enabled_if (= %{os_type} "Win32"))
+ (package opam)
+ (action
+  (with-stdout-to
+   %{targets}
+   (run ./run.exe %{exe:../../src/client/opamMain.exe.exe} %{dep:init-scripts.win32.test} %{read-lines:testing-env}))))
+
 (rule
  (alias reftest-init)
  (action

tests/reftests/init-scripts.unix.test

@@ -0,0 +1,124 @@
+N0REP0
+### rm -rf $OPAMROOT
+### opam init -na --bypass-checks --bare REPO --root root
+No configuration file found, using built-in defaults.
+
+<><> Fetching repository information ><><><><><><><><><><><><><><><><><><><><><>
+[default] Initialised
+### # we need the switch for variables file
+### opam switch create fake --empty --root root
+### ls root/opam-init | unordered
+complete.sh
+complete.zsh
+env_hook.csh
+env_hook.fish
+env_hook.sh
+env_hook.zsh
+hooks
+init.cmd
+init.csh
+init.fish
+init.ps1
+init.sh
+init.zsh
+variables.cmd
+variables.csh
+variables.fish
+variables.ps1
+variables.sh
+### : Init scripts :
+### cat root/opam-init/init.sh
+if [ -t 0 ]; then
+  test -r '${BASEDIR}/root/opam-init/complete.sh' && . '${BASEDIR}/root/opam-init/complete.sh'
+
+  test -r '${BASEDIR}/root/opam-init/env_hook.sh' && . '${BASEDIR}/root/opam-init/env_hook.sh'
+fi
+
+test -r '${BASEDIR}/root/opam-init/variables.sh' && . '${BASEDIR}/root/opam-init/variables.sh'
+### cat root/opam-init/init.zsh
+if [[ -o interactive ]]; then
+  [[ ! -r '${BASEDIR}/root/opam-init/complete.zsh' ]] || source '${BASEDIR}/root/opam-init/complete.zsh'
+
+  [[ ! -r '${BASEDIR}/root/opam-init/env_hook.zsh' ]] || source '${BASEDIR}/root/opam-init/env_hook.zsh'
+fi
+
+[[ ! -r '${BASEDIR}/root/opam-init/variables.sh' ]] || source '${BASEDIR}/root/opam-init/variables.sh'
+### cat root/opam-init/init.fish
+if status is-interactive
+  test -r '${BASEDIR}/root/opam-init/env_hook.fish' && source '${BASEDIR}/root/opam-init/env_hook.fish'
+end
+
+test -r '${BASEDIR}/root/opam-init/variables.fish' && source '${BASEDIR}/root/opam-init/variables.fish'
+### cat root/opam-init/init.csh
+if ( $?prompt ) then
+  if ( -f '${BASEDIR}/root/opam-init/env_hook.csh' ) source '${BASEDIR}/root/opam-init/env_hook.csh'
+endif
+
+if ( -f '${BASEDIR}/root/opam-init/variables.csh' ) source '${BASEDIR}/root/opam-init/variables.csh'
+### cat root/opam-init/init.cmd
+if exist "${BASEDIR}/root/opam-init/variables.cmd" call "${BASEDIR}/root/opam-init/variables.cmd"
+### cat root/opam-init/init.ps1
+if Test-Path "${BASEDIR}/root/opam-init/variables.ps1" { . "${BASEDIR}/root/opam-init/variables.ps1" }
+### : Variables scripts :
+### cat root/opam-init/variables.sh | grep -v man | grep -v MANPATH
+# Prefix of the current opam switch
+OPAM_SWITCH_PREFIX='${BASEDIR}/root/fake'; export OPAM_SWITCH_PREFIX;
+# Binary dir for opam switch fake
+PATH='${BASEDIR}/root/fake/bin':"$PATH"; export PATH;
+### test -f root/opam-init/variables.zsh
+# Return code 1 #
+### cat root/opam-init/variables.fish | grep -v man | grep -v MANPATH
+# Prefix of the current opam switch
+set -gx OPAM_SWITCH_PREFIX '${BASEDIR}/root/fake';
+# Binary dir for opam switch fake
+set -gx PATH '${BASEDIR}/root/fake/bin' $PATH;
+### cat root/opam-init/variables.csh | grep -v man | grep -v MANPATH
+# Prefix of the current opam switch
+if ( ! ${?OPAM_SWITCH_PREFIX} ) setenv OPAM_SWITCH_PREFIX ""
+setenv OPAM_SWITCH_PREFIX '${BASEDIR}/root/fake'
+# Binary dir for opam switch fake
+if ( ! ${?PATH} ) setenv PATH ""
+setenv PATH '${BASEDIR}/root/fake/bin':"$PATH"
+### cat root/opam-init/variables.cmd | grep -v man | grep -v MANPATH
+:: Prefix of the current opam switch
+set "OPAM_SWITCH_PREFIX=${BASEDIR}/root/fake"
+:: Binary dir for opam switch fake
+set "PATH=${BASEDIR}/root/fake/bin:%PATH%"
+### cat root/opam-init/variables.ps1 | grep -v man | grep -v MANPATH
+# Prefix of the current opam switch
+$env:OPAM_SWITCH_PREFIX='${BASEDIR}/root/fake'
+# Binary dir for opam switch fake
+$env:PATH='${BASEDIR}/root/fake/bin:' + "$env:PATH"
+### : Env hook scripts :
+### cat root/opam-init/env_hook.sh
+OPAMNOENVNOTICE=true; export OPAMNOENVNOTICE;
+_opam_env_hook() {
+ local previous_exit_status=$?;
+ eval $(opam env --shell=bash --readonly 2> /dev/null <&- );
+ return $previous_exit_status;
+};
+if ! [[ "$PROMPT_COMMAND" =~ _opam_env_hook ]]; then
+    PROMPT_COMMAND="_opam_env_hook;$PROMPT_COMMAND";
+fi
+### cat root/opam-init/env_hook.zsh
+OPAMNOENVNOTICE=true; export OPAMNOENVNOTICE;
+_opam_env_hook() {
+    eval $(opam env --shell=zsh --readonly 2> /dev/null <&-);
+}
+typeset -ag precmd_functions;
+if [[ -z ${precmd_functions[(r)_opam_env_hook]} ]]; then
+    precmd_functions+=_opam_env_hook;
+fi
+### cat root/opam-init/env_hook.fish
+set -gx OPAMNOENVNOTICE true;
+function __opam_env_export_eval --on-event fish_prompt
+    eval (opam env --shell=fish --readonly 2> /dev/null)
+end
+### cat root/opam-init/env_hook.csh
+if ( ! ${?OPAMNOENVNOTICE} ) setenv OPAMNOENVNOTICE ""
+setenv OPAMNOENVNOTICE true
+alias precmd 'eval `opam env --shell=csh --readonly`'
+### test -f root/opam-init/env_hook.cmd
+# Return code 1 #
+### test -f root/opam-init/env_hook.ps1
+# Return code 1 #