Fail `test_installation` on warnings, and remove deprecated license classifier by EliahKagan · Pull Request #2054 · gitpython-developers/GitPython

added 7 commits

June 15, 2025 19:04
So that the meaning of the `venv.sources` accesses in
`test_installation` is more readily clear.
This creates a function (technically, a callable `partial` object)
for `test_installation` to use instead of repeating `subproces.run`
keyword arguments all the time.

This relates directly to steps in `_set_up_venv`, and it's makes
about as much sense to do it there as in `test_installation`, so it
is placed (and described) in `_set_up_venv`.
By setting the `PYTHONWARNINGS` environment variable to `error` in
each of the subprocess invocations.

This is strictly stronger than passing `-Werror` for the `python`
commands, because it automatically applies to subprocesses (unless
they are created with a sanitized environment or otherwise with one
in which `PYTHONWARNINGS` has been customized), and because it
works for `pip` automatically.

Importantly, this will cause warnings internal to Python
subprocesses created by `pip` to be treated as errors. It should
thus surface any warnings coming from the `setuptools` backend.
Previously, it attempted to show stderr unless empty, first falling
back to stdout unless empty, then falling back to the prewritten
summary identifying the specific assertion.

This now has the `test_installation` assertions capture stderr as
well as stdout, handle standard streams as text rather than binary,
and show more information when failing, always distinguishing where
the information came from: the summary, then labeled captured
stdout (empty or not), then labeled captured stderr (empty or not).

That applies to all but the last assertion, which does not try to
show information differently when it fails, but is simplified to do
the right thing now that `subprocess.run` is using text streams.
(This subtly changes its semantics, but overall it should be as
effective as before at finding the `sys.path` woe it anticipates.)
GitPython project metadata specify the project's license primarily
through the value of `license`, currently passed as an argument to
`setuptools.setup`, which holds the string `"BSD-3-Clause"`. This
is an SPDX license identifier readily understood by both humans and
machines.

The PyPI trove classifier "License :: OSI Approved :: BSD License"
has also been specified in `setup.py`. However, this is not ideal,
because:

1. It does not identify a specific license. There are multiple
   "BSD" licenses in use, with BSD-2-Clause and BSD-3-Clause both
   in very wide use.

2. It is no longer recommended to use a trove classifier to
   indicate a license. The use of license classifiers (even
   unambiguous ones) has been deprecated. See:

   - https://packaging.python.org/en/latest/specifications/core-metadata/#classifier-multiple-use
   - https://peps.python.org/pep-0639/#deprecate-license-classifiers

This commit removes the classifier. The license itself is of course
unchanged, as is the `license` value of `"BSD-3-Clause"`.

(An expected effect of this change is that, starting in the next
release of GitPython, PyPI may show "License: BSD-3-Clause" instead
of the current text "License: BSD License (BSD-3-Clause)".)

This change fixes a warning issued by a subprocess of `pip` when
installing the package. The warning, until this change, could be
observed by running `pip install . -v` or `pip install -e . -v` and
examining the verbose output, or by running `pip install .` or
`pip install -e .` with the `PYTHONWARNINGS` environment variable
set to `error`:

    SetuptoolsDeprecationWarning: License classifiers are deprecated.
      !!

              ********************************************************************************
              Please consider removing the following classifiers in favor of a SPDX license expression:

              License :: OSI Approved :: BSD License

              See https://packaging.python.org/en/latest/guides/writing-pyproject-toml/#license for details.
              ********************************************************************************

      !!

(In preceding commits, `test_installation` has been augmented to
set that environment variable, surfacing the error. This change
should allow that test to pass, unless it finds other problems.)
It was originally made explicit like this, but it ended up becoming
position in my last few commits. This restores that clearer aspect
of how it was written before, while keeping all the other changes.