Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Support qemu in GitHub Actions #482
Support qemu in GitHub Actions #482
Changes from 3 commits
76fe472
90a14b9
ec99c01
0d29bf8
26b7b72
0c9af03
f73b608
7c86223
5aa85e8
0ca294e
91800e4
27f1a4e
8a1a0a4
b34bdee
eb03a11
3696f15
68dc286
3983fb2
460eaa7
f6ce1c9
6b00931
0893731
b5f5437
5ee663f
f434072
d65da2e
5b5078e
aa87128
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
The user can select which (additional linux) platforms they want to support with the above action.
In other CIs, they can simply execute
docker run --privileged --rm tonistiigi/binfmt --install all
(or any other platform selection), as perbinfmt
docsThis section, and more, can be added to the README for explanation.
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.
We aren't running a test that uses this yet, though. Should we add a build with ARM or PowerPC? Possibly trying to not add too much, since it's probably 5-10x slower?
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.
Shouldn't this only be appended if this is not specified manually?
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.
Yes, the
Architecture.i686 not in target_archs
part of the condition checks that it wasn't specified manually. I found this way is more explicit and allows to specify both, or i686 (without x86), or x86 (which includes both) on their own.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 I do
cibuildwheel .
then I get the native archs (x86_64 and i686) but if I specify manuallycibuildwheel --archs "x86_64" .
, I should only get the x86_64 build. So adding the i686 build should only happen if the user doesn't specify archs in their options.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 think "native", which is both the default and allowed in the list would be better.
native
would expand toi686
andx86_64
. Then you can do native + something else, or you can specify just one of these and not get the other.