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 MacOS ARM64 binaries with GHA #29

Merged
merged 5 commits into from
Feb 28, 2023
Merged

Build MacOS ARM64 binaries with GHA #29

merged 5 commits into from
Feb 28, 2023

Conversation

gau-nernst
Copy link
Contributor

@gau-nernst gau-nernst commented Feb 24, 2023

As communicated on Slack, I added workflow to build MacOS ARM64 binaries to support Apple Silicon. I downloaded the wheels and tested on my local M1. Installation worked fine and all pytest tests passed. I only tested Python 3.8-3.11, since there is no Python 3.7 MacOS ARM64 from conda. (I think no one builds Python 3.7 for MacOS ARM64? Anyway, Python 3.7 is reaching end-of-life this June so perhaps we can even stop building for Python 3.7).

This is the workflow run in my fork: https://github.com/gau-nernst/kornia-rs/actions/runs/4263731867

The Linux builds are failing because I don't have access to the Docker images.

Also, this closes #20

Copy link
Member

@edgarriba edgarriba left a comment

Choose a reason for hiding this comment

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

Amazing. @ducha-aiki can you somehow validate ?

@ducha-aiki
Copy link
Member

@edgarriba I don't see there any wheels built, so I cannot verify. However, yes, proper Python 3.7 support is absent here, and if we can cross-compile, that would be amazing.

@gau-nernst
Copy link
Contributor Author

@ducha-aiki If you go to my workflow run here (https://github.com/gau-nernst/kornia-rs/actions/runs/4263731867) and scroll down to the end, can you see and download the wheels (Artifact file)?

image

@gau-nernst
Copy link
Contributor Author

gau-nernst commented Feb 26, 2023

Also, I'm thinking about using matrix to merge mac x64 and arm64 job definition. It would look like this

  macos:
    runs-on: macos-latest
    strategy:
      matrix:
        python-version: ["3.7", "3.8", "3.9", "3.10", "3.11"]
        target: ["x86_64-apple-darwin", "aarch64-apple-darwin"]
    steps:
      - uses: ilammy/setup-nasm@v1
      - uses: actions/checkout@v3
      - uses: actions/setup-python@v2
        with:
          python-version: ${{ matrix.python-version }}
          architecture: x64
      - uses: actions-rs/toolchain@v1
        with:
          profile: minimal
          toolchain: stable
          target: ${{ matrix.target }}
          override: true
      - uses: messense/maturin-action@v1
        with:
          maturin-version: latest
          command: build
          args: --target ${{ matrix.target }} --release --out dist -i python${{ matrix.python-version }}
      - name: Upload wheels
        uses: actions/upload-artifact@v2
        with:
          name: wheels
          path: dist

Would you prefer this instead? @edgarriba

The available target names for rust can be found here: https://doc.rust-lang.org/nightly/rustc/platform-support.html

@edgarriba
Copy link
Member

Also, I'm thinking about using matrix to merge mac x64 and arm64 job definition. It would look like this

  macos:
    runs-on: macos-latest
    strategy:
      matrix:
        python-version: ["3.7", "3.8", "3.9", "3.10", "3.11"]
        target: ["x86_64-apple-darwin", "aarch64-apple-darwin"]
    steps:
      - uses: ilammy/setup-nasm@v1
      - uses: actions/checkout@v3
      - uses: actions/setup-python@v2
        with:
          python-version: ${{ matrix.python-version }}
          architecture: x64
      - uses: actions-rs/toolchain@v1
        with:
          profile: minimal
          toolchain: stable
          target: ${{ matrix.target }}
          override: true
      - uses: messense/maturin-action@v1
        with:
          maturin-version: latest
          command: build
          args: --target ${{ matrix.target }} --release --out dist -i python${{ matrix.python-version }}
      - name: Upload wheels
        uses: actions/upload-artifact@v2
        with:
          name: wheels
          path: dist

Would you prefer this instead? @edgarriba

The available target names for rust can be found here: https://doc.rust-lang.org/nightly/rustc/platform-support.html

Probably yes

@gau-nernst
Copy link
Contributor Author

@edgarriba I have updated the workflow. The workflow run can be found here: https://github.com/gau-nernst/kornia-rs/actions/runs/4277566348

@ducha-aiki To test the wheels, you can go to my workflow run, scroll to the bottom, and find the wheels artifact. I think you should be able to download the wheels from my branch. I don't see any permission control settings regarding the artifacts.

@edgarriba
Copy link
Member

@gau-nernst any idea why linux failed in your branch ?

@gau-nernst
Copy link
Contributor Author

@gau-nernst any idea why linux failed in your branch ?

I believe it is because I don't have permissions to the linux Docker images at ghcr.io/kornia

@ducha-aiki
Copy link
Member

@gau-nernst I have checked the wheels from release, they work.

@edgarriba
Copy link
Member

cool, i'll merge. @gau-nernst would be great to add a small new feature or something to make a new release and test the release workflow

@edgarriba edgarriba merged commit 76d0261 into kornia:main Feb 28, 2023
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.

Add M1 version to pypi :)
3 participants