Merge pull request #1787 from EliahKagan/deprecation

Deprecate Git.USE_SHELL

Author: Byron

git/cmd.py

@@ -280,20 +280,20 @@ 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``
     is unsafe unless the effect of shell expansions is fully considered and accounted
     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`.

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

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]:
         """

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

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

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,
                 )