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

Build for Linux AArch64 and other miscellaneous cleanups #710

Merged

Conversation

agriyakhetarpal
Copy link
Contributor

@agriyakhetarpal agriyakhetarpal commented Feb 18, 2025

This PR closes #288; it adds the new ubuntu-22.04-arm runner to build Linux AArch64/arm64 wheels. SSE2 and AVX2 x86-specific SIMD optimizations have been disabled at build time. This is in line with Spack downstream: spack/spack#40754. Additionally, the minimum macOS deployment target has been bumped from 10.9 to 10.13 (High Sierra), as CPython supports 10.13+ post 3.12.6, which is used by cibuildwheel's GitHub Action.

I've moved some code in setup.py to dedicated functions for now, but I agree with the direction in #464 wherein moving to a more modern build system in comparison to setuptools has been suggested; it sounds like a better long-term solution.

Additional context: https://github.blog/changelog/2025-01-16-linux-arm64-hosted-runners-now-available-for-free-in-public-repositories-public-preview/

TODO:

  • Unit tests and/or doctests in docstrings
  • Tests pass locally
  • Docstrings and API docs for any new/modified user-facing classes and functions
  • Changes documented in docs/release.rst
  • Docs build locally
  • GitHub Actions CI passes
  • Test coverage to 100% (Codecov passes)

@joshmoore
Copy link
Member

Thanks! Launched.

Copy link

codecov bot commented Feb 18, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.96%. Comparing base (a2bdbe5) to head (d00be4b).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #710   +/-   ##
=======================================
  Coverage   99.96%   99.96%           
=======================================
  Files          63       63           
  Lines        2754     2754           
=======================================
  Hits         2753     2753           
  Misses          1        1           

@joshmoore
Copy link
Member

Did not find Miniforge3-Linux-arm64.sh latest in cache

Perhaps see conda-incubator/setup-miniconda#385

@agriyakhetarpal
Copy link
Contributor Author

Thanks! That should be fixed with 6b05be5.

Also, I'm sorry – I didn't notice that there is an open PR for this: #315. I should have checked! I noticed a few prior attempts when scrolling through #288, but I didn't see that PR as it wasn't linked to the issue 🙈

I would be happy to go forward with whatever is suggested.

@agriyakhetarpal
Copy link
Contributor Author

In the meantime, I added a CHANGELOG entry for this change via 82a4565. I am happy to preserve authorship from the previous PRs or close my PR; either option is acceptable to me.

@joshmoore
Copy link
Member

joshmoore commented Feb 19, 2025

E   ImportError: /home/runner/work/numcodecs/numcodecs/numcodecs/blosc.cpython-311-aarch64-linux-gnu.so:
undefined symbol: pthread_atfork

@agriyakhetarpal
Copy link
Contributor Author

Ah, thanks – I'll have to take a look at that; I don't immediately see why that happens. The only reference to pthread_atfork is in https://github.com/Blosc/c-blosc/blob/dcf6813d46f9fcbf7af4395b5536f886dfc76dd0/blosc/blosc.c#L2225, but https://anaconda.org/conda-forge/blosc/ and https://anaconda.org/conda-forge/python-blosc/ successfully ship AArch64, so maybe there's something to uncover there. The recommendation is to switch to blosc-2, though: Blosc/c-blosc#268, xref: #413, and that might require supporting either both or dropping blosc-1 because of the lack of forward compatibility.

@agriyakhetarpal
Copy link
Contributor Author

I guess we aren't linking pthread properly, so the symbol isn't resolved.

@agriyakhetarpal
Copy link
Contributor Author

@joshmoore, the tests should be fixed now. They ran on my fork and passed (see logs); the failure sign there is just because the Codecov token won't be set up in my fork's settings. Could you please relaunch the workflows here?

@joshmoore
Copy link
Member

Launched.

@agriyakhetarpal
Copy link
Contributor Author

Thanks! :) The Ubuntu-3.11 failure looks like a sporadic one.

@basnijholt
Copy link

Thanks @agriyakhetarpal, I am excited for this to land!

When launching the devcontainer of pipefunc building numcodecs takes like 99% of the time, even though I am installing ≈200 dependencies.

Hope this PR will have the momentum to get merged (unlike #315 which seems inactive).

@agriyakhetarpal
Copy link
Contributor Author

Thanks, @basnijholt; I'm happy to address any review comments that might come up, though I think the PR is complete from my end. I intend to keep it up to date. 😃

@joshmoore
Copy link
Member

(Relaunched the flaky 3.11 build)

Copy link
Contributor

@dstansby dstansby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for this PR - build times are so much better now we can use a native runner! I just had one question where it looks like old code can be removed, but once that's dealt with I will approve, and hopefully we can get this into a release 🙌

@agriyakhetarpal
Copy link
Contributor Author

@dstansby, thanks for the review! All good to go, now.

@joshmoore
Copy link
Member

Relaunched.

@dstansby dstansby merged commit 0ad21f7 into zarr-developers:main Mar 3, 2025
30 checks passed
@dstansby dstansby added the needs changelog Needs a changelog entry writing label Mar 3, 2025
@agriyakhetarpal agriyakhetarpal deleted the ci/linux-aarch64-builds branch March 3, 2025 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs changelog Needs a changelog entry writing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support to release Linux aarch64 wheels
4 participants