Repository: gitpython-developers/GitPython
Pull request #1886 (2024-03-30 08:04:09)

Issue and test deprecation warnings
### Scope

Some deprecations in GitPython had no associated `DeprecationWarning`, and none had unit tests to verify that the warnings were issued under the intended circumstances. This adds...

- warnings,
- tests of the warnings, and
- where applicable, tests for subtle problems that can arise from adding the warnings as new runtime behavior on attribute access (see below for details)

...for everything in GitPython that is documented as deprecated in comments and/or docstrings, *except* where a `DeprecationWarning` is intentionally not issued either because some other kind of warning is emitted or because there is a specific reason not to do so:

- Deprecated attributes of the top-level `git` module that are listed in `__all__` for compatibility intentionally do not warn because it would lead to developers silencing warnings if they don't want to see them when exploring in a REPL and using a wildcard import. (Deprecated attributes of `git` that are not listed in `__all__` do warn.)
- Setting the `HIDE_WINDOWS_KNOWN_ERRORS` and `HIDE_WINDOWS_FREEZE_ERRORS` environment variables is deprecated, as noted in `git/util.py`, but this is expected to be entirely a user behavior rather than a behavior of code calling GitPython, and as such it already emits a warning with the `logging` framework. It may be worthwhile to test for this warning, but hopefully that code can simply be removed soon (and its existing tests thinned out further) by making the test suite entirely responsible for any special test-related `rmtree` behavior ([#790](https://github.com/gitpython-developers/GitPython/issues/790)).
- Passing `$var`, `${var}`, and or Windows `%var%` notation in paths to expand environment variables is deprecated since [#662](https://github.com/gitpython-developers/GitPython/pull/662). The current behavior is to try to issue `UserWarning` (which is seen in more situations than `DeprecationWarning`) except when `expand_vars` is available and `False` has been passed for it (suppressing the expansion). Besides that these are not `DeprecationWarning`s, I consider this to be its own issue, as there are design decisions to be made about multiple aspects of it, which should probably be made together.

This also intentionally causes three previously accepted usages to be treated as static type errors:

- The way the deprecation warning is added for `git.types.Lit_commit_ish` is selected to also make it always a type error to appear in any annotation. Due to [#1858](https://github.com/gitpython-developers/GitPython/issues/1858) and [#1859](https://github.com/gitpython-developers/GitPython/pull/1859), it is implausible that any annotation with `Lit_commit_ish` has been, or is currently, correct. (It is nonetheless not removed, since doing so could cause new exceptions at runtime, including in working code.) Note that this does not affect anything else in `git.types`; other types there, including `Commit_ish`, can be used correctly, are not deprecated, and continue to work, including in annotations.
- Although `mypy` does not consider nonpublic attribute access a type error or attempt to detect it, `pyright` does, and the `util` attribute of the top-level `git` module is now identified as nonpublic. The `git.util` module (`from git.util import X`) is of course public, but due to the effect of a past wildcard import and the continued need to avoid breaking code outside GitPython that my have ended up depending on it, the `util` attribute of `git` actually aliases the `git.index.util` module. Because `util` has never been listed in `git.__all__` (including when it had been dynamically generated), it is effectively nonpublic, but `pyright` treated it as public based on being temporarily set as the `git.util` module before being set to `git.index.util`. It is retained in `dir()` and its deprecation warning notes how it is a special case, but it is now seen as nonpublic.
- Some other aliases to modules that are not direct Python submodules of the top-level `git` module had in the past been created by wildcard imports. These are still accessible, for now, but since there has never been a reason for anyone to consider those aliases part of GitPython's public interface, they are now regarded as absent (and an error to access) by static type checkers. They are also now omitted from `dir()`, though this is to avoid confusion with actual direct Python submodules of `git` much more than to discourage their use.

Other typing-related efforts here are of the opposite kind: making sure things don't break or change significantly.

Two other notable aspects of the scope:

- The changes made to provide the runtime behavior of issuing `DeprecationWarning` when deprecated module-level attributes are accessed is, by its nature, immediately ready to also cover the case of a dynamically computed `git.__version__` attribute, as discussed in [#1716 (comment)](https://github.com/gitpython-developers/GitPython/issues/1716#issuecomment-1773746757). See also [this exploration of approaches to that](https://github.com/EliahKagan/version). But to avoid bloating the scope, that change is not included in this PR; instead, a comment notes where that logic would go.

- Building further on [#1782](https://github.com/gitpython-developers/GitPython/pull/1782) and [#1787](https://github.com/gitpython-developers/GitPython/pull/1787), I expanded the `Git.USE_SHELL` docstring to better explain the deprecation and to be more helpful in presenting the issues and what to do instead of setting it to `True`. However, I suspect it may benefit from some further expansion soon to clarify the risks (though maybe I can figure out a way to remove or abridge some parts so it doesn't keep growing). *I regard the `USE_SHELL` deprecation to be core to this whole PR*, because I think of everything deprecated in GitPython that it is where the greatest risk of usage that is unaware of the disadvantages may be.

  It is also a class attribute, which is naturally capable of being read and written on the `Git` class as well as read (but not written) through `Git` instances. Making class attribute accesses issue warnings without breaking anything else is nontrivial, and while the approach I have taken avoids most possible pitfalls, some potentially remain. But I believe the value of surfacing this deprecation and its rationale is sufficient to justify it, both in and of itself, and when considering the effect that issuing other deprecation warnings but not this one would have in creating the false appearance that this deprecation is less important (when really it is more). Further details on this are below, and also in the tests.

### Avoiding subtle problems

In many places, issuing a warning if it was not already issued, and testing it, were straightforward, because the warning should occur exactly when code in the body of some function runs, so the code to issue the warning can simply be added there, usually at the top of the function. The case of `git.util.Iterable` is less basic, involving a metaclass for the deprecated class, but that was already implemented, so all it needed were unit tests where derived classes were locally defined while capturing and verifying the expected `DeprecationWarning`. Furthermore, some deprecated attribute accesses were basic cases, because they were already properties, so where warnings were absent it was sufficient to warn in the already existing property accessor.

In contrast, on attribute accesses that are not already properties or otherwise dynamically customized, adding new runtime behavior, even just to issue a warning, should be done carefully--if at all, but see below on rationale--because there are three kinds of things it can break, that can easily go undetected unless tested for explicitly:

- Access to the attribute in less common ways that ought to work for the sake of correctness or practicality. Access that should not work, such as writing a read-only attribute, should fail with the same exception as before and the same or a similar message, and with no state change.

- Runtime metadata. For example, some ways of customizing attribute access cause the attribute not to appear in `dir()`, which is often unintentional, and some ways of attempting to fix that cause other conceptually unrelated attributes not to appear in `dir()`.

  Another example is generated documentation, which is sometimes okay to change but should remain useful: Sphinx must find and use docstrings, and generated help from the `help` builtin (or `pydoc`) should at minimum not *stop* indicating that an attribute is deprecated as a consequence of changes made to issue a warning. When possible, attributes with an important default value that is shown by `help()` should continue to show it.

- Static typing. Static type checkers such as `mypy` and `pyright` treat `__getattr__` and `__getattribute__` methods to mean that arbitrary attributes are permitted. This is often good, since for example it makes it so code accessing dynamic callable attributes of `Git` instances is not analyzed as having type errors. But when such a method is added where fully dynamic attribute lookup is not conceptually correct, if done in view of the type checker (i.e., not hidden behind `if not TYPE_CHECKING`), it causes attempts to use misspelled or otherwise bogus attributes to be analyzed as correct and returning the `Any` type, on which subsequent operations also are treated as `Any` and not warned about, and so on.

  The other subtlety about static typing is the distinction between analyzing GitPython and analyzing another codebase that *uses* GitPython. For example, [#1656](https://github.com/gitpython-developers/GitPython/issues/1656) was not observed by GitPython's own static analysis because the project only uses `mypy` internally but both `mypy` and `pyright` are popular static type checkers; and [#1349](https://github.com/gitpython-developers/GitPython/issues/1349), which happens with `mypy`, nonetheless was not internally observed because of differences between running `mypy` on GitPython and running it on another project that uses GitPython (which *partly* have to do with configuration).

The first and second of those points are covered in detail by substantial regression tests that engage with a number of their ramifications. The tests have docstrings and I believe they present the considerations, as well as why those considerations led to the specific implementation approaches used, clearly. But there are three exceptions to this:

- The effect on Sphinx-generated documentation and documentation displayed by `help()` I checked manually instead.
- Likewise, I manually checked exception messages, but this was based on a judgment that these were low risk in the design approach I took, where either the messages where written explicitly (in module-level `__getattr__`) or were delegated to Python through `super()` in overridden special methods. Because the implementation approach might change in the future, which is something the new tests usually try to accommodate, I do not consider the omission of exception-message assertions to be ideal. But the messages are not guaranteed to be the same in every version of Python and do change in some Python releases.
- The changes required to issue warnings on all accesses to `Git.USE_SHELL` are significant, and probably only justified because of the special value of issuing those particular warnings. (Their rationale is examined below.)

In view of the importance of maintaining robust and accurate static type checking at the boundary of GitPython and code that uses it, I took the somewhat unusual approach of [developing many of the tests in a separate project first](https://github.com/EliahKagan/GitPython-static-dynamic) while experimenting with and implementing some changes on my feature branch of GitPython. Then I used [git-filter-repo](https://github.com/newren/git-filter-repo) and a couple of rebases to include those commits in my GitPython feature branch, broken up into a few chunks, and interleaved with the changes they test, in what seemed like the most honest order.

Although CI results seem to be interesting on each commit and could be viewed in my fork in case interest arises, for those parts I'm pushing only the tips of each chunk, because there are many commits. Doing it that way also seems like it should make it easier to see where the chunks are.

The new tests should be type-checked within the GitPython project in the same way the code of the `git` module is type-checked (whatever that is, i.e., I am not advocating that `continue-on-error` be removed from the `mypy` step of `pythonpackage.yml` at this time). To facilitate this, I have put them in their own directory within `test/`, fully type-annotated them, configured `mypy` to be runnable with no arguments and scan both `git/` and `test/deprecation/`, and and modified the `pythonpackage.yml` CI workflow, `tox.ini`, and `README.md` with that simpler yet now slightly broader command.

This has the potential eventual disadvantage that splitting up tests of the same units of code into multiple test modules could be unwieldy, if many more new test subpackages alongside `test.performance` and `test.deprecation` end up popping up for separate unrelated reasons in the future. However, it has the advantage that all deprecations except those for which no `DeprecationWarning` is issued for some special reason (see above) can now be efficiently discerned by examining those tests. For this reason I think this is justified at the moment and potentially even permanently.

Running `mypy` in GitPython shows the same five errors as it did as of the reduction in #1859, none of which are in areas sufficiently related to anything touched here that they would stand to obscure problems introduced here.

### Rationale and design considerations of `USE_SHELL` warnings

I believe that what I have done for `Git.USE_SHELL` requires a specific rationale. The reason is that, to issue the deprecation warnings, *I have caused the `Git` class to have a custom metaclass*.

On the one hand, this should probably not break anything. As noted in the `test_git_cmd.py` module docstring, this could potentially break existing code [by causing a metaclass conflict](https://github.com/gitpython-developers/GitPython/blob/10e0fc2b862df63fbcaefbc965e3e55f19091c82/test/deprecation/test_cmd_git.py#L38), if code that uses GitPython is not only inheriting from the `Git` class, and not only using it in multiple inheritance, but using it in multiple inheritance with at least one sibling class that has its own unrelated custom metaclass. The narrow considerations are:

- This is arguably plausible, since `abc.ABC` uses the custom metaclass `abc.ABCMeta` and therefore all classes derived from abstract base classes have it, including concrete leaves in such a hierarchy. Also, explicitly naming a protocol as a base class also entails having a custom metaclass. (Satisfying a protocol in the usual way by having all the appropriate members of course does not entail gaining a custom metaclass.)
- However, this seems like a fairly low risk, especially considering the propensity for interference between the effectively unlimited number of dynamic "methods" of `Git` instances, and the functionality of other classes in multiple inheritance. I would question what successful applications there are of multiple inheritance with a custom metaclass and `Git` as a sibling.
- To be clear, the issue is not strictly that it breaks multiple inheritance with other unrelated custom metaclasses, but rather that it would be a breaking change to other such code that already exists, because that code would have to be modified to define and use a new metaclass that inherits from both metaclasses.
- It seems to me that it may be justified based on the idea [you have expressed there](https://github.com/gitpython-developers/GitPython/pull/1697#discussion_r1356428138) which, if I understand it correctly, entails at least that it is not of paramount importance to support unusual and inherently brittle forms of inheritance from `Git` that are not known or believed to be in current use.

But the broader consideration is... how can this possibly be? How can it be necessary to use a metaclass just to add a side effect to class attribute access?

Intuitively it feels like a descriptor, in the `Git` class, could do it. But a descriptor can only customize:

- All forms of access (getting, setting, and deleting) on an *instance* of the class it is placed in.
- *Only reading* on the class itself that it is placed in.

This is to say that:

- [`__get__`](https://docs.python.org/3/reference/datamodel.html#object.__get__) (not to be confused with `__getattr__` or `__getattribute__`) is called for both instance and class getattr operations (its `instance` parameter may be `None`).
- [`__set__`](https://docs.python.org/3/reference/datamodel.html#object.__set__) (not to be confused with `__setattr__`) is called only for instance setattr operations; getting the attribute from the class does not call `__set__` but simply replaces the descriptor object.
- [`__delete__`](https://docs.python.org/3/reference/datamodel.html#object.__delete__) (not to be confused with `__delattr__` or `__del__`) is analogous to `__set__`, being called only for instance delattr operations; deleting the attribute from the class does not call `__delete__` but simply removes the attribute (dropping the reference to the descriptor object).

Therefore, neither custom descriptors, nor properties (due to property objects being descriptors) will do this for us in the `Git` class itself.

Likewise, [`__getattr__`](https://docs.python.org/3/reference/datamodel.html#object.__getattr__), [`__getattribute__`](https://docs.python.org/3/reference/datamodel.html#object.__getattribute__), and [`__setattr__`](https://docs.python.org/3/reference/datamodel.html#object.__setattr__) won't do it, because they only ever customize attribute access on *instances* of the class they are defined in. (This differs from [`__getattr__`'s use in a module](https://docs.python.org/3/reference/datamodel.html#customizing-module-attribute-access), where it customizes access in that module.)

More precisely, none of these things will do it *by themselves*. They can warn on access to the `USE_SHELL` class attribute through instances (which I have done, with `__getattribute__` in `Git`). They could even warn on accesses by *reading* it from the class; but not about the most dangerous operation most meriting a warning, which is *setting* it on the class.

While the unit tests provide protection against technical pitfalls, as well as regression protection if the implementation strategy is changed in the future without accounting for all subtleties, they fundamentally cannot answer the underlying design question of whether the importance of warning about `USE_SHELL` is sufficient to justify the implementation. As I note in the `test_cmd_git.py` docstring:

https://github.com/gitpython-developers/GitPython/blob/10e0fc2b862df63fbcaefbc965e3e55f19091c82/test/deprecation/test_cmd_git.py#L54-L56

As mentioned above and elsewhere, I believe it is justified. However, I am willing to make changes, including if necessary removing `_GitMeta` altogether.

Assuming it is to be kept, a remaining question is whether it is better or worse to provide a public alias to the metaclass. It can of course be gotten with `type(Git)`, but static type checkers consider such an expression anywhere in any type annotation to be an error. An alias does not commit `Git` to continue to have a custom metaclass, because it can be documented as aliasing its metaclass whether that metaclass is `type` or a custom metaclass. I've done that. I've made the tests for that alias, and the code of the alias and its docstring itself (which cautions against writing code that would need it as well as against using it in a type annotation when `Type[Git]` is meant), be the last two commits here. That is so they can easily be dropped or reverted if you think having that public alias is not an improvement.


Patch:
diff --git a/.github/workflows/pythonpackage.yml b/.github/workflows/pythonpackage.yml
index 7cee0cd6..4c918a92 100644
--- a/.github/workflows/pythonpackage.yml
+++ b/.github/workflows/pythonpackage.yml
@@ -88,7 +88,7 @@ jobs:
 
     - name: Check types with mypy
       run: |
-        mypy --python-version=${{ matrix.python-version }} -p git
+        mypy --python-version=${{ matrix.python-version }}
       env:
         MYPY_FORCE_COLOR: "1"
         TERM: "xterm-256color"  # For color: https://github.com/python/mypy/issues/13817
diff --git a/README.md b/README.md
index 30af532d..9bedaaae 100644
--- a/README.md
+++ b/README.md
@@ -167,7 +167,7 @@ This includes the linting and autoformatting done by Ruff, as well as some other
 To typecheck, run:
 
 ```sh
-mypy -p git
+mypy
 ```
 
 #### CI (and tox)
diff --git a/SECURITY.md b/SECURITY.md
index d39425b7..cbfaafdd 100644
--- a/SECURITY.md
+++ b/SECURITY.md
@@ -11,4 +11,5 @@ Only the latest version of GitPython can receive security updates. If a vulnerab
 
 ## Reporting a Vulnerability
 
-Please report private portions of a vulnerability to <https://github.com/gitpython-developers/GitPython/security/advisories/new>. Doing so helps to receive updates and collaborate on the matter, without disclosing it publicliy right away.
+Please report private portions of a vulnerability to sebastian.thiel@icloud.com that would help to reproduce and fix it. To receive updates on progress and provide
+general information to the public, you can create an issue [on the issue tracker](https://github.com/gitpython-developers/GitPython/issues).
diff --git a/git/__init__.py b/git/__init__.py
index a1303045..1b2360e3 100644
--- a/git/__init__.py
+++ b/git/__init__.py
@@ -88,7 +88,12 @@ __all__ = [
 
 __version__ = "git"
 
-from typing import List, Optional, Sequence, TYPE_CHECKING, Tuple, Union
+from typing import Any, List, Optional, Sequence, TYPE_CHECKING, Tuple, Union
+
+if TYPE_CHECKING:
+    from types import ModuleType
+
+import warnings
 
 from gitdb.util import to_hex_sha
 
@@ -144,11 +149,6 @@ try:
         SymbolicReference,
         Tag,
         TagReference,
-        head,  # noqa: F401  # Nonpublic. May disappear! Use git.refs.head.
-        log,  # noqa: F401  # Nonpublic. May disappear! Use git.refs.log.
-        reference,  # noqa: F401  # Nonpublic. May disappear! Use git.refs.reference.
-        symbolic,  # noqa: F401  # Nonpublic. May disappear! Use git.refs.symbolic.
-        tag,  # noqa: F401  # Nonpublic. May disappear! Use git.refs.tag.
     )
     from git.diff import (  # @NoMove
         INDEX,
@@ -169,21 +169,9 @@ try:
         IndexEntry,
         IndexFile,
         StageType,
-        base,  # noqa: F401  # Nonpublic. May disappear! Use git.index.base.
-        fun,  # noqa: F401  # Nonpublic. May disappear! Use git.index.fun.
-        typ,  # noqa: F401  # Nonpublic. May disappear! Use git.index.typ.
-        #
-        # NOTE: The expression `git.util` evaluates to git.index.util, and the import
-        # `from git import util` imports git.index.util, NOT git.util. It may not be
-        # feasible to change this until the next major version, to avoid breaking code
-        # inadvertently relying on it. If git.index.util really is what you want, use or
-        # import from that name, to avoid confusion. To use the "real" git.util module,
-        # write `from git.util import ...`, or access it as `sys.modules["git.util"]`.
-        # (This differs from other historical indirect-submodule imports that are
-        # unambiguously nonpublic and are subject to immediate removal. Here, the public
-        # git.util module, even though different, makes it less discoverable that the
-        # expression `git.util` refers to a non-public attribute of the git module.)
-        util,  # noqa: F401
+        # NOTE: This tells type checkers what util resolves to. We delete it, and it is
+        # really resolved by __getattr__, which warns. See below on what to use instead.
+        util,
     )
     from git.util import (  # @NoMove
         Actor,
@@ -196,7 +184,79 @@ try:
 except GitError as _exc:
     raise ImportError("%s: %s" % (_exc.__class__.__name__, _exc)) from _exc
 
+
+def _warned_import(message: str, fullname: str) -> "ModuleType":
+    import importlib
+
+    warnings.warn(message, DeprecationWarning, stacklevel=3)
+    return importlib.import_module(fullname)
+
+
+def _getattr(name: str) -> Any:
+    # TODO: If __version__ is made dynamic and lazily fetched, put that case right here.
+
+    if name == "util":
+        return _warned_import(
+            "The expression `git.util` and the import `from git import util` actually "
+            "reference git.index.util, and not the git.util module accessed in "
+            '`from git.util import XYZ` or `sys.modules["git.util"]`. This potentially '
+            "confusing behavior is currently preserved for compatibility, but may be "
+            "changed in the future and should not be relied on.",
+            fullname="git.index.util",
+        )
+
+    for names, prefix in (
+        ({"head", "log", "reference", "symbolic", "tag"}, "git.refs"),
+        ({"base", "fun", "typ"}, "git.index"),
+    ):
+        if name not in names:
+            continue
+
+        fullname = f"{prefix}.{name}"
+
+        return _warned_import(
+            f"{__name__}.{name} is a private alias of {fullname} and subject to "
+            f"immediate removal. Use {fullname} instead.",
+            fullname=fullname,
+        )
+
+    raise AttributeError(f"module {__name__!r} has no attribute {name!r}")
+
+
+if not TYPE_CHECKING:
+    # NOTE: The expression `git.util` gives git.index.util and `from git import util`
+    # imports git.index.util, NOT git.util. It may not be feasible to change this until
+    # the next major version, to avoid breaking code inadvertently relying on it.
+    #
+    # - If git.index.util *is* what you want, use (or import from) that, to avoid
+    #   confusion.
+    #
+    # - To use the "real" git.util module, write `from git.util import ...`, or if
+    #   necessary access it as `sys.modules["git.util"]`.
+    #
+    # Note also that `import git.util` technically imports the "real" git.util... but
+    # the *expression* `git.util` after doing so is still git.index.util!
+    #
+    # (This situation differs from that of other indirect-submodule imports that are
+    # unambiguously non-public and subject to immediate removal. Here, the public
+    # git.util module, though different, makes less discoverable that the expression
+    # `git.util` refers to a non-public attribute of the git module.)
+    #
+    # This had originally come about by a wildcard import. Now that all intended imports
+    # are explicit, the intuitive but potentially incompatible binding occurs due to the
+    # usual rules for Python submodule bindings. So for now we replace that binding with
+    # git.index.util, delete that, and let __getattr__ handle it and issue a warning.
+    #
+    # For the same runtime behavior, it would be enough to forgo importing util, and
+    # delete util as created naturally; __getattr__ would behave the same. But type
+    # checkers would not know what util refers to when accessed as an attribute of git.
+    del util
+
+    # This is "hidden" to preserve static checking for undefined/misspelled attributes.
+    __getattr__ = _getattr
+
 # { Initialize git executable path
+
 GIT_OK = None
 
 
@@ -232,12 +292,9 @@ def refresh(path: Optional[PathLike] = None) -> None:
     GIT_OK = True
 
 
-# } END initialize git executable path
-
-
-#################
 try:
     refresh()
 except Exception as _exc:
     raise ImportError("Failed to initialize: {0}".format(_exc)) from _exc
-#################
+
+# } END initialize git executable path
diff --git a/git/cmd.py b/git/cmd.py
index 03e5d7ff..90fc39cd 100644
--- a/git/cmd.py
+++ b/git/cmd.py
@@ -5,7 +5,7 @@
 
 from __future__ import annotations
 
-__all__ = ["Git"]
+__all__ = ["GitMeta", "Git"]
 
 import contextlib
 import io
@@ -19,6 +19,7 @@ from subprocess import DEVNULL, PIPE, Popen
 import sys
 from textwrap import dedent
 import threading
+import warnings
 
 from git.compat import defenc, force_bytes, safe_decode
 from git.exc import (
@@ -307,8 +308,79 @@ def dict_to_slots_and__excluded_are_none(self: object, d: Mapping[str, Any], exc
 
 ## -- End Utilities -- @}
 
+_USE_SHELL_DEFAULT_MESSAGE = (
+    "Git.USE_SHELL is deprecated, because only its default value of False is safe. "
+    "It will be removed in a future release."
+)
+
+_USE_SHELL_DANGER_MESSAGE = (
+    "Setting Git.USE_SHELL to True is unsafe and insecure, as the effect of special "
+    "shell syntax cannot usually be accounted for. This can result in a command "
+    "injection vulnerability and arbitrary code execution. Git.USE_SHELL is deprecated "
+    "and will be removed in a future release."
+)
+
+
+def _warn_use_shell(extra_danger: bool) -> None:
+    warnings.warn(
+        _USE_SHELL_DANGER_MESSAGE if extra_danger else _USE_SHELL_DEFAULT_MESSAGE,
+        DeprecationWarning,
+        stacklevel=3,
+    )
+
+
+class _GitMeta(type):
+    """Metaclass for :class:`Git`.
+
+    This helps issue :class:`DeprecationWarning` if :attr:`Git.USE_SHELL` is used.
+    """
+
+    def __getattribute(cls, name: str) -> Any:
+        if name == "USE_SHELL":
+            _warn_use_shell(False)
+        return super().__getattribute__(name)
+
+    def __setattr(cls, name: str, value: Any) -> Any:
+        if name == "USE_SHELL":
+            _warn_use_shell(value)
+        super().__setattr__(name, value)
+
+    if not TYPE_CHECKING:
+        # To preserve static checking for undefined/misspelled attributes while letting
+        # the methods' bodies be type-checked, these are defined as non-special methods,
+        # then bound to special names out of view of static type checkers. (The original
+        # names invoke name mangling (leading "__") to avoid confusion in other scopes.)
+        __getattribute__ = __getattribute
+        __setattr__ = __setattr
+
+
+GitMeta = _GitMeta
+"""Alias of :class:`Git`'s metaclass, whether it is :class:`type` or a custom metaclass.
+
+Whether the :class:`Git` class has the default :class:`type` as its metaclass or uses a
+custom metaclass is not documented and may change at any time. This statically checkable
+metaclass alias is equivalent at runtime to ``type(Git)``. This should almost never be
+used. Code that benefits from it is likely to be remain brittle even if it is used.
 
-class Git:
+In view of the :class:`Git` class's intended use and :class:`Git` objects' dynamic
+callable attributes representing git subcommands, it rarely makes sense to inherit from
+:class:`Git` at all. Using :class:`Git` in multiple inheritance can be especially tricky
+to do correctly. Attempting uses of :class:`Git` where its metaclass is relevant, such
+as when a sibling class has an unrelated metaclass and a shared lower bound metaclass
+might have to be introduced to solve a metaclass conflict, is not recommended.
+
+:note:
+    The correct static type of the :class:`Git` class itself, and any subclasses, is
+    ``Type[Git]``. (This can be written as ``type[Git]`` in Python 3.9 later.)
+
+    :class:`GitMeta` should never be used in any annotation where ``Type[Git]`` is
+    intended or otherwise possible to use. This alias is truly only for very rare and
+    inherently precarious situations where it is necessary to deal with the metaclass
+    explicitly.
+"""
+
+
+class Git(metaclass=_GitMeta):
     """The Git class manages communication with the Git binary.
 
     It provides a convenient interface to calling the Git binary, such as in::
@@ -358,24 +430,53 @@ class Git:
     GIT_PYTHON_TRACE = os.environ.get("GIT_PYTHON_TRACE", False)
     """Enables debugging of GitPython's git commands."""
 
-    USE_SHELL = False
+    USE_SHELL: bool = False
     """Deprecated. If set to ``True``, a shell will be used when executing git commands.
 
+    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 syntax treated specially by the shell is fully
+    considered and accounted for, which is not possible under most circumstances. As
+    detailed below, it is also no longer needed, even where it had been in the past.
+
+    It is in many if not most cases a command injection vulnerability for an application
+    to set :attr:`USE_SHELL` to ``True``. Any attacker who can cause a specially crafted
+    fragment of text to make its way into any part of any argument to any git command
+    (including paths, branch names, etc.) can cause the shell to read and write
+    arbitrary files and execute arbitrary commands. Innocent input may also accidentally
+    contain special shell syntax, leading to inadvertent malfunctions.
+
+    In addition, how a value of ``True`` interacts with some aspects of GitPython's
+    operation is not precisely specified and may change without warning, even before
+    GitPython 4.0.0 when :attr:`USE_SHELL` may be removed. This includes:
+
+    * Whether or how GitPython automatically customizes the shell environment.
+
+    * Whether, outside of Windows (where :class:`subprocess.Popen` supports lists of
+      separate arguments even when ``shell=True``), this can be used with any GitPython
+      functionality other than direct calls to the :meth:`execute` method.
+
+    * Whether any GitPython feature that runs git commands ever attempts to partially
+      sanitize data a shell may treat specially. Currently this is not done.
+
     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.
+    Because Windows path search differs subtly based on whether a shell is used, in rare
+    cases changing this from ``True`` to ``False`` may keep an unusual git "executable",
+    such as a batch file, from being found. To fix this, set the command name or full
+    path in the :envvar:`GIT_PYTHON_GIT_EXECUTABLE` environment variable or pass the
+    full path to :func:`git.refresh` (or invoke the script using a ``.exe`` shim).
 
-    See:
+    Further reading:
 
-    - :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
+    * :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
+    * https://github.com/python/cpython/issues/91558#issuecomment-1100942950
+    * https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw
     """
 
     _git_exec_env_var = "GIT_PYTHON_GIT_EXECUTABLE"
@@ -868,6 +969,11 @@ class Git:
         self.cat_file_header: Union[None, TBD] = None
         self.cat_file_all: Union[None, TBD] = None
 
+    def __getattribute__(self, name: str) -> Any:
+        if name == "USE_SHELL":
+            _warn_use_shell(False)
+        return super().__getattribute__(name)
+
     def __getattr__(self, name: str) -> Any:
         """A convenience method as it allows to call the command as if it was an object.
 
@@ -1138,7 +1244,12 @@ class Git:
 
         stdout_sink = PIPE if with_stdout else getattr(subprocess, "DEVNULL", None) or open(os.devnull, "wb")
         if shell is None:
-            shell = self.USE_SHELL
+            # Get the value of USE_SHELL with no deprecation warning. Do this without
+            # warnings.catch_warnings, to avoid a race condition with application code
+            # configuring warnings. The value could be looked up in type(self).__dict__
+            # or Git.__dict__, but those can break under some circumstances. This works
+            # the same as self.USE_SHELL in more situations; see Git.__getattribute__.
+            shell = super().__getattribute__("USE_SHELL")
         _logger.debug(
             "Popen(%s, cwd=%s, stdin=%s, shell=%s, universal_newlines=%s)",
             redacted_command,
diff --git a/git/compat.py b/git/compat.py
index 4ede8c98..d7d9a55a 100644
--- a/git/compat.py
+++ b/git/compat.py
@@ -13,6 +13,7 @@ use some of these utilities, in some cases for compatibility across different pl
 import locale
 import os
 import sys
+import warnings
 
 from gitdb.utils.encoding import force_bytes, force_text  # noqa: F401
 
@@ -23,7 +24,9 @@ from typing import (
     AnyStr,
     Dict,  # noqa: F401
     IO,  # noqa: F401
+    List,
     Optional,
+    TYPE_CHECKING,
     Tuple,  # noqa: F401
     Type,  # noqa: F401
     Union,
@@ -33,7 +36,37 @@ from typing import (
 # ---------------------------------------------------------------------------
 
 
-is_win = os.name == "nt"
+_deprecated_platform_aliases = {
+    "is_win": os.name == "nt",
+    "is_posix": os.name == "posix",
+    "is_darwin": sys.platform == "darwin",
+}
+
+
+def _getattr(name: str) -> Any:
+    try:
+        value = _deprecated_platform_aliases[name]
+    except KeyError:
+        raise AttributeError(f"module {__name__!r} has no attribute {name!r}") from None
+
+    warnings.warn(
+        f"{__name__}.{name} and other is_<platform> aliases are deprecated. "
+        "Write the desired os.name or sys.platform check explicitly instead.",
+        DeprecationWarning,
+        stacklevel=2,
+    )
+    return value
+
+
+if not TYPE_CHECKING:  # Preserve static checking for undefined/misspelled attributes.
+    __getattr__ = _getattr
+
+
+def __dir__() -> List[str]:
+    return [*globals(), *_deprecated_platform_aliases]
+
+
+is_win: bool
 """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
@@ -45,7 +78,7 @@ used.
     Cygwin, use ``sys.platform == "cygwin"``.
 """
 
-is_posix = os.name == "posix"
+is_posix: bool
 """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
@@ -58,7 +91,7 @@ used.
     (Darwin).
 """
 
-is_darwin = sys.platform == "darwin"
+is_darwin: bool
 """Deprecated alias for ``sys.platform == "darwin"`` to check for macOS (Darwin).
 
 This is deprecated because it clearer to write out :attr:`os.name` or
diff --git a/git/diff.py b/git/diff.py
index 0e39fe7a..f89b12d9 100644
--- a/git/diff.py
+++ b/git/diff.py
@@ -7,6 +7,7 @@ __all__ = ["DiffConstants", "NULL_TREE", "INDEX", "Diffable", "DiffIndex", "Diff
 
 import enum
 import re
+import warnings
 
 from git.cmd import handle_process_output
 from git.compat import defenc
@@ -554,6 +555,11 @@ class Diff:
             This property is deprecated.
             Please use the :attr:`renamed_file` property instead.
         """
+        warnings.warn(
+            "Diff.renamed is deprecated, use Diff.renamed_file instead",
+            DeprecationWarning,
+            stacklevel=2,
+        )
         return self.renamed_file
 
     @property
diff --git a/git/objects/commit.py b/git/objects/commit.py
index 8de52980..d957c905 100644
--- a/git/objects/commit.py
+++ b/git/objects/commit.py
@@ -14,6 +14,7 @@ import re
 from subprocess import Popen, PIPE
 import sys
 from time import altzone, daylight, localtime, time, timezone
+import warnings
 
 from gitdb import IStream
 
@@ -399,6 +400,11 @@ class Commit(base.Object, TraversableIterableObj, Diffable, Serializable):
             Dictionary containing whitespace stripped trailer information.
             Only contains the latest instance of each trailer key.
         """
+        warnings.warn(
+            "Commit.trailers is deprecated, use Commit.trailers_list or Commit.trailers_dict instead",
+            DeprecationWarning,
+            stacklevel=2,
+        )
         return {k: v[0] for k, v in self.trailers_dict.items()}
 
     @property
diff --git a/git/types.py b/git/types.py
index a93ebdb4..58445014 100644
--- a/git/types.py
+++ b/git/types.py
@@ -7,14 +7,17 @@ from typing import (
     Any,
     Callable,
     Dict,
+    List,
     NoReturn,
     Optional,
     Sequence as Sequence,
     Tuple,
     TYPE_CHECKING,
+    Type,
     TypeVar,
     Union,
 )
+import warnings
 
 if sys.version_info >= (3, 8):
     from typing import (
@@ -127,21 +130,50 @@ See :manpage:`gitglossary(7)` on "object type":
 https://git-scm.com/docs/gitglossary#def_object_type
 """
 
-Lit_commit_ish = Literal["commit", "tag"]
-"""Deprecated. Type of literal strings identifying sometimes-commitish git object types.
+Lit_commit_ish: Type[Literal["commit", "tag"]]
+"""Deprecated. Type of literal strings identifying typically-commitish git object types.
 
 Prior to a bugfix, this type had been defined more broadly. Any usage is in practice
-ambiguous and likely to be incorrect. Instead of this type:
+ambiguous and likely to be incorrect. This type has therefore been made a static type
+error to appear in annotations. It is preserved, with a deprecated status, to avoid
+introducing runtime errors in code that refers to it, but it should not be used.
+
+Instead of this type:
 
 * For the type of the string literals associated with :class:`Commit_ish`, use
   ``Literal["commit", "tag"]`` or create a new type alias for it. That is equivalent to
-  this type as currently defined.
+  this type as currently defined (but usable in statically checked type annotations).
 
 * For the type of all four string literals associated with :class:`AnyGitObject`, use
   :class:`GitObjectTypeString`. That is equivalent to the old definition of this type
-  prior to the bugfix.
+  prior to the bugfix (and is also usable in statically checked type annotations).
 """
 
+
+def _getattr(name: str) -> Any:
+    if name != "Lit_commit_ish":
+        raise AttributeError(f"module {__name__!r} has no attribute {name!r}")
+
+    warnings.warn(
+        "Lit_commit_ish is deprecated. It is currently defined as "
+        '`Literal["commit", "tag"]`, which should be used in its place if desired. It '
+        'had previously been defined as `Literal["commit", "tag", "blob", "tree"]`, '
+        "covering all four git object type strings including those that are never "
+        "commit-ish. For that, use the GitObjectTypeString type instead.",
+        DeprecationWarning,
+        stacklevel=2,
+    )
+    return Literal["commit", "tag"]
+
+
+if not TYPE_CHECKING:  # Preserve static checking for undefined/misspelled attributes.
+    __getattr__ = _getattr
+
+
+def __dir__() -> List[str]:
+    return [*globals(), "Lit_commit_ish"]
+
+
 # Config_levels ---------------------------------------------------------
 
 Lit_config_levels = Literal["system", "global", "user", "repository"]
@@ -188,12 +220,12 @@ def assert_never(inp: NoReturn, raise_error: bool = True, exc: Union[Exception,
 
     :param inp:
         If all members are handled, the argument for `inp` will have the
-        :class:`~typing.Never`/:class:`~typing.NoReturn` type. Otherwise, the type will
-        mismatch and cause a mypy error.
+        :class:`~typing.Never`/:class:`~typing.NoReturn` type.
+        Otherwise, the type will mismatch and cause a mypy error.
 
     :param raise_error:
-        If ``True``, will also raise :exc:`ValueError` with a general "unhandled
-        literal" message, or the exception object passed as `exc`.
+        If ``True``, will also raise :exc:`ValueError` with a general
+        "unhandled literal" message, or the exception object passed as `exc`.
 
     :param exc:
         It not ``None``, this should be an already-constructed exception object, to be
diff --git a/git/util.py b/git/util.py
index 5839f972..8c1c2601 100644
--- a/git/util.py
+++ b/git/util.py
@@ -1285,7 +1285,7 @@ class IterableObj(Protocol):
 
 
 class IterableClassWatcher(type):
-    """Metaclass that issues :class:`DeprecationWarning` when :class:`git.util.Iterable`
+    """Metaclass that issues :exc:`DeprecationWarning` when :class:`git.util.Iterable`
     is subclassed."""
 
     def __init__(cls, name: str, bases: Tuple, clsdict: Dict) -> None:
diff --git a/pyproject.toml b/pyproject.toml
index 1dc1e6ae..6cb05f96 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -21,10 +21,11 @@ testpaths = "test"  # Space separated list of paths from root e.g test tests doc
 
 [tool.mypy]
 python_version = "3.8"
+files = ["git/", "test/deprecation/"]
 disallow_untyped_defs = true
 no_implicit_optional = true
 warn_redundant_casts = true
-warn_unused_ignores = true
+warn_unused_ignores = true  # Useful in general, but especially in test/deprecation.
 warn_unreachable = true
 implicit_reexport = true
 # strict = true
diff --git a/tox.ini b/tox.ini
index 33074a78..fc62fa58 100644
--- a/tox.ini
+++ b/tox.ini
@@ -30,7 +30,7 @@ description = Typecheck with mypy
 base_python = py{39,310,311,312,38,37}
 set_env =
     MYPY_FORCE_COLOR = 1
-commands = mypy -p git
+commands = mypy
 ignore_outcome = true
 
 [testenv:html]


Test patch:
diff --git a/test-requirements.txt b/test-requirements.txt
index e1f5e2ed..75e9e81f 100644
--- a/test-requirements.txt
+++ b/test-requirements.txt
@@ -8,3 +8,4 @@ pytest-cov
 pytest-instafail
 pytest-mock
 pytest-sugar
+typing-extensions ; python_version < "3.11"
diff --git a/test/deprecation/__init__.py b/test/deprecation/__init__.py
new file mode 100644
index 00000000..fec3126d
--- /dev/null
+++ b/test/deprecation/__init__.py
@@ -0,0 +1,19 @@
+# This module is part of GitPython and is released under the
+# 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/
+
+"""Tests of deprecation warnings and possible related attribute bugs.
+
+Most deprecation warnings are "basic" in the sense that there is no special complexity
+to consider, in introducing them. However, to issue deprecation warnings on mere
+attribute access can involve adding new dynamic behavior. This can lead to subtle bugs
+or less useful dynamic metadata. It can also weaken static typing, as happens if a type
+checker sees a method like ``__getattr__`` in a module or class whose attributes it did
+not already judge to be dynamic. This test.deprecation submodule covers all three cases:
+the basic cases, subtle dynamic behavior, and subtle static type checking issues.
+
+Static type checking is "tested" by a combination of code that should not be treated as
+a type error but would be in the presence of particular bugs, and code that *should* be
+treated as a type error and is accordingly marked ``# type: ignore[REASON]`` (for
+specific ``REASON``. The latter will only produce mypy errors when the expectation is
+not met if it is configured with ``warn_unused_ignores = true``.
+"""
diff --git a/test/deprecation/lib.py b/test/deprecation/lib.py
new file mode 100644
index 00000000..9fe623a3
--- /dev/null
+++ b/test/deprecation/lib.py
@@ -0,0 +1,27 @@
+# This module is part of GitPython and is released under the
+# 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/
+
+"""Support library for deprecation tests."""
+
+__all__ = ["assert_no_deprecation_warning", "suppress_deprecation_warning"]
+
+import contextlib
+import warnings
+
+from typing import Generator
+
+
+@contextlib.contextmanager
+def assert_no_deprecation_warning() -> Generator[None, None, None]:
+    """Context manager to assert that code does not issue any deprecation warnings."""
+    with warnings.catch_warnings():
+        warnings.simplefilter("error", DeprecationWarning)
+        warnings.simplefilter("error", PendingDeprecationWarning)
+        yield
+
+
+@contextlib.contextmanager
+def suppress_deprecation_warning() -> Generator[None, None, None]:
+    with warnings.catch_warnings():
+        warnings.simplefilter("ignore", DeprecationWarning)
+        yield
diff --git a/test/deprecation/test_basic.py b/test/deprecation/test_basic.py
new file mode 100644
index 00000000..6235a836
--- /dev/null
+++ b/test/deprecation/test_basic.py
@@ -0,0 +1,124 @@
+# This module is part of GitPython and is released under the
+# 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/
+
+"""Tests of assorted deprecation warnings when there are no extra subtleties to check.
+
+This tests deprecation warnings where all that needs be verified is that a deprecated
+property, function, or class issues a DeprecationWarning when used and, if applicable,
+that recommended alternatives do not issue the warning.
+
+This is in contrast to other modules within test.deprecation, which test warnings where
+there is a risk of breaking other runtime behavior, or of breaking static type checking
+or making it less useful, by introducing the warning or in plausible future changes to
+how the warning is implemented. That happens when it is necessary to customize attribute
+access on a module or class, in a way it was not customized before, to issue a warning.
+It is inapplicable to the deprecations whose warnings are tested in this module.
+"""
+
+import pytest
+
+from git.diff import NULL_TREE
+from git.objects.util import Traversable
+from git.repo import Repo
+from git.util import Iterable as _Iterable, IterableObj
+
+from .lib import assert_no_deprecation_warning
+
+# typing -----------------------------------------------------------------
+
+from typing import Generator, TYPE_CHECKING
+
+if TYPE_CHECKING:
+    from pathlib import Path
+
+    from git.diff import Diff
+    from git.objects.commit import Commit
+
+# ------------------------------------------------------------------------
+
+
+@pytest.fixture
+def commit(tmp_path: "Path") -> Generator["Commit", None, None]:
+    """Fixture to supply a one-commit repo's commit, enough for deprecation tests."""
+    (tmp_path / "a.txt").write_text("hello\n", encoding="utf-8")
+    repo = Repo.init(tmp_path)
+    repo.index.add(["a.txt"])
+    yield repo.index.commit("Initial commit")
+    repo.close()
+
+
+@pytest.fixture
+def diff(commit: "Commit") -> Generator["Diff", None, None]:
+    """Fixture to supply a single-file diff."""
+    (diff,) = commit.diff(NULL_TREE)  # Exactly one file in the diff.
+    yield diff
+
+
+def test_diff_renamed_warns(diff: "Diff") -> None:
+    """The deprecated Diff.renamed property issues a deprecation warning."""
+    with pytest.deprecated_call():
+        diff.renamed
+
+
+def test_diff_renamed_file_does_not_warn(diff: "Diff") -> None:
+    """The preferred Diff.renamed_file property issues no deprecation warning."""
+    with assert_no_deprecation_warning():
+        diff.renamed_file
+
+
+def test_commit_trailers_warns(commit: "Commit") -> None:
+    """The deprecated Commit.trailers property issues a deprecation warning."""
+    with pytest.deprecated_call():
+        commit.trailers
+
+
+def test_commit_trailers_list_does_not_warn(commit: "Commit") -> None:
+    """The nondeprecated Commit.trailers_list property issues no deprecation warning."""
+    with assert_no_deprecation_warning():
+        commit.trailers_list
+
+
+def test_commit_trailers_dict_does_not_warn(commit: "Commit") -> None:
+    """The nondeprecated Commit.trailers_dict property issues no deprecation warning."""
+    with assert_no_deprecation_warning():
+        commit.trailers_dict
+
+
+def test_traverse_list_traverse_in_base_class_warns(commit: "Commit") -> None:
+    """Traversable.list_traverse's base implementation issues a deprecation warning."""
+    with pytest.deprecated_call():
+        Traversable.list_traverse(commit)
+
+
+def test_traversable_list_traverse_override_does_not_warn(commit: "Commit") -> None:
+    """Calling list_traverse on concrete subclasses is not deprecated, does not warn."""
+    with assert_no_deprecation_warning():
+        commit.list_traverse()
+
+
+def test_traverse_traverse_in_base_class_warns(commit: "Commit") -> None:
+    """Traversable.traverse's base implementation issues a deprecation warning."""
+    with pytest.deprecated_call():
+        Traversable.traverse(commit)
+
+
+def test_traverse_traverse_override_does_not_warn(commit: "Commit") -> None:
+    """Calling traverse on concrete subclasses is not deprecated, does not warn."""
+    with assert_no_deprecation_warning():
+        commit.traverse()
+
+
+def test_iterable_inheriting_warns() -> None:
+    """Subclassing the deprecated git.util.Iterable issues a deprecation warning."""
+    with pytest.deprecated_call():
+
+        class Derived(_Iterable):
+            pass
+
+
+def test_iterable_obj_inheriting_does_not_warn() -> None:
+    """Subclassing git.util.IterableObj is not deprecated, does not warn."""
+    with assert_no_deprecation_warning():
+
+        class Derived(IterableObj):
+            pass
diff --git a/test/deprecation/test_cmd_git.py b/test/deprecation/test_cmd_git.py
new file mode 100644
index 00000000..e4449027
--- /dev/null
+++ b/test/deprecation/test_cmd_git.py
@@ -0,0 +1,391 @@
+# This module is part of GitPython and is released under the
+# 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/
+
+"""Tests for dynamic and static characteristics of Git class and instance attributes.
+
+Currently this all relates to the deprecated :attr:`Git.USE_SHELL` class attribute,
+which can also be accessed through instances. Some tests directly verify its behavior,
+including deprecation warnings, while others verify that other aspects of attribute
+access are not inadvertently broken by mechanisms introduced to issue the warnings.
+
+A note on multiprocessing
+=========================
+
+Because USE_SHELL has no instance state, this module does not include tests of pickling
+and multiprocessing:
+
+- Just as with a simple class attribute, when a class attribute with custom logic is set
+  to another value, even before a worker process is created that uses the class, the
+  worker process may see either the initial or new value, depending on the process start
+  method. With "fork", changes are preserved. With "spawn" or "forkserver", re-importing
+  the modules causes initial values to be set. Then the value in the parent at the time
+  it dispatches the task is only set in the children if the parent has the task set it,
+  or if it is set as a side effect of importing needed modules, or of unpickling objects
+  passed to the child (for example, if it is set in a top-level statement of the module
+  that defines the function submitted for the child worker process to call).
+
+- When an attribute gains new logic provided by a property or custom descriptor, and the
+  attribute involves instance-level state, incomplete or corrupted pickling can break
+  multiprocessing. (For example, when an instance attribute is reimplemented using a
+  descriptor that stores data in a global WeakKeyDictionary, pickled instances should be
+  tested to ensure they are still working correctly.) But nothing like that applies
+  here, because instance state is not involved. Although the situation is inherently
+  complex as described above, it is independent of the attribute implementation.
+
+- That USE_SHELL cannot be set on instances, and that when retrieved on instances it
+  always gives the same value as on the class, is covered in the tests here.
+
+A note on metaclass conflicts
+=============================
+
+The most important DeprecationWarning is for code like ``Git.USE_SHELL = True``, which
+is a security risk. But this warning may not be possible to implement without a custom
+metaclass. This is because a descriptor in a class can customize all forms of attribute
+access on its instances, but can only customize getting an attribute on the class.
+Retrieving a descriptor from a class calls its ``__get__`` method (if defined), but
+replacing or deleting it does not call its ``__set__`` or ``__delete__`` methods.
+
+Adding a metaclass is a potentially breaking change. This is because derived classes
+that use an unrelated metaclass, whether directly or by inheriting from a class such as
+abc.ABC that uses one, will raise TypeError when defined. These would have to be
+modified to use a newly introduced metaclass that is a lower bound of both. Subclasses
+remain unbroken in the far more typical case that they use no custom metaclass.
+
+The tests in this module do not establish whether the danger of setting Git.USE_SHELL to
+True is high enough, and applications of deriving from Git and using an unrelated custom
+metaclass marginal enough, to justify introducing a metaclass to issue the warnings.
+"""
+
+import logging
+import sys
+from typing import Generator
+import unittest.mock
+
+if sys.version_info >= (3, 11):
+    from typing import assert_type
+else:
+    from typing_extensions import assert_type
+
+import pytest
+from pytest import WarningsRecorder
+
+from git.cmd import Git, GitMeta
+
+from .lib import assert_no_deprecation_warning, suppress_deprecation_warning
+
+_USE_SHELL_DEPRECATED_FRAGMENT = "Git.USE_SHELL is deprecated"
+"""Text contained in all USE_SHELL deprecation warnings, and starting most of them."""
+
+_USE_SHELL_DANGEROUS_FRAGMENT = "Setting Git.USE_SHELL to True is unsafe and insecure"
+"""Beginning text of USE_SHELL deprecation warnings when USE_SHELL is set True."""
+
+_logger = logging.getLogger(__name__)
+
+
+@pytest.fixture
+def restore_use_shell_state() -> Generator[None, None, None]:
+    """Fixture to attempt to restore state associated with the USE_SHELL attribute.
+
+    This is used to decrease the likelihood of state changes leaking out and affecting
+    other tests. But the goal is not to assert implementation details of USE_SHELL.
+
+    This covers two of the common implementation strategies, for convenience in testing
+    both. USE_SHELL could be implemented in the metaclass:
+
+    * With a separate _USE_SHELL backing attribute. If using a property or other
+      descriptor, this is the natural way to do it, but custom __getattribute__ and
+      __setattr__ logic, if it does more than adding warnings, may also use that.
+    * Like a simple attribute, using USE_SHELL itself, stored as usual in the class
+      dictionary, with custom __getattribute__/__setattr__ logic only to warn.
+
+    This tries to save private state, tries to save the public attribute value, yields
+    to the test case, tries to restore the public attribute value, then tries to restore
+    private state. The idea is that if the getting or setting logic is wrong in the code
+    under test, the state will still most likely be reset successfully.
+    """
+    no_value = object()
+
+    # Try to save the original private state.
+    try:
+        old_private_value = Git._USE_SHELL  # type: ignore[attr-defined]
+    except AttributeError:
+        separate_backing_attribute = False
+        try:
+            old_private_value = type.__getattribute__(Git, "USE_SHELL")
+        except AttributeError:
+            old_private_value = no_value
+            _logger.error("Cannot retrieve old private _USE_SHELL or USE_SHELL value")
+    else:
+        separate_backing_attribute = True
+
+    try:
+        # Try to save the original public value. Rather than attempt to restore a state
+        # where the attribute is not set, if we cannot do this we allow AttributeError
+        # to propagate out of the fixture, erroring the test case before its code runs.
+        with suppress_deprecation_warning():
+            old_public_value = Git.USE_SHELL
+
+        # This doesn't have its own try-finally because pytest catches exceptions raised
+        # during the yield. (The outer try-finally catches exceptions in this fixture.)
+        yield
+
+        # Try to restore the original public value.
+        with suppress_deprecation_warning():
+            Git.USE_SHELL = old_public_value
+    finally:
+        # Try to restore the original private state.
+        if separate_backing_attribute:
+            Git._USE_SHELL = old_private_value  # type: ignore[attr-defined]
+        elif old_private_value is not no_value:
+            type.__setattr__(Git, "USE_SHELL", old_private_value)
+
+
+def test_cannot_access_undefined_on_git_class() -> None:
+    """Accessing a bogus attribute on the Git class remains a dynamic and static error.
+
+    This differs from Git instances, where most attribute names will dynamically
+    synthesize a "bound method" that runs a git subcommand when called.
+    """
+    with pytest.raises(AttributeError):
+        Git.foo  # type: ignore[attr-defined]
+
+
+def test_get_use_shell_on_class_default() -> None:
+    """USE_SHELL can be read as a class attribute, defaulting to False and warning."""
+    with pytest.deprecated_call() as ctx:
+        use_shell = Git.USE_SHELL
+
+    (message,) = [str(entry.message) for entry in ctx]  # Exactly one warning.
+    assert message.startswith(_USE_SHELL_DEPRECATED_FRAGMENT)
+
+    assert_type(use_shell, bool)
+
+    # This comes after the static assertion, just in case it would affect the inference.
+    assert not use_shell
+
+
+def test_get_use_shell_on_instance_default() -> None:
+    """USE_SHELL can be read as an instance attribute, defaulting to False and warning.
+
+    This is the same as test_get_use_shell_on_class_default above, but for instances.
+    The test is repeated, instead of using parametrization, for clearer static analysis.
+    """
+    instance = Git()
+
+    with pytest.deprecated_call() as ctx:
+        use_shell = instance.USE_SHELL
+
+    (message,) = [str(entry.message) for entry in ctx]  # Exactly one warning.
+    assert message.startswith(_USE_SHELL_DEPRECATED_FRAGMENT)
+
+    assert_type(use_shell, bool)
+
+    # This comes after the static assertion, just in case it would affect the inference.
+    assert not use_shell
+
+
+def _assert_use_shell_full_results(
+    set_value: bool,
+    reset_value: bool,
+    setting: WarningsRecorder,
+    checking: WarningsRecorder,
+    resetting: WarningsRecorder,
+    rechecking: WarningsRecorder,
+) -> None:
+    # The attribute should take on the values set to it.
+    assert set_value is True
+    assert reset_value is False
+
+    # Each access should warn exactly once.
+    (set_message,) = [str(entry.message) for entry in setting]
+    (check_message,) = [str(entry.message) for entry in checking]
+    (reset_message,) = [str(entry.message) for entry in resetting]
+    (recheck_message,) = [str(entry.message) for entry in rechecking]
+
+    # Setting it to True should produce the special warning for that.
+    assert _USE_SHELL_DEPRECATED_FRAGMENT in set_message
+    assert set_message.startswith(_USE_SHELL_DANGEROUS_FRAGMENT)
+
+    # All other operations should produce a usual warning.
+    assert check_message.startswith(_USE_SHELL_DEPRECATED_FRAGMENT)
+    assert reset_message.startswith(_USE_SHELL_DEPRECATED_FRAGMENT)
+    assert recheck_message.startswith(_USE_SHELL_DEPRECATED_FRAGMENT)
+
+
+def test_use_shell_set_and_get_on_class(restore_use_shell_state: None) -> None:
+    """USE_SHELL can be set and re-read as a class attribute, always warning."""
+    with pytest.deprecated_call() as setting:
+        Git.USE_SHELL = True
+    with pytest.deprecated_call() as checking:
+        set_value = Git.USE_SHELL
+    with pytest.deprecated_call() as resetting:
+        Git.USE_SHELL = False
+    with pytest.deprecated_call() as rechecking:
+        reset_value = Git.USE_SHELL
+
+    _assert_use_shell_full_results(
+        set_value,
+        reset_value,
+        setting,
+        checking,
+        resetting,
+        rechecking,
+    )
+
+
+def test_use_shell_set_on_class_get_on_instance(restore_use_shell_state: None) -> None:
+    """USE_SHELL can be set on the class and read on an instance, always warning.
+
+    This is like test_use_shell_set_and_get_on_class but it performs reads on an
+    instance. There is some redundancy here in assertions about warnings when the
+    attribute is set, but it is a separate test so that any bugs where a read on the
+    class (or an instance) is needed first before a read on an instance (or the class)
+    are detected.
+    """
+    instance = Git()
+
+    with pytest.deprecated_call() as setting:
+        Git.USE_SHELL = True
+    with pytest.deprecated_call() as checking:
+        set_value = instance.USE_SHELL
+    with pytest.deprecated_call() as resetting:
+        Git.USE_SHELL = False
+    with pytest.deprecated_call() as rechecking:
+        reset_value = instance.USE_SHELL
+
+    _assert_use_shell_full_results(
+        set_value,
+        reset_value,
+        setting,
+        checking,
+        resetting,
+        rechecking,
+    )
+
+
+@pytest.mark.parametrize("value", [False, True])
+def test_use_shell_cannot_set_on_instance(
+    value: bool,
+    restore_use_shell_state: None,  # In case of a bug where it does set USE_SHELL.
+) -> None:
+    instance = Git()
+    with pytest.raises(AttributeError):
+        instance.USE_SHELL = value  # type: ignore[misc]  # Name not in __slots__.
+
+
+@pytest.mark.filterwarnings("ignore::DeprecationWarning")
+@pytest.mark.parametrize("original_value", [False, True])
+def test_use_shell_is_mock_patchable_on_class_as_object_attribute(
+    original_value: bool,
+    restore_use_shell_state: None,
+) -> None:
+    """Asymmetric patching looking up USE_SHELL in ``__dict__`` doesn't corrupt state.
+
+    Code using GitPython may temporarily set Git.USE_SHELL to a different value. Ideally
+    it does not use unittest.mock.patch to do so, because that makes subtle assumptions
+    about the relationship between attributes and dictionaries. If the attribute can be
+    retrieved from the ``__dict__`` rather than directly, that value is assumed the
+    correct one to restore, even by a normal setattr.
+
+    The effect is that some ways of simulating a class attribute with added behavior can
+    cause a descriptor, such as a property, to be set as the value of its own backing
+    attribute during unpatching; then subsequent reads raise RecursionError. This
+    happens if both (a) setting it on the class is customized in a metaclass and (b)
+    getting it on instances is customized with a descriptor (such as a property) in the
+    class itself.
+
+    Although ideally code outside GitPython would not rely on being able to patch
+    Git.USE_SHELL with unittest.mock.patch, the technique is widespread. Thus, USE_SHELL
+    should be implemented in some way compatible with it. This test checks for that.
+    """
+    Git.USE_SHELL = original_value
+    if Git.USE_SHELL is not original_value:
+        raise RuntimeError("Can't set up the test")
+    new_value = not original_value
+
+    with unittest.mock.patch.object(Git, "USE_SHELL", new_value):
+        assert Git.USE_SHELL is new_value
+
+    assert Git.USE_SHELL is original_value
+
+
+def test_execute_without_shell_arg_does_not_warn() -> None:
+    """No deprecation warning is issued from operations implemented using Git.execute().
+
+    When no ``shell`` argument is passed to Git.execute, which is when the value of
+    USE_SHELL is to be used, the way Git.execute itself accesses USE_SHELL does not
+    issue a deprecation warning.
+    """
+    with assert_no_deprecation_warning():
+        Git().version()
+
+
+_EXPECTED_DIR_SUBSET = {
+    "cat_file_all",
+    "cat_file_header",
+    "GIT_PYTHON_TRACE",
+    "USE_SHELL",  # The attribute we get deprecation warnings for.
+    "GIT_PYTHON_GIT_EXECUTABLE",
+    "refresh",
+    "is_cygwin",
+    "polish_url",
+    "check_unsafe_protocols",
+    "check_unsafe_options",
+    "AutoInterrupt",
+    "CatFileContentStream",
+    "__init__",
+    "__getattr__",
+    "set_persistent_git_options",
+    "working_dir",
+    "version_info",
+    "execute",
+    "environment",
+    "update_environment",
+    "custom_environment",
+    "transform_kwarg",
+    "transform_kwargs",
+    "__call__",
+    "_call_process",  # Not currently considered public, but unlikely to change.
+    "get_object_header",
+    "get_object_data",
+    "stream_object_data",
+    "clear_cache",
+}
+"""Some stable attributes dir() should include on the Git class and its instances.
+
+This is intentionally incomplete, but includes substantial variety. Most importantly, it
+includes both ``USE_SHELL`` and a wide sampling of other attributes.
+"""
+
+
+def test_class_dir() -> None:
+    """dir() on the Git class includes its statically known attributes.
+
+    This tests that the mechanism that adds dynamic behavior to USE_SHELL accesses so
+    that all accesses issue warnings does not break dir() for the class, neither for
+    USE_SHELL nor for ordinary (non-deprecated) attributes.
+    """
+    actual = set(dir(Git))
+    assert _EXPECTED_DIR_SUBSET <= actual
+
+
+def test_instance_dir() -> None:
+    """dir() on Git objects includes its statically known attributes.
+
+    This is like test_class_dir, but for Git instances rather than the class itself.
+    """
+    instance = Git()
+    actual = set(dir(instance))
+    assert _EXPECTED_DIR_SUBSET <= actual
+
+
+def test_metaclass_alias() -> None:
+    """GitMeta aliases Git's metaclass, whether that is type or a custom metaclass."""
+
+    def accept_metaclass_instance(cls: GitMeta) -> None:
+        """Check that cls is statically recognizable as an instance of GitMeta."""
+
+    accept_metaclass_instance(Git)  # assert_type would expect Type[Git], not GitMeta.
+
+    # This comes after the static check, just in case it would affect the inference.
+    assert type(Git) is GitMeta
diff --git a/test/deprecation/test_compat.py b/test/deprecation/test_compat.py
new file mode 100644
index 00000000..2d7805e6
--- /dev/null
+++ b/test/deprecation/test_compat.py
@@ -0,0 +1,84 @@
+# This module is part of GitPython and is released under the
+# 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/
+
+"""Tests for dynamic and static characteristics of git.compat module attributes.
+
+These tests verify that the is_<platform> attributes are available, and are even listed
+in the output of dir(), but issue warnings, and that bogus (misspelled or unrecognized)
+attribute access is still an error both at runtime and with mypy. This is similar to
+some of the tests in test_toplevel, but the situation being tested here is simpler
+because it does not involve unintuitive module aliasing or import behavior. So this only
+tests attribute access, not "from" imports (whose behavior can be intuitively inferred).
+"""
+
+import os
+import sys
+
+if sys.version_info >= (3, 11):
+    from typing import assert_type
+else:
+    from typing_extensions import assert_type
+
+import pytest
+
+import git.compat
+
+_MESSAGE_LEADER = "{} and other is_<platform> aliases are deprecated."
+"""Form taken by the beginning of the warnings issued for is_<platform> access."""
+
+
+def test_cannot_access_undefined() -> None:
+    """Accessing a bogus attribute in git.compat remains a dynamic and static error."""
+    with pytest.raises(AttributeError):
+        git.compat.foo  # type: ignore[attr-defined]
+
+
+def test_is_platform() -> None:
+    """The is_<platform> attributes work, warn, and mypy accepts code accessing them."""
+    fully_qualified_names = [
+        "git.compat.is_win",
+        "git.compat.is_posix",
+        "git.compat.is_darwin",
+    ]
+
+    with pytest.deprecated_call() as ctx:
+        is_win = git.compat.is_win
+        is_posix = git.compat.is_posix
+        is_darwin = git.compat.is_darwin
+
+    assert_type(is_win, bool)
+    assert_type(is_posix, bool)
+    assert_type(is_darwin, bool)
+
+    messages = [str(entry.message) for entry in ctx]
+    assert len(messages) == 3
+
+    for fullname, message in zip(fully_qualified_names, messages):
+        assert message.startswith(_MESSAGE_LEADER.format(fullname))
+
+    # These assertions exactly reproduce the expressions in the code under test, so they
+    # are not good for testing that the values are correct. Instead, their purpose is to
+    # ensure that any dynamic machinery put in place in git.compat to cause warnings to
+    # be issued does not get in the way of the intended values being accessed.
+    assert is_win == (os.name == "nt")
+    assert is_posix == (os.name == "posix")
+    assert is_darwin == (sys.platform == "darwin")
+
+
+def test_dir() -> None:
+    """dir() on git.compat includes all public attributes, even if deprecated.
+
+    As dir() usually does, it also has nonpublic attributes, which should also not be
+    removed by a custom __dir__ function, but those are less important to test.
+    """
+    expected_subset = {
+        "is_win",
+        "is_posix",
+        "is_darwin",
+        "defenc",
+        "safe_decode",
+        "safe_encode",
+        "win_encode",
+    }
+    actual = set(dir(git.compat))
+    assert expected_subset <= actual
diff --git a/test/deprecation/test_toplevel.py b/test/deprecation/test_toplevel.py
new file mode 100644
index 00000000..74040819
--- /dev/null
+++ b/test/deprecation/test_toplevel.py
@@ -0,0 +1,233 @@
+# This module is part of GitPython and is released under the
+# 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/
+
+"""Tests for dynamic and static characteristics of top-level git module attributes.
+
+Provided mypy has ``warn_unused_ignores = true`` set, running mypy on these test cases
+checks static typing of the code under test. This is the reason for the many separate
+single-line attr-defined suppressions, so those should not be replaced with a smaller
+number of more broadly scoped suppressions, even where it is feasible to do so.
+
+Running pytest checks dynamic behavior as usual.
+"""
+
+import itertools
+import sys
+from typing import Type
+
+if sys.version_info >= (3, 11):
+    from typing import assert_type
+else:
+    from typing_extensions import assert_type
+
+import pytest
+
+import git
+import git.index.base
+import git.index.fun
+import git.index.typ
+import git.refs.head
+import git.refs.log
+import git.refs.reference
+import git.refs.symbolic
+import git.refs.tag
+
+
+def test_cannot_access_undefined() -> None:
+    """Accessing a bogus attribute in git remains a dynamic and static error."""
+    with pytest.raises(AttributeError):
+        git.foo  # type: ignore[attr-defined]
+
+
+def test_cannot_import_undefined() -> None:
+    """Importing a bogus attribute from git remains a dynamic and static error."""
+    with pytest.raises(ImportError):
+        from git import foo  # type: ignore[attr-defined]  # noqa: F401
+
+
+def test_util_alias_access() -> None:
+    """Accessing util in git works, warns, and mypy verifies it and its attributes."""
+    # The attribute access should succeed.
+    with pytest.deprecated_call() as ctx:
+        util = git.util
+
+    # There should be exactly one warning and it should have our util-specific message.
+    (message,) = [str(entry.message) for entry in ctx]
+    assert "git.util" in message
+    assert "git.index.util" in message
+    assert "should not be relied on" in message
+
+    # We check access through the util alias to the TemporaryFileSwap member, since it
+    # is slightly simpler to validate and reason about than the other public members,
+    # which are functions (specifically, higher-order functions for use as decorators).
+    from git.index.util import TemporaryFileSwap
+
+    assert_type(util.TemporaryFileSwap, Type[TemporaryFileSwap])
+
+    # This comes after the static assertion, just in case it would affect the inference.
+    assert util.TemporaryFileSwap is TemporaryFileSwap
+
+
+def test_util_alias_import() -> None:
+    """Importing util from git works, warns, and mypy verifies it and its attributes."""
+    # The import should succeed.
+    with pytest.deprecated_call() as ctx:
+        from git import util
+
+    # There may be multiple warnings. In CPython there will be currently always be
+    # exactly two, possibly due to the equivalent of calling hasattr to do a pre-check
+    # prior to retrieving the attribute for actual use. However, all warnings should
+    # have the same message, and it should be our util-specific message.
+    (message,) = {str(entry.message) for entry in ctx}
+    assert "git.util" in message, "Has alias."
+    assert "git.index.util" in message, "Has target."
+    assert "should not be relied on" in message, "Distinct from other messages."
+
+    # As above, we check access through the util alias to the TemporaryFileSwap member.
+    from git.index.util import TemporaryFileSwap
+
+    assert_type(util.TemporaryFileSwap, Type[TemporaryFileSwap])
+
+    # This comes after the static assertion, just in case it would affect the inference.
+    assert util.TemporaryFileSwap is TemporaryFileSwap
+
+
+_PRIVATE_MODULE_ALIAS_TARGETS = (
+    git.refs.head,
+    git.refs.log,
+    git.refs.reference,
+    git.refs.symbolic,
+    git.refs.tag,
+    git.index.base,
+    git.index.fun,
+    git.index.typ,
+)
+"""Targets of private aliases in the git module to some modules, not including util."""
+
+
+_PRIVATE_MODULE_ALIAS_TARGET_NAMES = (
+    "git.refs.head",
+    "git.refs.log",
+    "git.refs.reference",
+    "git.refs.symbolic",
+    "git.refs.tag",
+    "git.index.base",
+    "git.index.fun",
+    "git.index.typ",
+)
+"""Expected ``__name__`` attributes of targets of private aliases in the git module."""
+
+
+def test_alias_target_module_names_are_by_location() -> None:
+    """The aliases are weird, but their targets are normal, even in ``__name__``."""
+    actual = [module.__name__ for module in _PRIVATE_MODULE_ALIAS_TARGETS]
+    expected = list(_PRIVATE_MODULE_ALIAS_TARGET_NAMES)
+    assert actual == expected
+
+
+def test_private_module_alias_access() -> None:
+    """Non-util private alias access works but warns and is a deliberate mypy error."""
+    with pytest.deprecated_call() as ctx:
+        assert (
+            git.head,  # type: ignore[attr-defined]
+            git.log,  # type: ignore[attr-defined]
+            git.reference,  # type: ignore[attr-defined]
+            git.symbolic,  # type: ignore[attr-defined]
+            git.tag,  # type: ignore[attr-defined]
+            git.base,  # type: ignore[attr-defined]
+            git.fun,  # type: ignore[attr-defined]
+            git.typ,  # type: ignore[attr-defined]
+        ) == _PRIVATE_MODULE_ALIAS_TARGETS
+
+    # Each should have warned exactly once, and note what to use instead.
+    messages = [str(w.message) for w in ctx]
+
+    assert len(messages) == len(_PRIVATE_MODULE_ALIAS_TARGETS)
+
+    for fullname, message in zip(_PRIVATE_MODULE_ALIAS_TARGET_NAMES, messages):
+        assert message.endswith(f"Use {fullname} instead.")
+
+
+def test_private_module_alias_import() -> None:
+    """Non-util private alias import works but warns and is a deliberate mypy error."""
+    with pytest.deprecated_call() as ctx:
+        from git import head  # type: ignore[attr-defined]
+        from git import log  # type: ignore[attr-defined]
+        from git import reference  # type: ignore[attr-defined]
+        from git import symbolic  # type: ignore[attr-defined]
+        from git import tag  # type: ignore[attr-defined]
+        from git import base  # type: ignore[attr-defined]
+        from git import fun  # type: ignore[attr-defined]
+        from git import typ  # type: ignore[attr-defined]
+
+    assert (
+        head,
+        log,
+        reference,
+        symbolic,
+        tag,
+        base,
+        fun,
+        typ,
+    ) == _PRIVATE_MODULE_ALIAS_TARGETS
+
+    # Each import may warn multiple times. In CPython there will be currently always be
+    # exactly two warnings per import, possibly due to the equivalent of calling hasattr
+    # to do a pre-check prior to retrieving the attribute for actual use. However, for
+    # each import, all messages should be the same and should note what to use instead.
+    messages_with_duplicates = [str(w.message) for w in ctx]
+    messages = [message for message, _ in itertools.groupby(messages_with_duplicates)]
+
+    assert len(messages) == len(_PRIVATE_MODULE_ALIAS_TARGETS)
+
+    for fullname, message in zip(_PRIVATE_MODULE_ALIAS_TARGET_NAMES, messages):
+        assert message.endswith(f"Use {fullname} instead.")
+
+
+def test_dir_contains_public_attributes() -> None:
+    """All public attributes of the git module are present when dir() is called on it.
+
+    This is naturally the case, but some ways of adding dynamic attribute access
+    behavior can change it, especially if __dir__ is defined but care is not taken to
+    preserve the contents that should already be present.
+
+    Note that dir() should usually automatically list non-public attributes if they are
+    actually "physically" present as well, so the approach taken here to test it should
+    not be reproduced if __dir__ is added (instead, a call to globals() could be used,
+    as its keys list the automatic values).
+    """
+    expected_subset = set(git.__all__)
+    actual = set(dir(git))
+    assert expected_subset <= actual
+
+
+def test_dir_does_not_contain_util() -> None:
+    """The util attribute is absent from the dir() of git.
+
+    Because this behavior is less confusing than including it, where its meaning would
+    be assumed by users examining the dir() for what is available.
+    """
+    assert "util" not in dir(git)
+
+
+def test_dir_does_not_contain_private_module_aliases() -> None:
+    """Names from inside index and refs only pretend to be there and are not in dir().
+
+    The reason for omitting these is not that they are private, since private members
+    are usually included in dir() when actually present. Instead, these are only sort
+    of even there, no longer being imported and only being resolved dynamically for the
+    time being. In addition, it would be confusing to list these because doing so would
+    obscure the module structure of GitPython.
+    """
+    expected_absent = {
+        "head",
+        "log",
+        "reference",
+        "symbolic",
+        "tag",
+        "base",
+        "fun",
+        "typ",
+    }
+    actual = set(dir(git))
+    assert not (expected_absent & actual), "They should be completely disjoint."
diff --git a/test/deprecation/test_types.py b/test/deprecation/test_types.py
new file mode 100644
index 00000000..f97375a8
--- /dev/null
+++ b/test/deprecation/test_types.py
@@ -0,0 +1,69 @@
+# This module is part of GitPython and is released under the
+# 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/
+
+"""Tests for dynamic and static characteristics of git.types module attributes."""
+
+import sys
+
+if sys.version_info >= (3, 8):
+    from typing import Literal
+else:
+    from typing_extensions import Literal
+
+import pytest
+
+import git.types
+
+
+def test_cannot_access_undefined() -> None:
+    """Accessing a bogus attribute in git.types remains a dynamic and static error."""
+    with pytest.raises(AttributeError):
+        git.types.foo  # type: ignore[attr-defined]
+
+
+def test_can_access_lit_commit_ish_but_it_is_not_usable() -> None:
+    """Lit_commit_ish_can be accessed, but warns and is an invalid type annotation."""
+    # It would be fine to test attribute access rather than a "from" import. But a
+    # "from" import is more likely to appear in actual usage, so it is used here.
+    with pytest.deprecated_call() as ctx:
+        from git.types import Lit_commit_ish
+
+    # As noted in test_toplevel.test_util_alias_import, there may be multiple warnings,
+    # but all with the same message.
+    (message,) = {str(entry.message) for entry in ctx}
+    assert "Lit_commit_ish is deprecated." in message
+    assert 'Literal["commit", "tag", "blob", "tree"]' in message, "Has old definition."
+    assert 'Literal["commit", "tag"]' in message, "Has new definition."
+    assert "GitObjectTypeString" in message, "Has new type name for old definition."
+
+    _: Lit_commit_ish = "commit"  # type: ignore[valid-type]
+
+    # It should be as documented (even though deliberately unusable in static checks).
+    assert Lit_commit_ish == Literal["commit", "tag"]
+
+
+def test_dir() -> None:
+    """dir() on git.types includes public names, even ``Lit_commit_ish``.
+
+    It also contains private names that we don't test. See test_compat.test_dir.
+    """
+    expected_subset = {
+        "PathLike",
+        "TBD",
+        "AnyGitObject",
+        "Tree_ish",
+        "Commit_ish",
+        "GitObjectTypeString",
+        "Lit_commit_ish",
+        "Lit_config_levels",
+        "ConfigLevels_Tup",
+        "CallableProgress",
+        "assert_never",
+        "Files_TD",
+        "Total_TD",
+        "HSH_TD",
+        "Has_Repo",
+        "Has_id_attribute",
+    }
+    actual = set(dir(git.types))
+    assert expected_subset <= actual
diff --git a/test/performance/lib.py b/test/performance/lib.py
index 2ca3c409..c2459998 100644
--- a/test/performance/lib.py
+++ b/test/performance/lib.py
@@ -1,7 +1,7 @@
 # This module is part of GitPython and is released under the
 # 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/
 
-"""Support library for tests."""
+"""Support library for performance tests."""
 
 import logging
 import os
diff --git a/test/test_git.py b/test/test_git.py
index 112e2f0e..94e68ecf 100644
--- a/test/test_git.py
+++ b/test/test_git.py
@@ -669,13 +669,13 @@ class TestGit(TestBase):
             stack.enter_context(_patch_out_env("GIT_PYTHON_GIT_EXECUTABLE"))
 
             if sys.platform == "win32":
-                # On Windows, use a shell so "git" finds "git.cmd". (In the infrequent
-                # case that this effect is desired in production code, it should not be
-                # done with this technique. USE_SHELL=True is less secure and reliable,
-                # as unintended shell expansions can occur, and is deprecated. Instead,
-                # use a custom command, by setting the GIT_PYTHON_GIT_EXECUTABLE
-                # environment variable to git.cmd or by passing git.cmd's full path to
-                # git.refresh. Or wrap the script with a .exe shim.)
+                # On Windows, use a shell so "git" finds "git.cmd". The correct and safe
+                # ways to do this straightforwardly are to set GIT_PYTHON_GIT_EXECUTABLE
+                # to git.cmd in the environment, or call git.refresh with the command's
+                # full path. See the Git.USE_SHELL docstring for deprecation details.
+                # But this tests a "default" scenario where neither is done. The
+                # approach used here, setting USE_SHELL to True so PATHEXT is honored,
+                # should not be used in production code (nor even in most test cases).
                 stack.enter_context(mock.patch.object(Git, "USE_SHELL", True))
 
             new_git = Git()