fix(py): stamp py_pex_binary interpreter constraints from the binary's own interpreter#1344
Conversation
✨ Aspect Workflows Tasks📅 Wed Jul 22 21:21:52 UTC 2026 ✅ 40 successful tasks
⏱ Last updated Wed Jul 22 21:26:31 UTC 2026 · 📊 GitHub API quota 244/15,000 (2% used, resets in 56m) |
py_binary startup benchmark
sys.path quality
Bazel analysis benchmark
Auxiliary metrics
|
bf4e90a to
5e98541
Compare
|
@tamird (the robot) please check if there are any concerns with the proposed 2.x breaking changes here |
tamird
left a comment
tamird
left a comment
There was a problem hiding this comment.
The version transition is sound, but I found four correctness gaps around free-threaded interpreters, interpreter-file filtering, arbitrary runtime-data rules, and the documented py_venv input. OpenAI has no current py_pex_binary consumers; these affect the upstream API contract.
5e98541 to
a100f12
Compare
tamird
left a comment
tamird
left a comment
There was a problem hiding this comment.
Re-reviewed because Jason requested another pass in #ext-aspect-build. The current head still has the four correctness gaps from the prior review; the provider/aspect source files are byte-identical to the previously reviewed head, and the only changed PR file is a stronger PEX-content test.
Free-threaded binaries can still advertise an ABI-agnostic constraint with a pinned PEX version that predates the required constraint syntax; filtering still drops whole external interpreter repositories while retaining a supported main-repository interpreter; scalar/non-list data attributes can still fail aspect analysis; and the documented direct py_venv input still fails the single-entrypoint invariant. The original inline anchors remain current:
#1344 (comment)
#1344 (comment)
#1344 (comment)
#1344 (comment)
The version-transition regression and current CI are otherwise sound. These API-contract issues should be resolved or the supported surface narrowed before 2.x.
— tamirdex
tamird
left a comment
tamird
left a comment
There was a problem hiding this comment.
Two additional issues from the cumulative packaging pass:
A wheel behind the common filegroup(srcs=[...]) data wrapper is no longer recognized: the closure aspect walks only deps/data/actual/venv, so the expanded install tree is sent as loose --source content instead of a PEX distribution. The previous runfile/site-packages classifier handled this case. Please cover a wheel-under-filegroup input and either traverse label-bearing srcs/propagate the provider or retain a safe fallback (py/private/py_pex_binary.bzl:114-123,187-211).
The new source mapper also compares every expanded runfile child with every wheel-tree prefix, making packaging O(files x wheels). Large PEXes can have both many wheels and many installed files; use structural filtering or an indexed lookup before this becomes the 2.x path.
— tamirdex
cd0f0c9 to
2defdec
Compare
2defdec to
6e719c6
Compare
tamird
left a comment
tamird
left a comment
There was a problem hiding this comment.
Re-reviewed at Jason’s explicit request. The updated head fixes the earlier scalar-attribute and filegroup-wheel handling and correctly derives the interpreter version from the packaged binary.
One blocker remains in the existing ABI discussion: #1344 (comment). interpreter_files_and_version drops the interpreter ABI, so a free-threaded 3.13 binary containing cp313t extensions is advertised as CPython==3.13.*; PEX may then select ordinary cp313 and fail at runtime. The tool is still pinned to PEX 2.3.1, which predates Python 3.13 and free-threaded interpreter support. The added regression only inspects PEX-INFO and does not execute a free-threaded native-wheel PEX.
Please propagate the ABI and upgrade PEX to a version supporting Python 3.13 free-threaded constraints, or explicitly reject unsupported free-threaded inputs, and add a real cp313t execution regression. Exact-head CI is still pending.
— tamirdex
3 participants
Changes are visible to end-users: yes
No API changes. py_pex_binary no longer resolves a Python toolchain itself.
Test plan