-
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
feat: Add support for ubuntu-24.04-arm GHA runner #2135
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.
Assuming the above goes green (✅), this looks good to me!
examples/github-with-qemu.yml
Outdated
# configure cibuildwheel to build native archs ('auto'), and some | ||
# emulated ones |
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.
# configure cibuildwheel to build native archs ('auto'), and some | |
# emulated ones | |
# configure cibuildwheel on Linux to build native archs ('auto'), | |
# and to split the remaining architectures between the x86_64 and | |
# ARM runners |
I don't love this way of doing ternary expressions, but it's all we have in GHA workflows, so I think adding a comment is nice.
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 included your proposal and extended a bit on the auto armv7l
peculiarity. That might be a bit too verbose but that's strange enough to deserve a also comment (it's "native" in some circumstances hence not in auto
).
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 change looks good to me. Question for another PR, but it makes me wonder if we could detect armv7l support and include that in auto
on machines where that's possible.
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.
if we could detect armv7l support and include that in auto on machines where that's possible.
That might be possible:
- don't allow on platform other that Linux
- try to run a small statically linked Linux exe & check if it works.
Not sure this is a feature. Current cibuildwheel works fine with native ARM, already using it in several projects. :) |
Adding it to the example config upgrades it to a ✨ feature in my book :) - similar to if there was a CI platform that happened to work, as soon as we add it to the test matrix and document that it's supported by cibuildwheel, I think it would count as a feature. But yeah, there were no code changes required - previous versions of cibuildwheel work on native ARM perfectly :) |
@@ -59,7 +59,7 @@ Usage | |||
|
|||
| | Linux | macOS | Windows | Linux ARM | macOS ARM | Windows ARM | | |||
|-----------------|-------|-------|---------|-----------|-----------|-------------| | |||
| GitHub Actions | ✅ | ✅ | ✅ | ✅¹ | ✅ | ✅² | | |||
| GitHub Actions | ✅ | ✅ | ✅ | ✅ | ✅ | ✅² | |
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.
Should the actual ¹ text be dropped below as well?
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.
That footnote still applies to Gitlab, who don't yet offer ARM runners.
Run all tests on GitHub Actions Ubuntu 24.04 aarch64 runner.
The emulation tests are kept for now.
We could also run armv7l tests on those same runners but it's not part of
auto
arch because:1: not all aarch64 systems can run aarch32 binaries (we need EL0 aarch32 support - mac Mx SoC or Cavium ThunderX do not provide this for exemple).
2: part of the armv7l images are still experimental
If we want to run armv7l, we can probably amend the tests on GHA to request both aarch64 & armv7l architectures.
Fix #2131