BASH integration: No longer modify .bashrc to load shell integration

I think I have things setup robustly so that the shell integration
is loaded transparently via env vars and the normal bash startup files
are sourced, in the same way that vanilla bash does it. Let's hope I
haven't overlooked something.

docs/build.rst

@@ -190,13 +190,11 @@ update-checking
 
 shell-integration
    |kitty| by default injects its :ref:`shell_integration` code into the user's
-   shell using environment variables or (for bash only) modifying
+   shell using environment variables. For a package, it might make more sense
-   the user's :file:`~/.bashrc` file.
+   to distribute the shell integration scripts into the system-wide shell
-   For a package, it might make more sense to distribute the shell
+   vendor locations. The shell integration files are found in the
-   integration scripts into the system-wide shell vendor locations. The
+   :file:`shell-integration` directory. Copy them to the system wide shell
-   shell integration files are found in the :file:`shell-integration`
+   vendor locations for each shell, and use::
-   directory. Copy them to the system wide shell vendor locations for each
-   shell, and use::
 
        ./setup.py linux-package --shell-integration=enabled\ no-rc
 

docs/changelog.rst

@@ -78,6 +78,8 @@ Detailed list of changes
 0.24.3 [future]
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
+- BASH integration: No longer modify :file:`~/.bashrc` to load :ref:`shell integration <shell_integration>`
+
 - macOS: Allow kitty to handle various URL types. Can be configured via
   :ref:`launch_actions` (:pull:`4618`)
 

docs/shell-integration.rst

@@ -148,8 +148,10 @@ different shells.
 
 .. tab:: bash
 
-    For bash, kitty adds a couple of lines to the bottom of :file:`~/.bashrc`
+    For bash, kitty starts bash in POSIX mode and implements the loading of the
-    (in an atomic manner) to load the shell integration code.
+    bash startup files in the integration script itself, after disabling POSIX
+    mode. From the perspective of those scripts there should be no difference
+    to running vanilla bash.
 
 
 Then, when launching the shell, kitty sets the environment variable

kitty/child.py

@@ -196,7 +196,7 @@ class Child:
         allow_remote_control: bool = False
     ):
         self.allow_remote_control = allow_remote_control
-        self.argv = argv
+        self.argv = list(argv)
         if cwd_from is not None:
             try:
                 cwd = cwd_of_process(cwd_from)
@@ -233,7 +233,7 @@ class Child:
             opts = fast_data_types.get_options()
             if 'disabled' not in opts.shell_integration:
                 from .shell_integration import modify_shell_environ
-                modify_shell_environ(self.argv[0], opts, env)
+                modify_shell_environ(opts, env, self.argv)
             env = {k: v for k, v in env.items() if v is not DELETE_ENV_VAR}
         return env
 

kitty/main.py

@@ -30,7 +30,6 @@ from .options.types import Options
 from .options.utils import DELETE_ENV_VAR
 from .os_window_size import initial_window_size_func
 from .session import get_os_window_sizing_data
