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

feature: allow testing cp38-macosx_arm64 wheel #1283

Merged
merged 2 commits into from
Oct 4, 2022

Conversation

mayeut
Copy link
Member

@mayeut mayeut commented Sep 25, 2022

Per discussion in #1169, the default installer used for cp38 is an Intel installer. It makes sense to skip testing arm64 wheels in this case.

However, if the user choose to manually install the universal2 CPython version (c.f. #1278 (comment)), then, tests shall be run on arm64.

This allows users that either target 11.0+ on intel, universal2 or only build for arm64 to get the arm64 wheel tested on AppleSilicon.

I'm not a fan of the way I added the test here but there's a test. Maybe the test suite should be updated to run fully with both installers ?

mayeut and others added 2 commits September 25, 2022 12:34
Per discussion in pypa#1169, the default installer used for cp38 is an Intel installer.
It makes sense to skip testing arm64 wheels in this case.

However, if the user choose to manually install the universal2 CPython version, then, tests shall be run on arm64.

This allows users that either target 11.0+ on intel, universal2 or only build for arm64 to get the arm64 wheel tested on AppleSilicon.
@henryiii
Copy link
Contributor

I've got an idea here. I'll try to get to it in a day or so.

@joerick
Copy link
Contributor

joerick commented Sep 26, 2022

Thanks @mayeut. This provides a workaround for users with compiled dependencies that need native arm64 builds. I'm curious what @henryiii has in mind also...

I'm not a fan of the way I added the test here but there's a test. Maybe the test suite should be updated to run fully with both installers ?

Yeah, maybe. I'll save a full review until I've seen the approach henryiii has in mind. We could also limit the cp38-arm64 test job to test_0_basic, test_testing, test_macos_archs to save some CI time.

Copy link
Contributor

@henryiii henryiii left a comment

Choose a reason for hiding this comment

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

Playing with it a bit, I think we'll need to discuss some more, and I don't think this gets in the way & it might help.

Copy link
Contributor

@joerick joerick left a comment

Choose a reason for hiding this comment

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

Agreed. This is a good step, but maybe we'll do more here too.

@joerick
Copy link
Contributor

joerick commented Oct 4, 2022

Thank you @mayeut!

@joerick joerick merged commit c854501 into pypa:main Oct 4, 2022
@mayeut mayeut deleted the test-macos-cp38-arm64 branch October 5, 2022 07:01
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