Skip to content

MAINT: Modernize cibuildwheel infrastructure #54

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

Merged
merged 23 commits into from
Jun 24, 2024

Conversation

larsoner
Copy link
Collaborator

  1. Move to GHA for wheel building
  2. Add dependabot for updates
  3. Add release.yml to ignore dependabot in release notes

I already set up trusted publishing on PyPI so in theory once this is green and we merge and cut a release, all the new wheels should land on PyPI. This would be good because there are a bunch missing, e.g., not even Python 3.11 wheels are up there!

@larsoner
Copy link
Collaborator Author

Okay actions refused to run on this PR so I pushed the shell beginning parts directly to main (just up through the checkout step, which should trivially succeed hopefully!).

@larsoner
Copy link
Collaborator Author

Okay so now the benefits here (other than using GHA which is becoming more standard I think) are:

  1. macOS arm64 wheels are built and tested
  2. Linux aarch64 wheels are built but not tested (I could use manylinux_2_28 to get them to be tested, but it adds a bit of time and reduces compat so maybe not worth it 🤷? )

So the set of wheels built is a superset of the old ones, except dropping 3.6 and 3.7 and adding 3.11 and 3.12. The complete list of wheels is here.

@larsoner
Copy link
Collaborator Author

@mgeier feel free to review/merge if you're happy!

@larsoner
Copy link
Collaborator Author

@mgeier I'll merge this and try cutting a release shortly. If you have comments on the PR at some point I'm happy to do a follow-up though!

@larsoner larsoner merged commit 0337da9 into spatialaudio:master Jun 24, 2024
14 checks passed
@larsoner
Copy link
Collaborator Author

Seems to have worked as expected! 🎉

@larsoner larsoner deleted the cibw branch June 24, 2024 15:44
@mgeier
Copy link
Member

mgeier commented Jun 25, 2024

Thanks for your work on this, that's great!

Sorry for my lack of response, I was quite busy these days, so I haven't had time to look into this too closely, but I was wondering about one thing:

Is it really necessary to have wheels for different Python versions?

We are not using Python's C API (but CFFI interfacing with a native C library), so why would we be bound by Python C API compatibility?

Or am I missing something?

@mgeier
Copy link
Member

mgeier commented Jun 25, 2024

I don't really understand any of this, but I found this example: https://pypi.org/project/curl-cffi/#files

It only provides cp38-abi3-... wheels, which seem to be compatible with CPython 3.8+.

And I found another example that I understand even less: https://pypi.org/project/btclib_libsecp256k1/#files

This has py3-none-... wheels but also cp311-cp311-... (which was used on my system with pip install).
Maybe the first one is ABI mode and the second one API mode?

Anyway, all that is not very important, I was just wondering ...

@larsoner
Copy link
Collaborator Author

I am not knowledgeable about that either, but it sounds like we could try using ABI and Py_LIMITED_API to get abi3 wheels and test those instead. A fun Sunday project for someone to try :)

@larsoner
Copy link
Collaborator Author

... this post has a nice explanation I was reading a bit:

https://blog.trailofbits.com/2022/11/15/python-wheels-abi-abi3audit/

@mgeier
Copy link
Member

mgeier commented Jun 26, 2024

Thanks for the information!

In case somebody wants to try, I read that CFFI is already supposed to set Py_LIMITED_API, see https://cffi.readthedocs.io/en/latest/cdef.html#ffibuilder-compile-etc-compiling-out-of-line-modules.

And indeed, our latest wheels seem to contain files called _rtmixer.abi3.so.

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.

2 participants