From 1b4cf1fea7f67d0aa697ac08b194bd7679775279 Mon Sep 17 00:00:00 2001 From: Kovid Goyal Date: Sat, 14 May 2022 10:31:18 +0530 Subject: [PATCH] Remote file kitten: Integrate with the ssh kitten --- docs/changelog.rst | 4 ++ docs/kittens/remote_file.rst | 9 ++-- kittens/remote_file/main.py | 94 +++++++++++++++++++++++++----------- kitty/window.py | 19 ++++++-- 4 files changed, 89 insertions(+), 37 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 9e466a517..2ae373349 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -49,6 +49,10 @@ Detailed list of changes - Fix a regression in the previous release that caused mouse move events to be incorrectly reported as drag events even when a button is not pressed (:iss:`4992`) +- remote file kitten: Integrate with the ssh kitten for improved performance + and robustness. Re-uses the control master connection of the ssh kitten to + avoid round-trip latency. + - Fix tab selection when closing a new tab not correct in some scenarios (:iss:`4987`) - A new action :ac:`open_url` to open the specified URL (:pull:`5004`) diff --git a/docs/kittens/remote_file.rst b/docs/kittens/remote_file.rst index 847bd919a..1fba02349 100644 --- a/docs/kittens/remote_file.rst +++ b/docs/kittens/remote_file.rst @@ -27,13 +27,16 @@ supports hyperlinks. .. versionadded:: 0.19.0 .. note:: - Nested SSH sessions are not supported. The kitten will always try to copy - remote files from the first SSH host. This is because there is no way for + For best results, use this kitten with the :doc:`ssh kitten <./ssh>`. + Otherwise, nested SSH sessions are not supported. The kitten will always try to copy + remote files from the first SSH host. This is because, without the ssh + kitten, there is no way for |kitty| to detect and follow a nested SSH session robustly. Use the :doc:`transfer` kitten for such situations. .. note:: - If you have not setup automatic password-less SSH access, then, when editing + If you have not setup automatic password-less SSH access, and are not using + the ssh kitten, then, when editing starts you will be asked to enter your password just once, thereafter the SSH connection will be re-used. diff --git a/kittens/remote_file/main.py b/kittens/remote_file/main.py index a3221f8d6..cf7d0c348 100644 --- a/kittens/remote_file/main.py +++ b/kittens/remote_file/main.py @@ -27,6 +27,9 @@ from ..tui.operations import ( from ..tui.utils import get_key_press +is_ssh_kitten_sentinel = '!#*&$#($ssh-kitten)(##$' + + def key(x: str) -> str: return styled(x, bold=True, fg='green') @@ -53,10 +56,9 @@ The data used to connect over ssh. def show_error(msg: str) -> None: - print(styled(msg, fg='red')) + print(styled(msg, fg='red'), file=sys.stderr) print() - print('Press any key to quit') - sys.stdout.flush() + print('Press any key to quit', flush=True) with raw_mode(): while True: try: @@ -112,15 +114,25 @@ class ControlMaster: self.remote_path = remote_path self.dest = dest self.tdir = '' + self.last_error_log = '' self.cmd_prefix = cmd = [ conn_data.binary, '-o', f'ControlPath=~/.ssh/kitty-master-{os.getpid()}-%r@%h:%p', '-o', 'TCPKeepAlive=yes', '-o', 'ControlPersist=yes' ] - if conn_data.port: - cmd.extend(['-p', str(conn_data.port)]) - if conn_data.identity_file: - cmd.extend(['-i', conn_data.identity_file]) - self.batch_cmd_prefix = cmd + ['-o', 'BatchMode=yes'] + self.is_ssh_kitten = conn_data.binary is is_ssh_kitten_sentinel + if self.is_ssh_kitten: + del cmd[:] + self.batch_cmd_prefix = cmd + sk_cmdline = json.loads(conn_data.identity_file) + while '-t' in sk_cmdline: + sk_cmdline.remove('-t') + cmd.extend(sk_cmdline[:-2]) + else: + if conn_data.port: + cmd.extend(['-p', str(conn_data.port)]) + if conn_data.identity_file: + cmd.extend(['-i', conn_data.identity_file]) + self.batch_cmd_prefix = cmd + ['-o', 'BatchMode=yes'] def check_call(self, cmd: List[str]) -> None: p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, stdin=subprocess.DEVNULL) @@ -130,31 +142,37 @@ class ControlMaster: raise Exception(f'The ssh command: {shlex.join(cmd)} failed with exit code {p.returncode} and output: {out}') def __enter__(self) -> 'ControlMaster': - self.check_call( - self.cmd_prefix + ['-o', 'ControlMaster=auto', '-fN', self.conn_data.hostname]) - self.check_call( - self.batch_cmd_prefix + ['-O', 'check', self.conn_data.hostname]) + if not self.is_ssh_kitten: + self.check_call( + self.cmd_prefix + ['-o', 'ControlMaster=auto', '-fN', self.conn_data.hostname]) + self.check_call( + self.batch_cmd_prefix + ['-O', 'check', self.conn_data.hostname]) if not self.dest: self.tdir = tempfile.mkdtemp() self.dest = os.path.join(self.tdir, os.path.basename(self.remote_path)) return self def __exit__(self, *a: Any) -> None: - subprocess.Popen( - self.batch_cmd_prefix + ['-O', 'exit', self.conn_data.hostname], - stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, stdin=subprocess.DEVNULL - ).wait() + if not self.is_ssh_kitten: + subprocess.Popen( + self.batch_cmd_prefix + ['-O', 'exit', self.conn_data.hostname], + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, stdin=subprocess.DEVNULL + ).wait() if self.tdir: shutil.rmtree(self.tdir) @property def is_alive(self) -> bool: + if self.is_ssh_kitten: + return True return subprocess.Popen( self.batch_cmd_prefix + ['-O', 'check', self.conn_data.hostname], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, stdin=subprocess.DEVNULL ).wait() == 0 def check_hostname_matches(self) -> bool: + if self.is_ssh_kitten: + return True cp = subprocess.run(self.batch_cmd_prefix + [self.conn_data.hostname, 'hostname', '-f'], stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, stdin=subprocess.DEVNULL) if cp.returncode == 0: @@ -181,21 +199,35 @@ class ControlMaster: return response == 'y' return True + def show_error(self, msg: str) -> None: + if self.last_error_log: + print(self.last_error_log, file=sys.stderr) + self.last_error_log = '' + show_error(msg) + def download(self) -> bool: + cmdline = self.batch_cmd_prefix + [self.conn_data.hostname, 'cat', self.remote_path] with open(self.dest, 'wb') as f: - return subprocess.run( - self.batch_cmd_prefix + [self.conn_data.hostname, 'cat', self.remote_path], - stdout=f, stderr=subprocess.DEVNULL, stdin=subprocess.DEVNULL - ).returncode == 0 + cp = subprocess.run(cmdline, stdout=f, stderr=subprocess.PIPE, stdin=subprocess.DEVNULL) + if cp.returncode != 0: + self.last_error_log = f'The command: {shlex.join(cmdline)} failed\n' + cp.stderr.decode() + return False + return True def upload(self, suppress_output: bool = True) -> bool: cmd_prefix = self.cmd_prefix if suppress_output else self.batch_cmd_prefix cmd = cmd_prefix + [self.conn_data.hostname, 'cat', '>', self.remote_path] if not suppress_output: - print(' '.join(map(shlex.quote, cmd))) - redirect = subprocess.DEVNULL if suppress_output else None + print(shlex.join(cmd)) with open(self.dest, 'rb') as f: - return subprocess.run(cmd, stdout=redirect, stderr=redirect, stdin=f).returncode == 0 + if suppress_output: + cp = subprocess.run(cmd, stdin=f, capture_output=True) + if cp.returncode == 0: + return True + self.last_error_log = f'The command: {shlex.join(cmd)} failed\n' + cp.stdout.decode() + else: + return subprocess.run(cmd, stdin=f).returncode == 0 + return False Result = Optional[str] @@ -278,11 +310,15 @@ def save_as(conn_data: SSHConnectionData, remote_path: str, cli_opts: RemoteFile with ControlMaster(conn_data, remote_path, cli_opts, dest=dest) as master: if master.check_hostname_matches(): if not master.download(): - show_error('Failed to copy file from remote machine') + master.show_error('Failed to copy file from remote machine') def handle_action(action: str, cli_opts: RemoteFileCLIOptions) -> Result: - conn_data = SSHConnectionData(*json.loads(cli_opts.ssh_connection_data or '')) + cli_data = json.loads(cli_opts.ssh_connection_data or '') + if cli_data and cli_data[0] == is_ssh_kitten_sentinel: + conn_data = SSHConnectionData(is_ssh_kitten_sentinel, cli_data[-1], -1, identity_file=json.dumps(cli_data[1:])) + else: + conn_data = SSHConnectionData(*cli_data) remote_path = cli_opts.path or '' if action == 'open': print('Opening', cli_opts.path, 'from', cli_opts.hostname) @@ -291,7 +327,7 @@ def handle_action(action: str, cli_opts: RemoteFileCLIOptions) -> Result: if master.check_hostname_matches(): if master.download(): return dest - show_error('Failed to copy file from remote machine') + master.show_error('Failed to copy file from remote machine') elif action == 'edit': print('Editing', cli_opts.path, 'from', cli_opts.hostname) editor = get_editor() @@ -299,7 +335,7 @@ def handle_action(action: str, cli_opts: RemoteFileCLIOptions) -> Result: if not master.check_hostname_matches(): return None if not master.download(): - show_error(f'Failed to download {remote_path}') + master.show_error(f'Failed to download {remote_path}') return None mtime = os.path.getmtime(master.dest) print(reset_terminal(), end='', flush=True) @@ -314,9 +350,9 @@ def handle_action(action: str, cli_opts: RemoteFileCLIOptions) -> Result: print(reset_terminal(), end='', flush=True) if master.is_alive: if not master.upload(suppress_output=False): - show_error(f'Failed to upload {remote_path}') + master.show_error(f'Failed to upload {remote_path}') else: - show_error(f'Failed to upload {remote_path}, SSH master process died') + master.show_error(f'Failed to upload {remote_path}, SSH master process died') elif action == 'save': print('Saving', cli_opts.path, 'from', cli_opts.hostname) save_as(conn_data, remote_path, cli_opts) diff --git a/kitty/window.py b/kitty/window.py index 53d8c668d..c5e43b75e 100644 --- a/kitty/window.py +++ b/kitty/window.py @@ -829,12 +829,21 @@ class Window: set_clipboard_string(url) def handle_remote_file(self, netloc: str, remote_path: str) -> None: + from .utils import SSHConnectionData from kittens.ssh.main import get_connection_data - args = self.child.foreground_cmdline - conn_data = get_connection_data(args, self.child.foreground_cwd or self.child.current_cwd or '') - if conn_data is None: - get_boss().show_error('Could not handle remote file', f'No SSH connection data found in: {args}') - return + from kittens.remote_file.main import is_ssh_kitten_sentinel + args = self.ssh_kitten_cmdline() + conn_data: Union[None, List[str], SSHConnectionData] = None + if args: + ssh_cmdline = sorted(self.child.foreground_processes, key=lambda p: p['pid'])[-1]['cmdline'] or [''] + idx = ssh_cmdline.index('--') + conn_data = [is_ssh_kitten_sentinel] + list(ssh_cmdline[:idx + 2]) + else: + args = self.child.foreground_cmdline + conn_data = get_connection_data(args, self.child.foreground_cwd or self.child.current_cwd or '') + if conn_data is None: + get_boss().show_error('Could not handle remote file', f'No SSH connection data found in: {args}') + return get_boss().run_kitten( 'remote_file', '--hostname', netloc.partition(':')[0], '--path', remote_path, '--ssh-connection-data', json.dumps(conn_data)