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.

Sign up for GitHub

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

Jump to bottom

dist.yml: Build musllinux wheels, build linux aarch64 wheels via QEMU #38272

Merged
merged 8 commits into from
Jul 24, 2024
Merged

dist.yml: Build musllinux wheels, build linux aarch64 wheels via QEMU #38272

merged 8 commits into from
Jul 24, 2024

Conversation

mkoeppe
Copy link
Contributor

@mkoeppe mkoeppe commented Jun 25, 2024 •
edited
Loading

Other changes:

  • Reusing the built wheels for build dependencies. This speeds up the build when no compatible binary wheel is on PyPI yet.
  • Separate job steps for the various distributions, to improve readability of the output.

Test run: https://github.com/mkoeppe/sage/actions/runs/9666490294

The aarch64 build takes very long (e.g. 20 minutes for each wheel of sagemath-objects, total 2.5 hours for sagemath-objects, total 5 hours for all manylinux platforms) because of the CPU emulation. That's normal.

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

⌛ Dependencies

@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 25, 2024 •
edited
Loading

Documentation preview for this PR (built with commit 4e53812; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@mkoeppe mkoeppe changed the title .github/workflows/dist.yml: Build manylinux aarch64 wheels via QEMU dist.yml: Build musllinux wheels, build manylinux aarch64 wheels via QEMU Jun 25, 2024
@mkoeppe mkoeppe changed the title dist.yml: Build musllinux wheels, build manylinux aarch64 wheels via QEMU dist.yml: Build musllinux wheels, build linux aarch64 wheels via QEMU Jun 25, 2024
@mkoeppe mkoeppe self-assigned this Jun 25, 2024
@mkoeppe mkoeppe marked this pull request as ready for review June 25, 2024 03:37
@mkoeppe mkoeppe force-pushed the wheels_aarch64 branch 2 times, most recently from 7826649 to e00a529 Compare June 25, 2024 19:08
@mkoeppe mkoeppe marked this pull request as draft June 25, 2024 21:07
@mkoeppe mkoeppe marked this pull request as ready for review June 25, 2024 22:30
@mkoeppe mkoeppe added this to the sage-10.4 milestone Jun 25, 2024
@mkoeppe mkoeppe requested a review from vbraun June 25, 2024 22:40
@kwankyu
Copy link
Collaborator

kwankyu commented Jul 11, 2024

I see warnings like

  info: This container will host the build for cp39-manylinux_aarch64, cp310-manylinux_aarch64, cp311-manylinux_aarch64, cp312-manylinux_aarch64...
  WARNING: The requested image's platform (linux/arm64/v8) does not match the detected host platform (linux/amd64/v3) and no specific platform was requested

No problem or not fixable?

Would you rerun the test after rebase?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 11, 2024

I see warnings like

  info: This container will host the build for cp39-manylinux_aarch64, cp310-manylinux_aarch64, cp311-manylinux_aarch64, cp312-manylinux_aarch64...
  WARNING: The requested image's platform (linux/arm64/v8) does not match the detected host platform (linux/amd64/v3) and no specific platform was requested

That's normal. It warns about a slow operation (via CPU emulation), which is exactly what is intended here.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 11, 2024

Would you rerun the test after rebase?

Running at https://github.com/mkoeppe/sage/actions/runs/9891680565

@kwankyu
Copy link
Collaborator

kwankyu commented Jul 11, 2024 •
edited
Loading

I see warnings like

  info: This container will host the build for cp39-manylinux_aarch64, cp310-manylinux_aarch64, cp311-manylinux_aarch64, cp312-manylinux_aarch64...
  WARNING: The requested image's platform (linux/arm64/v8) does not match the detected host platform (linux/amd64/v3) and no specific platform was requested

That's normal. It warns about a slow operation (via CPU emulation), which is exactly what is intended here.

It seems to complain about ".../v8" vs ".../v3". I don't know what they are... (version?)

@kwankyu
Copy link
Collaborator

kwankyu commented Jul 11, 2024

Is it a docker image, a container of which runs on a platform emulated by qemu? The warning means that the target platform of the docker image does not match exactly with the emulated platform?

@kwankyu
Copy link
Collaborator

kwankyu commented Jul 11, 2024

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

Ah, it is rather about arm64 vs amd64...

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 11, 2024

Yes, the host (an amd64 machine) uses qemu to run the arm64 (= aarch64) Linux image. In the container running this image, we build the platform wheels for Linux aarch64.

@kwankyu
Copy link
Collaborator

kwankyu commented Jul 11, 2024

If the docker is running on the platform emulated by qemu and the message is from the docker, then the "detected host platform" is the platform emulated by qemu, which is amd64. Then the wheel is built for amd64. No?

If the wheel is verified as built for arm64, then obviously there is some error in the above reasoning. So if you could verify it somehow, just ignore me :-)

