From 2147e1770eb6e925065d2d08a72c48c2d3d318ef Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Tue, 11 Feb 2025 16:27:42 +1100 Subject: [PATCH] Set _PYTHON_HOST_PLATFORM when building wheels in CI (#21942) This follows the lead of `cibuildwheel` to explicitly correctly tag our wheels on macOS, via the `_PYTHON_HOST_PLATFORM` environment variable. This change ensures that the `pip` understands which platform each wheel is built for and compatible with, and thus external plugins using `pants_requirements(...)` work again. It appears the file name (including platform) tags of a wheel are derived from `sysconfig.get_platform()`, i.e. based on the build configuration of the current Python interpreter. Pants builds single platform wheels, so the wheels need to be tagged with the right CPU architecture. In #21655, we changed from tagging correctly to tagging incorrectly: - Before that change, our self-hosted runners had 'simple' Python interpreters, built for the current platform via Pyenv. `python -c 'import sysconfig; print(sysconfig.get_platform())' reports `macosx-10.15-x86_64` and `macosx-11.6-arm64`, and indeed the wheels were tagged like that. - After that change, we started using Python interpreters provided by GitHub-hosted runners. The interpreters report `macosx-10.9-universal2` on both the ARM64 and x86-64 runners. That is, they're built as universal binaries that can run on either architecture. This lead to the wheels being tagged as `universal2` as well. The `_PYTHON_HOST_PLATFORM` environment variable overrides the default `sysconfig.get_platform()` value, and appears to be designed for this sort of purpose (and is used by `cibuildwheel` for such): - https://github.com/python/cpython/blob/5505b91a684b0fc7ffcb3a5b325302671d74fb15/Lib/sysconfig.py#L652-L654 - https://github.com/pypa/cibuildwheel/blob/3805787fe7a0476391541d834fa548a721f0ab2e/cibuildwheel/macos.py#L318-L331 Fixes #21938 --- .github/workflows/release.yaml | 4 ++ .github/workflows/test.yaml | 5 +++ src/python/pants/BUILD | 4 ++ .../generate_github_workflows.py | 9 +++++ src/python/pants_release/release.py | 40 +++++++++++++------ 5 files changed, 50 insertions(+), 12 deletions(-) diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index fed5268e913..25028f1f5ac 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -254,10 +254,12 @@ jobs: go-version: 1.19.5 - env: ARCHFLAGS: -arch x86_64 + _PYTHON_HOST_PLATFORM: macosx-13.0-x86_64 name: Build wheels run: ./pants run src/python/pants_release/release.py -- build-wheels - env: ARCHFLAGS: -arch x86_64 + _PYTHON_HOST_PLATFORM: macosx-13.0-x86_64 name: Build Pants PEX run: ./pants package src/python/pants:pants-pex - continue-on-error: true @@ -351,10 +353,12 @@ jobs: go-version: 1.19.5 - env: ARCHFLAGS: -arch arm64 + _PYTHON_HOST_PLATFORM: macosx-14.0-arm64 name: Build wheels run: ./pants run src/python/pants_release/release.py -- build-wheels - env: ARCHFLAGS: -arch arm64 + _PYTHON_HOST_PLATFORM: macosx-14.0-arm64 name: Build Pants PEX run: ./pants package src/python/pants:pants-pex - continue-on-error: true diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index ea1b9b5acf5..eb703a52ed4 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -532,10 +532,12 @@ jobs: go-version: 1.19.5 - env: ARCHFLAGS: -arch x86_64 + _PYTHON_HOST_PLATFORM: macosx-13.0-x86_64 name: Build wheels run: ./pants run src/python/pants_release/release.py -- build-wheels - env: ARCHFLAGS: -arch x86_64 + _PYTHON_HOST_PLATFORM: macosx-13.0-x86_64 name: Build Pants PEX run: ./pants package src/python/pants:pants-pex - continue-on-error: true @@ -612,10 +614,12 @@ jobs: go-version: 1.19.5 - env: ARCHFLAGS: -arch arm64 + _PYTHON_HOST_PLATFORM: macosx-14.0-arm64 name: Build wheels run: ./pants run src/python/pants_release/release.py -- build-wheels - env: ARCHFLAGS: -arch arm64 + _PYTHON_HOST_PLATFORM: macosx-14.0-arm64 name: Build Pants PEX run: ./pants package src/python/pants:pants-pex - continue-on-error: true @@ -1870,6 +1874,7 @@ jobs: test_python_macos13_x86_64: env: ARCHFLAGS: -arch x86_64 + _PYTHON_HOST_PLATFORM: macosx-13.0-x86_64 if: (github.repository_owner == 'pantsbuild') && (needs.classify_changes.outputs.docs_only != 'true') name: Test Python (macOS13-x86_64) needs: diff --git a/src/python/pants/BUILD b/src/python/pants/BUILD index 804ed92243e..2afa12935a3 100644 --- a/src/python/pants/BUILD +++ b/src/python/pants/BUILD @@ -38,6 +38,10 @@ python_distribution( python_requires="==3.11.*", ), entry_points={"console_scripts": {"pants": "pants.bin.pants_loader:main"}}, + # We need to explicitly control the wheel tagging, rather than following whatever the current + # Python interpreter is tagged for (especially on macOS, where a 'universal' interpreter build + # can lead to single-platform wheels being tagged as `universal2` wheels, incorrectly) + env_vars=["_PYTHON_HOST_PLATFORM"], ) pex_binary( diff --git a/src/python/pants_release/generate_github_workflows.py b/src/python/pants_release/generate_github_workflows.py index 37c9f6b27ae..966c6adcf20 100644 --- a/src/python/pants_release/generate_github_workflows.py +++ b/src/python/pants_release/generate_github_workflows.py @@ -443,8 +443,17 @@ def platform_env(self): # Works around bad `-arch arm64` flag embedded in Xcode 12.x Python interpreters on # intel machines. See: https://github.com/giampaolo/psutil/issues/1832 ret["ARCHFLAGS"] = "-arch x86_64" + # Be explicit about the platform we've built our native code for: without this, Python + # builds and tags wheels based on the interpreter's build. + # + # At the time of writing, the GitHub-hosted runners' Pythons are built as + # macosx-10.9-universal2. Thus, without this env var, we tag our single-platform wheels + # as universal2... this is a lie and understandably leads to installing the wrong wheel + # and thus things do not work (see #21938 for an example). + ret["_PYTHON_HOST_PLATFORM"] = "macosx-13.0-x86_64" if self.platform in {Platform.MACOS14_ARM64}: ret["ARCHFLAGS"] = "-arch arm64" + ret["_PYTHON_HOST_PLATFORM"] = "macosx-14.0-arm64" if self.platform == Platform.LINUX_X86_64: # Currently we run Linux x86_64 CI on GitHub Actions-hosted hardware, and # these are weak dual-core machines. Default parallelism on those machines diff --git a/src/python/pants_release/release.py b/src/python/pants_release/release.py index 2eaffaf509a..15c78a9fa1e 100644 --- a/src/python/pants_release/release.py +++ b/src/python/pants_release/release.py @@ -532,19 +532,35 @@ def build_pants_wheels() -> None: for package in PACKAGES: found_wheels = sorted(Path("dist").glob(f"{package}-{version}-*.whl")) - # NB: For any platform-specific wheels, like pantsbuild.pants, we assume that the - # top-level `dist` will only have wheels built for the current platform. This - # should be safe because it is not possible to build native wheels for another - # platform. - if not is_cross_platform(found_wheels) and len(found_wheels) > 1: - die( - softwrap( - f""" - Found multiple wheels for {package} in the `dist/` folder, but was - expecting only one wheel: {sorted(wheel.name for wheel in found_wheels)}. - """ + if not is_cross_platform(found_wheels): + # NB: For any platform-specific wheels, like pantsbuild.pants, we assume that the + # top-level `dist` will only have wheels built for the current platform. This + # should be safe because it is not possible to build native wheels for another + # platform. + if len(found_wheels) > 1: + die( + softwrap( + f""" + Found multiple wheels for {package} in the `dist/` folder, but was + expecting only one wheel: {sorted(wheel.name for wheel in found_wheels)}. + """ + ) ) - ) + + # We also only build for a single architecture at a time, so lets confirm that the wheel + # isn't potentially reporting itself as applicable to arm64 and x86-64 ('universal2', in + # macOS parlance) (see #21938): + wheel = found_wheels[0] + if "universal2" in str(wheel): + die( + softwrap( + f""" + Found universal wheel for {package} in the `dist/` folder, but was + expecting a specific architecture: {wheel}. + """ + ) + ) + for wheel in found_wheels: wheel_dest = dest / wheel.name if not wheel_dest.exists():