Express conditional setuptools requirement statically#2043
Merged
EliahKagan merged 2 commits intogitpython-developers:mainfrom Jun 7, 2025
Merged
Express conditional setuptools requirement statically#2043EliahKagan merged 2 commits intogitpython-developers:mainfrom
setuptools requirement statically#2043EliahKagan merged 2 commits intogitpython-developers:mainfrom
Conversation
The `VirtualEnvironment` test helper is used in multiple places, but it is only told to install/upgrade `pip` when used from `test_installation`. It implements the functionality it would ideally obtain by `upgrade_deps`, since the `upgrade_deps` parameter is only avaiilable in `venv` when using Python 3.9 and later. When `pip` is installed, `upgrade_deps` would install `setuptools` when using Python 3.11 or lower, but not when using Python 3.12 or higher. `VirtualEnvironment` does the same. (The reason for this is not just to avoid needlessly departing from what `upgrade_deps` would do. Rather, it should not generally be necessary to have `setuptools` installed for package management since Python 3.12, and if it were necessary then this would a bug we would want to detect while running tests.) Previously this conditional specification of `setuptools` was done by building different lists of package arguments to pass to `pip`, by checking `sys.version_info` to decide whether to append the string `setuptools`. This commit changes how it is done, to use a static list of package arguments instead. (The Python intepreter path continues to be obtained dynamically, but all its positional arguments, including those specifying packages, are now string literals.) The conditional `setuptools` requirement is now expressed statically using notation recognized by `pip`, as the string `setuptools; python_version<"3.12"`.
`setuptools` may potentially be needed for installation to work fully as desired prior to Python 3.12, so in those versions it is installed automatically in any virtual environment that is created with `pip` available. This is a behavior of the `venv` module that is not specific to CI. However, on CI we upgrade packages that are preinstalled in the virtual environment, or that we may otherwise wish to be present. This took the form of unconditionally installing/upgrading the `pip` and `wheel` packages, but conditionally upgrading the `setuptools` package only if we find that it is already installed. This commit changes the behavior to statically specify the same list of package specifications to `pip` in all environments and in all versions of Python, but to use the static notation recognized by `pip` to indicate that `setuptools` is to be instaled/upgraded only if the Python version is strictly less than Python 3.12. This seems to be more readable. It also avoids using unquoted shell parameter expansion in a way that is potentially confusing (for example, if we were running our CI script steps through ShellCheck, then it would automatically balk at that construction). It is also more consistent with how `test_installation` sets up its environment (especially since 31e1c03, but actually even before that, because it was still conditioning `setuptools` on the Python version rather than whether it was already installed). Finally, this behavior is what the preexisting comment already described. This also adjusts the shell quoting style slightly in other related commands (in the same workflows) that pass package specifications to `pip`, for consistency. (For now, `".[test]"` rather than `.[test]` remains written in the readme because it works in `cmd.exe` as well as other shells, but it may be changed there in the future too.)
setuptools staticallysetuptools requirement statically
EliahKagan
added a commit
to EliahKagan/GitPython
that referenced
this pull request
Jun 14, 2025
- In the Cygwin CI workflow, move `runs-on` below `strategy`, for greater consistency with other workflows. - In the Cygwin CI jobs, use `pwsh` rather than `bash` for the `git config` command run outside of Cygwin, since `pwsh` is the default shell for such commands, it's the shell the Cygwin setup action uses, and it avoids creating the wrong impression that `bash` is needed. - Use "virtual environment" instead of "virtualenv" in some step names to avoid possible confusion with the `virtualenv` pacakge. - Remove comments in the PyPA package upgrade steps, which are more self-documenting since 727f4e9 (gitpython-developers#2043).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In
pipcommands that conditionally install/upgrade or disregardsetuptools, this uses the static but version-discriminating specificationsetuptools; python_version<"3.12", rather than conditionally including or omitting asetuptoolsargument as was done before.This covers the two scenarios where this applies, in separate commits:
VirtualEnvironmenttest helper class (where this functionality is relevant in howtest_installationusesVirtualEnvironment).