-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Stop injecting wheel
as a build dep fallback
#12449
Conversation
PEP 517 doesn't mandate depending on `wheel` when a `__legacy__` setuptools fallback is used. Historically, it used to be assumed as necessary, but later it turned out to be wrong. The reason is that `setuptools`' `get_requires_for_build_wheel()` hook already injects this dependency when building wheels is requested [[1]]. It also used to have this hint in the docs, but it was corrected earlier [[2]]. It could be argued that this is an optimization as `pip` will request building wheels anyway. However, it also shows up in the docs, giving the readers a wrong impression of what to put into `[build-system].requires` when they start a new project using setuptools. This patch removes `wheel` from said `requires` list fallback in the docs and the actual runtime. [1]: https://github.com/pypa/setuptools/blob/v40.8.0/setuptools/build_meta.py#L130 [2]: pypa/setuptools#3056
b6dae52
to
3769ad7
Compare
Motivation/backstory: I was reading https://gregoryszorc.com/blog/2023/10/30/my-user-experience-porting-off-setup.py/ (it was shared @ pypa/packaging.python.org#1468) and it got me puzzled — I thought I'd gotten rid of all the prominent places newbies may copy-and-paste the suggestion to depend on |
Similar PR in build: pypa/build#716. |
This is also still recommended in PyPA's sample project: https://github.com/pypa/sampleproject/blob/main/pyproject.toml#L153 |
That should be raised separately in that repo. |
Thanks for this, @webknjaz! |
Thanks, @pfmoore! Is there any chance that could be related to this patch? It doesn't seem to be the cause but is still worth checking... |
It should be easy to verify by restarting the CI (or at least one job) of the previous commit — https://github.com/pypa/pip/commit/e88d39ae49ab11a8b80609a018e5a36ea4ccad89/checks. |
I don't believe it was this run, but I've hit restart just to check. |
FYI CI runs are failing because of changes in setuptools which had a significant impact on Pip tests: #12458 |
@notatallshaw thanks for confirming! I had a hunch that maybe the test env isn't pinned well enough, but was not fully sure. |
Thanks @notatallshaw - I had a suspicion it was that. @webknjaz I'm not sure we should be pinning setuptools in the test env. The occasional failure, while annoying, is overall probably less so than having to update our pins at the sort of rate setuptools changes (and in general, I think we do want to know we work with the latest setuptools). |
Understandably so. OTOH, there's a middle ground — use pins for PRs and merges but add unpinned tests for nightly/cron runs. |
PEP 517 doesn't mandate depending on
wheel
when a__legacy__
setuptools fallback is used. Historically, it used to be assumed as necessary, but later it turned out to be wrong. The reason is thatsetuptools
'get_requires_for_build_wheel()
hook already injects this dependency when building wheels is requested [1]. It also used to have this hint in the docs, but it was corrected earlier [2].It could be argued that this is an optimization as
pip
will request building wheels anyway. However, it also shows up in the docs, giving the readers a wrong impression of what to put into[build-system].requires
when they start a new project using setuptools.This patch removes
wheel
from saidrequires
list fallback in the docs and the actual runtime.