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

chore: drop deprecated options related to CIBW_ENABLE #2095

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

mayeut
Copy link
Member

@mayeut mayeut commented Nov 23, 2024

Drop deprecated options related to CIBW_ENABLE.

based on #1912

Comment on lines -160 to -165
parser.add_argument(
"--prerelease-pythons",
action="store_true",
help="Enable pre-release Python versions if available.",
)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be keeping the command line option but turn it in an enable in options.py ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we instead support --enable=<opt> --enable=<opt>? We also now have --only, which you can use to build a pre-release wheel, so maybe we don't need a CLI option for this anymore? (It also is supported statically now)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only a few projects are using this in their CI and those could probably move to the static definition (despite our recommendation not to deliver wheels in pre-ABI stability, for now at least).

I liked --prerelease-pythons better than --only because it allows to check musl/glibc, gil/no-gil and multiple architectures in just one pass.

For UNIX users debugging locally, CIBW_ENABLE=cpython-prerelease cibuildwheel ... works quite easily so probably not worth adding an --enable flag. It might be a bit more painful on Windows if you don't want to leave the environment variable after execution (don't know if there's a one liner there) but we can still probably live without a new option as well.

So, maybe just remove the option for now ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be okay with that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be in favour of a --enable= command line option, because the ergonomics make sense to me - you could do one cibuildwheel execution for the PyPI artifacts, then another with experimental stuff enabled, that might go to a different place.

Copy link
Contributor

@joerick joerick Jan 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed a commit to add the enable cmdline option.

@henryiii henryiii added the Hold for future release This PR might be complete, but is scheduled to be merged in a future release. Don't merge yet. label Nov 24, 2024
@mayeut mayeut force-pushed the enable-legacy-removal branch from b24ba79 to 4097d72 Compare November 24, 2024 22:37
@mayeut mayeut force-pushed the enable-legacy-removal branch from 4097d72 to 6d9ce82 Compare November 25, 2024 07:28
@joerick joerick removed the Hold for future release This PR might be complete, but is scheduled to be merged in a future release. Don't merge yet. label Jan 5, 2025
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