Skip to content
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

Add Linux ARM64 (aarch64) wheel support #415

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

aoirint
Copy link
Contributor

@aoirint aoirint commented Oct 23, 2023

This pull request is taking over #382. Thanks @imWildCat ( #382 (comment) )!

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.

Currently, this pull request does not update the submodule _soundfile_data to keep the reference for bastibe's repository.
Updating submodule after bastibe/libsndfile-binaries#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 ( bastibe/libsndfile-binaries#29 fixed one). This binary successfully passes pytest.

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 aoirint changed the title Add Linux ARM64 (aarch64) wheel Add Linux ARM64 (aarch64) wheel support Oct 23, 2023
@bastibe
Copy link
Owner

bastibe commented Oct 25, 2023

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?

@aoirint
Copy link
Contributor Author

aoirint commented Oct 25, 2023

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.

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

@bastibe
Copy link
Owner

bastibe commented Oct 27, 2023

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
Copy link
Owner

bastibe commented Dec 10, 2023

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.

@aoirint
Copy link
Contributor Author

aoirint commented Dec 10, 2023

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.

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 .

@bastibe
Copy link
Owner

bastibe commented Dec 12, 2023

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.

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.

3 participants