-from .shell_integration import setup_shell_integration
 from .types import SingleKey
 from .utils import (
     detach, expandvars, log_error, single_instance,
@@ -322,7 +321,6 @@ def setup_environment(opts: Options, cli_opts: CLIOptions) -> None:
         os.environ['KITTY_LISTEN_ON'] = cli_opts.listen_on
     env = opts.env.copy()
     ensure_kitty_in_path()
-    setup_shell_integration(opts, env)
     kitty_path = shutil.which('kitty')
     if kitty_path:
         child_path = env.get('PATH')

kitty/shell_integration.py

@@ -3,65 +3,14 @@
 
 
 import os
-from contextlib import suppress
+from typing import Optional, Dict, List
-from typing import Optional, Union, Dict
 
 from .options.types import Options
-from .config import atomic_save
 from .constants import shell_integration_dir
-from .utils import log_error, resolved_shell
+from .utils import log_error
-
-posix_template = '''\
-# BEGIN_KITTY_SHELL_INTEGRATION
-if test -n "$KITTY_INSTALLATION_DIR" -a -e "$KITTY_INSTALLATION_DIR/{path}"; then source "$KITTY_INSTALLATION_DIR/{path}"; fi
-# END_KITTY_SHELL_INTEGRATION\
-'''
 
 
-def atomic_write(path: str, data: Union[str, bytes]) -> None:
+def setup_fish_env(env: Dict[str, str], argv: List[str]) -> None:
-    if isinstance(data, str):
-        data = data.encode('utf-8')
-    atomic_save(data, path)
-
-
-def safe_read(path: str) -> str:
-    with suppress(FileNotFoundError):
-        with open(path) as f:
-            return f.read()
-    return ''
-
-
-def rc_inset(shell_name: str = 'bash', template: str = posix_template) -> str:
-    return template.format(path=f"shell-integration/{shell_name}/kitty.{shell_name}")
-
-
-def setup_integration(shell_name: str, rc_path: str, template: str = posix_template) -> None:
-    import re
-    rc_path = os.path.realpath(rc_path)
-    rc = safe_read(rc_path)
-    integration = rc_inset(shell_name, template)
-    newrc, num_subs = re.subn(
-        r'^# BEGIN_KITTY_SHELL_INTEGRATION.+?^# END_KITTY_SHELL_INTEGRATION',
-        integration, rc, flags=re.DOTALL | re.MULTILINE)
-    if num_subs < 1:
-        newrc = newrc.rstrip() + '\n\n' + integration
-    if newrc != rc:
-        atomic_write(rc_path, newrc)
-
-
-def setup_zsh_integration(env: Dict[str, str]) -> None:
-    pass  # this is handled in the zsh env modifier
-
-
-def setup_bash_integration(env: Dict[str, str]) -> None:
-    setup_integration('bash', os.path.expanduser('~/.bashrc'))
-
-
-def setup_fish_integration(env: Dict[str, str]) -> None:
-    pass  # this is handled in the fish env modifier
-
-
-def setup_fish_env(env: Dict[str, str]) -> None:
     val = env.get('XDG_DATA_DIRS')
     env['KITTY_FISH_XDG_DATA_DIR'] = shell_integration_dir
     if not val:
@@ -88,7 +37,7 @@ def is_new_zsh_install(env: Dict[str, str]) -> bool:
     return True
 
 
-def setup_zsh_env(env: Dict[str, str]) -> None:
+def setup_zsh_env(env: Dict[str, str], argv: List[str]) -> None:
     if is_new_zsh_install(env):
         # dont prevent zsh-newuser-install from running
         # zsh-newuser-install never runs as root but we assume that it does
@@ -103,48 +52,58 @@ def setup_zsh_env(env: Dict[str, str]) -> None:
     env['ZDOTDIR'] = os.path.join(shell_integration_dir, 'zsh')
 
 
-SUPPORTED_SHELLS = {
+def setup_bash_env(env: Dict[str, str], argv: List[str]) -> None:
-    'zsh': setup_zsh_integration,
+    inject = {'1'}
-    'bash': setup_bash_integration,
+    posix_env = rcfile = ''
-    'fish': setup_fish_integration,
+    remove_args = set()
-}
+    for i in range(1, len(argv)):
+        arg = argv[i]
+        if arg == '--posix':
+            inject.add('posix')
+            posix_env = env.get('ENV', '')
+            remove_args.add(i)
+        elif arg == '--norc':
+            inject.add('no-rc')
+            remove_args.add(i)
+        elif arg == '--noprofile':
+            inject.add('no-profile')
+            remove_args.add(i)
+        elif arg in ('--rcfile', '--init-file') and i + 1 < len(argv):
+            rcfile = argv[i+1]
+            remove_args |= {i, i+1}
+    env['ENV'] = os.path.join(shell_integration_dir, 'bash', 'kitty.bash')
+    env['KITTY_BASH_INJECT'] = ' '.join(inject)
+    if posix_env:
+        env['KITTY_BASH_POSIX_ENV'] = posix_env
+    if rcfile:
+        env['KITTY_BASH_RCFILE'] = rcfile
+    for i in sorted(remove_args, reverse=True):
+        del argv[i]
+    argv.insert(1, '--posix')
+
+
 ENV_MODIFIERS = {
     'fish': setup_fish_env,
     'zsh': setup_zsh_env,
+    'bash': setup_bash_env,
 }
 
 
 def get_supported_shell_name(path: str) -> Optional[str]:
-    name = os.path.basename(path).split('.')[0].lower()
+    name = os.path.basename(path)
-    name = name.replace('-', '')
+    if name.lower().endswith('.exe'):
-    if name in SUPPORTED_SHELLS:
+        name = name.rpartition('.')[0]
-        return name
+    if name.startswith('-'):
-    return None
+        name = name[1:]
+    return name if name in ENV_MODIFIERS else None
 
 
 def shell_integration_allows_rc_modification(opts: Options) -> bool:
     return not (opts.shell_integration & {'disabled', 'no-rc'})
 
 
-def setup_shell_integration(opts: Options, env: Dict[str, str]) -> bool:
+def modify_shell_environ(opts: Options, env: Dict[str, str], argv: List[str]) -> None:
-    if not shell_integration_allows_rc_modification(opts):
+    shell = get_supported_shell_name(argv[0])
-        return False
-    shell = get_supported_shell_name(resolved_shell(opts)[0])
-    if shell is None:
-        return False
-    func = SUPPORTED_SHELLS[shell]
-    try:
-        func(env)
-    except Exception:
-        import traceback
-        traceback.print_exc()
-        log_error(f'Failed to setup shell integration for: {shell}')
-        return False
-    return True
-
-
-def modify_shell_environ(argv0: str, opts: Options, env: Dict[str, str]) -> None:
-    shell = get_supported_shell_name(argv0)
     if shell is None or 'disabled' in opts.shell_integration:
         return
     env['KITTY_SHELL_INTEGRATION'] = ' '.join(opts.shell_integration)
@@ -153,7 +112,7 @@ def modify_shell_environ(argv0: str, opts: Options, env: Dict[str, str]) -> None
     f = ENV_MODIFIERS.get(shell)
     if f is not None:
         try:
-            f(env)
+            f(env, argv)
         except Exception:
             import traceback
             traceback.print_exc()

kitty_tests/shell_integration.py

@@ -3,19 +3,20 @@
 
 
 import os
+import shlex
 import shutil
 import tempfile
 import unittest
 from contextlib import contextmanager
 
-from kitty.constants import kitty_base_dir, terminfo_dir, is_macos
+from kitty.constants import is_macos, kitty_base_dir, terminfo_dir
 from kitty.fast_data_types import CURSOR_BEAM
-from kitty.shell_integration import rc_inset, setup_zsh_env
+from kitty.shell_integration import setup_bash_env, setup_zsh_env
 
 from . import BaseTest
 
 
-def safe_env_for_running_shell(home_dir, rc='', shell='zsh'):
+def safe_env_for_running_shell(argv, home_dir, rc='', shell='zsh'):
     ans = {
         'PATH': os.environ['PATH'],
         'HOME': home_dir,
@@ -33,38 +34,29 @@ def safe_env_for_running_shell(home_dir, rc='', shell='zsh'):
             print('unset GLOBAL_RCS', file=f)
         with open(os.path.join(home_dir, '.zshrc'), 'w') as f:
             print(rc + '\n', file=f)
-        setup_zsh_env(ans)
+        setup_zsh_env(ans, argv)
     elif shell == 'bash':
-        ans['ENV'] = '~/.bashrc'
+        setup_bash_env(ans, argv)
-        with open(os.path.join(home_dir, '.bashrc'), 'w') as f:
+        ans['KITTY_BASH_INJECT'] += ' posix'
-            # get out of POSIX mode
+        ans['KITTY_BASH_POSIX_ENV'] = os.path.join(home_dir, '.bashrc')
-            print('set +o posix', file=f)
+        with open(ans['KITTY_BASH_POSIX_ENV'], 'w') as f:
             # ensure LINES and COLUMNS are kept up to date
             print('shopt -s checkwinsize', file=f)
             if rc:
                 print(rc, file=f)
-            print(rc_inset('bash'), file=f)
     return ans
 
 
-def launch_cmd_for_shell(shell):
-    if shell == 'bash':
-        # Sadly we cannot use --noprofile as the idiotic Linux distros compile
-        # bash with -DSYS_BASHRC which causes it to unconditionally source the
-        # system wide bashrc file (which is distro dependent). So we use POSIX
-        # mode.
-        return 'bash --posix'
-    return shell
-
-
 class ShellIntegration(BaseTest):
 
     @contextmanager
     def run_shell(self, shell='zsh', rc='', cmd=''):
         home_dir = os.path.realpath(tempfile.mkdtemp())
-        cmd = cmd or launch_cmd_for_shell(shell)
+        cmd = cmd or shell
+        cmd = shlex.split(cmd.format(**locals()))
+        env = safe_env_for_running_shell(cmd, home_dir, rc=rc, shell=shell)
         try:
-            pty = self.create_pty(cmd.format(**locals()), cwd=home_dir, env=safe_env_for_running_shell(home_dir, rc=rc, shell=shell))
+            pty = self.create_pty(cmd, cwd=home_dir, env=env)
             i = 10
             while i > 0 and not pty.screen_contents().strip():
                 pty.process_input_from_child()

shell-integration/bash/kitty.bash

@@ -25,6 +25,44 @@ _ksi_main() {
         # "
     }
 
+    _ksi_safe_source() {
+        if [[ -f "$1" && -r "$1" ]]; then 
+            builtin source "$1"; 
+            builtin return 0; 
+        fi
+        builtin return 1;
+    }
+
+    if [[ -n "$KITTY_BASH_INJECT" ]]; then
+        if [[ "$KITTY_BASH_INJECT" == *"posix"* ]]; then 
+            if [[ -n "$KITTY_BASH_POSIX_ENV" && -r "$KITTY_BASH_POSIX_ENV" ]]; then
+                builtin source "$KITTY_BASH_POSIX_ENV";
+            fi
+        else
+            set +o posix; 
+            # See run_startup_files() in shell.c in the BASH source code
+            if builtin shopt -q login_shell; then
+                if [[ "$KITTY_BASH_INJECT" != *"no-profile"* ]]; then
+                    _ksi_safe_source "/etc/profile";
+                    _ksi_safe_source "$HOME/.bash_profile" || _ksi_safe_source "$HOME/.bash_login" || _ksi_safe_source "$HOME/.profile"; 
+                fi
+            else
+                if [[ "$KITTY_BASH_INJECT" != *"no-rc"* ]]; then
+                    # Linux distros build bash with -DSYS_BASHRC. Unfortunately, there is
+                    # no way to to probe bash for it and different distros use different files
+                    _ksi_safe_source "/etc/bash.bashrc"  # Arch, Debian, Ubuntu
+                    # Fedora uses /etc/bashrc sourced from ~/.bashrc instead of SYS_BASHRC
+                    if [[ -z "$KITTY_BASH_RCFILE" ]]; then KITTY_BASH_RCFILE="$HOME/.bashrc"; fi
+                    _ksi_safe_source "$KITTY_BASH_RCFILE";
+                fi
+            fi
+        fi
+        builtin unset KITTY_BASH_RCFILE;
+        builtin unset KITTY_BASH_POSIX_ENV;
+        builtin unset KITTY_BASH_INJECT;
+    fi
+    builtin unset -f _ksi_safe_source
+
     _ksi_set_mark() { 
         _ksi_prompt["${1}_mark"]="\[\e]133;k;${1}_kitty\a\]" 
     }