Skip to content
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

fix: do not pre-seed setuptools / wheel in virtual environment #1819

Merged
merged 1 commit into from
May 11, 2024

Conversation

mayeut
Copy link
Member

@mayeut mayeut commented May 10, 2024

Since 9879937, setuptools & wheel are not pinned/installed anymore.
We should not pre-seed the virtual environment with those either.

Since pypa@9879937, setuptools & wheel are not pinned/installed anymore.
We should not pre-seed the virtual environment with those either.
@henryiii
Copy link
Contributor

This is only in draft due to needing #1818 (merged)?

@mayeut mayeut marked this pull request as ready for review May 11, 2024 05:38
@mayeut
Copy link
Member Author

mayeut commented May 11, 2024

this is only in draft due to needing #1818 (merged)?

yes

@henryiii henryiii merged commit cf18014 into pypa:main May 11, 2024
19 checks passed
@mayeut mayeut deleted the no-setuptools-injection branch May 11, 2024 21:30
@EwoutH
Copy link
Contributor

EwoutH commented Aug 11, 2024

Any chance this error has to do with this chance? It appeared when updating from cibuildwheel 2.17.0 to 2.20.0, and this PR was included in 2.18.0.

If so, what would be the best way to resolve it?

@mayeut
Copy link
Member Author

mayeut commented Aug 11, 2024

yes, it seems to be related.
you need to install your dependencies. for wheel cli, this can most likely run with pipx.

@EwoutH
Copy link
Contributor

EwoutH commented Aug 11, 2024

Thanks for getting back so quickly! I think we fixed it by simply installing it explicitly before build: scipy/scipy@f20a0b4.

To confirm, it was intended that we now need to do this explicitly? (since that could be considered a breaking change, which would go against semantic versioning in a minor release)

@mayeut
Copy link
Member Author

mayeut commented Aug 11, 2024

This PR is a fix. What you were using was not intended to work after 9879937. 2.17.0 probably should have been called 3.0.0 but it's too late for that.

@EwoutH
Copy link
Contributor

EwoutH commented Aug 11, 2024

image

No worries, it's always difficult how to handle fixes to unintended and undocumented behavior. Thanks for getting back and explaining the situation :)

@henryiii
Copy link
Contributor

Cibuildwheel does not follow SemVer (many other Python libraries do not, including Python itself). That’s why we don’t allow you to pin to a major version in GHA, but only to a minor version. We do try very hard not to break anyone in patch versions. Most projects will not break in minor versions, but we add and remove python versions, which definitely can break your workflow. If you don’t support Python 3.13, 2.20 is a breaking release. If you need Python 3.6, I’m sure we will eventually drop it. Etc.

I would consider this basically a user bug; if you needed wheel, you should’ve been making sure it is installed. Modern setuptools doesn’t even need wheel anymore. It looks like you were doing exactly the right thing, though, pinning to a minor version and then updating regularly!

PS: in general, please do not rely on setuptools or wheel being present! They should be treated like any other dependency. They need to be removed from the default environments; in some places they were removed in 3.12 (venv and virtualenv), and some other places in 3.13 (manylinux), but it is happening! It speeds up environment creation if they aren’t needed, and gives the user more version freedom if they are.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants