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

[draft] Sketched out an explicit QEMU mode #1773

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

Conversation

jonded94
Copy link

@jonded94 jonded94 commented Mar 2, 2024

This is a very primitive sketch of forcing cibuildwheel to execute a given docker build container in the platform mode that corresponds to the wanted wheel architecture. This should only have an effect when doing non-native platform wheel builds through QEMU.

Possibly interesting since I noticed that all build containers run commands are given no further platform information, even if an image could not be executed natively and has to be run through QEMU. This should work because of the binfmt_misc feature of the linux kernel, but for some reason I saw this crash catastrophically in this issue in a Github Action: #1771.

This would also remove these warning messages that docker prints out:

WARNING: The requested image's platform (linux/arm64) does not match the detected host platform (linux/amd64/v3) and no specific platform was requested

Note that this is just a sketch and not intended to be merged. Let me know what you think about this in general.

Jonas Dedden and others added 3 commits March 2, 2024 16:50
@joerick joerick marked this pull request as draft March 2, 2024 17:56
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.

Thanks for putting this together. my thoughts-

  • no need to put this behind a new option, IMO. We can add the --platform always
  • we shouldn't fiddle with the container_engine_config, instead, pass the architecture into OCIContainer, and map it as appropriate in there
  • For arm64, I see you specified linux/arm64/v8, I wonder if we can be more vague and leave out the variant here and just say e.g. linux/arm64

@jonded94
Copy link
Author

jonded94 commented Mar 3, 2024

Implemented your suggested changes. Unfortunately I'm getting failed tests in CircleCI when setting --platform everywhere:

"--platform" is only supported on a Docker daemon with experimental features enabled

Also, this is failing: https://dev.azure.com/joerick0429/cibuildwheel/_build/results?buildId=5690&view=logs&j=65f37ddb-1a35-539a-d91f-4ab7f01a34d0&t=677308e5-f586-5eb1-7f4b-3c9712904030&l=1767. Seems like a i686 image is actually built in amd64 mode?

image with reference quay.io/pypa/manylinux1_i686:2024-02-17-6f383ed was found but does not match the specified platform: wanted linux/386, actual: linux/amd64
Error: Command ['docker', 'create', '--platform=linux/386', '--env=CIBUILDWHEEL', '--env=SOURCE_DATE_EPOCH', '--name=cibuildwheel-dae485b4-0570-47a4-a254-296030275f4a', '--interactive', '--volume=/:/host', 'quay.io/pypa/manylinux1_i686:2024-02-17-6f383ed', '/bin/bash'] failed with code 1. None

I wonder if we can be more vague and leave out the variant here and just say e.g. linux/arm64

Since arm64/v8 seems to be the first ARM platform iteration to be 64-bit, I think leaving the version number out is perfectly possible since aarch64 obviously means 64-bit anyways.

@joerick
Copy link
Contributor

joerick commented Mar 15, 2024

Also, this is failing: https://dev.azure.com/joerick0429/cibuildwheel/_build/results?buildId=5690&view=logs&j=65f37ddb-1a35-539a-d91f-4ab7f01a34d0&t=677308e5-f586-5eb1-7f4b-3c9712904030&l=1767. Seems like a i686 image is actually built in amd64 mode?

Hmm. this kind of detail makes me less sure that manually specifying the platform is the right approach.

"--platform" is only supported on a Docker daemon with experimental features enabled

It's not experimental since Docker API v1.32, guess Circle are lagging pretty bad there. But given this route is still in the process of rolling out, I think a better option would be for you to use create_args on container-engine for this. I can make a change to container-engine so it's overridable per-platform, which should make it easy to specify the right flags using overrides. It could be a useful feature for other use-cases too.

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.

None yet

2 participants