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

Build macOS arm64 and musllinux wheels #173

Merged
merged 7 commits into from
Aug 9, 2022
Merged

Build macOS arm64 and musllinux wheels #173

merged 7 commits into from
Aug 9, 2022

Conversation

Jackenmen
Copy link
Contributor

@Jackenmen Jackenmen commented Jul 20, 2022 •
edited

This PR enables building of musllinux wheels and macOS universal2 wheels which support both x86_64 and the new arm64 platforms

While trying to build the wheels, I've ran into an issue with the legacy setuptools backend so I updated this project to use PEP 517 compatible build system (a matter of adding pyproject.toml with setuptools specified and some minor changes related to the build isolation).

Additionally I made some changes that were not strictly necessary:

  • updated macOS version from 10.15 to 11 as the former is deprecated on GH Actions (The macOS 10.15 Actions runner image will begin deprecation on 5/31/22 and will be fully unsupported by 12/1/22 for GitHub and ADO actions/runner-images#5583)
  • updated cibuildwheel version because it seemed like a good idea to bump that though I have not run into any bugs with cibuildwheel 2.0.1
  • migrated cibuildwheel configuration from Actions workflows to pyproject.toml to de-duplicate it
  • removed CIBW_PRERELEASE_PYTHONS flag from release-wheels.yml workflow to avoid publishing wheels for Python versions that have not reached stable ABI yet
  • simplified the list in CIBW_BUILD from cp36-* cp37-* cp38-* cp39-* cp310-* to cp3*
    • if there's ever a need to skip any builds, CIBW_SKIP option can easily be used so this is IMO a better way of doing it
    • cibuilwheel is still being pinned so this can't actually cause it to start failing out of nowhere without you explicitly bumping the version

If you think I should split any part of the PR into a separate one, let me know.

@Jackenmen
Build macOS arm64 wheels
e3ad28c 
- use macOS 11 to build wheels
- update cibuildwheel version
- add universal2 tag to the macOS architectures
@Jackenmen
Use PEP 517 build system
d119ab1 
- switch to PEP 517 build system
- switch CI to use `pip install .` rather than `setup.py install`
@Jackenmen
Switch to macos-11 in test.yml workflow as well
665baf3
@Jackenmen Jackenmen changed the title Build macos arm64 wheels Build macOS arm64 wheels Jul 20, 2022
@saghul
Copy link
Owner

saghul commented Jul 21, 2022

This looks great, thank you!

@saghul
Copy link
Owner

saghul commented Jul 21, 2022

I think we need to change the yum command to apt. Not sure why that was there TBH.

@Jackenmen
Copy link
Contributor Author

I think we need to change the yum command to apt. Not sure why that was there TBH.

Ehh, it's more complicated than that. Other than manylinux_2_24 which uses Debian (I don't think that image is used here anyway so it should be fine), all other manylinux images use RHEL-based distro and rightfully use dnf.
What this fails on however, is musllinux images which use Alpine which would have to use apk I imaginr. I don't know why I haven't ran into that while testing it myself yesterday though.

@Jackenmen Jackenmen changed the title Build macOS arm64 wheels Build macOS arm64 and musllinux wheels Jul 21, 2022
@saghul
Copy link
Owner

saghul commented Jul 21, 2022

Aha! Do you know how to solve this?

@Jackenmen
Copy link
Contributor Author

Aha! Do you know how to solve this?

Yes, I actually am waiting for the build to finish before pushing it here and responding 😄

@Jackenmen
Copy link
Contributor Author

I don't know why I haven't ran into that while testing it myself yesterday though.

Weirdly enough, I might have actually not tested this exact combination with the bumped version of cibuildwheel which explains why I haven't caught that earlier since musllinux support is relatively new (though not as recent as I thought 😄).

Either way, I've pushed a fix for this and updated the PR's title to mention musllinux wheels as it seems I have added a musllinux build as well without even knowing :)

Additionally, I've done a bunch of smaller changes that weren't strictly necessary for this to work.
I moved cibuildwheel configuration over to pyproject.toml to de-duplicate it. While doing that, I removed the CIBW_PRERELEASE_PYTHONS env var from release-wheels.yml as according to cibuild wheel documentation, it should only be used for testing purposes because ABI of Python is not stable while in alpha and beta phase and cibuildwheel does get updated to work without that flag whenever Python release reaches that stage.

I have also described all the changes that this PR makes in the PR description to hopefully make it easier to look through.

One thing I noticed is that this practically doubles (or maybe even triples?) the time that is needed to build everything and I feel like this could benefit from splitting manylinux and musllinux in the CI matrix (and maybe dropping Python 3.6 support since it has already been EOL for 7 months?). If this is something that you want, I can help out but IMO it would be better to leave it for a separate PR because I already kind of feel like I might have overreached with the changes that I've done here and I'm not sure I should be adding even more to that.

To finish off, here's a link to the finished working build that's equivalent to what I've just pushed:
https://github.com/jack1142/pycares/actions/runs/2711821071

I'm going to assume you'll just wait for this PR's build to finish anyway but in the meantime, I figured it might be useful.

@Jackenmen
Copy link
Contributor Author

@saghul hi, hope I'm not bothering you. Just wanted to ping you to see if you didn't forget about this PR. If you're busy then sorry, it's just that you were pretty quick to respond when I was working on this so I felt that maybe this got lost in your backlog rather than it is due to you not having time :)

@saghul
Copy link
Owner

saghul commented Aug 8, 2022

Sorry, I had indeed forgotten. Let's see if the CI is happy now.

@Jackenmen
Copy link
Contributor Author

Sorry, I had indeed forgotten.

No problem, it's an open source project after all 😄

Sorry, I had indeed forgotten. Let's see if the CI is happy now.

Seems like it's happy.

@saghul
Copy link
Owner

saghul commented Aug 9, 2022

Awesome, let's go!

@saghul saghul merged commit 1f59aff into saghul:master Aug 9, 2022
@saghul
Copy link
Owner

saghul commented Aug 9, 2022

I'll kick off a release in a bit.

@Jackenmen Jackenmen deleted the build_macos_arm64_wheels branch August 9, 2022 06:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@Jackenmen @saghul