Content-Length: 31857 | pFad | http://github.com/gitpython-developers/GitPython/pull/1688.patch

thub.com From b79198a880982e6768fec4d0ef244338420efbdc Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 3 Oct 2023 04:58:13 -0400 Subject: [PATCH 01/11] Document Git.execute parameters in definition order - Reorder the items in the git.cmd.Git.execute docstring that document its parameters, to be in the same order the parameters are actually defined in. - Use consistent spacing, by having a blank line between successive items that document parameters. Before, most of them were separated this way, but some of them were not. - Reorder the elements of execute_kwargs (which list all those parameters except the first parameter, command) to be listed in that order as well. These were mostly in order, but a couple were out of order. This is just about the order they appear in the definition, since sets in Python (unlike dicts) have no key order guarantees. --- git/cmd.py | 46 ++++++++++++++++++++++++++-------------------- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/git/cmd.py b/git/cmd.py index 53b8b9050..4d772e909 100644 --- a/git/cmd.py +++ b/git/cmd.py @@ -66,10 +66,10 @@ "with_extended_output", "with_exceptions", "as_process", - "stdout_as_string", "output_stream", - "with_stdout", + "stdout_as_string", "kill_after_timeout", + "with_stdout", "universal_newlines", "shell", "env", @@ -883,6 +883,27 @@ def execute( decoded into a string using the default encoding (usually utf-8). The latter can fail, if the output contains binary data. + :param kill_after_timeout: + To specify a timeout in seconds for the git command, after which the process + should be killed. This will have no effect if as_process is set to True. It is + set to None by default and will let the process run until the timeout is + explicitly specified. This feature is not supported on Windows. It's also worth + noting that kill_after_timeout uses SIGKILL, which can have negative side + effects on a repository. For example, stale locks in case of git gc could + render the repository incapable of accepting changes until the lock is manually + removed. + + :param with_stdout: + If True, default True, we open stdout on the created process + + :param universal_newlines: + if True, pipes will be opened as text, and lines are split at + all known line endings. + + :param shell: + Whether to invoke commands through a shell (see `Popen(..., shell=True)`). + It overrides :attr:`USE_SHELL` if it is not `None`. + :param env: A dictionary of environment variables to be passed to `subprocess.Popen`. @@ -891,29 +912,14 @@ def execute( one invocation of write() method. If the given number is not positive then the default value is used. + :param strip_newline_in_stdout: + Whether to strip the trailing ``\\n`` of the command stdout. + :param subprocess_kwargs: Keyword arguments to be passed to subprocess.Popen. Please note that some of the valid kwargs are already set by this method, the ones you specify may not be the same ones. - :param with_stdout: If True, default True, we open stdout on the created process - :param universal_newlines: - if True, pipes will be opened as text, and lines are split at - all known line endings. - :param shell: - Whether to invoke commands through a shell (see `Popen(..., shell=True)`). - It overrides :attr:`USE_SHELL` if it is not `None`. - :param kill_after_timeout: - To specify a timeout in seconds for the git command, after which the process - should be killed. This will have no effect if as_process is set to True. It is - set to None by default and will let the process run until the timeout is - explicitly specified. This feature is not supported on Windows. It's also worth - noting that kill_after_timeout uses SIGKILL, which can have negative side - effects on a repository. For example, stale locks in case of git gc could - render the repository incapable of accepting changes until the lock is manually - removed. - :param strip_newline_in_stdout: - Whether to strip the trailing ``\\n`` of the command stdout. :return: * str(output) if extended_output = False (Default) * tuple(int(status), str(stdout), str(stderr)) if extended_output = True From 13e1593fc6a3c218451e29dd6b4a58b3a44afee3 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 3 Oct 2023 05:30:22 -0400 Subject: [PATCH 02/11] Don't say Git.execute uses a shell, in its summary The top line of the Git.execute docstring said that it used a shell, which is not necessarily the case (and is not usually the case, since the default is not to use one). This removes that claim while keeping the top-line wording otherwise the same. It also rephrases the description of the command parameter, in a way that does not change its meaning but reflects the more common practice of passing a sequence of arguments (since portable calls that do not use a shell must do that). --- git/cmd.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/git/cmd.py b/git/cmd.py index 4d772e909..e324db7e2 100644 --- a/git/cmd.py +++ b/git/cmd.py @@ -842,12 +842,12 @@ def execute( strip_newline_in_stdout: bool = True, **subprocess_kwargs: Any, ) -> Union[str, bytes, Tuple[int, Union[str, bytes], str], AutoInterrupt]: - """Handles executing the command on the shell and consumes and returns - the returned information (stdout) + """Handles executing the command and consumes and returns the returned + information (stdout) :param command: The command argument list to execute. - It should be a string, or a sequence of program arguments. The + It should be a sequence of program arguments, or a string. The program to execute is the first item in the args sequence or string. :param istream: From 74b68e9b621a729a3407b2020b0a48d7921fb1e9 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 3 Oct 2023 05:55:54 -0400 Subject: [PATCH 03/11] Copyedit Git.execute docstring These are some small clarity and consistency revisions to the docstring of git.cmd.Git.execute that didn't specifically fit in the narrow topics of either of the preceding two commits. --- git/cmd.py | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/git/cmd.py b/git/cmd.py index e324db7e2..f20cd77af 100644 --- a/git/cmd.py +++ b/git/cmd.py @@ -851,7 +851,7 @@ def execute( program to execute is the first item in the args sequence or string. :param istream: - Standard input filehandle passed to subprocess.Popen. + Standard input filehandle passed to `subprocess.Popen`. :param with_extended_output: Whether to return a (status, stdout, stderr) tuple. @@ -862,8 +862,7 @@ def execute( :param as_process: Whether to return the created process instance directly from which streams can be read on demand. This will render with_extended_output and - with_exceptions ineffective - the caller will have - to deal with the details himself. + with_exceptions ineffective - the caller will have to deal with the details. It is important to note that the process will be placed into an AutoInterrupt wrapper that will interrupt the process once it goes out of scope. If you use the command in iterators, you should pass the whole process instance @@ -876,25 +875,25 @@ def execute( always be created with a pipe due to issues with subprocess. This merely is a workaround as data will be copied from the output pipe to the given output stream directly. - Judging from the implementation, you shouldn't use this flag ! + Judging from the implementation, you shouldn't use this parameter! :param stdout_as_string: - if False, the commands standard output will be bytes. Otherwise, it will be - decoded into a string using the default encoding (usually utf-8). + If False, the command's standard output will be bytes. Otherwise, it will be + decoded into a string using the default encoding (usually UTF-8). The latter can fail, if the output contains binary data. :param kill_after_timeout: - To specify a timeout in seconds for the git command, after which the process + Specifies a timeout in seconds for the git command, after which the process should be killed. This will have no effect if as_process is set to True. It is set to None by default and will let the process run until the timeout is explicitly specified. This feature is not supported on Windows. It's also worth noting that kill_after_timeout uses SIGKILL, which can have negative side - effects on a repository. For example, stale locks in case of git gc could + effects on a repository. For example, stale locks in case of ``git gc`` could render the repository incapable of accepting changes until the lock is manually removed. :param with_stdout: - If True, default True, we open stdout on the created process + If True, default True, we open stdout on the created process. :param universal_newlines: if True, pipes will be opened as text, and lines are split at @@ -916,19 +915,19 @@ def execute( Whether to strip the trailing ``\\n`` of the command stdout. :param subprocess_kwargs: - Keyword arguments to be passed to subprocess.Popen. Please note that - some of the valid kwargs are already set by this method, the ones you + Keyword arguments to be passed to `subprocess.Popen`. Please note that + some of the valid kwargs are already set by this method; the ones you specify may not be the same ones. :return: * str(output) if extended_output = False (Default) * tuple(int(status), str(stdout), str(stderr)) if extended_output = True - if output_stream is True, the stdout value will be your output stream: + If output_stream is True, the stdout value will be your output stream: * output_stream if extended_output = False * tuple(int(status), output_stream, str(stderr)) if extended_output = True - Note git is executed with LC_MESSAGES="C" to ensure consistent + Note that git is executed with ``LC_MESSAGES="C"`` to ensure consistent output regardless of system language. :raise GitCommandError: From 133271bb3871baff3ed772fbdea8bc359f115fd6 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 3 Oct 2023 06:34:05 -0400 Subject: [PATCH 04/11] Other copyediting in the git.cmd module (Not specific to git.cmd.Git.execute.) --- git/cmd.py | 43 ++++++++++++++++++++----------------------- 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/git/cmd.py b/git/cmd.py index f20cd77af..a6d287986 100644 --- a/git/cmd.py +++ b/git/cmd.py @@ -105,7 +105,7 @@ def handle_process_output( ) -> None: """Registers for notifications to learn that process output is ready to read, and dispatches lines to the respective line handlers. - This function returns once the finalizer returns + This function returns once the finalizer returns. :return: result of finalizer :param process: subprocess.Popen instance @@ -294,9 +294,7 @@ def __setstate__(self, d: Dict[str, Any]) -> None: @classmethod def refresh(cls, path: Union[None, PathLike] = None) -> bool: - """This gets called by the refresh function (see the top level - __init__). - """ + """This gets called by the refresh function (see the top level __init__).""" # discern which path to refresh with if path is not None: new_git = os.path.expanduser(path) @@ -446,9 +444,9 @@ def polish_url(cls, url: str, is_cygwin: Union[None, bool] = None) -> PathLike: if is_cygwin: url = cygpath(url) else: - """Remove any backslahes from urls to be written in config files. + """Remove any backslashes from urls to be written in config files. - Windows might create config-files containing paths with backslashed, + Windows might create config files containing paths with backslashes, but git stops liking them as it will escape the backslashes. Hence we undo the escaping just to be sure. """ @@ -464,8 +462,8 @@ def check_unsafe_protocols(cls, url: str) -> None: Check for unsafe protocols. Apart from the usual protocols (http, git, ssh), - Git allows "remote helpers" that have the form `::

`, - one of these helpers (`ext::`) can be used to invoke any arbitrary command. + Git allows "remote helpers" that have the form ``::

``, + one of these helpers (``ext::``) can be used to invoke any arbitrary command. See: @@ -517,7 +515,7 @@ def __init__(self, proc: Union[None, subprocess.Popen], args: Any) -> None: self.status: Union[int, None] = None def _terminate(self) -> None: - """Terminate the underlying process""" + """Terminate the underlying process.""" if self.proc is None: return @@ -572,7 +570,7 @@ def wait(self, stderr: Union[None, str, bytes] = b"") -> int: """Wait for the process and return its status code. :param stderr: Previously read value of stderr, in case stderr is already closed. - :warn: may deadlock if output or error pipes are used and not handled separately. + :warn: May deadlock if output or error pipes are used and not handled separately. :raise GitCommandError: if the return status is not 0""" if stderr is None: stderr_b = b"" @@ -605,13 +603,12 @@ def read_all_from_possibly_closed_stream(stream: Union[IO[bytes], None]) -> byte # END auto interrupt class CatFileContentStream(object): - """Object representing a sized read-only stream returning the contents of an object. It behaves like a stream, but counts the data read and simulates an empty stream once our sized content region is empty. - If not all data is read to the end of the objects's lifetime, we read the - rest to assure the underlying stream continues to work""" + If not all data is read to the end of the object's lifetime, we read the + rest to assure the underlying stream continues to work.""" __slots__: Tuple[str, ...] = ("_stream", "_nbr", "_size") @@ -740,11 +737,11 @@ def __getattr__(self, name: str) -> Any: def set_persistent_git_options(self, **kwargs: Any) -> None: """Specify command line options to the git executable - for subsequent subcommand calls + for subsequent subcommand calls. :param kwargs: is a dict of keyword arguments. - these arguments are passed as in _call_process + These arguments are passed as in _call_process but will be passed to the git command rather than the subcommand. """ @@ -775,7 +772,7 @@ def version_info(self) -> Tuple[int, int, int, int]: """ :return: tuple(int, int, int, int) tuple with integers representing the major, minor and additional version numbers as parsed from git version. - This value is generated on demand and is cached""" + This value is generated on demand and is cached.""" return self._version_info @overload @@ -843,7 +840,7 @@ def execute( **subprocess_kwargs: Any, ) -> Union[str, bytes, Tuple[int, Union[str, bytes], str], AutoInterrupt]: """Handles executing the command and consumes and returns the returned - information (stdout) + information (stdout). :param command: The command argument list to execute. @@ -1213,7 +1210,7 @@ def _unpack_args(cls, arg_list: Sequence[str]) -> List[str]: def __call__(self, **kwargs: Any) -> "Git": """Specify command line options to the git executable - for a subcommand call + for a subcommand call. :param kwargs: is a dict of keyword arguments. @@ -1251,7 +1248,7 @@ def _call_process( self, method: str, *args: Any, **kwargs: Any ) -> Union[str, bytes, Tuple[int, Union[str, bytes], str], "Git.AutoInterrupt"]: """Run the given git command with the specified arguments and return - the result as a String + the result as a string. :param method: is the command. Contained "_" characters will be converted to dashes, @@ -1260,7 +1257,7 @@ def _call_process( :param args: is the list of arguments. If None is included, it will be pruned. This allows your commands to call git more conveniently as None - is realized as non-existent + is realized as non-existent. :param kwargs: It contains key-values for the following: @@ -1390,7 +1387,7 @@ def get_object_header(self, ref: str) -> Tuple[str, str, int]: return self.__get_object_header(cmd, ref) def get_object_data(self, ref: str) -> Tuple[str, str, int, bytes]: - """As get_object_header, but returns object data as well + """As get_object_header, but returns object data as well. :return: (hexsha, type_string, size_as_int, data_string) :note: not threadsafe""" @@ -1400,10 +1397,10 @@ def get_object_data(self, ref: str) -> Tuple[str, str, int, bytes]: return (hexsha, typename, size, data) def stream_object_data(self, ref: str) -> Tuple[str, str, int, "Git.CatFileContentStream"]: - """As get_object_header, but returns the data as a stream + """As get_object_header, but returns the data as a stream. :return: (hexsha, type_string, size_as_int, stream) - :note: This method is not threadsafe, you need one independent Command instance per thread to be safe !""" + :note: This method is not threadsafe, you need one independent Command instance per thread to be safe!""" cmd = self._get_persistent_cmd("cat_file_all", "cat_file", batch=True) hexsha, typename, size = self.__get_object_header(cmd, ref) cmd_stdout = cmd.stdout if cmd.stdout is not None else io.BytesIO() From fc755dae6866b9c9e0aa347297b693fec2c5b415 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 3 Oct 2023 06:38:54 -0400 Subject: [PATCH 05/11] Avoid having a local function seem to be a method The kill_process local function defined in the Git.execute method is a local function and not a method, but it was easy to misread as a method for two reasons: - Its docstring described it as a method. - It was named with a leading underscore, as though it were a nonpublic method. But this name is a local variable, and local variables are always nonpublic (except when they are function parameters, in which case they are in a sense public). A leading underscore in a local variable name usually means the variable is unused in the function. This fixes the docstring and drops the leading underscore from the name. If this is ever extracted from the Git.execute method and placed at class or module scope, then the name can be changed back. --- git/cmd.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/git/cmd.py b/git/cmd.py index a6d287986..1d8c70f32 100644 --- a/git/cmd.py +++ b/git/cmd.py @@ -1012,8 +1012,8 @@ def execute( if as_process: return self.AutoInterrupt(proc, command) - def _kill_process(pid: int) -> None: - """Callback method to kill a process.""" + def kill_process(pid: int) -> None: + """Callback to kill a process.""" p = Popen( ["ps", "--ppid", str(pid)], stdout=PIPE, @@ -1046,7 +1046,7 @@ def _kill_process(pid: int) -> None: if kill_after_timeout is not None: kill_check = threading.Event() - watchdog = threading.Timer(kill_after_timeout, _kill_process, args=(proc.pid,)) + watchdog = threading.Timer(kill_after_timeout, kill_process, args=(proc.pid,)) # Wait for the process to return status = 0 From 2d1efdca84e266a422f4298ee94ee9b8dae6c32e Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 3 Oct 2023 07:55:35 -0400 Subject: [PATCH 06/11] Test that git.cmd.execute_kwargs is correct --- test/test_git.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/test_git.py b/test/test_git.py index 583c74fa3..723bf624b 100644 --- a/test/test_git.py +++ b/test/test_git.py @@ -5,6 +5,7 @@ # This module is part of GitPython and is released under # the BSD License: https://opensource.org/license/bsd-3-clause/ import contextlib +import inspect import logging import os import os.path as osp @@ -364,3 +365,11 @@ def counter_stderr(line): self.assertEqual(count[1], line_count) self.assertEqual(count[2], line_count) + + def test_execute_kwargs_set_agrees_with_method(self): + parameter_names = inspect.signature(cmd.Git.execute).parameters.keys() + self_param, command_param, *most_params, extra_kwargs_param = parameter_names + self.assertEqual(self_param, "self") + self.assertEqual(command_param, "command") + self.assertEqual(set(most_params), cmd.execute_kwargs) # Most important. + self.assertEqual(extra_kwargs_param, "subprocess_kwargs") From a8a43fe6f8d6f0a7f9067149859634d624406bb1 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 3 Oct 2023 08:50:17 -0400 Subject: [PATCH 07/11] Simplify shell test helper with with_exceptions=False Instead of swallowing GitCommandError exceptions in the helper used by test_it_uses_shell_or_not_as_specified and test_it_logs_if_it_uses_a_shell, this modifies the helper so it prevents Git.execute from raising the exception in the first place. --- test/test_git.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/test/test_git.py b/test/test_git.py index 723bf624b..8d269f38a 100644 --- a/test/test_git.py +++ b/test/test_git.py @@ -4,7 +4,6 @@ # # This module is part of GitPython and is released under # the BSD License: https://opensource.org/license/bsd-3-clause/ -import contextlib import inspect import logging import os @@ -97,10 +96,8 @@ def _do_shell_combo(self, value_in_call, value_from_class): # git.cmd gets Popen via a "from" import, so patch it there. with mock.patch.object(cmd, "Popen", wraps=cmd.Popen) as mock_popen: # Use a command with no arguments (besides the program name), so it runs - # with or without a shell, on all OSes, with the same effect. Since git - # errors out when run with no arguments, we swallow that error. - with contextlib.suppress(GitCommandError): - self.git.execute(["git"], shell=value_in_call) + # with or without a shell, on all OSes, with the same effect. + self.git.execute(["git"], with_exceptions=False, shell=value_in_call) return mock_popen From 9fa1ceef2c47c9f58e10d8925cc166fdfd6b5183 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 3 Oct 2023 09:45:45 -0400 Subject: [PATCH 08/11] Extract a _assert_logged_for_popen method This extracts the logic of searching log messages, and asserting that (at least) one matches a pattern for the report of a Popen call with a given argument, from test_it_logs_if_it_uses_a_shell into a new nonpublic test helper method _assert_logged_for_popen. The extracted version is modified to make it slightly more general, and slightly more robust. This is still not extremely robust: the notation used to log Popen calls is informal, so it wouldn't make sense to really parse it as code. But this no longer assumes that the representation of a value ends at a word boundary, nor that the value is free of regular expression metacharacters. --- test/test_git.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/test/test_git.py b/test/test_git.py index 8d269f38a..10b346e4d 100644 --- a/test/test_git.py +++ b/test/test_git.py @@ -40,6 +40,13 @@ def tearDown(self): gc.collect() + def _assert_logged_for_popen(self, log_watcher, name, value): + re_name = re.escape(name) + re_value = re.escape(str(value)) + re_line = re.compile(fr"DEBUG:git.cmd:Popen$.*\b{re_name}={re_value}[,)]") + match_attempts = [re_line.match(message) for message in log_watcher.output] + self.assertTrue(any(match_attempts), repr(log_watcher.output)) + @mock.patch.object(Git, "execute") def test_call_process_calls_execute(self, git): git.return_value = "" @@ -113,14 +120,9 @@ def test_it_uses_shell_or_not_as_specified(self, case): def test_it_logs_if_it_uses_a_shell(self, case): """``shell=`` in the log message agrees with what is passed to `Popen`.""" value_in_call, value_from_class = case - with self.assertLogs(cmd.log, level=logging.DEBUG) as log_watcher: mock_popen = self._do_shell_combo(value_in_call, value_from_class) - - popen_shell_arg = mock_popen.call_args.kwargs["shell"] - expected_message = re.compile(rf"DEBUG:git.cmd:Popen\(.*\bshell={popen_shell_arg}\b.*$") - match_attempts = [expected_message.fullmatch(message) for message in log_watcher.output] - self.assertTrue(any(match_attempts), repr(log_watcher.output)) + self._assert_logged_for_popen(log_watcher, "shell", mock_popen.call_args.kwargs["shell"]) def test_it_executes_git_and_returns_result(self): self.assertRegex(self.git.execute(["git", "version"]), r"^git version [\d\.]{2}.*$") From 790a790f49a2548c620532ee2b9705b09fb33855 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 3 Oct 2023 09:59:28 -0400 Subject: [PATCH 09/11] Log stdin arg as such, and test that this is done This changes how the Popen call debug logging line shows the informal summary of what kind of thing is being passed as the stdin argument to Popen, showing it with stdin= rather than istream=. The new test, with "istream" in place of "stdin", passed before the code change in the git.cmd module, failed once "istream" was changed to "stdin" in the test, and then, as expected, passed again once "istream=" was changed to "stdin=" in the log.debug call in git.cmd.Git.execute. --- git/cmd.py | 2 +- test/test_git.py | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/git/cmd.py b/git/cmd.py index 1d8c70f32..e545ba160 100644 --- a/git/cmd.py +++ b/git/cmd.py @@ -979,7 +979,7 @@ def execute( if shell is None: shell = self.USE_SHELL log.debug( - "Popen(%s, cwd=%s, universal_newlines=%s, shell=%s, istream=%s)", + "Popen(%s, cwd=%s, universal_newlines=%s, shell=%s, stdin=%s)", redacted_command, cwd, universal_newlines, diff --git a/test/test_git.py b/test/test_git.py index 10b346e4d..1ee7b3642 100644 --- a/test/test_git.py +++ b/test/test_git.py @@ -124,6 +124,16 @@ def test_it_logs_if_it_uses_a_shell(self, case): mock_popen = self._do_shell_combo(value_in_call, value_from_class) self._assert_logged_for_popen(log_watcher, "shell", mock_popen.call_args.kwargs["shell"]) + @ddt.data( + ("None", None), + ("", subprocess.PIPE), + ) + def test_it_logs_istream_summary_for_stdin(self, case): + expected_summary, istream_argument = case + with self.assertLogs(cmd.log, level=logging.DEBUG) as log_watcher: + self.git.execute(["git", "version"], istream=istream_argument) + self._assert_logged_for_popen(log_watcher, "stdin", expected_summary) + def test_it_executes_git_and_returns_result(self): self.assertRegex(self.git.execute(["git", "version"]), r"^git version [\d\.]{2}.*$") From c3fde7fb8dcd48d17ee24b78db7b0dd25d2348ab Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 3 Oct 2023 10:16:10 -0400 Subject: [PATCH 10/11] Log args in the order they are passed to Popen This is still not including all or even most of the arguments, nor are all the logged arguments literal (nor should either of those things likely be changed). It is just to facilitate easier comparison of what is logged to the Popen call in the code. --- git/cmd.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/git/cmd.py b/git/cmd.py index e545ba160..c35e56703 100644 --- a/git/cmd.py +++ b/git/cmd.py @@ -979,12 +979,12 @@ def execute( if shell is None: shell = self.USE_SHELL log.debug( - "Popen(%s, cwd=%s, universal_newlines=%s, shell=%s, stdin=%s)", + "Popen(%s, cwd=%s, stdin=%s, shell=%s, universal_newlines=%s)", redacted_command, cwd, - universal_newlines, - shell, istream_ok, + shell, + universal_newlines, ) try: with maybe_patch_caller_env: From ab958865d89b3146bb953f826f30da11dc59139a Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Tue, 3 Oct 2023 10:30:58 -0400 Subject: [PATCH 11/11] Eliminate istream_ok variable In Git.execute, the stdin argument to Popen is the only one where a compound expression (rather than a single term) is currently passed. So having that be the same in the log message makes it easier to understand what is going on, as well as to see how the information shown in the log corresponds to what Popen receives. --- git/cmd.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/git/cmd.py b/git/cmd.py index c35e56703..7c448e3f2 100644 --- a/git/cmd.py +++ b/git/cmd.py @@ -973,16 +973,13 @@ def execute( # end handle stdout_sink = PIPE if with_stdout else getattr(subprocess, "DEVNULL", None) or open(os.devnull, "wb") - istream_ok = "None" - if istream: - istream_ok = "" if shell is None: shell = self.USE_SHELL log.debug( "Popen(%s, cwd=%s, stdin=%s, shell=%s, universal_newlines=%s)", redacted_command, cwd, - istream_ok, + "" if istream else "None", shell, universal_newlines, )

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier! Saves Data!