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

thub.com From a0fa2bd40f163b959d67a76c011b8b3c6d0a37c8 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 25 Dec 2023 06:01:20 -0500 Subject: [PATCH 1/2] Improve deprecation messages and other docstrings - Fix spacing in deprecation messages. - Update versions in deprecation messages to reflect SemVer. - Add and fix up docstrings for methods with some deprecated uses. - Add a couple missing docstrings on nearby attributes and methods. - Convert is_ deprecation comment to three docstrings. - Avoid wrongly claiming is_darwin still uses os.name (see #1732). - Convert some other comments to docstrings. - Make disembodied """-strings docstrings or comments, per intent. - Move some comments that were misplaced during auto-formatting. This commit builds on the changes in 94a85d1 (#1782). --- git/compat.py | 37 ++++++++++++---- git/diff.py | 15 ++++--- git/objects/commit.py | 4 +- git/objects/util.py | 98 ++++++++++++++++++++++++------------------- git/util.py | 8 ++-- 5 files changed, 99 insertions(+), 63 deletions(-) diff --git a/git/compat.py b/git/compat.py index 6b1b4f547..920e44b7f 100644 --- a/git/compat.py +++ b/git/compat.py @@ -28,18 +28,41 @@ # --------------------------------------------------------------------------- -# DEPRECATED attributes providing shortcuts to operating system checks based on os.name. -# -# These are deprecated because it is clearer, and helps avoid bugs, to write out the -# os.name or sys.platform checks explicitly, especially in cases where it matters which -# is used. For example, is_win is False on Cygwin, but is often assumed True. To detect -# Cygwin, use sys.platform == "cygwin". (Also, in the past, is_darwin was unreliable.) -# is_win = os.name == "nt" +"""Deprecated alias for ``os.name == "nt"`` to check for native Windows. + +This is deprecated because it is clearer to write out :attr:`os.name` or +:attr:`sys.platform` checks explicitly, especially in cases where it matters which is +used. + +:note: ``is_win`` is ``False`` on Cygwin, but is often wrongly assumed ``True``. To + detect Cygwin, use ``sys.platform == "cygwin"``. +""" + is_posix = os.name == "posix" +"""Deprecated alias for ``os.name == "posix"`` to check for Unix-like ("POSIX") systems. + +This is deprecated because it clearer to write out :attr:`os.name` or +:attr:`sys.platform` checks explicitly, especially in cases where it matters which is +used. + +:note: For POSIX systems, more detailed information is available in + :attr:`sys.platform`, while :attr:`os.name` is always ``"posix"`` on such systems, + including macOS (Darwin). +""" + is_darwin = sys.platform == "darwin" +"""Deprecated alias for ``sys.platform == "darwin"`` to check for macOS (Darwin). + +This is deprecated because it clearer to write out :attr:`os.name` or +:attr:`sys.platform` checks explicitly. + +:note: For macOS (Darwin), ``os.name == "posix"`` as in other Unix-like systems, while + ``sys.platform == "darwin"`. +""" defenc = sys.getfilesystemencoding() +"""The encoding used to convert between Unicode and bytes filenames.""" @overload diff --git a/git/diff.py b/git/diff.py index 3c6c3fb13..aba1a1080 100644 --- a/git/diff.py +++ b/git/diff.py @@ -4,6 +4,7 @@ # 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/ import re + from git.cmd import handle_process_output from git.compat import defenc from git.util import finalize_process, hex_to_bin @@ -208,13 +209,15 @@ class DiffIndex(List[T_Diff]): The class improves the diff handling convenience. """ - # Change type invariant identifying possible ways a blob can have changed: - # A = Added - # D = Deleted - # R = Renamed - # M = Modified - # T = Changed in the type change_type = ("A", "C", "D", "R", "M", "T") + """Change type invariant identifying possible ways a blob can have changed: + + * ``A`` = Added + * ``D`` = Deleted + * ``R`` = Renamed + * ``M`` = Modified + * ``T`` = Changed in the type + """ def iter_change_type(self, change_type: Lit_change_type) -> Iterator[T_Diff]: """ diff --git a/git/objects/commit.py b/git/objects/commit.py index 7310d66b0..5a069848c 100644 --- a/git/objects/commit.py +++ b/git/objects/commit.py @@ -352,8 +352,8 @@ def stats(self) -> Stats: def trailers(self) -> Dict[str, str]: """Get the trailers of the message as a dictionary - :note: This property is deprecated, please use either ``Commit.trailers_list`` - or ``Commit.trailers_dict``. + :note: This property is deprecated, please use either :attr:`trailers_list` or + :attr:`trailers_dict``. :return: Dictionary containing whitespace stripped trailer information. Only contains diff --git a/git/objects/util.py b/git/objects/util.py index c52dc7564..b25a3f5ff 100644 --- a/git/objects/util.py +++ b/git/objects/util.py @@ -62,9 +62,9 @@ class TraverseNT(NamedTuple): T_TIobj = TypeVar("T_TIobj", bound="TraversableIterableObj") # For TraversableIterableObj.traverse() TraversedTup = Union[ - Tuple[Union["Traversable", None], "Traversable"], # For commit, submodule - "TraversedTreeTup", -] # for tree.traverse() + Tuple[Union["Traversable", None], "Traversable"], # For Commit, Submodule. + "TraversedTreeTup", # For Tree.traverse(). +] # -------------------------------------------------------------------- @@ -380,11 +380,15 @@ class Tree:: (cls, Tree) -> Tuple[Tree, ...] @abstractmethod def list_traverse(self, *args: Any, **kwargs: Any) -> Any: - """ """ + """Traverse self and collect all items found. + + Calling this directly only the abstract base class, including via a ``super()`` + proxy, is deprecated. Only overridden implementations should be called. + """ warnings.warn( "list_traverse() method should only be called from subclasses." - "Calling from Traversable abstract class will raise NotImplementedError in 3.1.20" - "Builtin sublclasses are 'Submodule', 'Tree' and 'Commit", + " Calling from Traversable abstract class will raise NotImplementedError in 4.0.0." + " The concrete subclasses in GitPython itself are 'Commit', 'RootModule', 'Submodule', and 'Tree'.", DeprecationWarning, stacklevel=2, ) @@ -393,12 +397,14 @@ def list_traverse(self, *args: Any, **kwargs: Any) -> Any: def _list_traverse( self, as_edge: bool = False, *args: Any, **kwargs: Any ) -> IterableList[Union["Commit", "Submodule", "Tree", "Blob"]]: - """ + """Traverse self and collect all items found. + :return: IterableList with the results of the traversal as produced by - traverse() - Commit -> IterableList['Commit'] - Submodule -> IterableList['Submodule'] - Tree -> IterableList[Union['Submodule', 'Tree', 'Blob']] + :meth:`traverse`:: + + Commit -> IterableList['Commit'] + Submodule -> IterableList['Submodule'] + Tree -> IterableList[Union['Submodule', 'Tree', 'Blob']] """ # Commit and Submodule have id.__attribute__ as IterableObj. # Tree has id.__attribute__ inherited from IndexObject. @@ -421,11 +427,15 @@ def _list_traverse( @abstractmethod def traverse(self, *args: Any, **kwargs: Any) -> Any: - """ """ + """Iterator yielding items found when traversing self. + + Calling this directly on the abstract base class, including via a ``super()`` + proxy, is deprecated. Only overridden implementations should be called. + """ warnings.warn( "traverse() method should only be called from subclasses." - "Calling from Traversable abstract class will raise NotImplementedError in 3.1.20" - "Builtin sublclasses are 'Submodule', 'Tree' and 'Commit", + " Calling from Traversable abstract class will raise NotImplementedError in 4.0.0." + " The concrete subclasses in GitPython itself are 'Commit', 'RootModule', 'Submodule', and 'Tree'.", DeprecationWarning, stacklevel=2, ) @@ -441,7 +451,7 @@ def _traverse( ignore_self: int = 1, as_edge: bool = False, ) -> Union[Iterator[Union["Traversable", "Blob"]], Iterator[TraversedTup]]: - """:return: Iterator yielding items found when traversing self + """Iterator yielding items found when traversing self. :param predicate: f(i,d) returns False if item i at depth d should not be included in the result. @@ -471,18 +481,18 @@ def _traverse( if True, return a pair of items, first being the source, second the destination, i.e. tuple(src, dest) with the edge spanning from source to destination - """ - """ - Commit -> Iterator[Union[Commit, Tuple[Commit, Commit]] - Submodule -> Iterator[Submodule, Tuple[Submodule, Submodule]] - Tree -> Iterator[Union[Blob, Tree, Submodule, - Tuple[Union[Submodule, Tree], Union[Blob, Tree, Submodule]]] - - ignore_self=True is_edge=True -> Iterator[item] - ignore_self=True is_edge=False --> Iterator[item] - ignore_self=False is_edge=True -> Iterator[item] | Iterator[Tuple[src, item]] - ignore_self=False is_edge=False -> Iterator[Tuple[src, item]] + :return: Iterator yielding items found when traversing self:: + + Commit -> Iterator[Union[Commit, Tuple[Commit, Commit]] + Submodule -> Iterator[Submodule, Tuple[Submodule, Submodule]] + Tree -> Iterator[Union[Blob, Tree, Submodule, + Tuple[Union[Submodule, Tree], Union[Blob, Tree, Submodule]]] + + ignore_self=True is_edge=True -> Iterator[item] + ignore_self=True is_edge=False --> Iterator[item] + ignore_self=False is_edge=True -> Iterator[item] | Iterator[Tuple[src, item]] + ignore_self=False is_edge=False -> Iterator[Tuple[src, item]] """ visited = set() @@ -547,7 +557,7 @@ class Serializable(Protocol): def _serialize(self, stream: "BytesIO") -> "Serializable": """Serialize the data of this object into the given data stream. - :note: A serialized object would ``_deserialize`` into the same object. + :note: A serialized object would :meth:`_deserialize` into the same object. :param stream: a file-like object @@ -627,24 +637,24 @@ def traverse( ignore_self: int = 1, as_edge: bool = False, ) -> Union[Iterator[T_TIobj], Iterator[Tuple[T_TIobj, T_TIobj]], Iterator[TIobj_tuple]]: - """For documentation, see util.Traversable._traverse()""" + """For documentation, see :meth:`Traversable._traverse`.""" + + ## To typecheck instead of using cast: + # + # import itertools + # from git.types import TypeGuard + # def is_commit_traversed(inp: Tuple) -> TypeGuard[Tuple[Iterator[Tuple['Commit', 'Commit']]]]: + # for x in inp[1]: + # if not isinstance(x, tuple) and len(x) != 2: + # if all(isinstance(inner, Commit) for inner in x): + # continue + # return True + # + # ret = super(Commit, self).traverse(predicate, prune, depth, branch_first, visit_once, ignore_self, as_edge) + # ret_tup = itertools.tee(ret, 2) + # assert is_commit_traversed(ret_tup), f"{[type(x) for x in list(ret_tup[0])]}" + # return ret_tup[0] - """ - # To typecheck instead of using cast. - import itertools - from git.types import TypeGuard - def is_commit_traversed(inp: Tuple) -> TypeGuard[Tuple[Iterator[Tuple['Commit', 'Commit']]]]: - for x in inp[1]: - if not isinstance(x, tuple) and len(x) != 2: - if all(isinstance(inner, Commit) for inner in x): - continue - return True - - ret = super(Commit, self).traverse(predicate, prune, depth, branch_first, visit_once, ignore_self, as_edge) - ret_tup = itertools.tee(ret, 2) - assert is_commit_traversed(ret_tup), f"{[type(x) for x in list(ret_tup[0])]}" - return ret_tup[0] - """ return cast( Union[Iterator[T_TIobj], Iterator[Tuple[Union[None, T_TIobj], T_TIobj]]], super()._traverse(predicate, prune, depth, branch_first, visit_once, ignore_self, as_edge), # type: ignore diff --git a/git/util.py b/git/util.py index b18dccdfe..c5afea241 100644 --- a/git/util.py +++ b/git/util.py @@ -1233,10 +1233,10 @@ def __init__(cls, name: str, bases: Tuple, clsdict: Dict) -> None: for base in bases: if type(base) is IterableClassWatcher: warnings.warn( - f"GitPython Iterable subclassed by {name}. " - "Iterable is deprecated due to naming clash since v3.1.18" - " and will be removed in 3.1.20, " - "Use IterableObj instead \n", + f"GitPython Iterable subclassed by {name}." + " Iterable is deprecated due to naming clash since v3.1.18" + " and will be removed in 4.0.0." + " Use IterableObj instead.", DeprecationWarning, stacklevel=2, ) From 3af3d4314031890744ef008eda376bb32e23de66 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Mon, 25 Dec 2023 06:59:02 -0500 Subject: [PATCH 2/2] Deprecate USE_SHELL; caution in Git.execute shell doc This builds on 106bbe6 (#1782) to actually deprecate Git.USE_SHELL. Its docstring is updated to state this, but no DeprecationWarning is issued for its use at this time. The documentation for the `shell` parameter of Git.execute is also updated to caution against passing shell=True, but the deprecation is not currently extended to that parameter. Some information is duplicated across the two docstrings. I have done this because the the USE_SHELL attribute's docstring, while it is displayed by many editors, doesn't currently appear in generated HTML documentation (see #1734). --- git/cmd.py | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/git/cmd.py b/git/cmd.py index 7297e4ae6..488c0dea8 100644 --- a/git/cmd.py +++ b/git/cmd.py @@ -280,13 +280,12 @@ def __setstate__(self, d: Dict[str, Any]) -> None: """Enables debugging of GitPython's git commands.""" USE_SHELL = False - """If True, a shell will be used when executing git commands. + """Deprecated. If set to True, a shell will be used when executing git commands. - This exists to avoid breaking old code that may access it, but it is no longer - needed and should rarely if ever be used. Prior to GitPython 2.0.8, it had a narrow - purpose in suppressing console windows in graphical Windows applications. In 2.0.8 - and higher, it provides no benefit, as GitPython solves that problem more robustly - and safely by using the ``CREATE_NO_WINDOW`` process creation flag on Windows. + Prior to GitPython 2.0.8, this had a narrow purpose in suppressing console windows + in graphical Windows applications. In 2.0.8 and higher, it provides no benefit, as + GitPython solves that problem more robustly and safely by using the + ``CREATE_NO_WINDOW`` process creation flag on Windows. Code that uses ``USE_SHELL = True`` or that passes ``shell=True`` to any GitPython functions should be updated to use the default value of ``False`` instead. ``True`` @@ -294,6 +293,7 @@ def __setstate__(self, d: Dict[str, Any]) -> None: for, which is not possible under most circumstances. See: + - :meth:`Git.execute` (on the ``shell`` parameter). - https://github.com/gitpython-developers/GitPython/commit/0d9390866f9ce42870d3116094cd49e0019a970a - https://learn.microsoft.com/en-us/windows/win32/procthread/process-creation-flags """ @@ -919,7 +919,15 @@ def execute( :param shell: Whether to invoke commands through a shell (see `Popen(..., shell=True)`). - It overrides :attr:`USE_SHELL` if it is not `None`. + If this is not `None`, it overrides :attr:`USE_SHELL`. + + Passing ``shell=True`` to this or any other GitPython function should be + avoided, as it is unsafe under most circumstances. This is because it is + typically not feasible to fully consider and account for the effect of shell + expansions, especially when passing ``shell=True`` to other methods that + forward it to :meth:`Git.execute`. Passing ``shell=True`` is also no longer + needed (nor useful) to work around any known operating system specific + issues. :param env: A dictionary of environment variables to be passed to :class:`subprocess.Popen`.

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