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

Set _PYTHON_HOST_PLATFORM when building wheels in CI #21942

Merged
merged 4 commits into from
Feb 11, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions .github/workflows/release.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions .github/workflows/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
4 changes: 4 additions & 0 deletions src/python/pants/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
9 changes: 9 additions & 0 deletions src/python/pants_release/generate_github_workflows.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Copy link
Contributor

@tdyas tdyas Feb 10, 2025

Choose a reason for hiding this comment

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

Should we also have logic in the GHA workflow to check that the wheel's filename is as expected? I.e. that no universal2 wheels were generated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, yeah, I'll add some extra checks to release.py

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
Expand Down
40 changes: 28 additions & 12 deletions src/python/pants_release/release.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand Down
Loading