@kwankyu
Copy link
Collaborator

kwankyu commented Jul 11, 2024

The "detected host platform" seems to mean the host platform running the github workflow. And the platform emulated by qemu is arm64. All as you said. OK.

@kwankyu
Copy link
Collaborator

kwankyu commented Jul 11, 2024

ChatGPT says we can avoid the warning message with

export CIBW_ENVIRONMENT="DOCKER_DEFAULT_PLATFORM=linux/amd64"

I don't know, but you may try if you want...

@kwankyu
Copy link
Collaborator

kwankyu commented Jul 11, 2024

From the new run:

4 wheels produced in 139 minutes:
  sagemath_objects-10.4rc2-cp310-cp310-manylinux_2_17_aarch64.manylinux2014_aarch64.whl   14,961 kB
  sagemath_objects-10.4rc2-cp311-cp311-manylinux_2_17_aarch64.manylinux2014_aarch64.whl   16,116 kB
  sagemath_objects-10.4rc2-cp312-cp312-manylinux_2_17_aarch64.manylinux2014_aarch64.whl   16,200 kB
  sagemath_objects-10.4rc2-cp39-cp39-manylinux_2_17_aarch64.manylinux2014_aarch64.whl     15,044 kB

Note that repeated "cp...-cp...". Is this correct?

The job name is "wheels manylinux*_aarch64". How about just wheels manylinux_aarch64?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 12, 2024

Note that repeated "cp...-cp...". Is this correct?

Yes, compare with the files at https://pypi.org/project/numpy/#files for example and https://peps.python.org/pep-0491/#file-name-convention for a description of the file name format.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 12, 2024

avoid the warning message with

export CIBW_ENVIRONMENT="DOCKER_DEFAULT_PLATFORM=linux/amd64"

I think I'll pass on that.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 12, 2024

The job name is "wheels manylinux*_aarch64". How about just wheels manylinux_aarch64?

The * expresses that manylinux will be followed by a version number

@kwankyu
Copy link
Collaborator

kwankyu commented Jul 12, 2024

The job name is "wheels manylinux*_aarch64". How about just wheels manylinux_aarch64?

The * expresses that manylinux will be followed by a version number

I think no one can read that meaning from a single asterisk... Not a big deal, anyway.

kwankyu
kwankyu approved these changes Jul 12, 2024
View reviewed changes
Copy link
Collaborator

@kwankyu kwankyu left a comment

Choose a reason for hiding this comment

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

Otherwise lgtm.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 13, 2024

Thanks

@mkoeppe mkoeppe removed this from the sage-10.4 milestone Jul 21, 2024
@vbraun vbraun merged commit 76a0869 into sagemath:develop Jul 24, 2024
20 of 21 checks passed
@mkoeppe mkoeppe added this to the sage-10.5 milestone Jul 25, 2024
@mkoeppe mkoeppe deleted the wheels_aarch64 branch July 25, 2024 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kwankyu kwankyu kwankyu approved these changes

@vbraun vbraun Awaiting requested review from vbraun

Assignees

@mkoeppe mkoeppe

Labels
c: distribution c: scripts v: small
Projects
None yet
Milestone
sage-10.5
Development

Successfully merging this pull request may close these issues.
3 participants
@mkoeppe @kwankyu @vbraun