Skip to content

Setup cibuildwheel with Linux, macOS, Windows wheels #66

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

Setup cibuildwheel with Linux, macOS, Windows wheels #66

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
28c3b04
Setup cibuildwheel with Linux, macOS, Windows wheels
alugowski Feb 3, 2023
c270d51
Merge branch 'GraphBLAS:main' into main
alugowski Feb 10, 2023
148d04d
Merge branch 'main' into HEAD
jim22k Mar 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
167 changes: 140 additions & 27 deletions .github/workflows/wheels.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,36 +4,149 @@ on:
release:
types: [created]

# Enable Run Workflow button in GitHub UI
workflow_dispatch:

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

jobs:
wheels:
build_sdist:
name: Build SDist
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
Copy link
Member

Choose a reason for hiding this comment

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

Let's add

        with:
          fetch-depth: 0

to make sure it can always find a tag to use as the version.


- name: Build SDist
run: pipx run build --sdist

- name: Check metadata
run: pipx run twine check dist/*
Comment on lines +21 to +25
Copy link
Member

Choose a reason for hiding this comment

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

What are the versions of pip, build, and twine used here? Should we always update them such as with

          python -m pip install --upgrade pip
          python -m pip install build twine

or the pipx equivalent (note: I don't use or really understand pipx).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

AFAIK you always get the latest.


- uses: actions/upload-artifact@v3
with:
path: dist/*.tar.gz


build_wheels:
name: Wheels on ${{ matrix.platform_id }} - ${{ matrix.os }}
runs-on: ${{ matrix.os }}
defaults:
run:
shell: bash -l {0}
strategy:
fail-fast: false
matrix:
os: ["ubuntu-latest"]
# Loosely based on scikit-learn's config:
# https://github.com/scikit-learn/scikit-learn/blob/main/.github/workflows/wheels.yml
include:
- os: windows-latest
python-version: "3.8"
platform_id: win_amd64

- os: ubuntu-latest
python-version: "3.8"
platform_id: manylinux_x86_64
manylinux_image: manylinux2014
- os: ubuntu-latest
python-version: "3.8"
platform_id: manylinux_aarch64
manylinux_image: manylinux2014

# Use x86 macOS runner to build both x86 and ARM. GitHub does not offer M1/M2 yet (only self-hosted).
- os: macos-latest
python-version: "3.8"
platform_id: macosx_x86_64

steps:
- name: Checkout
uses: actions/checkout@v3
- name: Set up Python
uses: actions/setup-python@v4
with:
python-version: '3.8'
- name: Upgrade pip
run: |
python -m pip install --upgrade pip
- name: Build manylinux Python wheels
uses: RalfG/python-wheels-manylinux-build@v0.4.2-manylinux2014_x86_64
with:
python-versions: 'cp38-cp38 cp39-cp39'
build-requirements: 'cffi numpy>=1.19,<1.20 cython'
pre-build-command: ${{ format('sh suitesparse.sh {0}', github.ref) }}
- name: Publish wheels to PyPI
env:
TWINE_USERNAME: __token__
TWINE_PASSWORD: ${{ secrets.PYPI_TOKEN }}
run: |
pip install twine
twine upload dist/*-manylinux*.whl
- uses: actions/checkout@v3
Copy link
Member

Choose a reason for hiding this comment

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

And here:

        with:
          fetch-depth: 0


- uses: actions/setup-python@v4
with:
python-version: ${{ matrix.python-version }}

- name: Install tools (macOS)
if: contains(matrix.os, 'macos')
# * install coreutils which includes `nproc` used by `make -j` in suitesparse.sh
# * GitHub actions comes with libomp already installed. It's listed here to explicitly list this requirement.
run: |
brew install coreutils
brew install libomp

- name: Build Wheels
env:
# very verbose
CIBW_BUILD_VERBOSITY: 3

# Build SuiteSparse
CIBW_BEFORE_ALL: bash suitesparse.sh ${{ github.ref }}
# CIBW_BEFORE_ALL: bash suitesparse.sh refs/tags/7.4.3.0

# Ask suitesparse.sh to build libraries in MSVC style on Windows.
CIBW_ENVIRONMENT_WINDOWS: CMAKE_GNUtoMS=ON

# macOS libomp setup.
# FindOpenMP doesn't find brew's libomp, so set the necessary configs manually.
# The OpenMP_* variables need to be passed to CMake as -D defines, that is done by suitesparse.sh
CIBW_ENVIRONMENT_MACOS: LDFLAGS="-L$(brew --prefix libomp)/lib" OpenMP_C_FLAGS="-Xclang -fopenmp -I$(brew --prefix libomp)/include" OpenMP_C_LIB_NAMES="libomp" OpenMP_libomp_LIBRARY="omp"

# Uncomment to only build CPython wheels
# CIBW_BUILD: "cp*"

# macOS: build x86_64 and arm64
CIBW_ARCHS_MACOS: "x86_64 arm64"

# No 32-bit builds
CIBW_SKIP: "*-win32 *_i686"

# Use delvewheel on Windows.
# This copies graphblas.dll into the wheel. "repair" in cibuildwheel parlance includes copying any shared
# libraries from the build host into the wheel to make the wheel self-contained.
# Cibuildwheel includes tools for this for Linux and macOS, and they recommend delvewheel for Windows.
CIBW_BEFORE_BUILD_WINDOWS: "pip install delvewheel"
CIBW_REPAIR_WHEEL_COMMAND_WINDOWS: "delvewheel repair --add-path \"C:\\Program Files (x86)\\bin\" --no-mangle \"libgomp-1.dll;libgcc_s_seh-1.dll\" -w {dest_dir} {wheel}"

# Test dependencies
CIBW_TEST_REQUIRES: "pytest"

# run tests
CIBW_TEST_COMMAND: "pytest {project}/suitesparse_graphblas/tests"

# GitHub Actions macOS Intel runner cannot run ARM tests.
CIBW_TEST_SKIP: "*-macosx_arm64"

run: |
python -m pip install cibuildwheel
python -m cibuildwheel --output-dir wheelhouse .
shell: bash

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

Copy link
Member

Choose a reason for hiding this comment

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

For sanity check, maybe add

          if-no-files-found: error


upload_all:
name: Upload to PyPI
needs: [build_wheels, build_sdist]
runs-on: ubuntu-latest
# if: github.event_name == 'release' && github.event.action == 'published'

steps:
- uses: actions/setup-python@v4
with:
python-version: "3.x"

- uses: actions/download-artifact@v3
with:
name: artifact
path: dist

- uses: pypa/gh-action-pypi-publish@release/v1
with:
# PyPI does not allow replacing a file. Without this flag the entire action fails if even a single duplicate exists.
skip_existing: true
verbose: true
# Real PyPI:
# password: ${{ secrets.PYPI_TOKEN }}

# Test PyPI:
password: ${{ secrets.TEST_PYPI_API_TOKEN }}
repository_url: https://test.pypi.org/legacy/
58 changes: 56 additions & 2 deletions suitesparse.sh
100644 → 100755
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,68 @@
#!/bin/bash

# parse SuiteSparse version from first argument, a git tag that ends in the version (no leading v)
if [[ $1 =~ refs/tags/([0-9]*\.[0-9]*\.[0-9]*)\..*$ ]];
then
VERSION=${BASH_REMATCH[1]}
else
echo "Specify a SuiteSparse version, such as: $0 refs/tags/7.4.3.0"
exit -1
fi
echo VERSION: $VERSION

NPROC="$(nproc)"
if [ -z "${NPROC}" ]; then
# Default for platforms that don't have nproc. Mostly Windows.
NPROC="2"
fi

cmake_params=()
if [ -n "${OpenMP_C_FLAGS}" ]; then
# macOS OpenMP flags.
# wheels.yml sets them as env vars, but CMake needs them as -D defs.
cmake_params+=(-DOpenMP_C_FLAGS="${OpenMP_C_FLAGS}")
cmake_params+=(-DOpenMP_C_LIB_NAMES="${OpenMP_C_LIB_NAMES}")
cmake_params+=(-DOpenMP_libomp_LIBRARY="${OpenMP_libomp_LIBRARY}")
fi

if [ -n "${CMAKE_GNUtoMS}" ]; then
# Windows needs .lib libraries, not .a
cmake_params+=(-DCMAKE_GNUtoMS=ON)
# Windows expects 'graphblas.lib', not 'libgraphblas.lib'
cmake_params+=(-DCMAKE_SHARED_LIBRARY_PREFIX=)
cmake_params+=(-DCMAKE_STATIC_LIBRARY_PREFIX=)
fi

curl -L https://github.com/DrTimothyAldenDavis/GraphBLAS/archive/refs/tags/v${VERSION}.tar.gz | tar xzf -
cd GraphBLAS-${VERSION}/build
cmake .. -DCMAKE_BUILD_TYPE=Release
make -j$(nproc)

# Disable optimizing some rarely-used types for significantly faster builds and significantly smaller wheel size.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If experimenting with the workflow I recommend disabling all the types. That brings the GraphBLAS build times into the 15 minute range on GitHub Actions.

Copy link
Member

Choose a reason for hiding this comment

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

I have a non-documented feature that you can use for this. Turn on the CMAKE_CUDA_DEV flag in the GraphBLAS CmakeLists.txt. That disables all compilation of the kernels in Source/Generated2.

I don't recommend that for production so I don't document it. The resulting GraphBLAS will be pretty slow. But it's nice for development.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@DrTimothyAldenDavis I thought I was going crazy because -DCMAKE_CUDA_DEV=1 didn't seem to make a difference, but I just noticed CMakeLIsts.txt has set ( CMAKE_CUDA_DEV off ) on lines 56 and 61, so CMAKE_CUDA_DEV is only usable if one is able to edit CMakeLists.txt directly. I supoose it's possible to do with sed, but sed inplace support varies between OSes and such edits are a PITA.

Copy link
Collaborator Author

@alugowski alugowski Feb 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@DrTimothyAldenDavis Just FYI, compiling that combinatorial explosion of types really does make working with GraphBLAS very difficult. This PR would not happen if GitHub's accounting wasn't broken, as the CI time should have cost me several hundred dollars. With the types disabled as seen here, building just the macOS wheels one time costs about $3.50 according to GitHub's published price list ($5 for all OSes). I'm estimating well over $20/run for all types enabled. I don't mind because GitHub doesn't have my credit card and they aren't counting anything against my free quota, but that's just luck on our part.

Copy link
Member

Choose a reason for hiding this comment

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

That's intentional. It is not meant for anyone else to use but me. It is supposed be very hard to turn on GBCUDA_DEV, because turning it on slows down GraphBLAS immensely (run time, not compile time).

If it's easy to turn on GBCUDA_DEV, then I worry that it would be compiled that way for production and distribution, as in a linux distro or conda package. That would be awful.

If you like, you could instead compile with

-DGxB_NO_BOOL=1 -DGxB_NO_INT8=1 -DGxB_NO_INT16=1 ... (etc).

I do not recommend that for a production build.

See GraphBLAS/Source/GB_control.h:
Note that I already disable quite a few semirings via that file.

GraphBLAS requires a surprisingly large amount of semirings and operators when used in production.

I'm working on a JIT where I can compile these at run-time, however. That will keep the performance up (and even improve it) while at the same time reducing the initial compile time. Distros will have to package my source code for that to work, however.

# Also the build with all types enabled sometimes stalls on GitHub Actions. Probably due to exceeded resource limits.
# These can still be used, they'll just have reduced performance (AFAIK similar to UDTs).
#echo "#define GxB_NO_BOOL 1" >> ../Source/GB_control.h #
#echo "#define GxB_NO_FP32 1" >> ../Source/GB_control.h #
#echo "#define GxB_NO_FP64 1" >> ../Source/GB_control.h #
#echo "#define GxB_NO_FC32 1" >> ../Source/GB_control.h #
#echo "#define GxB_NO_FC64 1" >> ../Source/GB_control.h #
echo "#define GxB_NO_INT16 1" >> ../Source/GB_control.h
echo "#define GxB_NO_INT32 1" >> ../Source/GB_control.h
#echo "#define GxB_NO_INT64 1" >> ../Source/GB_control.h #
echo "#define GxB_NO_INT8 1" >> ../Source/GB_control.h
echo "#define GxB_NO_UINT16 1" >> ../Source/GB_control.h
echo "#define GxB_NO_UINT32 1" >> ../Source/GB_control.h
echo "#define GxB_NO_UINT64 1" >> ../Source/GB_control.h
echo "#define GxB_NO_UINT8 1" >> ../Source/GB_control.h

cmake .. -DCMAKE_BUILD_TYPE=Release -G 'Unix Makefiles' "${cmake_params[@]}"
make -j$NPROC
make install

if [ -n "${CMAKE_GNUtoMS}" ]; then
# Windows:
# CMAKE_STATIC_LIBRARY_PREFIX is sometimes ignored, possibly when the MinGW toolchain is selected.
# Drop the 'lib' prefix manually.
echo "manually removing lib prefix"
mv "C:/Program Files (x86)/lib/libgraphblas.lib" "C:/Program Files (x86)/lib/graphblas.lib"
mv "C:/Program Files (x86)/lib/libgraphblas.dll.a" "C:/Program Files (x86)/lib/graphblas.dll.a"
cp "C:/Program Files (x86)/bin/libgraphblas.dll" "C:/Program Files (x86)/bin/graphblas.dll"
fi
Empty file removed suitesparse_graphblas/tests/__init__.py
View file
Open in desktop
Empty file.