From 0e2cb172e63020f6cbfb973c2e25a494a8f41e38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Tue, 26 Mar 2024 12:07:15 +0100 Subject: [PATCH 1/6] cli/ssh: allocate tty by default -t is only enabled when the local ssh command is also connected to a tty, so it seems to be enabled by default. --- pkgs/clan-cli/clan_cli/ssh/__init__.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/pkgs/clan-cli/clan_cli/ssh/__init__.py b/pkgs/clan-cli/clan_cli/ssh/__init__.py index b0c3b3f02..89e38b5ab 100644 --- a/pkgs/clan-cli/clan_cli/ssh/__init__.py +++ b/pkgs/clan-cli/clan_cli/ssh/__init__.py @@ -400,6 +400,7 @@ class Host: check: bool = True, verbose_ssh: bool = False, timeout: float = math.inf, + tty: bool = True, ) -> subprocess.CompletedProcess[str]: """ Command to run on the host via ssh @@ -444,7 +445,7 @@ class Host: bash_cmd += cmd # FIXME we assume bash to be present here? Should be documented... ssh_cmd = [ - *self.ssh_cmd(verbose_ssh=verbose_ssh), + *self.ssh_cmd(verbose_ssh=verbose_ssh, tty=tty), "--", f"{sudo}bash -c {quote(bash_cmd)} -- {' '.join(map(quote, bash_args))}", ] @@ -462,6 +463,7 @@ class Host: def ssh_cmd( self, verbose_ssh: bool = False, + tty: bool = True, ) -> list[str]: if self.user is not None: ssh_target = f"{self.user}@{self.host}" @@ -484,6 +486,8 @@ class Host: ssh_opts.extend(["-o", "UserKnownHostsFile=/dev/null"]) if verbose_ssh or self.verbose_ssh: ssh_opts.extend(["-v"]) + if tty: + ssh_opts.extend(["-t"]) return ["ssh", ssh_target, *ssh_opts] @@ -547,6 +551,7 @@ class HostGroup: check: bool = True, verbose_ssh: bool = False, timeout: float = math.inf, + tty: bool = True, ) -> None: try: proc = host.run_local( @@ -575,6 +580,7 @@ class HostGroup: check: bool = True, verbose_ssh: bool = False, timeout: float = math.inf, + tty: bool = True, ) -> None: try: proc = host.run( @@ -586,6 +592,7 @@ class HostGroup: check=check, verbose_ssh=verbose_ssh, timeout=timeout, + tty=tty, ) results.append(HostResult(host, proc)) except Exception as e: @@ -617,6 +624,7 @@ class HostGroup: cwd: None | str | Path = None, check: bool = True, verbose_ssh: bool = False, + tty: bool = True, timeout: float = math.inf, ) -> Results: results: Results = [] @@ -636,6 +644,7 @@ class HostGroup: check=check, verbose_ssh=verbose_ssh, timeout=timeout, + tty=tty, ), ) thread.start() @@ -659,6 +668,7 @@ class HostGroup: check: bool = True, verbose_ssh: bool = False, timeout: float = math.inf, + tty: bool = True, ) -> Results: """ Command to run on the remote host via ssh @@ -679,6 +689,7 @@ class HostGroup: check=check, verbose_ssh=verbose_ssh, timeout=timeout, + tty=True, ) def run_local( From c5db14dea8fbcd5fa3a8b8e0a69407d7bf56e48e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Tue, 26 Mar 2024 12:39:27 +0100 Subject: [PATCH 2/6] ssh: add interactive flag --- pkgs/clan-cli/clan_cli/ssh/__init__.py | 64 ++++++++++++++++---------- 1 file changed, 40 insertions(+), 24 deletions(-) diff --git a/pkgs/clan-cli/clan_cli/ssh/__init__.py b/pkgs/clan-cli/clan_cli/ssh/__init__.py index 89e38b5ab..2d3ef34fe 100644 --- a/pkgs/clan-cli/clan_cli/ssh/__init__.py +++ b/pkgs/clan-cli/clan_cli/ssh/__init__.py @@ -270,30 +270,37 @@ class Host: cwd: None | str | Path = None, check: bool = True, timeout: float = math.inf, + interactive: bool = False, ) -> subprocess.CompletedProcess[str]: with ExitStack() as stack: read_std_fd, write_std_fd = (None, None) read_err_fd, write_err_fd = (None, None) - if stdout is None or stderr is None: + if not interactive and (stdout is None or stderr is None): read_std_fd, write_std_fd = stack.enter_context(_pipe()) read_err_fd, write_err_fd = stack.enter_context(_pipe()) - if stdout is None: + if interactive: stdout_read = None - stdout_write = write_std_fd - elif stdout == subprocess.PIPE: - stdout_read, stdout_write = stack.enter_context(_pipe()) - else: - raise Exception(f"unsupported value for stdout parameter: {stdout}") - - if stderr is None: stderr_read = None - stderr_write = write_err_fd - elif stderr == subprocess.PIPE: - stderr_read, stderr_write = stack.enter_context(_pipe()) + stdout_write: IO[str] | None = sys.stdout + stderr_write: IO[str] | None = sys.stderr else: - raise Exception(f"unsupported value for stderr parameter: {stderr}") + if stdout is None: + stdout_read = None + stdout_write = write_std_fd + elif stdout == subprocess.PIPE: + stdout_read, stdout_write = stack.enter_context(_pipe()) + else: + raise Exception(f"unsupported value for stdout parameter: {stdout}") + + if stderr is None: + stderr_read = None + stderr_write = write_err_fd + elif stderr == subprocess.PIPE: + stderr_read, stderr_write = stack.enter_context(_pipe()) + else: + raise Exception(f"unsupported value for stderr parameter: {stderr}") env = os.environ.copy() env.update(extra_env) @@ -319,14 +326,17 @@ class Host: stderr_write.close() start = time.time() - stdout_data, stderr_data = self._prefix_output( - displayed_cmd, - read_std_fd, - read_err_fd, - stdout_read, - stderr_read, - timeout, - ) + if interactive: + stdout_data, stderr_data = "", "" + else: + stdout_data, stderr_data = self._prefix_output( + displayed_cmd, + read_std_fd, + read_err_fd, + stdout_read, + stderr_read, + timeout, + ) try: ret = p.wait(timeout=max(0, timeout - (time.time() - start))) except subprocess.TimeoutExpired: @@ -356,6 +366,7 @@ class Host: cwd: None | str | Path = None, check: bool = True, timeout: float = math.inf, + interactive: bool = False, ) -> subprocess.CompletedProcess[str]: """ Command to run locally for the host @@ -387,6 +398,7 @@ class Host: cwd=cwd, check=check, timeout=timeout, + interactive=interactive, ) def run( @@ -398,8 +410,9 @@ class Host: extra_env: dict[str, str] = {}, cwd: None | str | Path = None, check: bool = True, - verbose_ssh: bool = False, timeout: float = math.inf, + interactive: bool = False, + verbose_ssh: bool = False, tty: bool = True, ) -> subprocess.CompletedProcess[str]: """ @@ -458,6 +471,7 @@ class Host: cwd=cwd, check=check, timeout=timeout, + interactive=interactive, ) def ssh_cmd( @@ -623,9 +637,10 @@ class HostGroup: extra_env: dict[str, str] = {}, cwd: None | str | Path = None, check: bool = True, + timeout: float = math.inf, + interactive: bool = False, verbose_ssh: bool = False, tty: bool = True, - timeout: float = math.inf, ) -> Results: results: Results = [] threads = [] @@ -642,8 +657,9 @@ class HostGroup: extra_env=extra_env, cwd=cwd, check=check, - verbose_ssh=verbose_ssh, timeout=timeout, + interactive=interactive, + verbose_ssh=verbose_ssh, tty=tty, ), ) From 10a12eb85c06e541af3d2492c8327b94b826abb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Tue, 26 Mar 2024 12:45:26 +0100 Subject: [PATCH 3/6] ruff: switch to check subcommand --- formatter.nix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/formatter.nix b/formatter.nix index 1e2e1c35d..fc884feff 100644 --- a/formatter.nix +++ b/formatter.nix @@ -37,7 +37,7 @@ options = [ "-eucx" '' - ${lib.getExe pkgs.ruff} --fix "$@" + ${lib.getExe pkgs.ruff} check --fix "$@" ${lib.getExe pkgs.ruff} format "$@" '' "--" # this argument is ignored by bash From 54f0526c5bec6870dda44d29e078ecef0ac9ead9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Tue, 26 Mar 2024 12:45:33 +0100 Subject: [PATCH 4/6] update nixos-generators --- flake.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/flake.lock b/flake.lock index 68cbea4d0..b3f18b2ee 100644 --- a/flake.lock +++ b/flake.lock @@ -63,11 +63,11 @@ ] }, "locked": { - "lastModified": 1711327729, - "narHash": "sha256-RzOXI1kBlK7HkeZfRjUnsBUJEmlMYgLEG7CziZN0lgs=", + "lastModified": 1711375484, + "narHash": "sha256-+d4HqehyQvuHUKR8Nv9HGGd/SP5wjg3MA/hEYJBWQq0=", "owner": "nix-community", "repo": "nixos-generators", - "rev": "d3e8145766dad6b47f6e37ce28731a05144dec26", + "rev": "2b3720c7af2271be8cee713cd2f69c5127b0a8e4", "type": "github" }, "original": { From 7b8a49bf6c7a46f2262c389cae7dcac4f76c45f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Tue, 26 Mar 2024 13:05:11 +0100 Subject: [PATCH 5/6] ssh: default tty to False nix behaves weird when the terminal is interactive because we are also do line buffering. --- pkgs/clan-cli/clan_cli/ssh/__init__.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/pkgs/clan-cli/clan_cli/ssh/__init__.py b/pkgs/clan-cli/clan_cli/ssh/__init__.py index 2d3ef34fe..66a8715ab 100644 --- a/pkgs/clan-cli/clan_cli/ssh/__init__.py +++ b/pkgs/clan-cli/clan_cli/ssh/__init__.py @@ -413,7 +413,7 @@ class Host: timeout: float = math.inf, interactive: bool = False, verbose_ssh: bool = False, - tty: bool = True, + tty: bool = False, ) -> subprocess.CompletedProcess[str]: """ Command to run on the host via ssh @@ -477,7 +477,7 @@ class Host: def ssh_cmd( self, verbose_ssh: bool = False, - tty: bool = True, + tty: bool = False, ) -> list[str]: if self.user is not None: ssh_target = f"{self.user}@{self.host}" @@ -565,7 +565,7 @@ class HostGroup: check: bool = True, verbose_ssh: bool = False, timeout: float = math.inf, - tty: bool = True, + tty: bool = False, ) -> None: try: proc = host.run_local( @@ -594,7 +594,7 @@ class HostGroup: check: bool = True, verbose_ssh: bool = False, timeout: float = math.inf, - tty: bool = True, + tty: bool = False, ) -> None: try: proc = host.run( @@ -640,7 +640,7 @@ class HostGroup: timeout: float = math.inf, interactive: bool = False, verbose_ssh: bool = False, - tty: bool = True, + tty: bool = False, ) -> Results: results: Results = [] threads = [] @@ -684,7 +684,7 @@ class HostGroup: check: bool = True, verbose_ssh: bool = False, timeout: float = math.inf, - tty: bool = True, + tty: bool = False, ) -> Results: """ Command to run on the remote host via ssh @@ -705,7 +705,7 @@ class HostGroup: check=check, verbose_ssh=verbose_ssh, timeout=timeout, - tty=True, + tty=tty, ) def run_local( From 80abeef994066b574cabf02dde76e03d90e5095f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Tue, 26 Mar 2024 13:13:35 +0100 Subject: [PATCH 6/6] Revert "ssh: add interactive flag" This reverts commit c5db14dea8fbcd5fa3a8b8e0a69407d7bf56e48e. --- pkgs/clan-cli/clan_cli/ssh/__init__.py | 58 ++++++++++---------------- 1 file changed, 21 insertions(+), 37 deletions(-) diff --git a/pkgs/clan-cli/clan_cli/ssh/__init__.py b/pkgs/clan-cli/clan_cli/ssh/__init__.py index 66a8715ab..fb434a2fd 100644 --- a/pkgs/clan-cli/clan_cli/ssh/__init__.py +++ b/pkgs/clan-cli/clan_cli/ssh/__init__.py @@ -270,37 +270,30 @@ class Host: cwd: None | str | Path = None, check: bool = True, timeout: float = math.inf, - interactive: bool = False, ) -> subprocess.CompletedProcess[str]: with ExitStack() as stack: read_std_fd, write_std_fd = (None, None) read_err_fd, write_err_fd = (None, None) - if not interactive and (stdout is None or stderr is None): + if stdout is None or stderr is None: read_std_fd, write_std_fd = stack.enter_context(_pipe()) read_err_fd, write_err_fd = stack.enter_context(_pipe()) - if interactive: + if stdout is None: stdout_read = None - stderr_read = None - stdout_write: IO[str] | None = sys.stdout - stderr_write: IO[str] | None = sys.stderr + stdout_write = write_std_fd + elif stdout == subprocess.PIPE: + stdout_read, stdout_write = stack.enter_context(_pipe()) else: - if stdout is None: - stdout_read = None - stdout_write = write_std_fd - elif stdout == subprocess.PIPE: - stdout_read, stdout_write = stack.enter_context(_pipe()) - else: - raise Exception(f"unsupported value for stdout parameter: {stdout}") + raise Exception(f"unsupported value for stdout parameter: {stdout}") - if stderr is None: - stderr_read = None - stderr_write = write_err_fd - elif stderr == subprocess.PIPE: - stderr_read, stderr_write = stack.enter_context(_pipe()) - else: - raise Exception(f"unsupported value for stderr parameter: {stderr}") + if stderr is None: + stderr_read = None + stderr_write = write_err_fd + elif stderr == subprocess.PIPE: + stderr_read, stderr_write = stack.enter_context(_pipe()) + else: + raise Exception(f"unsupported value for stderr parameter: {stderr}") env = os.environ.copy() env.update(extra_env) @@ -326,17 +319,14 @@ class Host: stderr_write.close() start = time.time() - if interactive: - stdout_data, stderr_data = "", "" - else: - stdout_data, stderr_data = self._prefix_output( - displayed_cmd, - read_std_fd, - read_err_fd, - stdout_read, - stderr_read, - timeout, - ) + stdout_data, stderr_data = self._prefix_output( + displayed_cmd, + read_std_fd, + read_err_fd, + stdout_read, + stderr_read, + timeout, + ) try: ret = p.wait(timeout=max(0, timeout - (time.time() - start))) except subprocess.TimeoutExpired: @@ -366,7 +356,6 @@ class Host: cwd: None | str | Path = None, check: bool = True, timeout: float = math.inf, - interactive: bool = False, ) -> subprocess.CompletedProcess[str]: """ Command to run locally for the host @@ -398,7 +387,6 @@ class Host: cwd=cwd, check=check, timeout=timeout, - interactive=interactive, ) def run( @@ -411,7 +399,6 @@ class Host: cwd: None | str | Path = None, check: bool = True, timeout: float = math.inf, - interactive: bool = False, verbose_ssh: bool = False, tty: bool = False, ) -> subprocess.CompletedProcess[str]: @@ -471,7 +458,6 @@ class Host: cwd=cwd, check=check, timeout=timeout, - interactive=interactive, ) def ssh_cmd( @@ -638,7 +624,6 @@ class HostGroup: cwd: None | str | Path = None, check: bool = True, timeout: float = math.inf, - interactive: bool = False, verbose_ssh: bool = False, tty: bool = False, ) -> Results: @@ -658,7 +643,6 @@ class HostGroup: cwd=cwd, check=check, timeout=timeout, - interactive=interactive, verbose_ssh=verbose_ssh, tty=tty, ),