Improve Python version and OS compatibility, fixing deprecations by EliahKagan · Pull Request #1654 · gitpython-developers/GitPython (original) (raw)

Fixes #1640
Fixes #1651
Fixes #1652
Fixes #1653

Overview

This changes installation instructions, test code, scripts including setup.py, and CI workflows to support Python 3.12, to fix a test that always failed on native Windows systems, and to avoid using deprecated setuptools features or recommending their use. I've attempted to fix problems and replace the use of deprecated features in a way that increases rather than decreasing robustness, clarity, and ease of installation. Besides the practical overlap (see below), the conceptual thread that holds all these changes together is that they are about improving compatibility with current and future Python installations.

The actual GitPython library code itself seems already compatible with 3.12, and this PR does not change anything in git/. It modifies README.md including to change the recommend way to install GitPython from a downloaded source code archive or cloned repository; edits scripts related to installation and building, including setup.py; and modifies a test to fix some OS and Python version compatibilities, to make it test the changed installation procedure in the readme, and to make it slightly more robust; and modifies the two CI workflows that run tests.

It seemed to me that the four issues mentioned above, although they are distinct issues with none being a duplicate of any others, inherently overlap and are best fixed in a way that involves overlapping code changes as well as overlapping considerations for reviewing the changes. So although I am a bit concerned about the scope of this pull request, I've done these changes together in one PR. However, they are separated across a number of narrowly scoped commits, with most of the commit messages detailing the change the commit makes and its purpose.

Documentation changes

In parts of the readme that had to be changed to replace python setup.py with pip install, I applied other updates and clarifications too. I avoided doing this in any other parts of the readme. I reorganized the installation instructions for clarity, subdividing them into separate <h4> sections so that the distinctions between different installation approaches is readily apparent and readers can immediately find the part of the instructions they are looking for.

When making those changes, I included the tag-fetching step that was recently added in one place but not another where it is relevant, explicitly mentioned that the instructions should usually be carried out in a virtual environment, and addressed forks so users less familiar with common GitHub workflows would not be misled into thinking they should clone the upstream repository to propose changes. (I was a bit uncomfortable doing the latter as part of this already broad PR, but it actually relates to the tag-fetching step: if the gh command is used for cloning, tags on the upstream repo are available even if the fork does not have them, allowing local tests to pass.)

I did not also update the documentation in doc/. Although this should be done, that documentation is already considerably out of date in other ways including with respect to installation and dependencies, and this PR is already fairly large in scope. Note that the old approach of running python setup.py install does still work in all the cases where it worked before. So this does not break the old documentation, it just doesn't bring it up to date.

CI changes

This adds 3.12, which is currently at RC2, to be tested on CI, permitting setup-python to install prereleases for 3.12 but not for other versions. That only affects the Ubuntu workflow.

It also updates both CI test workflows to test the new installation procedure and to harmonize them with the updated README.md instructions, and this attempts to make them clearer both in terms of the workflow files' own readability and in terms of the output generated in the GitHub Actions web-based interface.

This does not add any new Windows tests. While it would be valuable to have native (non-Cygwin) Windows tests on CI, and it would be valuable (if it does not cause CI checks to take too long) to have Cygwin and non-Cygwin Windows tests on multiple Python versions (as well as to test on macOS), this PR does not propose any such changes. This would be nontrivial, at least in the case of native Windows tests, and in my opinion significantly beyond the scope of this PR. I also did not want to wait to fix #1640, since Python 3.12 is already at RC2, with stable 3.12.0 coming out in a month.

Because there are still no non-Cygwin Windows tests on CI, the CI tests results shown for this pull request do not demonstrate that it resolves #1651. However, I have shown the failure in that issue, and I have verified that the changes here fix it, causing test_installation to pass. I tested on Windows 10 with Python 3.11.5, and also on the same system with Python 3.12.0rc2 to verify that it works for the combination of 3.12 and Windows.

One of the setup.py changes deserves special scrutiny in review

The main change in this PR that I anticipate might not be wanted is the addition of a test extra. This approach seemed best to me, but only by a very narrow margin, and I am not at all sure that I am right. The reason I did this, as well as why some other approach might be preferred, are detailed in #1652.

My rationale hinges on the assumption that it is a goal for there to be a way to install the package for local development that also takes care of installing test dependencies. Although test dependencies are not installed unless the test extra is called for, it may nonetheless be surprising for an extra to exist that provides dependencies that none of the code in the PyPI package uses. (A possible counterargument is that running the tests is a way of using the code under test, and the code under test is part of the PyPI package.)

If the test extra is not wanted, I would be pleased to remove it and to update the readme and CI workflows accordingly. This could still resolve #1652, because I could appropriately weaken the claim it makes about automatic dependency installation.