search

matthew-brett / delocate

Find and copy needed dynamic libraries into python wheels
BSD 2-Clause "Simplified" License
262 stars 59 forks source link

MRG: update wheel examples for Big Sur #111

Closed matthew-brett closed 2 years ago

matthew-brett commented 2 years ago

Rebuild wheel examples and libraries on Big Sur.

Adapt code to universal2 dual arch builds, and arm64 as second architecture, instead of i386.

Refactor to allow for Big Sur's push of system libraries into a dynamic linker cache, so the libraries are no longer on the file system.

HexDecimal commented 2 years ago

Is there anything I should be doing to help?

You can replace assert_equal(x, y) with assert x == y then Pytest will tell you what the variables were in its error message in Python 3.

matthew-brett commented 2 years ago

@HexDecimal - thanks - that is helpful. Would you consider doing a review?

HexDecimal commented 2 years ago

Could to try adding define_macros=[("CYTHON_LIMITED_API", "0x03060000"), ("Py_LIMITED_API", "0x03060000")] to the Extension classes in the test wheels in order to enable the Stable ABI? This should set a consistent name for the extension file no matter which version of Python 3 is used to compile it.

You could then append --py-limited-api=cp36 to the setup.py commands of wheels with extensions so that the wheels are given the same names and can be run from any later version of Python 3.

matthew-brett commented 2 years ago

Thanks - nice idea. Trying this now though, it does set the wheel name to e.g.:

fakepkg1-1.0-cp36-abi3-macosx_10_9_universal2.whl

but the extension filenames in the wheel stay the same:

fakepkg/subpkg/module2.cpython-39-darwin.so

Is that what you were expecting?

HexDecimal commented 2 years ago

--py-limited-api assumes the extensions are correct and module2.cpython-39-darwin.so is wrong as long as it still has cpython-39 in it.

I can only assume that Cython is having trouble with using the stable ABI. This is something that could be put off until a later PR.

HexDecimal commented 2 years ago

I've seen a suggestion to add py_limited_api=True to Extension, maybe could affect it?

HexDecimal commented 2 years ago

It could also be that the setup scripts are using distutils instead of setuptools.

HexDecimal commented 2 years ago

I think distutils is most likely the issue. from setuptools import setup, Extension should be used instead.

matthew-brett commented 2 years ago

Doesn't seem to make any difference to the file extension - any of the suggestions and various other things I tried... If you have any other suggestions, happy to try them.

HexDecimal commented 2 years ago

I looked into it more. It's an experimental feature that's in the pre-release versions of Cython. With the latest pre-release I couldn't get a local test to build.

An option is to use an alternative to Cython. I have experience with cffi which I know to work. Also a plain C extension might be possible since the example is simple. These things can be done in a later PR, since it's gotten pretty far from how easy I initially thought this would be.

matthew-brett commented 2 years ago

OK - I switched to using the bare Python C-API - that does work for the limited API.

HexDecimal commented 2 years ago

Good job. The extensions end with abi3.so now, which is what I would've expected.

matthew-brett commented 2 years ago

I think it's ready now; tests should be passing, the limited API is definitely an improvement.

HexDecimal commented 2 years ago

I think it looks fine. I can't follow the code as well without type hints so my review is mostly just me glancing at the code.