polars icon indicating copy to clipboard operation
polars copied to clipboard

ci[python]: Use maturin-action for Windows/macOS release

Open stinodego opened this issue 3 years ago • 8 comments
trafficstars

Changes:

  • Use maturin-action for the Windows/macOS workflow, rather than manually running commands.
    • I see no reason not to use the action, like we do in other release workflows. But maybe I am missing something?
  • Some minor reformatting, also in the other macOS workflow

This is untested, as I don't really know how to test this without actually publishing the wheel. And I don't want to do that since I don't know enough yet about the release process / if that could potentially break something.

stinodego avatar Sep 13 '22 21:09 stinodego

Let's do that one time when we are live in the office and try to get a release out. Then we can iterate quickly.

There is also a test.pypi. So we can set the architecture up for this so that we can test this properly.

ritchie46 avatar Sep 14 '22 08:09 ritchie46

Let's do that one time when we are live in the office and try to get a release out. Then we can iterate quickly.

There is also a test.pypi. So we can set the architecture up for this so that we can test this properly.

Yes, I think that's the way to go!

stinodego avatar Sep 14 '22 08:09 stinodego

As another point of reference, I'm generally very fond of cibuildwheel for making wheels on CI. I've used it more for non-Rust projects, but in geopolars I also got it working to wrap maturin. It ended up being pretty straightforward. The gist is:

  build_wheels:
    name: Build wheel on ${{ matrix.os }}
    runs-on: ${{ matrix.os }}
    strategy:
      matrix:
        os: [ubuntu-latest, windows-latest, macos-latest]

    steps:
      - uses: actions/checkout@v2

      - name: Build wheels
        uses: pypa/[email protected]
        env:
          # Python wheel support for mac M1 started as of 3.8. So if we _only_ build universal2
          # wheels, we don't build a wheel for 3.7. Therefore, we set CIBW_ARCHS_MACOS to both, but
          # explicitly skip x86-64-only wheels for 3.8 and higher.
          CIBW_SKIP: "cp2* cp35* cp36* cp38*macosx*x86_64 cp39*macosx*x86_64 cp310*macosx*x86_64 pp* *win32 *i686 *musllinux*"
          CIBW_ARCHS_MACOS: x86_64 universal2
          CIBW_ENVIRONMENT: 'PATH="$PATH:$HOME/.cargo/bin"'
          CIBW_BEFORE_ALL: "curl https://sh.rustup.rs -sSf | sh -s -- --default-toolchain stable -y"
          CIBW_BEFORE_ALL_MACOS: "rustup target add aarch64-apple-darwin"
        with:
          package-dir: py-geopolars

      - uses: actions/upload-artifact@v2
        with:
          path: ./wheelhouse/*.whl

cibuildwheel uses the pyproject.toml setup, so it uses maturin's pep 517 support.

At least in the linked setup, it's not hard to test without making a release; you just need to comment out the automatic publishing.

kylebarron avatar Sep 14 '22 20:09 kylebarron

Polars is able to distribute only a few wheels because it uses pyo3's abi3 support, but those docs also say:

The downside of this is that PyO3 can't use optimizations which rely on being compiled against a known exact Python version.

Since polars is very performance focused, maybe it would make sense to remove the abi3 support defined here: https://github.com/pola-rs/polars/blob/527ae86f7996bea60b7ae1bb4be48229d515e115/py-polars/Cargo.toml#L34 and distribute Python-version-specific wheels? I'm curious if that could lead to a perf improvement for newer versions of Python.

kylebarron avatar Sep 14 '22 20:09 kylebarron

Interesting, thanks for the input @kylebarron ! I'll ponder this for a bit 😄

stinodego avatar Sep 14 '22 21:09 stinodego

cibuildwheel uses the pyproject.toml setup, so it uses maturin's pep 517 support.

Yes, if anything goes wrong, feel free to report to maturin.

BTW, I think cibuildwheel is not a good fit for building non-x86 on GitHub Actions, fortunately cibuildwheel v2.10.0 added support for building wheels on Cirrus CI, so you can move Linux aarch64, macOS M1 builds to Cirrus CI to build on its native platforms now!

This might also help https://github.com/pola-rs/polars/pull/4848 since you can use the official manylinux2014 aarch64 docker image on Cirrus CI which contains a very recent GCC toolchain.

messense avatar Sep 15 '22 09:09 messense

BTW, I think cibuildwheel is not a good fit for building non-x86 on GitHub Actions

By "good fit" do you mean unable to build for non-x86 or just longer build times (The mac arm build times on Github Actions can indeed be quite long)? I don't know too much about non-x86 but my friend's M1 machine was at least able to load and install a geopolars universal2 wheel built in my cibuildwheel setup 🤷‍♂️

kylebarron avatar Sep 15 '22 09:09 kylebarron

By "good fit" do you mean unable to build for non-x86 or just longer build times

M1 is different, it's actually cross compiling which is fast enough. The QEMU based Linux builds are usually 6x slower than native build, it seems to be around 150 mins for polars for each build target.

messense avatar Sep 15 '22 09:09 messense