From bc946b919cac6f25a199a526da571638cfde109f Mon Sep 17 00:00:00 2001 From: Nicholas Junge Date: Mon, 15 Apr 2024 18:44:09 +0200 Subject: [PATCH] Modernize wheel building job config (#1783) It is now possible to build Mac wheels on native machines in Github Actions, so ARM64 Mac wheels are now built and tested on M1 machines. Also, the artifact up-/download was migrated to v4, which made it necessary to upload wheels to unique artifact names, and then later stitch them together again in a subsequent job. The cross-platform Mac build injection in setup.py was removed, since it is no longer necessary. I relanded a monkey-patching of Bazel build files, this time for MODULE.bazel. This is because `rules_python` does not allow running as the root user, which is the case in cibuildwheel+Linux (happens in a Docker container). Since I did not see a quick way of switching to rootless containers, and did not want to hardcode the config change (it can apparently cause cache misses and build failures), I inject the "ignore_root_user_error" flag into the MODULE.bazel file when running in cibuildwheel on Linux. --- .github/install_bazel.sh | 9 +++--- .github/workflows/wheels.yml | 50 +++++++++++++++++----------- MODULE.bazel | 4 --- setup.py | 63 +++++++++++++++++++++++++++--------- 4 files changed, 83 insertions(+), 43 deletions(-) diff --git a/.github/install_bazel.sh b/.github/install_bazel.sh index d07db0e7..1b0d63c9 100644 --- a/.github/install_bazel.sh +++ b/.github/install_bazel.sh @@ -3,11 +3,10 @@ if ! bazel version; then if [ "$arch" == "aarch64" ]; then arch="arm64" fi - echo "Installing wget and downloading $arch Bazel binary from GitHub releases." - yum install -y wget - wget "https://github.com/bazelbuild/bazel/releases/download/6.4.0/bazel-6.4.0-linux-$arch" -O /usr/local/bin/bazel - chmod +x /usr/local/bin/bazel + echo "Downloading $arch Bazel binary from GitHub releases." + curl -L -o $HOME/bin/bazel --create-dirs "https://github.com/bazelbuild/bazel/releases/download/7.1.1/bazel-7.1.1-linux-$arch" + chmod +x $HOME/bin/bazel else - # bazel is installed for the correct architecture + # Bazel is installed for the correct architecture exit 0 fi diff --git a/.github/workflows/wheels.yml b/.github/workflows/wheels.yml index a36d312a..8b772cd8 100644 --- a/.github/workflows/wheels.yml +++ b/.github/workflows/wheels.yml @@ -15,16 +15,16 @@ jobs: uses: actions/checkout@v4 with: fetch-depth: 0 - - name: Install Python 3.11 + - name: Install Python 3.12 uses: actions/setup-python@v5 with: - python-version: 3.11 + python-version: 3.12 - run: python -m pip install build - name: Build sdist run: python -m build --sdist - - uses: actions/upload-artifact@v3 + - uses: actions/upload-artifact@v4 with: - name: dist + name: dist-sdist path: dist/*.tar.gz build_wheels: @@ -32,7 +32,7 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - os: [ubuntu-latest, macos-latest, windows-latest] + os: [ubuntu-latest, macos-13, macos-14, windows-latest] steps: - name: Check out Google Benchmark @@ -47,32 +47,44 @@ jobs: platforms: all - name: Build wheels on ${{ matrix.os }} using cibuildwheel - uses: pypa/cibuildwheel@v2.16.2 + uses: pypa/cibuildwheel@v2.17 env: - CIBW_BUILD: 'cp38-* cp39-* cp310-* cp311-* cp312-*' + CIBW_BUILD: "cp38-* cp39-* cp310-* cp311-* cp312-*" CIBW_SKIP: "*-musllinux_*" - CIBW_TEST_SKIP: "*-macosx_arm64" - CIBW_ARCHS_LINUX: x86_64 aarch64 - CIBW_ARCHS_MACOS: x86_64 arm64 - CIBW_ARCHS_WINDOWS: AMD64 + CIBW_TEST_SKIP: "cp38-macosx_*:arm64" + CIBW_ARCHS_LINUX: auto64 aarch64 + CIBW_ARCHS_WINDOWS: auto64 CIBW_BEFORE_ALL_LINUX: bash .github/install_bazel.sh + # Grab the rootless Bazel installation inside the container. + CIBW_ENVIRONMENT_LINUX: PATH=$PATH:$HOME/bin CIBW_TEST_COMMAND: python {project}/bindings/python/google_benchmark/example.py - name: Upload Google Benchmark ${{ matrix.os }} wheels - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 + with: + name: dist-${{ matrix.os }} + path: wheelhouse/*.whl + + merge_wheels: + name: Merge all built wheels into one artifact + runs-on: ubuntu-latest + needs: build_wheels + steps: + - name: Merge wheels + uses: actions/upload-artifact/merge@v4 with: name: dist - path: wheelhouse/*.whl + pattern: dist-* + delete-merged: true pypi_upload: name: Publish google-benchmark wheels to PyPI - needs: [build_sdist, build_wheels] + needs: [merge_wheels] runs-on: ubuntu-latest permissions: id-token: write steps: - - uses: actions/download-artifact@v3 - with: - name: dist - path: dist - - uses: pypa/gh-action-pypi-publish@v1.8.11 + - uses: actions/download-artifact@v4 + with: + path: dist + - uses: pypa/gh-action-pypi-publish@v1 diff --git a/MODULE.bazel b/MODULE.bazel index ca7bff65..95db0b12 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -27,10 +27,6 @@ python.toolchain( is_default = True, python_version = "3.12", ) -use_repo( - python, - python = "python_versions", -) pip = use_extension("@rules_python//python/extensions:pip.bzl", "pip", dev_dependency = True) pip.parse( diff --git a/setup.py b/setup.py index 910383c7..40cdc8d3 100644 --- a/setup.py +++ b/setup.py @@ -1,14 +1,17 @@ +import contextlib import os import platform +import re import shutil from pathlib import Path -from typing import Any +from typing import Any, Generator import setuptools from setuptools.command import build_ext IS_WINDOWS = platform.system() == "Windows" IS_MAC = platform.system() == "Darwin" +IS_LINUX = platform.system() == "Linux" # hardcoded SABI-related options. Requires that each Python interpreter # (hermetic or not) participating is of the same major-minor version. @@ -17,6 +20,46 @@ py_limited_api = version_tuple >= (3, 12) options = {"bdist_wheel": {"py_limited_api": "cp312"}} if py_limited_api else {} +def is_cibuildwheel() -> bool: + return os.getenv("CIBUILDWHEEL") is not None + + +@contextlib.contextmanager +def _maybe_patch_toolchains() -> Generator[None, None, None]: + """ + Patch rules_python toolchains to ignore root user error + when run in a Docker container on Linux in cibuildwheel. + """ + + def fmt_toolchain_args(matchobj): + suffix = "ignore_root_user_error = True" + callargs = matchobj.group(1) + # toolchain def is broken over multiple lines + if callargs.endswith("\n"): + callargs = callargs + " " + suffix + ",\n" + # toolchain def is on one line. + else: + callargs = callargs + ", " + suffix + return "python.toolchain(" + callargs + ")" + + CIBW_LINUX = is_cibuildwheel() and IS_LINUX + try: + if CIBW_LINUX: + module_bazel = Path("MODULE.bazel") + content: str = module_bazel.read_text() + module_bazel.write_text( + re.sub( + r"python.toolchain\(([\w\"\s,.=]*)\)", + fmt_toolchain_args, + content, + ) + ) + yield + finally: + if CIBW_LINUX: + module_bazel.write_text(content) + + class BazelExtension(setuptools.Extension): """A C/C++ extension that is defined as a Bazel BUILD target.""" @@ -73,21 +116,11 @@ class BuildBazelExtension(build_ext.build_ext): for library_dir in self.library_dirs: bazel_argv.append("--linkopt=/LIBPATH:" + library_dir) elif IS_MAC: - if platform.machine() == "x86_64": - # C++17 needs macOS 10.14 at minimum - bazel_argv.append("--macos_minimum_os=10.14") + # C++17 needs macOS 10.14 at minimum + bazel_argv.append("--macos_minimum_os=10.14") - # cross-compilation for Mac ARM64 on GitHub Mac x86 runners. - # ARCHFLAGS is set by cibuildwheel before macOS wheel builds. - archflags = os.getenv("ARCHFLAGS", "") - if "arm64" in archflags: - bazel_argv.append("--cpu=darwin_arm64") - bazel_argv.append("--macos_cpus=arm64") - - elif platform.machine() == "arm64": - bazel_argv.append("--macos_minimum_os=11.0") - - self.spawn(bazel_argv) + with _maybe_patch_toolchains(): + self.spawn(bazel_argv) if IS_WINDOWS: suffix = ".pyd"