Improve Python version and OS compatibility, fixing deprecations #1654
Conversation
Starting in Python 3.12, global and virtual Python environments no longer automatically ship setuptools (per the "ensurepip" item in https://docs.python.org/3.12/whatsnew/3.12.html#removed). Projects that use setuptools as a build backend are still supported, including with setup.py using techniques such as "pip install .". In Windows, the "bin" subdir of a virtual environment dir is called "Scripts" instead. Unlike in a global environment (where no names are universal, and "python3" and "pip3" are more common for the Python 3 commands on some popular Unix-like systems), in a virtual environment the "python" and "pip" commands are always present and "python3" and "pip3" are not guaranteed to be present. This commit changes test_installation accordingly. The CI workflows and documentation still need to be updated.
This changes the installation instructions in README.md to recommend "pip install ." instead of "python setup.py install". The former is compatible with Python 3.12 which doesn't have setuptools installed by default (so setup.py, which imports it, can be indirectly but not directly used). This also matches the corresponding change made in the installation unit test. While doing so, I've also clarified the instructions, and added the implied "cd" command as well as the "git fetch --tags" command in the position where a later section was recently updated to mention it should have been run. Using "pip install ." creates the opportunity to pass "-e" to make an editable install, which users who clone the repository to work on changes should do, because the effect of an editable install is only partially simulated by pytest, and so that manual testing of changes actually uses the changes intended for testing. This increases the length and detail of the instructions, so I've added h4 subsections to clarify the separations between them and make it easier for readers to find the part they're looking for. In doing so, I've reordered these subsections accordingly. Because greater detail can create the impression that all important steps are mentioned, I've made the general good advice to use a virtual environment explicit. For brevity, I have not added venv commands.
This adds a #! line to the top of setup.py, because it is a script with the executable bit set. Although neither recent nor current documentation in the project recommends to run "./setup.py", this should probably have the intuitive effect of attempting to run the script with a Python interpreter rather than a Unix-style shell. It also uses the "env trick" in init-tests-after-clone.sh so that script runs with whatever bash interpreter is found in a normal PATH search. While "sh" is expected to be found in /bin on all Unix-like systems, that is not always the case for "bash". This change slightly improves compatibility by supporting systems that don't ship with bash but on which it has been installed.
- Remove "gitdb" from test-requirements.txt, because it already a dependency of the project (listed in requirements.txt, which is used to build the value passed for install_requires in setup.py). - Remove "black" from requirements-dev.txt, because it is listed in test-requirement.txt (which requirements-dev.txt sources).
Because tests_require is deprecated since setuptools 41.5.0 with the intention that it will be removed in some future version (noted in https://setuptools.pypa.io/en/latest/references/keywords.html). It is somewhat unintuitive for GitPython to have a "test" extra, as it makes it so "GitPython[test]" can be specified for installation from PyPI to get test dependencies, even though the PyPI package doesn't include the unit test themselves. However, this makes the statement in README.md that the installer takes care of both requirements.txt and test-requirements.txt dependencies fully true, instead of moving further away from that. Because it is now possible to make an editable GitPython install with test as well as minimal dependencies installed, this commit also updates the readme to document and recommend this.
Instead of directly running setup.py. This allows Python 3.12 (as well as previous versions) to be used for building. Although setuptools could be added as a development dependency to run setup.py, using "build" instead is recommended in https://setuptools.pypa.io/en/latest/userguide/quickstart.html. Those docs likewise recommend only listing "wheel" in the build-system section of pyproject.toml if setup.py actually imports the wheel module. So this removes that. (Running "make release", which now uses "build", will continue to build wheels.) The "build" package is not conceptually a testing dependency, but test-requirements.txt is currently the de facto list of all stable development dependencies for regular use.
This is cleanup related to the previous commit. As that file grows, it is harder to tell immediately if a particular package is in it when not alphabetized. (The groups were also not intuitive, with ddt listed separately from other unit test related dependencies.)
This removes the step in test_installation that did the equivalent of "pip install -r requirements.txt", because installing GitPython is sufficient to install all its required dependencies, and it is more important to test that than to test requirements.txt directly. Removing this causes the test to fail if installing the project doesn't entail installation of the requirements necessary to import the git module or to cause gitdb to be found in a sys.path search.
Key changes: - Update the two CI workflows to install the project and its dependencies in accordance with the changed recommendations in README.md. (This is to test that those recommendations work, which the changed test_installation test case partially but not completely tests. The old approach to installation still works too, so this change on CI is not required to keep CI working.) - Add Python 3.12 to the CI test matrix in pythonpackage.yml, testing it on Ubuntu. (The Cygwin workflow still tests only 3.9.) Maintenance changes, made to avoid decreasing readability with the other changes (and hopefully even increase it somewhat): - Separate commands into more steps, grouping them by more specific purposes. - Decrease the ways the two workflows differ from each other that do not represent actual intended behavioral differences. This is to make the important differences easier to stop, and to make it easier to determine when the same change has or has not been made to both workflows.
The omission of "set -x" was intentional and is currently necessary on Cygwin (but not on Ubuntu), per aafb92a.
EliahKagan
changed the title
EliahKagan
changed the title
Setting the "fetch-depth" to 0 does a deep (i.e., ordinary) fetch, fetching all commits and tags. Setting "submodules" to "recursive" clones and checks out all submodules. These options allow commands that were doing those things to be removed from the later steps.
This also adds "--noprofile --norc" to the Cygwin shell command as a speed optimization (bash doesn't need to source its scripts). That only changes the Cygwin workflow; in the Ubuntu workflow, "--noprofile --norc" had already been included by default when no shell was specified, so having it there is to *keep* the optimized behavior that was already in use.
There was a problem hiding this comment.
That's great work, thank again.
I find your ability to explain changes downright super-human, along with keeping track of their internal and external dependencies, combining it all in various orthogonal issues along with a PR to fixing all of them, explaining even why this is chosen over alternatives. This is certainly appreciated, as it forces me to follow your train of thought while catching up with python. There is still a part of me that simply wants to give you write access to main and say: You know what's best, go ahead :).
With that said, I would definitely appreciated if native Windows testing could, one day, be added to CI. The time budget of about 20 minutes seems quite feasible, a bound that is currently set by cygwin, and I would be surprised if native windows manages to be worse.
For me it's most important that I can still make releases, and I think I will try to perform one from this branch and if it works, merge it right away.
|
It looks like the script currently expects Maybe I am holding it wrong. Should I follow the installation instructions to get a local dev environment, and then try again from there? I might add that Thanks for your help. |
|
Note: Edited to fix a badly unclear part. I don't think we need to add Although following the instructions to get a virtual environment and running In virtual environments (and Docker, CI, etc.) You would still need If you're on a system that has a global |
There the system python interpreter is referred to as `python3`, at least when installed by homebrew.
|
Thanks for elaborating. I have changed the In order to run Maybe a venv installation is best to solve both problems. Could you add a commit that adjusts the Thank you 😅 |
If a virtual environment (created by venv or virtualenv) is active, running "make release" or "make force_release" now automatically installs/upgrades the "build" and "twine" packages in it. This is only done if "make" is run in a virtual environment. This can be a fresh environment: neither the project nor its dependencies need to be installed in it. Because the "build" module is not currently used in any tests and running "make" in a virtual environment takes care of installing "build" (and "twine"), "build" is now removed from test-requirements.txt. The publishing instructions in the readme are updated accordingly, to mention the optional step of creating and activating a virtual environment, and to briefly clarify why one might want to do that. Running "make" outside a virtual environment remains supported, except that, due to recent changes, whatever environment it is run in needs to have a usable "build" module.
Good idea. I have now done this in 5343aa0. There are various approaches that could be taken for this. The approach I picked was:
I have tested this (as best as I can) on Ubuntu with Python 3.11, in virtual environments created with I have kept the change of The installation instructions and release instructions link to two different pages on virtual environments. This is intentional, but I'd be pleased to change if it you prefer. Because |
|
Thank you so much! The trick really was to run I just created a new release which, in hindsight, I was unnecessary as it doesn't contain user-facing changes. Regarding |
I'm not sure if this should've had a release or not. Starting in 3.1.36, it is possible to run When I saw d99b2d4, I thought to ask if it should be a post release instead. But I realized that, due to the new extra, that should probably not be done (since I think it is best for post releases to carry zero installed difference, even in situations that are unusual or not recommended). On the other hand, it might've been best to avoid it, because it's not necessary to upgrade to it, and because the resulting downstream 3.1.36 in conda-forge really is identical to the corresponding downstream 3.1.35. I don't know what the best practice is in this situation.
On GNU/Linux systems including my Ubuntu system, If |
|
Replying to #1654 (review):
Thanks! :) :)
It's an honor to hear that such a thing has even crossed your mind! However, I recommend against doing any such thing, at least at this time. The reason is that I actually have only minimal knowledge of GitPython. I use GitPython indirectly--because, as a very popular library, it is a dependency of other projects I use directly, especially nbdime. As a result, if I did have write access, it would be the current situation, just with extra steps: I would not be comfortable making any unreviewed changes. That's the origin of my interest in GitPython. I learned of CVE-2023-40590 (#1635) from security alerts on a couple of my repositories that declare
Yes, this is something I am very much interested in doing. (Though I certainly do not want to stop anybody else who wants to work on it!) I'm interested in figuring out (a) which, of tests that do not pass for me on Windows, should pass because they fail due to a problem specific to my environment or fork, and, (b) of others that are not already skipped on Windows, if the bugs that break them on native Windows I'm also hoping to find some way to make it significantly faster than the Cygwin test job (currently) is, even if not nearly as fast as the Ubuntu test jobs. If the tests can be made to run fast enough on native Windows, then a matrix strategy could be used as in the Ubuntu test workflow to test on all Python versions GitPython supports (possibly even as part of that workflow, by adding an OS dimension to the existing matrix, depending on how much Window-specific preparation ends up having to be done). Because each job gets its own runner and the runners operate in parallel, six 20-minute checks are no problem for one push, but could slow things down a lot with many pushes in a short time, due to limits on the number of GitHub Actions runners per user or organization. If worse comes to worse, there is always the middle ground of testing a smaller number of versions on native Windows but still more than one. Perhaps 3.8 and 3.12. |
macOS Ventura doesn't sort according to SemVer: ❯ sort --version
2.3-Apple (154)
❯ git tag | sort -nr
3.1.9
3.1.8
3.1.7
3.1.6
3.1.5
3.1.4
3.1.36
3.1.35
3.1.34
3.1.33
3.1.32
3.1.31
3.1.30
3.1.3
3.1.29
3.1.28
3.1.27
3.1.26
3.1.25
3.1.24
3.1.23
3.1.22
3.1.20
3.1.2
3.1.19
3.1.18
3.1.17
3.1.16
3.1.15
3.1.14
3.1.13
3.1.12
3.1.11
3.1.10
3.1.1
3.1.0
3.0.9
3.0.8
3.0.7
3.0.6
3.0.5
3.0.4
3.0.3
3.0.2
3.0.1
3.0.0
2.1.9
2.1.8
2.1.7
2.1.6
2.1.5
2.1.4
2.1.3
2.1.2
2.1.15
2.1.14
2.1.13
2.1.12
2.1.11
2.1.10
2.1.1
2.1.0
2.0.9
2.0.8
2.0.7
2.0.6
2.0.5
2.0.4
2.0.3
2.0.2
2.0.1
2.0.0
1.0.2
1.0.1
1.0.0
0.3.7
0.3.6
0.3.5
0.3.4
0.3.3
0.3.2.1-patched
0.3.2.1
0.3.2-RC1
0.3.2
0.3.1-beta2
0.3.1-beta1
0.3.0-beta2
0.3.0-beta1
0.2.0-beta1
0.1.7
0.1.6
0.1.5
0.1.4-pre
0.1.4
winerr_show |
|
Thanks for sharing your "origin story" - I start liking CVEs much more now, given they made you aware of this project and caused the first contribution :).
If I recall correctly, most test-failures on windows arise from its 'unique' way of handling open files, which is quite incompatible with the way files are handled on Unix systems. That said, by now things might have changed and it's probably worth to at least retry the ignored tests to see if some of them work now. I also think that having any kind of native windows CI job is orthogonal to making more tests pass - the latter is entirely optional to me and definitely nothing more than 'nice to have' at this point. CI on native python on Windows would be a great step forward already.
These days there isn't too much CI activity here, but if it was a problem I am willing to cancel some jobs by hand. It's something I do quite regularly in |
|
@hugovk Thanks--I find that very helpful, because it tells me Based on that, I looked for alternatives to git -c versionsort.suffix=-alpha -c versionsort.suffix=-beta -c versionsort.suffix=-pre -c versionsort.suffix=-rc -c versionsort.suffix=-RC tag --sort=-v:refnameIf git's git $(printf ' -c versionsort.suffix=-%s' alpha beta pre rc RC) tag --sort=-v:refnameThen, to also filter out non-version tags (they're at the top now, so must be dropped before git $(printf ' -c versionsort.suffix=-%s' alpha beta pre rc RC) tag -l '[0-9]*' --sort=-v:refname(Where the |
Thanks! That might make sense if/when I have learned more about the codebase (though I would still likely request reviews on many changes).
That makes sense. If it's okay for native Windows CI to be set up even with tests failing that are not currently skipped or documented to fail on Windows, then I may be able to do it sooner. I think there's still some stuff I need to figure out, though. I think some of the tests that fail on my Windows system are failing only because not everything is set up properly. Furthermore, none of the changes I've made that needed to be tested on Windows required me to set up Since I believe Windows GitHub Actions runners include full Git for Windows installations, and Git for Windows does ship |
This is absolutely amazing! I know
Yes, absolutely. I think it's better to split the task into CI Setup and Tests themselves. That way, Tests can simply be ignored when found failing at first until the is time to see if they could also work. Additionally, I think it's perfectly alright to only test the borders of the supported version range of Python, and for that test different Windows versions. I know that GHA supports quite some parallelism, but I'd prefer to conserve the compute anyway and optimize for actual coverage. Limiting oneself somewhat when setting up additional jobs seems particularly easy here since this GitPython seems to have been fine without any of it for more than a decade now. |
[](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [GitPython](https://togithub.com/gitpython-developers/GitPython) | `==3.1.36` -> `==3.1.37` | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>gitpython-developers/GitPython (GitPython)</summary> ### [`v3.1.37`](https://togithub.com/gitpython-developers/GitPython/releases/tag/3.1.37): - a proper fix CVE-2023-41040 [Compare Source](https://togithub.com/gitpython-developers/GitPython/compare/3.1.36...3.1.37) #### What's Changed - Improve Python version and OS compatibility, fixing deprecations by [@​EliahKagan](https://togithub.com/EliahKagan) in [https://github.com/gitpython-developers/GitPython/pull/1654](https://togithub.com/gitpython-developers/GitPython/pull/1654) - Better document env_case test/fixture and cwd by [@​EliahKagan](https://togithub.com/EliahKagan) in [https://github.com/gitpython-developers/GitPython/pull/1657](https://togithub.com/gitpython-developers/GitPython/pull/1657) - Remove spurious executable permissions by [@​EliahKagan](https://togithub.com/EliahKagan) in [https://github.com/gitpython-developers/GitPython/pull/1658](https://togithub.com/gitpython-developers/GitPython/pull/1658) - Fix up checks in Makefile and make them portable by [@​EliahKagan](https://togithub.com/EliahKagan) in [https://github.com/gitpython-developers/GitPython/pull/1661](https://togithub.com/gitpython-developers/GitPython/pull/1661) - Fix URLs that were redirecting to another license by [@​EliahKagan](https://togithub.com/EliahKagan) in [https://github.com/gitpython-developers/GitPython/pull/1662](https://togithub.com/gitpython-developers/GitPython/pull/1662) - Assorted small fixes/improvements to root dir docs by [@​EliahKagan](https://togithub.com/EliahKagan) in [https://github.com/gitpython-developers/GitPython/pull/1663](https://togithub.com/gitpython-developers/GitPython/pull/1663) - Use venv instead of virtualenv in test_installation by [@​EliahKagan](https://togithub.com/EliahKagan) in [https://github.com/gitpython-developers/GitPython/pull/1664](https://togithub.com/gitpython-developers/GitPython/pull/1664) - Omit py_modules in setup by [@​EliahKagan](https://togithub.com/EliahKagan) in [https://github.com/gitpython-developers/GitPython/pull/1665](https://togithub.com/gitpython-developers/GitPython/pull/1665) - Don't track code coverage temporary files by [@​EliahKagan](https://togithub.com/EliahKagan) in [https://github.com/gitpython-developers/GitPython/pull/1666](https://togithub.com/gitpython-developers/GitPython/pull/1666) - Configure tox by [@​EliahKagan](https://togithub.com/EliahKagan) in [https://github.com/gitpython-developers/GitPython/pull/1667](https://togithub.com/gitpython-developers/GitPython/pull/1667) - Format tests with black and auto-exclude untracked paths by [@​EliahKagan](https://togithub.com/EliahKagan) in [https://github.com/gitpython-developers/GitPython/pull/1668](https://togithub.com/gitpython-developers/GitPython/pull/1668) - Upgrade and broaden flake8, fixing style problems and bugs by [@​EliahKagan](https://togithub.com/EliahKagan) in [https://github.com/gitpython-developers/GitPython/pull/1673](https://togithub.com/gitpython-developers/GitPython/pull/1673) - Fix rollback bug in SymbolicReference.set_reference by [@​EliahKagan](https://togithub.com/EliahKagan) in [https://github.com/gitpython-developers/GitPython/pull/1675](https://togithub.com/gitpython-developers/GitPython/pull/1675) - Remove `@NoEffect` annotations by [@​EliahKagan](https://togithub.com/EliahKagan) in [https://github.com/gitpython-developers/GitPython/pull/1677](https://togithub.com/gitpython-developers/GitPython/pull/1677) - Add more checks for the validity of refnames by [@​facutuesca](https://togithub.com/facutuesca) in [https://github.com/gitpython-developers/GitPython/pull/1672](https://togithub.com/gitpython-developers/GitPython/pull/1672) **Full Changelog**: gitpython-developers/GitPython@3.1.36...3.1.37 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/allenporter/flux-local). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi45Ny4xIiwidXBkYXRlZEluVmVyIjoiMzYuOTcuMSIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
](https://renovatebot.com){kind=link}
Bump gitpython from 3.1.35 to 3.1.37 Bumps gitpython from 3.1.35 to 3.1.37. Release notes Sourced from gitpython's releases. 3.1.37 - a proper fix CVE-2023-41040 What's Changed Improve Python version and OS compatibility, fixing deprecations by @EliahKagan in gitpython-developers/GitPython#1654 Better document env_case test/fixture and cwd by @EliahKagan in gitpython-developers/GitPython#1657 Remove spurious executable permissions by @EliahKagan in gitpython-developers/GitPython#1658 Fix up checks in Makefile and make them portable by @EliahKagan in gitpython-developers/GitPython#1661 Fix URLs that were redirecting to another license by @EliahKagan in gitpython-developers/GitPython#1662 Assorted small fixes/improvements to root dir docs by @EliahKagan in gitpython-developers/GitPython#1663 Use venv instead of virtualenv in test_installation by @EliahKagan in gitpython-developers/GitPython#1664 Omit py_modules in setup by @EliahKagan in gitpython-developers/GitPython#1665 Don't track code coverage temporary files by @EliahKagan in gitpython-developers/GitPython#1666 Configure tox by @EliahKagan in gitpython-developers/GitPython#1667 Format tests with black and auto-exclude untracked paths by @EliahKagan in gitpython-developers/GitPython#1668 Upgrade and broaden flake8, fixing style problems and bugs by @EliahKagan in gitpython-developers/GitPython#1673 Fix rollback bug in SymbolicReference.set_reference by @EliahKagan in gitpython-developers/GitPython#1675 Remove @NoEffect annotations by @EliahKagan in gitpython-developers/GitPython#1677 Add more checks for the validity of refnames by @facutuesca in gitpython-developers/GitPython#1672 Full Changelog: gitpython-developers/GitPython@3.1.36...3.1.37 Commits b27a89f fix makefile to compare commit hashes only 0bd2890 prepare next release 832b6ee remove unnecessary list comprehension to fix CI e98f57b Merge pull request #1672 from trail-of-forks/robust-refname-checks 1774f1e Merge pull request #1677 from EliahKagan/no-noeffect a4701a0 Remove @NoEffect annotations d40320b Merge pull request #1675 from EliahKagan/rollback d1c1f31 Merge pull request #1673 from EliahKagan/flake8 e480985 Tweak rollback logic in log.to_file ff84b26 Refactor try-finally cleanup in git/ Additional commits viewable in compare view Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase. Dependabot commands and options You can trigger Dependabot actions by commenting on this PR: @dependabot rebase will rebase this PR @dependabot recreate will recreate this PR, overwriting any edits that have been made to it @dependabot merge will merge this PR after your CI passes on it @dependabot squash and merge will squash and merge this PR after your CI passes on it @dependabot cancel merge will cancel a previously requested merge and block automerging @dependabot reopen will reopen this PR if it is closed @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually @dependabot show <dependency name> ignore conditions will show all of the ignore conditions of the specified dependency @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) You can disable automated security fix PRs for this repo from the Security Alerts page. Reviewed-by: Vladimir Vshivkov
8 of the tests that fail on native Windows systems fail due to IndexFile.from_tree being broken on Windows, causing gitpython-developers#1630. This commit marks those tests as xfail. This is part, though not all, of the changes to get CI test jobs for native Windows that are passing, to guard against new regressions and to allow the code and tests to be gradually fixed (see discussion in gitpython-developers#1654). When fixing the bug, this commit can be reverted.
8 of the tests that fail on native Windows systems fail due to IndexFile.from_tree being broken on Windows, causing gitpython-developers#1630. This commit marks those tests as xfail. This is part, though not all, of the changes to get CI test jobs for native Windows that are passing, to guard against new regressions and to allow the code and tests to be gradually fixed (see discussion in gitpython-developers#1654). When fixing the bug, this commit can be reverted.
8 of the tests that fail on native Windows systems fail due to IndexFile.from_tree being broken on Windows, causing gitpython-developers#1630. This commit marks those tests as xfail. This is part, though not all, of the changes to get CI test jobs for native Windows that are passing, to guard against new regressions and to allow the code and tests to be gradually fixed (see discussion in gitpython-developers#1654). When fixing the bug, this commit can be reverted.
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 deprecatedsetuptoolsfeatures 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 modifiesREADME.mdincluding to change the recommend way to install GitPython from a downloaded source code archive or cloned repository; edits scripts related to installation and building, includingsetup.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.pywithpip 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
ghcommand 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 runningpython setup.py installdoes 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-pythonto 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.mdinstructions, 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_installationto 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.pychanges deserves special scrutiny in reviewThe main change in this PR that I anticipate might not be wanted is the addition of a
testextra. 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
testextra 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
testextra 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.