From 88556278af74afc079409c5b88abdd18e5848bd9 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sat, 28 May 2022 12:00:46 -0500 Subject: [PATCH 1/6] chore(run): Add *args, **kwargs passthrough to subprocess.Popen --- libvcs/_internal/run.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libvcs/_internal/run.py b/libvcs/_internal/run.py index 7cadbed23..72df3557c 100644 --- a/libvcs/_internal/run.py +++ b/libvcs/_internal/run.py @@ -150,6 +150,8 @@ def run( log_in_real_time: bool = True, check_returncode: bool = True, callback: Optional[ProgressCallbackProtocol] = None, + *args, + **kwargs ): """Run 'cmd' in a shell and return the combined contents of stdout and stderr (Blocking). Throws an exception if the command exits non-zero. @@ -192,6 +194,8 @@ def progress_cb(output, timestamp): stderr=subprocess.PIPE, stdout=subprocess.PIPE, cwd=cwd, + *args, + **kwargs ) all_output = [] From 4aa0b52e45a6c53be4094b9a39512e3b26b03fc2 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sat, 28 May 2022 13:12:34 -0500 Subject: [PATCH 2/6] refactor!(run): Match subprocess.Popen --- libvcs/_internal/run.py | 110 +++++++++++++++++++++++++++++++++++++--- libvcs/cmd/git.py | 2 +- libvcs/cmd/hg.py | 5 +- libvcs/cmd/svn.py | 5 +- 4 files changed, 109 insertions(+), 13 deletions(-) diff --git a/libvcs/_internal/run.py b/libvcs/_internal/run.py index 72df3557c..847cecafa 100644 --- a/libvcs/_internal/run.py +++ b/libvcs/_internal/run.py @@ -13,7 +13,20 @@ import os import subprocess import sys -from typing import Optional, Protocol, Union +from typing import ( + IO, + Any, + Callable, + Iterable, + Mapping, + Optional, + Protocol, + Sequence, + Union, + overload, +) + +from typing_extensions import TypeAlias from .. import exc from ..types import StrOrBytesPath @@ -143,15 +156,97 @@ def __call__(self, output: Union[str, bytes], timestamp: datetime.datetime): ... +if sys.platform == "win32": + _ENV: TypeAlias = Mapping[str, str] +else: + _ENV: TypeAlias = Mapping[bytes, StrOrBytesPath] | Mapping[str, StrOrBytesPath] + +_CMD = Union[StrOrBytesPath, Sequence[StrOrBytesPath]] +_FILE: TypeAlias = None | int | IO[Any] + +# def run( +# args: _CMD, +# bufsize: int = -1, +# executable: StrOrBytesPath | None = None, +# stdin: _FILE | None = None, +# stdout: _FILE | None = None, +# stderr: _FILE | None = None, +# preexec_fn: Callable[[], Any] | None = None, +# close_fds: bool = True, +# shell: bool = False, +# cwd: StrOrBytesPath | None = None, +# env: _ENV | None = None, +# universal_newlines: bool = False, +# startupinfo: Any | None = None, +# creationflags: int = 0, +# restore_signals: bool = True, +# start_new_session: bool = False, +# pass_fds: Any = None, +# *, +# text: bool | None = None, +# encoding: str = "utf-8", +# errors: str | None = None, +# user: str | int | None = None, +# group: str | int | None = None, +# extra_groups: Iterable[str | int] | None = None, +# umask: int = -1, +# pipesize: int = -1, +# +# +# # custom +# log_in_real_time: bool = True, +# check_returncode: bool = True, +# callback: Optional[ProgressCallbackProtocol] = None, +# ): + + +@overload def run( - cmd: Union[str, list[str]], + args: _CMD, + bufsize: int = ..., + executable: StrOrBytesPath | None = ..., + stdin: _FILE | None = ..., + stdout: _FILE | None = ..., + stderr: _FILE | None = ..., + preexec_fn: Callable[[], Any] | None = ..., + close_fds: bool = ..., + shell: bool = ..., + cwd: Optional[StrOrBytesPath] = ..., + env: _ENV | None = ..., + universal_newlines: bool = ..., + startupinfo: Any | None = ..., + creationflags: int = ..., + restore_signals: bool = ..., + start_new_session: bool = ..., + pass_fds: Any = ..., + *, + text: bool | None = ..., + encoding: str = "utf-8", + errors: str | None = ..., + user: str | int | None = ..., + group: str | int | None = ..., + extra_groups: Iterable[str | int] | None = ..., + umask: int = ..., + pipesize: int = ..., + # custom + log_in_real_time: bool = True, + check_returncode: bool = True, + callback: Optional[ProgressCallbackProtocol] = None, +): + ... + + +def run( + # args: Union[str, list[str]], + args: _CMD, shell: bool = False, cwd: Optional[StrOrBytesPath] = None, + *, + # custom log_in_real_time: bool = True, check_returncode: bool = True, callback: Optional[ProgressCallbackProtocol] = None, - *args, - **kwargs + **kwargs, ): """Run 'cmd' in a shell and return the combined contents of stdout and stderr (Blocking). Throws an exception if the command exits non-zero. @@ -189,13 +284,12 @@ def progress_cb(output, timestamp): run(['git', 'pull'], callback=progress_cb) """ proc = subprocess.Popen( - cmd, + args, shell=shell, stderr=subprocess.PIPE, stdout=subprocess.PIPE, cwd=cwd, - *args, - **kwargs + **kwargs, ) all_output = [] @@ -220,5 +314,5 @@ def progress_cb(output, timestamp): all_output = console_to_str(b"".join(stderr_lines)) output = "".join(all_output) if code != 0 and check_returncode: - raise exc.CommandError(output=output, returncode=code, cmd=cmd) + raise exc.CommandError(output=output, returncode=code, cmd=args) return output diff --git a/libvcs/cmd/git.py b/libvcs/cmd/git.py index 47ef047c3..cc7491ce3 100644 --- a/libvcs/cmd/git.py +++ b/libvcs/cmd/git.py @@ -181,7 +181,7 @@ def run( if no_optional_locks is True: cli_args.append("--no-optional-locks") - return run(cmd=cli_args, **kwargs) + return run(args=cli_args, **kwargs) def clone( self, diff --git a/libvcs/cmd/hg.py b/libvcs/cmd/hg.py index 0b116010a..aed789df0 100644 --- a/libvcs/cmd/hg.py +++ b/libvcs/cmd/hg.py @@ -2,9 +2,10 @@ import pathlib from typing import Optional, Sequence, Union -from ..types import StrOrBytesPath, StrPath from libvcs._internal.run import run +from ..types import StrOrBytesPath, StrPath + _CMD = Union[StrOrBytesPath, Sequence[StrOrBytesPath]] @@ -156,7 +157,7 @@ def run( if help is True: cli_args.append("--help") - return run(cmd=cli_args, **kwargs) + return run(args=cli_args, **kwargs) def clone( self, diff --git a/libvcs/cmd/svn.py b/libvcs/cmd/svn.py index 836f8ded8..9bfbd4dcc 100644 --- a/libvcs/cmd/svn.py +++ b/libvcs/cmd/svn.py @@ -1,9 +1,10 @@ import pathlib from typing import Literal, Optional, Sequence, Union -from ..types import StrOrBytesPath, StrPath from libvcs._internal.run import run +from ..types import StrOrBytesPath, StrPath + _CMD = Union[StrOrBytesPath, Sequence[StrOrBytesPath]] DepthLiteral = Union[Literal["infinity", "empty", "files", "immediates"], None] @@ -107,7 +108,7 @@ def run( if config_option is not None: cli_args.append("--config-option {config_option}") - return run(cmd=cli_args, **kwargs) + return run(args=cli_args, **kwargs) def checkout( self, From 1da287382199ac1c02a02e862f250aa892ea9064 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sat, 28 May 2022 15:35:40 -0500 Subject: [PATCH 3/6] refactor!(run): Simplify params --- libvcs/_internal/run.py | 123 +++++++++++++++------------------------- 1 file changed, 45 insertions(+), 78 deletions(-) diff --git a/libvcs/_internal/run.py b/libvcs/_internal/run.py index 847cecafa..fe192ad37 100644 --- a/libvcs/_internal/run.py +++ b/libvcs/_internal/run.py @@ -23,7 +23,6 @@ Protocol, Sequence, Union, - overload, ) from typing_extensions import TypeAlias @@ -164,89 +163,38 @@ def __call__(self, output: Union[str, bytes], timestamp: datetime.datetime): _CMD = Union[StrOrBytesPath, Sequence[StrOrBytesPath]] _FILE: TypeAlias = None | int | IO[Any] -# def run( -# args: _CMD, -# bufsize: int = -1, -# executable: StrOrBytesPath | None = None, -# stdin: _FILE | None = None, -# stdout: _FILE | None = None, -# stderr: _FILE | None = None, -# preexec_fn: Callable[[], Any] | None = None, -# close_fds: bool = True, -# shell: bool = False, -# cwd: StrOrBytesPath | None = None, -# env: _ENV | None = None, -# universal_newlines: bool = False, -# startupinfo: Any | None = None, -# creationflags: int = 0, -# restore_signals: bool = True, -# start_new_session: bool = False, -# pass_fds: Any = None, -# *, -# text: bool | None = None, -# encoding: str = "utf-8", -# errors: str | None = None, -# user: str | int | None = None, -# group: str | int | None = None, -# extra_groups: Iterable[str | int] | None = None, -# umask: int = -1, -# pipesize: int = -1, -# -# -# # custom -# log_in_real_time: bool = True, -# check_returncode: bool = True, -# callback: Optional[ProgressCallbackProtocol] = None, -# ): - - -@overload -def run( - args: _CMD, - bufsize: int = ..., - executable: StrOrBytesPath | None = ..., - stdin: _FILE | None = ..., - stdout: _FILE | None = ..., - stderr: _FILE | None = ..., - preexec_fn: Callable[[], Any] | None = ..., - close_fds: bool = ..., - shell: bool = ..., - cwd: Optional[StrOrBytesPath] = ..., - env: _ENV | None = ..., - universal_newlines: bool = ..., - startupinfo: Any | None = ..., - creationflags: int = ..., - restore_signals: bool = ..., - start_new_session: bool = ..., - pass_fds: Any = ..., - *, - text: bool | None = ..., - encoding: str = "utf-8", - errors: str | None = ..., - user: str | int | None = ..., - group: str | int | None = ..., - extra_groups: Iterable[str | int] | None = ..., - umask: int = ..., - pipesize: int = ..., - # custom - log_in_real_time: bool = True, - check_returncode: bool = True, - callback: Optional[ProgressCallbackProtocol] = None, -): - ... - def run( - # args: Union[str, list[str]], args: _CMD, + bufsize: int = -1, + executable: StrOrBytesPath | None = None, + stdin: _FILE | None = None, + stdout: _FILE | None = None, + stderr: _FILE | None = None, + preexec_fn: Callable[[], Any] | None = None, + close_fds: bool = True, shell: bool = False, - cwd: Optional[StrOrBytesPath] = None, + cwd: StrOrBytesPath | None = None, + env: _ENV | None = None, + universal_newlines: bool | None = None, + startupinfo: Any | None = None, + creationflags: int = 0, + restore_signals: bool = True, + start_new_session: bool = False, + pass_fds: Any = (), *, + text: bool | None = None, + encoding: str | None = None, + errors: str | None = None, + user: str | int | None = None, + group: str | int | None = None, + extra_groups: Iterable[str | int] | None = None, + umask: int = -1, + pipesize: int = -1, # custom log_in_real_time: bool = True, check_returncode: bool = True, callback: Optional[ProgressCallbackProtocol] = None, - **kwargs, ): """Run 'cmd' in a shell and return the combined contents of stdout and stderr (Blocking). Throws an exception if the command exits non-zero. @@ -285,11 +233,30 @@ def progress_cb(output, timestamp): """ proc = subprocess.Popen( args, + bufsize=bufsize, + executable=executable, + stdin=stdin, + stdout=stdout or subprocess.PIPE, + stderr=stderr or subprocess.PIPE, + preexec_fn=preexec_fn, + close_fds=close_fds, shell=shell, - stderr=subprocess.PIPE, - stdout=subprocess.PIPE, cwd=cwd, - **kwargs, + env=env, + universal_newlines=universal_newlines, + startupinfo=startupinfo, + creationflags=creationflags, + restore_signals=restore_signals, + start_new_session=start_new_session, + pass_fds=pass_fds, + text=text, + encoding=encoding, + errors=errors, + user=user, + group=group, + extra_groups=extra_groups, + umask=umask, + pipesize=pipesize, ) all_output = [] From 709599abad31a42e6e258bd69ec27e09afde7ec0 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 29 May 2022 07:58:37 -0500 Subject: [PATCH 4/6] fix!(run): Python 3.9 support --- libvcs/_internal/run.py | 40 +++++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/libvcs/_internal/run.py b/libvcs/_internal/run.py index fe192ad37..8ec447c4e 100644 --- a/libvcs/_internal/run.py +++ b/libvcs/_internal/run.py @@ -158,39 +158,42 @@ def __call__(self, output: Union[str, bytes], timestamp: datetime.datetime): if sys.platform == "win32": _ENV: TypeAlias = Mapping[str, str] else: - _ENV: TypeAlias = Mapping[bytes, StrOrBytesPath] | Mapping[str, StrOrBytesPath] + _ENV: TypeAlias = Union[ + Mapping[bytes, StrOrBytesPath], Mapping[str, StrOrBytesPath] + ] _CMD = Union[StrOrBytesPath, Sequence[StrOrBytesPath]] -_FILE: TypeAlias = None | int | IO[Any] +_FILE: TypeAlias = Optional[Union[int, IO[Any]]] def run( args: _CMD, bufsize: int = -1, - executable: StrOrBytesPath | None = None, - stdin: _FILE | None = None, - stdout: _FILE | None = None, - stderr: _FILE | None = None, - preexec_fn: Callable[[], Any] | None = None, + executable: Optional[StrOrBytesPath] = None, + stdin: Optional[_FILE] = None, + stdout: Optional[_FILE] = None, + stderr: Optional[_FILE] = None, + preexec_fn: Optional[Callable[[], Any]] = None, close_fds: bool = True, shell: bool = False, - cwd: StrOrBytesPath | None = None, - env: _ENV | None = None, - universal_newlines: bool | None = None, - startupinfo: Any | None = None, + cwd: Optional[StrOrBytesPath] = None, + env: Optional[_ENV] = None, + universal_newlines: Optional[bool] = None, + startupinfo: Optional[Any] = None, creationflags: int = 0, restore_signals: bool = True, start_new_session: bool = False, pass_fds: Any = (), *, - text: bool | None = None, - encoding: str | None = None, - errors: str | None = None, - user: str | int | None = None, - group: str | int | None = None, - extra_groups: Iterable[str | int] | None = None, + text: Optional[bool] = None, + encoding: Optional[str] = None, + errors: Optional[str] = None, + user: Optional[Union[str, int]] = None, + group: Optional[Union[str, int]] = None, + extra_groups: Optional[Iterable[Union[str, int]]] = None, umask: int = -1, - pipesize: int = -1, + # Not until sys.version_info >= (3, 10) + # pipesize: int = -1, # custom log_in_real_time: bool = True, check_returncode: bool = True, @@ -256,7 +259,6 @@ def progress_cb(output, timestamp): group=group, extra_groups=extra_groups, umask=umask, - pipesize=pipesize, ) all_output = [] From 8aa4a5b744c24c0818b436775e909d367915e7f6 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 29 May 2022 08:06:36 -0500 Subject: [PATCH 5/6] docs(run): Update docstring --- libvcs/_internal/run.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/libvcs/_internal/run.py b/libvcs/_internal/run.py index 8ec447c4e..b83280ba8 100644 --- a/libvcs/_internal/run.py +++ b/libvcs/_internal/run.py @@ -199,20 +199,21 @@ def run( check_returncode: bool = True, callback: Optional[ProgressCallbackProtocol] = None, ): - """Run 'cmd' in a shell and return the combined contents of stdout and - stderr (Blocking). Throws an exception if the command exits non-zero. + """Run 'args' in a shell and return the combined contents of stdout and + stderr (Blocking). Throws an exception if the command exits non-zero. + + Keyword arguments are passthrough to {class}`subprocess.Popen`. Parameters ---------- - cmd : list or str, or single str, if shell=True + args : list or str, or single str, if shell=True the command to run shell : boolean boolean indicating whether we are using advanced shell features. Use only when absolutely necessary, since this allows a lot more freedom which could be exploited by malicious code. See the - warning here: - http://docs.python.org/library/subprocess.html#popen-constructor + warning here: http://docs.python.org/library/subprocess.html#popen-constructor cwd : str dir command is run from. Defaults to ``path``. From b4b7a5d17c5d03986571fa650bacc9990ac71801 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 29 May 2022 08:04:00 -0500 Subject: [PATCH 6/6] docs(CHANGES): Note run() change --- CHANGES | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES b/CHANGES index 1ff5be8b7..405da4959 100644 --- a/CHANGES +++ b/CHANGES @@ -17,6 +17,10 @@ $ pip install --user --upgrade --pre libvcs - {issue}`343`: `libvcs.cmd.core` (including {func}`~libvcs._internal.run.run`) have been moved to `libvcs._internal.run`. It will be supported as an unstable, internal API. +- {issue}`361`: {class}`~libvcs._internal.run.run`'s params are now a pass-through to + {class}`subprocess.Popen`. + + - `run(cmd, ...)` is now `run(args, ...)` to match `Popen`'s convention. ### What's new