From b81406e11ae7ed41a9f3fda5a8daa9c11511111a Mon Sep 17 00:00:00 2001 From: Jan Lindemann Date: Thu, 26 Feb 2026 15:55:52 +0100 Subject: [PATCH] run_cmd() and friends: Make args a list[str] This is a code maintenance commit: some run_xxx() helper functions take a string, some a list, and some just digest all arguments and pass them on as a list to exec() to be executed. That's highly inconsistent. This commit changes that to list-only. Except for the run_cmd() method of SSHClient, which is still run as a shell method, because, erm, it's a shell. Might be changed in the future for consistency reasons. Signed-off-by: Jan Lindemann --- src/python/jw/pkg/cmds/distro/backend/Backend.py | 2 +- src/python/jw/pkg/cmds/distro/backend/Util.py | 2 +- src/python/jw/pkg/cmds/distro/backend/suse/Delete.py | 2 +- src/python/jw/pkg/cmds/distro/backend/suse/Dup.py | 2 +- .../jw/pkg/cmds/distro/backend/suse/Install.py | 2 +- .../jw/pkg/cmds/distro/backend/suse/Refresh.py | 2 +- src/python/jw/pkg/cmds/distro/backend/suse/Util.py | 6 +++--- src/python/jw/pkg/cmds/distro/lib/rpm.py | 9 ++++----- src/python/jw/pkg/cmds/projects/CmdBuild.py | 2 +- src/python/jw/pkg/cmds/projects/CmdGetAuthInfo.py | 2 +- src/python/jw/pkg/lib/SSHClient.py | 2 +- src/python/jw/pkg/lib/util.py | 12 ++++++------ 12 files changed, 22 insertions(+), 23 deletions(-) diff --git a/src/python/jw/pkg/cmds/distro/backend/Backend.py b/src/python/jw/pkg/cmds/distro/backend/Backend.py index 8b46926d..fec5c6f9 100644 --- a/src/python/jw/pkg/cmds/distro/backend/Backend.py +++ b/src/python/jw/pkg/cmds/distro/backend/Backend.py @@ -3,7 +3,7 @@ from __future__ import annotations from typing import TYPE_CHECKING -from ....lib.util import run_cmd, run_sudo +from ....lib.util import run_sudo if TYPE_CHECKING: from ..Cmd import Cmd diff --git a/src/python/jw/pkg/cmds/distro/backend/Util.py b/src/python/jw/pkg/cmds/distro/backend/Util.py index 7a4fbc8d..4ce63934 100644 --- a/src/python/jw/pkg/cmds/distro/backend/Util.py +++ b/src/python/jw/pkg/cmds/distro/backend/Util.py @@ -3,7 +3,7 @@ from __future__ import annotations from typing import TYPE_CHECKING -from ....lib.util import run_cmd, run_sudo +from ....lib.util import run_sudo if TYPE_CHECKING: from ..Cmd import Cmd diff --git a/src/python/jw/pkg/cmds/distro/backend/suse/Delete.py b/src/python/jw/pkg/cmds/distro/backend/suse/Delete.py index e6d67fc0..9f138365 100644 --- a/src/python/jw/pkg/cmds/distro/backend/suse/Delete.py +++ b/src/python/jw/pkg/cmds/distro/backend/suse/Delete.py @@ -11,4 +11,4 @@ class Delete(Base): super().__init__(parent) async def run(self, args: Namespace): - return await self.util.rpm('-e', *args.packages, sudo=True) + return await self.util.rpm(['-e', *args.packages], sudo=True) diff --git a/src/python/jw/pkg/cmds/distro/backend/suse/Dup.py b/src/python/jw/pkg/cmds/distro/backend/suse/Dup.py index 206544a1..d8f26bfe 100644 --- a/src/python/jw/pkg/cmds/distro/backend/suse/Dup.py +++ b/src/python/jw/pkg/cmds/distro/backend/suse/Dup.py @@ -11,4 +11,4 @@ class Dup(Base): super().__init__(parent) async def run(self, args: Namespace): - return await self.util.zypper('dup', '--force-resolution', '--auto-agree-with-licenses') + return await self.util.zypper(['dup', '--force-resolution', '--auto-agree-with-licenses']) diff --git a/src/python/jw/pkg/cmds/distro/backend/suse/Install.py b/src/python/jw/pkg/cmds/distro/backend/suse/Install.py index 9e99e789..665657b0 100644 --- a/src/python/jw/pkg/cmds/distro/backend/suse/Install.py +++ b/src/python/jw/pkg/cmds/distro/backend/suse/Install.py @@ -11,4 +11,4 @@ class Install(Base): super().__init__(parent) async def run(self, args: Namespace): - return await self.util.zypper('install', *args.packages) + return await self.util.zypper(['install', *args.packages]) diff --git a/src/python/jw/pkg/cmds/distro/backend/suse/Refresh.py b/src/python/jw/pkg/cmds/distro/backend/suse/Refresh.py index e9f0269d..f92fe773 100644 --- a/src/python/jw/pkg/cmds/distro/backend/suse/Refresh.py +++ b/src/python/jw/pkg/cmds/distro/backend/suse/Refresh.py @@ -11,4 +11,4 @@ class Refresh(Base): super().__init__(parent) async def run(self, args: Namespace): - return await self.util.zypper('refresh') + return await self.util.zypper(['refresh']) diff --git a/src/python/jw/pkg/cmds/distro/backend/suse/Util.py b/src/python/jw/pkg/cmds/distro/backend/suse/Util.py index e5c44eb9..38ac66dc 100644 --- a/src/python/jw/pkg/cmds/distro/backend/suse/Util.py +++ b/src/python/jw/pkg/cmds/distro/backend/suse/Util.py @@ -10,7 +10,7 @@ class Util(Base): def __init__(self, parent: Cmd): super().__init__(parent) - async def zypper(self, *args): + async def zypper(self, args: list[str]): cmd = ['/usr/bin/zypper'] if not self.interactive: cmd.extend(['--non-interactive', '--gpg-auto-import-keys', '--no-gpg-checks']) @@ -18,5 +18,5 @@ class Util(Base): # Run sudo --login in case /etc/profile modifies ZYPP_CONF return await self._sudo(cmd, opts=['--login']) - async def rpm(self, *args, sudo=False): - return await run_rpm(*args, sudo=sudo) + async def rpm(self, args: list[str], sudo=False): + return await run_rpm(args, sudo=sudo) diff --git a/src/python/jw/pkg/cmds/distro/lib/rpm.py b/src/python/jw/pkg/cmds/distro/lib/rpm.py index e4823048..f7537353 100644 --- a/src/python/jw/pkg/cmds/distro/lib/rpm.py +++ b/src/python/jw/pkg/cmds/distro/lib/rpm.py @@ -6,13 +6,12 @@ from ....lib.util import run_cmd, run_sudo from .Package import Package -async def run_rpm(*args, sudo=False): # export +async def run_rpm(args: list[str], sudo: bool=False): # export cmd = ['/usr/bin/rpm'] cmd.extend(args) - # FIXME: usage of run_xxx() (unpacked vs regular list is highly inconsistent) if sudo: return await run_sudo(cmd) - return await run_cmd(*cmd) + return await run_cmd(cmd) async def all_installed_packages() -> Iterable[Package]: # export ret: list[Package] = [] @@ -25,7 +24,7 @@ async def all_installed_packages() -> Iterable[Package]: # export opts.append('--queryformat') tag_str = '|'.join([f'%{{{tag}}}' for tag in query_tags]) + r'\n' opts.append(tag_str) - package_list_str, stderr = await run_rpm('-qa', *opts, sudo=False) + package_list_str, stderr = await run_rpm(['-qa', *opts], sudo=False) for package_str in package_list_str.splitlines(): tags = package_str.split('|') ret.append(Package(name=tags[0], vendor=tags[1], packager=tags[2], url=tags[3])) @@ -33,5 +32,5 @@ async def all_installed_packages() -> Iterable[Package]: # export async def list_files(pkg: str) -> list[str]: # export opts: list[str] = [] - file_list_str, stderr = await run_rpm('-ql', pkg, *opts, sudo=False) + file_list_str, stderr = await run_rpm(['-ql', pkg, *opts], sudo=False) return file_list_str.splitlines() diff --git a/src/python/jw/pkg/cmds/projects/CmdBuild.py b/src/python/jw/pkg/cmds/projects/CmdBuild.py index 7f0f0514..e8e20e28 100644 --- a/src/python/jw/pkg/cmds/projects/CmdBuild.py +++ b/src/python/jw/pkg/cmds/projects/CmdBuild.py @@ -116,7 +116,7 @@ class CmdBuild(Cmd): # export try: stdout, stderr = await run_cmd( - *make_cmd, + make_cmd, wd=wd, throw=True, verbose=True, diff --git a/src/python/jw/pkg/cmds/projects/CmdGetAuthInfo.py b/src/python/jw/pkg/cmds/projects/CmdGetAuthInfo.py index a37f0b6c..eaae7d78 100644 --- a/src/python/jw/pkg/cmds/projects/CmdGetAuthInfo.py +++ b/src/python/jw/pkg/cmds/projects/CmdGetAuthInfo.py @@ -29,7 +29,7 @@ class CmdGetAuthInfo(Cmd): # export if not os.path.isdir(jw_pkg_dir + '/.git'): log(DEBUG, f'jw-pkg directory is not a Git repo: {jw_pkg_dir}') return - remotes, stderr = await run_cmd('git', '-C', jw_pkg_dir, 'remote', '-v') + remotes, stderr = await run_cmd(['git', '-C', jw_pkg_dir, 'remote', '-v']) result: dict[str, str] = {} for line in remotes.splitlines(): name, url, typ = re.split(r'\s+', line) diff --git a/src/python/jw/pkg/lib/SSHClient.py b/src/python/jw/pkg/lib/SSHClient.py index 233fee67..8790677a 100644 --- a/src/python/jw/pkg/lib/SSHClient.py +++ b/src/python/jw/pkg/lib/SSHClient.py @@ -97,5 +97,5 @@ class SSHClientCmd(SSHClient): # export self.__init_askpass() cmd_arr = ['ssh'] cmd_arr.append(self.hostname) - stdout, stderr = await run_cmd('ssh', self.hostname, cmd) + stdout, stderr = await run_cmd(['ssh', self.hostname, cmd]) return stdout diff --git a/src/python/jw/pkg/lib/util.py b/src/python/jw/pkg/lib/util.py index edfb638c..f916b4cc 100644 --- a/src/python/jw/pkg/lib/util.py +++ b/src/python/jw/pkg/lib/util.py @@ -26,7 +26,7 @@ def pretty_cmd(cmd: list[str], wd=None): return ret async def run_cmd( - *args: str, + args: list[str], wd: str|None = None, throw: bool = True, verbose: bool = False, @@ -39,7 +39,7 @@ async def run_cmd( Run a command asynchronously and return its output Args: - *args: Command and arguments + args: Command and arguments wd: Optional working directory throw: Raise an exception on non-zero exit status if True verbose: Emit log output while the command runs @@ -225,7 +225,7 @@ async def run_curl(args: list[str], parse_json: bool=True, wd=None, throw=None, if not verbose: cmd.append('-s') cmd.extend(args) - ret, stderr = await run_cmd(*cmd, wd=wd, throw=throw, verbose=verbose, cmd_input=cmd_input) + ret, stderr = await run_cmd(cmd, wd=wd, throw=throw, verbose=verbose, cmd_input=cmd_input) if parse_json: try: return json.loads(ret) @@ -259,7 +259,7 @@ async def run_askpass(askpass_env: list[str], key: AskpassKey, host: str|None=No continue # Can't get user name from SSH_ASKPASS case AskpassKey.Password: exe_arg += 'Password' - ret, stderr = await run_cmd(exe, exe_arg, throw=False) + ret, stderr = await run_cmd([exe, exe_arg], throw=False) if ret is not None: return ret return None @@ -279,7 +279,7 @@ async def run_sudo(cmd: list[str], mod_env: dict[str, str] = {}, opts: list[str] cmdline.extend(cmd) if interactive: cmd_input = "mode:interactive" - stdout, stderr = await run_cmd(*cmdline, throw=True, verbose=verbose, env=env, cmd_input=cmd_input) + stdout, stderr = await run_cmd(cmdline, throw=True, verbose=verbose, env=env, cmd_input=cmd_input) return stdout, stderr async def get_username(args: Namespace|None=None, url: str|None=None, askpass_env: list[str]=[]) -> str: # export @@ -328,7 +328,7 @@ async def get_profile_env(throw: bool=True, keep: Iterable[str]|bool=False) -> d } # Run bash as a login shell, which sources /etc/profile, then print environment as NUL-separated key=value pairs cmd = ['/usr/bin/env', '-i', '/bin/bash', '-lc', 'env -0'] - stdout, stderr = await run_cmd(*cmd, throw=throw, output_encoding="bytes", verbose=True, env=env) + stdout, stderr = await run_cmd(cmd, throw=throw, output_encoding="bytes", verbose=True, env=env) ret: dict[str, str] = {} for entry in stdout.split(b"\0"): if not entry: