From 88091b4ab3cb8016dc80dda4c0327e857a9fdcdc Mon Sep 17 00:00:00 2001 From: Kovid Goyal Date: Tue, 22 Feb 2022 21:24:51 +0530 Subject: [PATCH] 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 | 12 ++- docs/changelog.rst | 2 + docs/shell-integration.rst | 6 +- kitty/child.py | 4 +- kitty/main.py | 2 - kitty/shell_integration.py | 129 ++++++++++-------------------- kitty_tests/shell_integration.py | 34 +++----- shell-integration/bash/kitty.bash | 38 +++++++++ 8 files changed, 108 insertions(+), 119 deletions(-) diff --git a/docs/build.rst b/docs/build.rst index 5c00ea0e4..7bab4f011 100644 --- a/docs/build.rst +++ b/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 - the user's :file:`~/.bashrc` file. - For a package, it might make more sense to distribute the shell - integration scripts into the system-wide shell vendor locations. The - shell integration files are found in the :file:`shell-integration` - directory. Copy them to the system wide shell vendor locations for each - shell, and use:: + shell using environment variables. For a package, it might make more sense + to distribute the shell integration scripts into the system-wide shell + vendor locations. The shell integration files are found in the + :file:`shell-integration` directory. Copy them to the system wide shell + vendor locations for each shell, and use:: ./setup.py linux-package --shell-integration=enabled\ no-rc diff --git a/docs/changelog.rst b/docs/changelog.rst index a5e314b2f..a70ab7bc0 100644 --- a/docs/changelog.rst +++ b/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 ` + - macOS: Allow kitty to handle various URL types. Can be configured via :ref:`launch_actions` (:pull:`4618`) diff --git a/docs/shell-integration.rst b/docs/shell-integration.rst index 14d398346..5629841e0 100644 --- a/docs/shell-integration.rst +++ b/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` - (in an atomic manner) to load the shell integration code. + For bash, kitty starts bash in POSIX mode and implements the loading of the + 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 diff --git a/kitty/child.py b/kitty/child.py index f56b3de47..f9fb2fa6e 100644 --- a/kitty/child.py +++ b/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 diff --git a/kitty/main.py b/kitty/main.py index dc209a313..4dfee9fae 100644 --- a/kitty/main.py +++ b/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') diff --git a/kitty/shell_integration.py b/kitty/shell_integration.py index a6445351c..5b2745361 100644 --- a/kitty/shell_integration.py +++ b/kitty/shell_integration.py @@ -3,65 +3,14 @@ import os -from contextlib import suppress -from typing import Optional, Union, Dict +from typing import Optional, Dict, List from .options.types import Options -from .config import atomic_save from .constants import shell_integration_dir -from .utils import log_error, resolved_shell - -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\ -''' +from .utils import log_error -def atomic_write(path: str, data: Union[str, bytes]) -> 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: +def setup_fish_env(env: Dict[str, str], argv: List[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 = { - 'zsh': setup_zsh_integration, - 'bash': setup_bash_integration, - 'fish': setup_fish_integration, -} +def setup_bash_env(env: Dict[str, str], argv: List[str]) -> None: + inject = {'1'} + posix_env = rcfile = '' + 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 = name.replace('-', '') - if name in SUPPORTED_SHELLS: - return name - return None + name = os.path.basename(path) + if name.lower().endswith('.exe'): + name = name.rpartition('.')[0] + if name.startswith('-'): + 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: - if not shell_integration_allows_rc_modification(opts): - 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) +def modify_shell_environ(opts: Options, env: Dict[str, str], argv: List[str]) -> None: + shell = get_supported_shell_name(argv[0]) 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() diff --git a/kitty_tests/shell_integration.py b/kitty_tests/shell_integration.py index bc533a77e..e447d2a1c 100644 --- a/kitty_tests/shell_integration.py +++ b/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' - with open(os.path.join(home_dir, '.bashrc'), 'w') as f: - # get out of POSIX mode - print('set +o posix', file=f) + setup_bash_env(ans, argv) + ans['KITTY_BASH_INJECT'] += ' posix' + ans['KITTY_BASH_POSIX_ENV'] = os.path.join(home_dir, '.bashrc') + 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() diff --git a/shell-integration/bash/kitty.bash b/shell-integration/bash/kitty.bash index c8991efc7..3d17e9311 100644 --- a/shell-integration/bash/kitty.bash +++ b/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\]" }