-
Notifications
You must be signed in to change notification settings - Fork 247
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
Add support for Cirrus CI #1191
Conversation
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.
Exciting! I'd love to have native macos-arm64 CI, and another native linux-aarch64 is no bad thing either!
I've tried to add the service to this repo, but it seems that it requires a pypa admin. @pradyunsg would you mind adding the free tier of CirrusCI to this repo? https://github.com/marketplace/cirrus-ci
CIBW_TEST_COMMAND
isn't ran under Rosetta when using pipes
#1193
Is this ready to review/go? Might be nice to get it in for the upcoming release? |
Sadly as I mentioned in the Discord server, my account on Cirrus CI got suspended, most likely by an automated system, so I can't really do anything until their support responds. I emailed them about this 2 days ago, but no response yet. It's possible that if Pradyun added the Cirrus CI app to the PyPA org, it would be possible to see the build output but it's also equally if not more likely that it just wouldn't run the build when it sees me as the one triggering it. |
@pradyunsg has added cirrus ci to this repo, so let's try again... perhaps try pushing an empty commit and see if cirrus picks it up? |
My account has actually been unsuspended earlier today after I re-asked on their GitHub rather than their support email so their CI should just work now for me again. I'm going to merge Note that Linux arm64 build still doesn't work due to cirruslabs/cirrus-ci-docs#1034, I think it might make sense to leave that work for a future PR. |
pypa/wheel - issue 387 and PR 390
Co-authored-by: Joe Rickerby <joerick@mac.com>
Co-authored-by: Joe Rickerby <joerick@mac.com>
Alright, all review comments should be addressed and the tests pass. I dropped Linux arm64 support - I don't think it's worth blocking over this and once this PR is merged, a new issue should be created for that with explanation what is a potential blocker for it. I updated the PR description appropriately and included links to successful builds for example files. This PR should now be ready for review. |
Linux arm64 might also be solved by cirruslabs/cirrus-ci-docs#1027, whichever comes first. It makes to leave that work for a future PR. |
I was hoping to re-trigger CircleCI check due to https://status.circleci.com/incidents/3d31fnpthj7y but looks like that didn't work... Gonna try an empty commit. |
Well, that definitely made the failing check disappear, I guess that's solved then 🤔 |
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.
This all looks good to me!
I went ahead and made #1240 to track Linux arm64 support on Cirrus CI. |
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.
I guess the intel Mac example is separate from the rest of the example because the arm64 one cross-compiles Intel wheels? At least for now, I'd guess the ideal workflow for most projects on Cirrus would be to have native wheels & run both runners - cross compiling is harder than native compiling. Eventually, as Intel runners become hard to find, then this workflow would be better.
Just a comment, looks great!
The cirrus-ci-minimal example file does not do any cross-compilation since the default arch on Apple Silicon is only arm64 (which was done in cibuildwheel 2.9.0).
Cirrus CI does not have an Intel runner. And in case of Apple Silicon, it's not really cross-compilation process per se since it uses Rosetta2 emulation and so it doesn't involve a typical cross-compilation toolchain. |
|
||
<sup>¹ [Requires emulation](https://cibuildwheel.readthedocs.io/en/stable/faq/#emulation), distributed separately. Other services may also support Linux ARM through emulation or third-party build hosts, but these are not tested in our CI.</sup><br> | ||
<sup>² [Uses cross-compilation](https://cibuildwheel.readthedocs.io/en/stable/faq/#universal2). It is not possible to test `arm64` and the `arm64` part of a `universal2` wheel on this CI platform.</sup><br> | ||
<sup>³ [Uses cross-compilation](https://cibuildwheel.readthedocs.io/en/stable/faq/#universal2). Thanks to Rosetta 2 emulation, it is possible to test `x86_64` and both parts of a `universal2` wheel on this CI platform.</sup><br> |
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.
As I said in my previous comment, I'm not sure cross-compilation is the right term here, could anyone confirm whether "emulation" makes more sense here?
<sup>³ [Uses cross-compilation](https://cibuildwheel.readthedocs.io/en/stable/faq/#universal2). Thanks to Rosetta 2 emulation, it is possible to test `x86_64` and both parts of a `universal2` wheel on this CI platform.</sup><br> | |
<sup>³ [Uses Rosetta 2 emulation](https://cibuildwheel.readthedocs.io/en/stable/faq/#universal2). Thanks to the emulation, it is possible to test `x86_64` and both parts of a `universal2` wheel on this CI platform.</sup><br> |
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.
Well, technically it's both! Cross-compilation is right, the wheel is built using cross-compilation, but we can still test the wheel using Rosetta 2 emulation.
I think the existing copy is fine as-is.
Good news, the linked issue got resolved and I've already made a configuration that so far appears to just work, I'm waiting for the tests on my fork to finish before pushing it here. |
All tests ran in Cirrus CI succeeded, I've also updated the PR description with the new link to a build from It seems that one of the GitHub workflows failed due to an intermittent pip/PyPI issue but would have worked otherwise. I can't re-run it myself. |
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.
Great! The Cirrus aarch64 builder is is 1.5x faster than the Travis one, and 5x faster than emulation on GHA!
Co-authored-by: Joe Rickerby <joerick@mac.com>
PR merged. 🎉 Stellar work on this @jack1142! Thank you. |
Resolves #145 and resolves #1240
For macOS arm64, there was a need for some changes to the test suite:
cp36-*
) and on macOS arm64, you can only use CPython 3.8 and above (4bc6bd3
,b39dc5b
)wheel
0.36.1 and earlier, generated wheel filenames had a wrong minimum macOS version (3c246c7
)cp38-macosx_arm64
wheels without testing them which caused it to not delete the wheel when the test expected it to (2b35f3c
)Link to a Cirrus CI test suite build: https://cirrus-ci.com/build/6463822541094912
Link to a Cirrus CI minimal example build: https://cirrus-ci.com/build/6073962235953152
Link to a Cirrus CI macOS Intel example build: https://cirrus-ci.com/build/6736767930859520