Don't exclude setuptools, distribute & wheel from freeze output on Python 3.12+ #12032
Conversation
SpecLad
force-pushed
the
freeze-setuptools
branch
from
fe72585 to
6e263eb
Compare
| """Test pip shows itself""" | ||
| """Test that pip shows itself only when --all is used""" | ||
| result = script.pip("freeze") | ||
| assert "pip==" not in result.stdout |
There was a problem hiding this comment.
This also doesn't seem directly related to this PR. It's a sensible addition, but why is it included here?
There was a problem hiding this comment.
Just making sure that pip is still hidden by default.
There was a problem hiding this comment.
A comment mentioning pip should hide itself from freeze would be useful
There was a problem hiding this comment.
Doesn't the test docstring say exactly that?
tests/functional/test_freeze.py
Outdated
| """ | ||
|
|
||
| result = script.pip("freeze") | ||
| if sys.version_info >= (3, 12): |
There was a problem hiding this comment.
Can we do this test by mocking the value of sys.version_info, rather than having the test do different things depending on what Python version it's run under? That may be overkill, and may be difficult to achieve because the test runs pip in a subprocess, so I'm not suggesting that this is essential, but it's worth thinking about (especially as CI currently won't check this change at all).
There was a problem hiding this comment.
Seems like overkill, frankly. 3.12 will be added to CI sooner or later anyway.
There was a problem hiding this comment.
+1 to mocking, doing conditional here makes running tests locally unnecessarily difficult.
There was a problem hiding this comment.
I gave it more thought, and mocking sys.version_info really does not seem like a good idea. Imagine that pip (or one of its dependencies) does something like this in the code:
if sys.version_info >= (3, 12):
# use feature that is only available in 3.12+Then if you set sys.version_info to 3.12.x while running on, say, 3.8.x, then this code will break.
There was a problem hiding this comment.
Or two tests with different skip_if decorators?
There was a problem hiding this comment.
Instead of mocking sys.version_info directly, I’d add a function that toggles freeze output, and mock that.
There was a problem hiding this comment.
Instead of mocking
sys.version_infodirectly, I’d add a function that toggles freeze output, and mock that.
Okay, I did it.
SpecLad
force-pushed
the
freeze-setuptools
branch
from
6e263eb to
28217cc
Compare
SpecLad
force-pushed
the
freeze-setuptools
branch
from
28217cc to
7b6a81b
Compare
…thon 3.12+ Due to the advent of build isolation, it is no longer necessary to install setuptools and wheel in an environment just to install other packages. Moreover, on Python 3.12 both ensurepip [1] and virtualenv [2] are to stop installing setuptools & wheel by default. This means that when those packages are present in a Python 3.12+ environment, it is reasonable to assume that they are runtime dependencies of the user's project, and therefore should be included in freeze output. distribute is just obsolete. [1] python/cpython#95299 [2] pypa/virtualenv#2558
This makes it possible to test both branches on any Python version.
SpecLad
force-pushed
the
freeze-setuptools
branch
from
9610d5b to
393ccfb
Compare
Co-authored-by: Tzu-ping Chung <[email protected]>
| " {}".format(", ".join(DEV_PKGS)) | ||
| " {}".format(", ".join(_dev_pkgs())) |
There was a problem hiding this comment.
Just a minor note, it’d be a good idea to add some sorting here so the order package appear is more stable. But honestly nobody cares that much about these help messages anyway so it’s fine to keep things as-is.
|
@pradyunsg will this be released in |
|
It's been merged, so it will be in 23.2. |
Due to the advent of build isolation, it is no longer necessary to install setuptools and wheel in an environment just to install other packages. Moreover, on Python 3.12 both ensurepip (python/cpython#95299) and virtualenv (pypa/virtualenv#2558) are to stop installing setuptools & wheel by default. This means that when those packages are present in a Python 3.12+ environment, it is reasonable to assume that they are runtime dependencies of the user's project, and therefore should be included in freeze output.
distribute is just obsolete.
Closes #4256.