Closed Jackenmen closed 2 years ago
This looks great, thank you!
I think we need to change the yum command to apt. Not sure why that was there TBH.
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.
Aha! Do you know how to solve this?
Aha! Do you know how to solve this?
Yes, I actually am waiting for the build to finish before pushing it here and responding :smile:
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 :smile:).
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.
@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 :)
Sorry, I had indeed forgotten. Let's see if the CI is happy now.
Sorry, I had indeed forgotten.
No problem, it's an open source project after all :smile:
Sorry, I had indeed forgotten. Let's see if the CI is happy now.
Seems like it's happy.
Awesome, let's go!
I'll kick off a release in a bit.
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
withsetuptools
specified and some minor changes related to the build isolation).Additionally I made some changes that were not strictly necessary:
CIBW_PRERELEASE_PYTHONS
flag fromrelease-wheels.yml
workflow to avoid publishing wheels for Python versions that have not reached stable ABI yetCIBW_BUILD
fromcp36-* cp37-* cp38-* cp39-* cp310-*
tocp3*
CIBW_SKIP
option can easily be used so this is IMO a better way of doing itIf you think I should split any part of the PR into a separate one, let me know.