René's URL Explorer Experiment

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

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

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

Description: The license trove classifier was imprecise and deprecated The main way we indicate the license in the project's metadata is: GitPython/setup.py Line 72 in 46c439b license="BSD-3-Clause", However, we have for some time, until this PR, also had a trove classifier indicating "BSD License": GitPython/setup.py Line 98 in 46c439b "License :: OSI Approved :: BSD License", This has a couple of problems. One is that there is no specific "BSD License", but instead various BSD licenses, of which two are very popular: BSD-3-Clause, which this project uses, and BSD-2-Clause, which is not what this project uses. (No PyPI-recognized trove classifiers for those specific BSD licenses exist.) The other problem is that all usage of License :: ... trove classifiers has been deprecated. When the package is installed, the build backend generates a warning. The warning is typically not shown when using pip, but it will be shown if -v is passed, and it will be shown and cause an error if PYTHONWARNINGS=error is set in the environment. (Running python -Werror -m pip … is not sufficient to produce it, because it is not passed down to the subprocess running setuptools, which is where the warning occurs.) 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. ******************************************************************************** !! The "BSD-3-Clause" value of the license argument we pass to setuptools.setup is already such an SDPX license expression. So all that is needed to fix both these problems is to remove the deprecated trove classifier. However, there are other related problems, some of which make sense to fix at the same time. The installation test didn't detect warnings, nor report errors clearly test_installation seeks to detect problems installing GitPython, and with #2053 it is back running and passing on all platforms we test. But it has not treated warnings during installation as errors. Especially considering the small number of dependencies GitPython has (when not installing the test or doc extra), we shouldn't ordinarily have any warnings on any supported platform and Python version, and anytime we do, we probably want to know about it. I think it would not be reasonable to set PYTHONWARNINGS=error in the top-level pip install '.[test]' commands in the CI test workflows, but that this is a good thing to do in test_installation. Setting it in test_installation does reveal that an error occurs, but without making other changes to the test, it does not reveal in a readily understandable way what error occurs. test_installation contains this code (and some other code like it): GitPython/test/test_installation.py Lines 16 to 25 in 46c439b result = subprocess.run( [venv.pip, "install", "."], stdout=subprocess.PIPE, cwd=venv.sources, ) self.assertEqual( 0, result.returncode, msg=result.stderr or result.stdout or "Can't install project", ) The intent of result.stderr or result.stdout or "Can't install project" is to show what, if anything, was written to stderr; otherwise show what, if anything, was written to stdout; and otherwise show the prewritten message. It's true that the most important information to understand an error is often on stderr. The problem is that it will never show what was written to stderr, because the call to subprocess.run captures only stdout, then attempts to access stderr as though it had been captured too. Here's a simplified demonstration of that problem: >>> from subprocess import run, PIPE >>> result = run(["sh", "-c", "echo 'this is stdout'; echo 'this is stderr' >&2"], stdout=PIPE) this is stderr >>> result.stdout b'this is stdout\n' >>> result.stderr >>> Depending on how the test runner (usually pytest, which is the only runner the whole test suite supports) is run, the stderr message may be captured by the test runner and not shown, captured by the test runner and shown later where it may not be readily identified, or not captured by the test runner and possibly seen at some point either at the intuitive position or interleaved elsewhere. The fixes This PR: Refactors test_installation and its helpers to decrease code duplication and to clarify what the steps that set up the new virtual environment are doing, fixes the test bug where stderr was not captured, interacts with the python and pip subprocesses assuming text-based streams and includes in the assertion message the prewritten summary as well as stdout and stderr (labeling each), and runs the subprocesses with PYTHONWARNINGS=error. This surfaces the deprecation warning and causes it to fail test_installation with fully detailed output reporting the problem. Removes the deprecated trove classifier from setup.py. This is sufficient to make the test pass, as there are no other diagnostic messages of warning or higher level. Outscoped Improve license metadata by Hugo van Kemenade, though not at all specific to GitPython, covers both the problem with some license classifiers (including "BSD License") being unclear and the deprecation, pointing out that an SPDX string should be used to identify in metadata what license a project has chosen. However, it recommends that one also specify the paths to one's license files using license-files. This is already something we were not doing, and removing the deprecated classifier does not worsen that situation. Our LICENSE file also does not have customized elements besides a listed author, who is also one of the authors listed in metadata about authorship. Most importantly, this is just about metadata: the LICENSE file is part of the project and listed in MANIFEST.in (and various files in the project also carry notices). Nonetheless, it would be good to do something like this. I don't know if we can straightforwardly achieve the effect of license-files in setup.py. That blog post covers static project definitions in pyproject.toml. In addition, that meaning in pyproject.toml of license and license-files is only recognized in sufficiently new versions of build backends. Usually this isn't a problem, but some of these changes to the way licenses are expected to be indicated are actually fairly recent. GitPython supports some old versions of Python that are older than supported by the oldest version of setuptools that recognizes pyproject.toml with a license value that is an SPDX string (even aside from supporting license-files). The packaging guide shows the current status for this (PEP 639). One implication here is that it looks like we will have to choose between: Not using pyproject.toml to specify license as an SPDX identifier (and I think maybe not switching to pyproject.toml at all, and maybe not being able to specify license-files or equivalent at all). Dropping Python 3.7 and 3.8 support. This is not unreasonable and will definitely happen eventually--they are end-of-life, after all--but I'd rather the reason we stop supporting them relate either to something that matters at runtime, broader maintainability considerations, or major benefits of unconditionally using newer features. GitPython has a lot of users, and while hopefully just about everyone is using a supported Python, I'd be interested in moving slowly on dropping old versions, at least as far as 3.8 is concerned. Using pyproject.toml with an old, less useful license value, which will generate deprecation warnings, and which will eventually--sometime in 2026, I believe--stop the project from being able to be installed and/or published properly. (This deprecation is a more serious deprecation than the one fixed here with classifiers.) Switching from the setuptools build backend to another build backend that continues to support older Python versions and also supports modern license and license-files, such as flit-core or (if we drop 3.7 by then) hatchling. Although the original plan was to stick with setuptools per #1716 (comment), some time has passed since then and I suspect there may end up being other reasons to switch build backends. I think it may be best to decide all this at the same time, and when actually making the project definition declarative. Therefore, I have not attempted to add license-files or equivalent metadata in this PR.

Open Graph Description: The license trove classifier was imprecise and deprecated The main way we indicate the license in the project's metadata is: GitPython/setup.py Line 72 i...

X Description: The license trove classifier was imprecise and deprecated The main way we indicate the license in the project's metadata is: GitPython/setup.py Line 72 ...

Opengraph URL: https://github.com/gitpython-developers/GitPython/pull/2054

X: @github

direct link

Domain: github.com

Links:

Viewport: width=device-width