fix(whl_install): use rules_python exec tools toolchain for pyc compilation#903
Merged
Merged
Conversation
gregmagolan
force-pushed
the
fix/whl-install-exec-tools-toolchain
branch
from
3a4b9a0 to
da49db2
Compare
|
dzbarsky
reviewed
Apr 4, 2026
dzbarsky
approved these changes
Apr 4, 2026
gregmagolan
force-pushed
the
fix/whl-install-exec-tools-toolchain
branch
3 times, most recently
from
2cd9e85 to
d4016ef
Compare
…lation Replace the `resolved_py_toolchain` workaround (a cfg=exec label attr that re-exported the Python toolchain) with the proper `EXEC_TOOLS_TOOLCHAIN_TYPE` from rules_python, whose `exec_runtime` field provides an RBE-compatible exec-platform interpreter via correct toolchain resolution. This required bumping rules_python to 1.9.0, which introduced `PyExecToolsInfo.exec_runtime` in aspect-build/rules_python#3539. The old workaround and the new approach solve the same cross-compilation problem: `ctx.toolchains[PY_TOOLCHAIN]` resolves the Python interpreter for the *target* platform, which isn't executable on the build host. The exec tools toolchain is registered with `exec_compatible_with` set to the platform constraints (rather than `target_compatible_with`), so toolchain resolution selects the host-runnable interpreter automatically. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
gregmagolan
force-pushed
the
fix/whl-install-exec-tools-toolchain
branch
from
d4016ef to
073a9f2
Compare
gregmagolan
added a commit
that referenced
this pull request
Apr 4, 2026
UNPACK_TOOLCHAIN is registered in repo.bzl with exec_compatible_with (via the cfg="exec" TOOL_CFGS entry), so ctx.toolchains[UNPACK_TOOLCHAIN] already resolves the exec-platform binary without any indirection. The resolved_unpack_toolchain rule and its cfg="exec" _unpack attr in whl_install were an unnecessary workaround, analogous to resolved_py_toolchain which was removed in #903. Remove resolved_unpack_toolchain from tools.bzl and BUILD.bazel, and replace the ctx.attr._unpack indirection in rule.bzl with a direct toolchain lookup. Add an analysis-time regression test (exec_toolchain_resolution_test) that confirms UNPACK_TOOLCHAIN resolves the exec-platform binary even when the target platform is arm64. The test writes the resolved binary path to a file for both native and arm64-transitioned targets and asserts they are identical, proving exec-platform resolution is honoured throughout platform transitions. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
3 tasks
gregmagolan
added a commit
that referenced
this pull request
Apr 5, 2026
UNPACK_TOOLCHAIN is registered in repo.bzl with exec_compatible_with (via the cfg="exec" TOOL_CFGS entry), so ctx.toolchains[UNPACK_TOOLCHAIN] already resolves the exec-platform binary without any indirection. The resolved_unpack_toolchain rule and its cfg="exec" _unpack attr in whl_install were an unnecessary workaround, analogous to resolved_py_toolchain which was removed in #903. Remove resolved_unpack_toolchain from tools.bzl and BUILD.bazel, and replace the ctx.attr._unpack indirection in rule.bzl with a direct toolchain lookup. Add an analysis-time regression test (exec_toolchain_resolution_test) that confirms UNPACK_TOOLCHAIN resolves the exec-platform binary even when the target platform is arm64. The test writes the resolved binary path to a file for both native and arm64-transitioned targets and asserts they are identical, proving exec-platform resolution is honoured throughout platform transitions. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
gregmagolan
added a commit
that referenced
this pull request
Apr 5, 2026
UNPACK_TOOLCHAIN is registered in repo.bzl with exec_compatible_with (via the cfg="exec" TOOL_CFGS entry), so ctx.toolchains[UNPACK_TOOLCHAIN] already resolves the exec-platform binary without any indirection. The resolved_unpack_toolchain rule and its cfg="exec" _unpack attr in whl_install were an unnecessary workaround, analogous to resolved_py_toolchain which was removed in #903. Remove resolved_unpack_toolchain from tools.bzl and BUILD.bazel, and replace the ctx.attr._unpack indirection in rule.bzl with a direct toolchain lookup. Add an analysis-time regression test (exec_toolchain_resolution_test) that confirms UNPACK_TOOLCHAIN resolves the exec-platform binary even when the target platform is arm64. The test writes the resolved binary path to a file for both native and arm64-transitioned targets and asserts they are identical, proving exec-platform resolution is honoured throughout platform transitions.
gregmagolan
added a commit
that referenced
this pull request
Apr 5, 2026
UNPACK_TOOLCHAIN is registered in repo.bzl with exec_compatible_with (via the cfg="exec" TOOL_CFGS entry), so ctx.toolchains[UNPACK_TOOLCHAIN] already resolves the exec-platform binary without any indirection. The resolved_unpack_toolchain rule and its cfg="exec" _unpack attr in whl_install were an unnecessary workaround, analogous to resolved_py_toolchain which was removed in #903. Remove resolved_unpack_toolchain from tools.bzl and BUILD.bazel, and replace the ctx.attr._unpack indirection in rule.bzl with a direct toolchain lookup. Add an analysis-time regression test (exec_toolchain_resolution_test) that confirms UNPACK_TOOLCHAIN resolves the exec-platform binary even when the target platform is arm64. The test writes the resolved binary path to a file for both native and arm64-transitioned targets and asserts they are identical, proving exec-platform resolution is honoured throughout platform transitions.
gregmagolan
added a commit
that referenced
this pull request
Apr 5, 2026
UNPACK_TOOLCHAIN is registered in repo.bzl with exec_compatible_with (via the cfg="exec" TOOL_CFGS entry), so ctx.toolchains[UNPACK_TOOLCHAIN] already resolves the exec-platform binary without any indirection. The resolved_unpack_toolchain rule and its cfg="exec" _unpack attr in whl_install were an unnecessary workaround, analogous to resolved_py_toolchain which was removed in #903. Remove resolved_unpack_toolchain from tools.bzl and BUILD.bazel, and replace the ctx.attr._unpack indirection in rule.bzl with a direct toolchain lookup. Add an analysis-time regression test (exec_toolchain_resolution_test) that confirms UNPACK_TOOLCHAIN resolves the exec-platform binary even when the target platform is arm64. The test writes the resolved binary path to a file for both native and arm64-transitioned targets and asserts they are identical, proving exec-platform resolution is honoured throughout platform transitions.
gregmagolan
added a commit
that referenced
this pull request
Apr 5, 2026
) ## Summary - `UNPACK_TOOLCHAIN` is registered with `exec_compatible_with` (via `cfg="exec"` in `TOOL_CFGS`), so `ctx.toolchains[UNPACK_TOOLCHAIN]` already resolves the exec-platform binary — no indirection needed. - Remove `resolved_unpack_toolchain` rule from `tools.bzl` and `BUILD.bazel`. - Replace the `cfg="exec"` `_unpack` attr + `ctx.attr._unpack[ToolchainInfo].bin.bin` pattern in `whl_install/rule.bzl` with a direct `ctx.toolchains[UNPACK_TOOLCHAIN].bin.bin` lookup. - Add `exec_toolchain_resolution_test` to the `uv-deps-650/crossbuild` e2e case: writes the resolved unpack binary path for both a native target and an arm64-transitioned target, then asserts the paths are identical (proving exec-platform resolution is preserved across target-platform transitions). This is a follow-on cleanup to #903, which removed the analogous `resolved_py_toolchain` workaround. ## Test plan - [x] Existing e2e suite passes (crossbuild_compile_pyc_test still green) - [x] New `exec_toolchain_resolution_test` passes on Linux CI (arm64 and amd64 transitions resolve the same exec-platform binary) - [x] No references to `resolved_unpack_toolchain` remain
jbedard
reviewed
Apr 5, 2026
| return """\ | ||
| load("@rules_python//python:py_runtime.bzl", "py_runtime") | ||
| load("@rules_python//python:py_runtime_pair.bzl", "py_runtime_pair") | ||
| load("@rules_python//python/private:py_exec_tools_toolchain.bzl", "py_exec_tools_toolchain") |
Member
There was a problem hiding this comment.
We should not be importing private stuff from rules_python, isn't this also exposed publicly?
gregmagolan
added a commit
that referenced
this pull request
Apr 5, 2026
…, drop min version to 1.0.0 (#913) ## Summary #903 bumped the minimum `rules_python` version from `1.0.0` to `1.9.0` to gain access to `PyExecToolsInfo.exec_runtime` (added in rules_python 1.9.0) and loaded `py_exec_tools_toolchain` from `rules_python`'s private API. This PR reverses the version bump by hoisting the necessary code into rules_py and removes all imports from `rules_python/private`. ## Changes **New `py/private/exec_tools/defs.bzl`** — hoisted from rules_python: - `PyExecToolsInfo` provider with only an `exec_runtime` field. We own the provider so we can include `exec_runtime` (the field added in 1.9.0) regardless of which rules_python version the user has. - `py_exec_tools_toolchain` rule — instantiated in each PBS interpreter repo; derives `exec_runtime` from `exec_interpreter[ToolchainInfo].py3_runtime` (the pre-1.9.0 equivalent). - `current_interpreter_executable` rule — resolves `PY_TOOLCHAIN` to create an exec-configured interpreter symlink; defaults for the above rule. **`py/private/toolchain/BUILD.bazel`** — added `exec_tools_toolchain_type` toolchain type. We declare our own type rather than reusing `@rules_python//python:exec_tools_toolchain_type`, so our `PyExecToolsInfo` never needs to satisfy rules_python's internal interface (e.g. the `precompiler` field that `maybe_precompile` checks for). **`py/private/toolchain/types.bzl`** — `EXEC_TOOLS_TOOLCHAIN` now points to `@aspect_rules_py//py/private/toolchain:exec_tools_toolchain_type`. **`py/private/interpreter/repository.bzl`** — generated interpreter BUILD files now load `py_exec_tools_toolchain` from `@aspect_rules_py//py/private/exec_tools:defs.bzl` and register under our own toolchain type instead of `@rules_python//python:exec_tools_toolchain_type`. **`uv/private/whl_install/rule.bzl`** — `EXEC_TOOLS_TOOLCHAIN` now loaded from our own `types.bzl`. **`MODULE.bazel` / `e2e/MODULE.bazel`** — `rules_python` and `rules_python_gazelle_plugin` dropped back to `1.0.0`. --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: aspect-marvin[bot] <marvin@aspect.build>
2 tasks
gregmagolan
added a commit
that referenced
this pull request
Apr 6, 2026
…oolchain (#924) ## Problem The BCR presubmit fails on Bazel 9 for `//cases/uv-deps-650/crossbuild:crossbuild_compile_pyc_test`: ``` While resolving toolchains for target //cases/uv-deps-650/crossbuild:app_bin: No matching toolchains found for types: @@bazel_tools//tools/python:toolchain_type ``` The toolchain resolution debug log shows why — when evaluating `app_bin` for an arm64 target on an x86_64 exec host, the arm64 Python interpreter toolchain is rejected with: ``` Rejected toolchain python_3_12_aarch64_unknown_linux_gnu; mismatching values: aarch64 ``` This is Bazel 9 enforcing `exec_compatible_with` strictly. The aarch64 Python interpreter was registered with `exec_compatible_with = ["@platforms//cpu:aarch64", ...]`, which doesn't match the x86_64 exec platform. Bazel 8 ignored this mismatch; Bazel 9 enforces it. ## Root cause Since the original [python-build-standalone](https://github.com/astral-sh/python-build-standalone) (PBS) interpreter commit (`b2feee5`), every Python interpreter toolchain has been registered with: ```python toolchain( name = "python_3_12_aarch64_unknown_linux_gnu", exec_compatible_with = ["@platforms//os:linux", "@platforms//cpu:aarch64"], # WRONG target_compatible_with = ["@platforms//os:linux", "@platforms//cpu:aarch64"], ... toolchain_type = "@bazel_tools//tools/python:toolchain_type", ) ``` Setting `exec_compatible_with` on a `@bazel_tools//tools/python:toolchain_type` toolchain is semantically wrong. The Python interpreter here is a **runtime dependency** — it ends up inside the virtualenv and runs on the **target machine**, not on the build host. The exec platform constraint is irrelevant and actively harmful during cross-compilation. This is distinct from `exec_tools_toolchain_type`, which represents the interpreter that runs **build actions** (e.g. `compileall` in `whl_install`). That toolchain correctly uses `exec_compatible_with` — it must run on the exec host, and #903 was specifically designed to handle this case properly. This fix does not touch that path. ## Fix Remove `exec_compatible_with` from the `@bazel_tools//tools/python:toolchain_type` registration. `target_compatible_with` alone is sufficient to select the right interpreter for the target platform. ## Test plan - [x] All e2e tests continue to pass (native builds unaffected — target constraint still selects the right interpreter) - [x] `//cases/uv-deps-650/crossbuild:crossbuild_compile_pyc_test` still passes (whl_install exec interpreter still resolved via `exec_tools_toolchain_type`) Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
rules_python(andrules_python_gazelle_plugin) from1.0.0to1.9.0resolved_py_toolchainworkaround inwhl_installwithEXEC_TOOLS_TOOLCHAIN_TYPEfrom rules_python, usingPyExecToolsInfo.exec_runtimeto get an exec-platform interpreter forcompile_pycactionsresolved_py_toolchainrule and its BUILD target frompy/private/toolchain/exec_tools_toolchain_typeregistrations in rules_py's interpreter provisioning soEXEC_TOOLS_TOOLCHAIN_TYPEresolves for all PBS-provisioned interpretersPossible breaking change
Minimum
rules_pythonversion is now1.9.0(previously1.0.0). Users must update theirMODULE.bazel:rules_python 1.9.0 should be generally compatible with 1.0.0 but we're calling this out here just incase it is not for some users.
Background
When
compile_pyc = True,whl_installneeds to runcompileallas a build action, which requires an interpreter that executes on the build host, not the target platform. In cross-arch builds (e.g. x86 host → arm64 target),ctx.toolchains[PY_TOOLCHAIN]gives the arm64 interpreter, which can't run on the host.The previous fix (#881) worked around this with a
resolved_py_toolchainindirection rule — acfg="exec"label attr that re-exported the Python toolchain in exec configuration. This was a correct workaround, but rules_python already has the proper solution:EXEC_TOOLS_TOOLCHAIN_TYPE(//python:exec_tools_toolchain_type), which is registered withexec_compatible_withset to the platform constraints so toolchain resolution naturally selects the host-runnable interpreter.PyExecToolsInfo.exec_runtime(the RBE-compatible field we need) was added in rules_python #3539, which landed in 1.9.0.rules_py's own interpreter provisioning (
py/private/interpreter/) previously only generatedTARGET_TOOLCHAIN_TYPEregistrations. This PR also generatesexec_tools_toolchain_typetoolchains for each PBS-provisioned interpreter so the newwhl_installdependency resolves correctly.Test plan
crossbuild_compile_pyc_testine2e/cases/uv-deps-650/crossbuild/builds arm64 wheels withcompile_pyc=Trueon the host platform — fails with "Exec format error" if the wrong interpreter is selected