-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Stop including 'wheel', setuptools 70.1 has native bdist_wheel support #2868
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
Conversation
bce9ebb
to
d0cb03a
Compare
The two blockers are resolved. Note that this PR now bumps the bundled pip (except for Python 3.8, which I assume we'll remove shortly). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR still failing.
…these tests Previously we relied on pip to build the packages in non-PEP517 mode, which implied no build isolation. The latest `virtualenv` (with pypa/virtualenv#2868) won't include `wheel` in the virtualenv, which will mean that pip uses PEP-517 mode, which is isolated by default.
Ahh, I'm actually adding Edit: nevermind for that too, checking the version looks pretty easy, so I'll just do that. |
April next year. |
May I suggest an alternative approach? What if it was never removed? Keeping a dummy no-action option is not a maintenance burden. Removing its usage from countless places on the other end seems not worth it. |
See pypa/virtualenv#2868. It looks easy to check the virtualenv version, so let's just do that. Signed-off-by: Henry Schreiner <[email protected]>
Hi, I'm writing here because this issue was linked to the change in changelog for "v20.31.0 (2025-05-05)". I have a suspicion this broke the airspeed-velocity library, because there they used the How to fix this? I skimmed through the changelog, but not 100% sure what to do to make the fix backwards compatible:
|
I think you should remove wheel=bundle on Python 3.8+. What is your motivation to use it? The general idea is that it should not be needed with the updated setuptools and pip. (However I relize that it's a distruption, perhaps it should warn but not fail?) |
This is breaking our builds using Poetry 1.8.5, which I don't expect the Poetry team to update for this breaking change. We'll update to Poetry 2.x at some point but I'd rather not be forced to by this change. |
I managed to fix it by omitting the The problematic code is here: https://github.com/airspeed-velocity/asv/blob/01f7cfa3da86f6d98ef68984ecc663ac6ff7dfaa/asv/plugins/virtualenv.py#L136 And here is how I fixed it: util.check_call([
sys.executable,
"-mvirtualenv",
# Omit `--wheel=bundle` from py3.9+
# See https://github.com/airspeed-velocity/asv/issues/1484
*(["--wheel=bundle"] if sys.version_info < (3, 9) else []),
"--setuptools=bundle",
"-p",
self._executable,
self._path], env=env) As for why that command is called with those arguments, I don't know. I'm maintainer at django-components and went to report / complain at airspeed-velocity, which brought me here 😄 |
I think this is breaking any Poetry project when at least one dependency needs a wheel to be built from source: see python-poetry/poetry#10378 (comment) |
I think a solution for the poetry problem (and similar) might be:
|
It's indeed a breaking change for any project (poetry and others) having the |
I was looking into this, but fundamentally at: virtualenv/src/virtualenv/seed/embed/base_embed.py Lines 23 to 34 in e9b6ba7
We would have all the options, and I am not entirely sure how to check |
Yeah, we should probably make it a no op with a warning on newer Python interpreters. cc @stefanor |
This is my draft PR (which doesn't work, as I am stuck). #2884 |
This also risks upsetting users. But, I suppose the What got us into this pickle was |
Note that there have been no major bumps since the rewrite. I guess because behavioural changes have been keyed to Python versions. |
Well, the other option was to keep the last known good version of wheel. (The new one was yanked BTW, so we might just as well do that for now.) |
self.app_data = options.app_data | ||
self.periodic_update = not options.no_periodic_update | ||
|
||
if options.no_wheel: | ||
warn( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stefanor I would also like to point out that as an user, I did not see this warning when I tested the option.
Not an option in the distribution contexts, where we don't include the bundled wheels, but rather wheels built from our own wheel source package. That's what brought me here. |
Oh, also, the current setuptools is not compatible with the old wheel version from before the packaging de-vendor, IIRC. Even worse, it's all tied up in a single enormous refactoring commit :( (in wheel) |
What's the conclusion here? Can I just wait for my poetry 1.x pipelines to suddenly work again or do I need to take action? /edit: nvm, fixed by upgrading to poetry 2 - removing (?) a CLI arg should've been a breaking change |
https://github.com/pypa/virtualenv/releases/tag/20.31.2 thanks @hroncok for the fix 🤟 |
setuptools has native
bdist_wheel
support these days, so I don't think it's necessary to bundle thewheel
library any more. Or am I missing something?Possibly we could include a noop
--no-wheel
argument (but I'm not sure where that would be best defined).Thanks for contributing, make sure you address all the checklists (for details on how see development documentation)
tox -e fix
)docs/changelog
folder