Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitbdabb21

Browse files
committed
Expand USE_SHELL docstring; clarify a test usage
In the USE_SHELL docstring:- Restore the older wording "when executing git commands" rather than "to execute git commands". I've realized that longer phrase, which dates back to the introduction of USE_SHELL in1c2dd54, is clearer, because readers less familiar with GitPython's overall design and operation will still not be misled into thinking USE_SHELL ever affects whether GitPython uses an external git command, versus some other mechanism, to do something.- Give some more information about why USE_SHELL = True is unsafe (though further revision or clarification may be called for).- Note some non-obvious aspects of USE_SHELL, that some of the way it interacts with other aspects of GitPython is not part of what is or has been documented about it, and in practice changes over time. The first example relates to#1792; the second may help users understand why code that enables USE_SHELL on Windows, in addition to being unsafe there, often breaks immediately on other platforms; the third is included so the warnings in the expanded docstring are not interpreted as a new commitment that any shell syntax that may have a *desired* effect in some application will continue to have the same effect in the future.- Cover a second application that might lead, or have led, to setting USE_SHELL to True, and explain what to do instead.In test_successful_default_refresh_invalidates_cached_version_info:- Rewrite the commented explanation of a special variant of that second application, where the usual easy alternatives are not used because part of the goal of the test is to check a "default" scenario that does not include either of them. This better explains why that choice is made in the test, and also hopefully will prevent anyone from thinking that test is a model for another situation (which, as noted, is unlikely to be the case even in unit tests).
1 parent3da47c2 commitbdabb21

File tree

2 files changed

+38
-16
lines changed

2 files changed

+38
-16
lines changed

‎git/cmd.py

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -405,23 +405,45 @@ def __setstate__(self, d: Dict[str, Any]) -> None:
405405
"""Enables debugging of GitPython's git commands."""
406406

407407
USE_SHELL:bool=False
408-
"""Deprecated. If set to ``True``, a shell will be used to execute git commands.
408+
"""Deprecated. If set to ``True``, a shell will be used when executing git commands.
409+
410+
Code that uses ``USE_SHELL = True`` or that passes ``shell=True`` to any GitPython
411+
functions should be updated to use the default value of ``False`` instead. ``True``
412+
is unsafe unless the effect of syntax treated specially by the shell is fully
413+
considered and accounted for, which is not possible under most circumstances. As
414+
detailed below, it is also no longer needed, even where it had been in the past.
415+
416+
In addition, how a value of ``True`` interacts with some aspects of GitPython's
417+
operation is not precisely specified and may change without warning, even before
418+
GitPython 4.0.0 when :attr:`USE_SHELL` may be removed. This includes:
419+
420+
* Whether or how GitPython automatically customizes the shell environment.
421+
422+
* Whether, outside of Windows (where :class:`subprocess.Popen` supports lists of
423+
separate arguments even when ``shell=True``), this can be used with any GitPython
424+
functionality other than direct calls to the :meth:`execute` method.
425+
426+
* Whether any GitPython feature that runs git commands ever attempts to partially
427+
sanitize data a shell may treat specially. Currently this is not done.
409428
410429
Prior to GitPython 2.0.8, this had a narrow purpose in suppressing console windows
411430
in graphical Windows applications. In 2.0.8 and higher, it provides no benefit, as
412431
GitPython solves that problem more robustly and safely by using the
413432
``CREATE_NO_WINDOW`` process creation flag on Windows.
414433
415-
Code that uses ``USE_SHELL = True`` or that passes ``shell=True`` to any GitPython
416-
functions should be updated to use the default value of ``False`` instead. ``True``
417-
is unsafe unless the effect of shell expansions is fully considered and accounted
418-
for, which is not possible under most circumstances.
434+
Because Windows path search differs subtly based on whether a shell is used, in rare
435+
cases changing this from ``True`` to ``False`` may keep an unusual git "executable",
436+
such as a batch file, from being found. To fix this, set the command name or full
437+
path in the :envvar:`GIT_PYTHON_GIT_EXECUTABLE` environment variable or pass the
438+
full path to :func:`git.refresh` (or invoke the script using a ``.exe`` shim).
419439
420-
See:
440+
Further reading:
421441
422-
- :meth:`Git.execute` (on the ``shell`` parameter).
423-
- https://github.com/gitpython-developers/GitPython/commit/0d9390866f9ce42870d3116094cd49e0019a970a
424-
- https://learn.microsoft.com/en-us/windows/win32/procthread/process-creation-flags
442+
* :meth:`Git.execute` (on the ``shell`` parameter).
443+
* https://github.com/gitpython-developers/GitPython/commit/0d9390866f9ce42870d3116094cd49e0019a970a
444+
* https://learn.microsoft.com/en-us/windows/win32/procthread/process-creation-flags
445+
* https://github.com/python/cpython/issues/91558#issuecomment-1100942950
446+
* https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw
425447
"""
426448

427449
_git_exec_env_var="GIT_PYTHON_GIT_EXECUTABLE"

‎test/test_git.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -669,13 +669,13 @@ def test_successful_default_refresh_invalidates_cached_version_info(self):
669669
stack.enter_context(_patch_out_env("GIT_PYTHON_GIT_EXECUTABLE"))
670670

671671
ifsys.platform=="win32":
672-
# On Windows, use a shell so "git" finds "git.cmd".(In the infrequent
673-
#case thatthiseffect is desired in production code, it should not be
674-
#done with this technique. USE_SHELL=True is less secure and reliable,
675-
#as unintended shell expansions can occur, and is deprecated. Instead,
676-
#use a custom command, by setting the GIT_PYTHON_GIT_EXECUTABLE
677-
#environment variable to git.cmd or by passing git.cmd's full path to
678-
#git.refresh. Or wrap the script with a .exe shim.)
672+
# On Windows, use a shell so "git" finds "git.cmd".The correct and safe
673+
#ways to dothisstraightforwardly are to set GIT_PYTHON_GIT_EXECUTABLE
674+
#to git.cmd in the environment, or call git.refresh with the command's
675+
#full path. See the Git.USE_SHELL docstring for deprecation details.
676+
#But this tests a "default" scenario where neither is done. The
677+
#approach used here, setting USE_SHELL to True so PATHEXT is honored,
678+
#should not be used in production code (nor even in most test cases).
679679
stack.enter_context(mock.patch.object(Git,"USE_SHELL",True))
680680

681681
new_git=Git()

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp