`import deflate` fails immediately on MacOS

[jpivarski]

I first noticed this in CI:

____________________________ test_file_header[True] ____________________________

use_deflate = True

    @pytest.mark.parametrize("use_deflate", [False, True])
    def test_file_header(use_deflate):
        if use_deflate:
>           pytest.importorskip("deflate")
E           pytest.PytestDeprecationWarning: 
E           Module 'deflate' was found, but when imported by pytest it raised:
E               ImportError("dlopen(/Users/runner/miniconda3/envs/test/lib/python3.9/site-packages/deflate.cpython-39-darwin.so, 0x0002): symbol not found in flat namespace '_libdeflate_arm_cpu_features'")
E           In pytest 9.1 this warning will become an error by default.
E           You can fix the underlying problem, or alternatively overwrite this behavior and silence this warning by passing exc_type=ImportError explicitly.
E           See https://docs.pytest.org/en/stable/deprecations.html#pytest-importorskip-default-behavior-regarding-importerror

(perhaps because GitHub just upgraded to ARM/M1/M2 Macs), but I also see it on a (M2) Mac laptop:

>>> import deflate
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ImportError: dlopen(/Users/jpivarski/mambaforge/lib/python3.10/site-packages/deflate.cpython-310-darwin.so, 0x0002): symbol not found in flat namespace '_libdeflate_arm_cpu_features'

This is deflate 0.6.0, freshly installed from PyPI.

My version of libdeflate is 1.17, installed through conda. (Overall, it's a conda environment, with deflate being the only package installed by pip within that environment.)

[henryiii]

FYI, macos-latest is being moved from macos-12 (Intel) to macos-14 (ARM). The best way to handle this in cibuildwheel is probably to have both a macos-13 (Intel) and a macos-14 job, and remove the explicit ARCH setting, removing the need to cross compile at all.

Do you have any CPython code in the library? If not, you should make a generic wheel, not a CPython specific one, that will work on all Pythons and won't require rereleasing every time there's a new CPython release.

[henryiii]

Also, I notice there's a CMakeLists in the library you are wrapping, I'd highly recommend looking at scikit-build-core.

[dcwatson]

Yikes, thanks for the report. And thanks for the tips @henryiii. I do have C code wrapping libdeflate, so I don't think a generic wheel is an option? I'm also not terribly keen on learning (and keeping up with) a new build system at the moment. I will have a look at getting this fixed ASAP though.

[henryiii]

https://github.com/dcwatson/deflate/blob/d32c41307d9a0bcdb60ab1fd675e609a6c9c42dd/deflate.c#L3C1-L3C20

Ahh, yes. Though might be worth checking the Stable ABI out.

[dcwatson]

Splitting the macos runners for cibuildwheel seems to do the trick. At least installing the wheel it built on my M3 doesn't fail 😄 I yanked 0.6.0 and uploaded 0.6.1 to PyPI - please let me know if you have any more trouble with it. And thanks again to both of you.

[henryiii]

FYI, this was the bug:

https://github.com/dcwatson/deflate/blob/87272e9dfc76ec776611d32cc6855ccccf846d4f/setup.py#L44

That's going to get the wrong file when cross compiling.

[henryiii]

FYI, I played with the stable ABI (using #43 as a base, since I know how to do that), and it looks like PyBuffer is used, which requires Python 3.11 as the lowest Stable ABI version (fine), but then I hit _PyBytes_Resize, which is not part of the stable ABI. (In fact, given it starts with an underscore, it's not part of the normal ABI either). Is it possible to avoid that private function?

It is documented, though

[jpivarski]

Thanks for the quick fix! I've reverted Uproot to include it in all tests (scikit-hep/uproot5#1206).

[dcwatson]

@henryiii I use _PyBytes_Resize to resize the output bytes without having to create a second buffer for libdeflate to decompress into and then copy. A memoryview might have been a better choice for this - users could always copy them to bytes if they wanted, but likely wouldn't need to in many cases. I may revisit that for a 1.0.

[jpivarski]

It looks like it's still not working for Python 3.8 (only):

=================================== FAILURES ===================================
____________________________ test_file_header[True] ____________________________

use_deflate = True

    @pytest.mark.parametrize("use_deflate", [False, True])
    def test_file_header(use_deflate):
        if use_deflate:
>           pytest.importorskip("deflate")
E           pytest.PytestDeprecationWarning: 
E           Module 'deflate' was found, but when imported by pytest it raised:
E               ImportError("dlopen(/Users/runner/miniconda3/envs/test/lib/python3.8/site-packages/deflate.cpython-38-darwin.so, 0x0002): symbol not found in flat namespace '_libdeflate_arm_cpu_features'")
E           In pytest 9.1 this warning will become an error by default.
E           You can fix the underlying problem, or alternatively overwrite this behavior and silence this warning by passing exc_type=ImportError explicitly.
E           See https://docs.pytest.org/en/stable/deprecations.html#pytest-importorskip-default-behavior-regarding-importerror

use_deflate = True

tests/test_0008_start_interpretation.py:53: PytestDeprecationWarning

This test has deflate 0.6.1 installed:

2024-05-07T12:41:35.8221780Z Downloading deflate-0.6.1-cp38-cp38-macosx_11_0_arm64.whl (43 kB)
2024-05-07T12:41:35.8324180Z    ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 43.4/43.4 kB 4.3 MB/s eta 0:00:00

[henryiii]

Python 3.8 is tricky, as it just barely supported ARM. In cibuildwheel, we use the Intel version due to the final and only Universal release not being compiled in a compatible manor. But I think that's causing the code linked above to be incorrect.

Scikit-build-core would have handled this correctly, by the way - the issues is due to hacking this into setuptools. :)

[henryiii]

You can check ARCHFLAGS or _PYTHON_HOST_PLATFORM; those will tell you what you are compiling for. Or just switch to scikit-build-core, which handles all this sort of stuff for you. See https://github.com/pypa/cibuildwheel/blob/98d57d9547203fa3b5676ef6960d639989295cf8/cibuildwheel/macos.py#L248-L260

FYI, it's actually correct to include x86 & arm sources always, they are protected internally. That's what the upstream library does: https://github.com/ebiggers/libdeflate/blob/275aa5141db6eda3587214e0f1d3a134768f557d/CMakeLists.txt#L99-L106

FYI, cibuildwheel can't test 3.8 wheels on AS due to using a Python binary that doesn't support Apple Silicon, so it can't catch issues with 3.8.

[dcwatson]

I just pushed out 0.7.0, built with scikit-build-core. Thanks again for the report and the help 🎉

[henryiii]

I know this was pulled, but:

* Use only [stable Python API](https://docs.python.org/3/c-api/stable.html)
    * _Note that this can't actually be enabled until Python 3.11 is the minimum
      supported version._

That's not correct. If you set cp311, then the stable ABI will be built on 3.11, and used on 3.12 and 3.13b1, but 3.8-3.10 will have traditional wheels. Scikit-build-core and cibuildwheel will do this for you.