python-soundfile icon indicating copy to clipboard operation
python-soundfile copied to clipboard

Add Linux ARM64 (aarch64) wheel support

Open aoirint opened this issue 1 year ago • 6 comments

This pull request is taking over #382. Thanks @imWildCat ( https://github.com/bastibe/python-soundfile/pull/382#issuecomment-1774979298 )!

Add Linux ARM64 (aarch64) wheel to build_wheels.py and fix some platform dependent code.

Here is the main points of fixes.

With only this pull request, pytest is not passed due to an issue of libsndfile_arm64.so distributed in bastibe/libsndfile-binaries. Please see the GitHub Issue below.

  • https://github.com/bastibe/libsndfile-binaries/issues/29

Currently, this pull request does not update the submodule _soundfile_data to keep the reference for bastibe's repository. Updating submodule after https://github.com/bastibe/libsndfile-binaries/issues/29 fixed is required for working properly either before or after merging this pull request.

Here is an experimental ARM64 (aarch64) binary wheel build using my own libsndfile_arm64.so binary ( https://github.com/bastibe/libsndfile-binaries/issues/29 fixed one). This binary successfully passes pytest.

  • https://github.com/aoirint/python-soundfile/releases/tag/0.12.2.dev2%2Baoirint.addlinuxarm64
    • Download soundfile-0.12.2.dev2+aoirint.addlinuxarm64-py2.py3-none-manylinux_2_17_aarch64.whl
Full log of pytest using the experimental wheel (successfully passed)
$ python -m pytest
================================================= test session starts ==================================================
platform linux -- Python 3.11.6, pytest-7.4.2, pluggy-1.3.0
rootdir: /code
collected 324 items

tests/test_argspec.py ....                                                                                       [  1%]
tests/test_soundfile.py ........................................................................................ [ 28%]
................................................................................................................ [ 62%]
................................................................................................................ [ 97%]
........                                                                                                         [100%]

=================================================== warnings summary ===================================================
tests/test_soundfile.py::test_file_truthiness[obj]
  /home/user/.local/lib/python3.11/site-packages/_pytest/unraisableexception.py:78: PytestUnraisableExceptionWarning: Exception ignored from cffi callback <function SoundFile._init_virtual_io.<locals>.vio_write at 0x5518f63ec0>: None

  Traceback (most recent call last):
    File "/home/user/.local/lib/python3.11/site-packages/_pytest/fixtures.py", line 596, in _get_active_fixturedef
      return self._fixture_defs[argname]
             ~~~~~~~~~~~~~~~~~~^^^^^^^^^
  KeyError: 'request'

  During handling of the above exception, another exception occurred:

  Traceback (most recent call last):
    File "/home/user/.local/lib/python3.11/site-packages/_pytest/fixtures.py", line 599, in _get_active_fixturedef
      fixturedef = self._getnextfixturedef(argname)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/home/user/.local/lib/python3.11/site-packages/_pytest/fixtures.py", line 473, in _getnextfixturedef
      raise FixtureLookupError(argname, self)
  _pytest.fixtures.FixtureLookupError: ('request', <SubRequest 'file_w' for <Function test_file_truthiness[obj]>>)

  During handling of the above exception, another exception occurred:

  Traceback (most recent call last):
    File "/code/soundfile.py", line 1261, in vio_write
      written = file.write(data)
                ^^^^^^^^^^^^^^^^
  ValueError: I/O operation on closed file

    warnings.warn(pytest.PytestUnraisableExceptionWarning(msg))

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
============================================ 324 passed, 1 warning in 3.49s ============================================

If you are using Docker, you may test my ARM64 (aarch64) wheel build/fix on x86_64 machine with QEMU CPU emulation.

Commands
# On host machine
git clone https://github.com/aoirint/python-soundfile.git
cd python-soundfile
git checkout 5f2f5b37cd210005f0510f3d11cd9e4e0efa06e4
git submodule update --init

sudo docker run --rm -it --volume ".:/code" --platform "linux/arm64" arm64v8/python:3.11 bash
# As root user in Docker container
useradd --uid 1000 --create-home user
su -l user -s /bin/bash
# As general user in Docker container
cd /code

# If you want to build your own wheel
python build_wheels.py

# Please replace the wheel file name for your own wheel
pip install ./dist/soundfile-0.12.2.dev2+aoirint.addlinuxarm64-py2.py3-none-manylinux_2_17_aarch64.whl

pip install numpy pytest
python -m pytest

aoirint avatar Oct 23 '23 12:10 aoirint

Awesome to see progress on this! The changes so far look entirely reasonable.

Are you working on the aarch64 build scripts over in libsndfile-binaries as well?

bastibe avatar Oct 25 '23 15:10 bastibe

Are you working on the aarch64 build scripts over in libsndfile-binaries as well?

We don't need any new build script for libsndfile-binaries. It has already been merged in the pull request below.

  • https://github.com/bastibe/libsndfile-binaries/pull/24

It seems you just need to run the GitHub Actions workflow manually, then upload/push the Linux ARM64 build artifact libsndfile.so as libsndfile_arm64.so into bastibe/libsndfile-binaries (overwrite existing libsndfile_arm64.so).

image_concat

image_005

aoirint avatar Oct 25 '23 17:10 aoirint

Thank you for the heads-up! I probably won't have time to look into this in the next few days, but next weekend or so looks promising.

bastibe avatar Oct 27 '23 16:10 bastibe

I have just updated libsndfile-binaries to the newest version of libsndfile, which should include support for Vorbis in ARM64 in #423.

However, in order to test these things, we'd need a test suite that runs uses the precompiled binaries, in addition to the platform-supplied libsndfile. Would you, by any chance, like to contribute such a test suite? I'm afraid I don't have enough time to do that in my spare time at the moment.

bastibe avatar Dec 10 '23 11:12 bastibe

Thanks for creating the updated shared libraries.

Would you, by any chance, like to contribute such a test suite?

OK. I will try work on it.

  • Add test for Linux environment using libsndfile from OS package
    • Remove _soundfile_data before installing soundfile package

Also, python-soundfile is not currently tested for the ARM64 platform. We should test this if possible.

  • Add test for Linux ARM64 environment emulated on Linux x64

However, it is complicated to test ARM64 on GitHub Actions because there are only the x64-native CI runners. We might need to add a separated CI job instead of adding arm64 to strategy.matrix.architecture.

We have some options. If you don't mind, I prefer to use Docker because it is easier to implement for me.

  • Docker multi-platform image
  • Use pguyot/arm-runner-action@v2 like the build job of libsndfile_arm64.so
    • https://github.com/bastibe/libsndfile-binaries/blob/27341b2ec76a7ccccb2826e76dc938159bfb1370/.github/workflows/build-libs.yml#L106-L127

Example implementation using Docker:

docker run --name "emulator" --volume ".:/code" --platform "linux/arm64" --detach arm64v8/python:3.11 tail -f
docker exec "emulator" useradd --create-home user
docker exec "emulator" apt-get update
docker exec "emulator" apt-get install -y libsndfile1
docker exec -u user "emulator" pip install numpy pytest cffi>=1.0
docker exec -u user "emulator" python soundfile_build.py
docker exec -u user -w /code "emulator" pip install --editable . --verbose
docker exec -u user -w /code "emulator" python -m pytest .

aoirint avatar Dec 10 '23 16:12 aoirint

OK. I will try work on it.

Thank you very much! This is highly appreciated!

Also, python-soundfile is not currently tested for the ARM64 platform. We should test this if possible.

Indeed, that is a problem. The use of docker for this is absolutely fine. Thank you for bringing this up, I wasn't aware of that option. Do you want to address this as part of this PR, or in a new PR?

Out of curiosity, does it run faster than the pguyot/arm-runner-action@v2-action? IIRC, that build step was very slow compared to the others in libsndfile-binaries.

bastibe avatar Dec 12 '23 06:12 bastibe