fix(packaging): build the .publish twine binary for the exec platform#3909
fix(packaging): build the .publish twine binary for the exec platform#3909rdelfin wants to merge 1 commit into
Conversation
The .publish target generated by py_wheel wraps twine in a native_binary whose src attribute uses cfg = "target". When the build uses a cross-compilation platform (e.g. --platforms=linux_aarch64), this causes bazel to resolve twine for the target platform rather than the host, which fails because rules_python_publish_deps only provides wheels for the host platform. Twine is a developer tool that uploads wheels to PyPI; it only ever runs on the developer's workstation and should always be built for the exec platform regardless of the target. Replace the native_binary call with a minimal inline rule, _py_wheel_publish_binary, that is identical except its src attribute uses cfg = "exec".
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request replaces the usage of bazel_skylib's native_binary with a custom _py_wheel_publish_binary rule in python/packaging.bzl to ensure the twine binary is built for the execution platform. The review feedback suggests adding allow_files = True to the src attribute of the new rule to maintain compatibility with native_binary and prevent analysis-phase errors when passing file targets.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| "src": attr.label( | ||
| executable = True, | ||
| mandatory = True, | ||
| cfg = "exec", | ||
| ), |
There was a problem hiding this comment.
To ensure full compatibility with bazel_skylib's native_binary and allow passing pre-built binary files directly as src, the src attribute should explicitly set allow_files = True. Without this, passing a file target to src will result in an analysis-phase error.
| "src": attr.label( | |
| executable = True, | |
| mandatory = True, | |
| cfg = "exec", | |
| ), | |
| "src": attr.label( | |
| executable = True, | |
| allow_files = True, | |
| mandatory = True, | |
| cfg = "exec", | |
| ), |
There was a problem hiding this comment.
Pull request overview
This PR aims to make the .publish target generated by py_wheel reliably runnable during cross-compilation by ensuring the twine executable is built for the exec (host) platform, not the target platform.
Changes:
- Remove the
@bazel_skylibnative_binarydependency frompython/packaging.bzl. - Add an inline
_py_wheel_publish_binaryrule intended to mirrornative_binary, but withsrcbuilt usingcfg = "exec". - Switch the
.publishtarget generation inpy_wheel()to use_py_wheel_publish_binary.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| _py_wheel_publish_binary = rule( | ||
| implementation = _py_wheel_publish_binary_impl, | ||
| executable = True, | ||
| attrs = { | ||
| "src": attr.label( | ||
| executable = True, | ||
| mandatory = True, | ||
| cfg = "exec", | ||
| ), | ||
| "data": attr.label_list(allow_files = True), | ||
| "out": attr.string(), | ||
| }, |
aignas
left a comment
aignas
left a comment
There was a problem hiding this comment.
+1 on the direction here and thank you for the fix.
We have lot's of analysis tests in the tests folder and it would be very useful to add a regression test for this. With AI help it should be very doable.
That and please add a news/3909.fixed.md file summarising the fixes here.
| **copy_propagating_kwargs(kwargs) | ||
| ) | ||
| elif twine: | ||
| py_binary( |
There was a problem hiding this comment.
Don't we need to update this as well?
The .publish target generated by py_wheel wraps twine in a native_binary whose src attribute uses cfg = "target". When the build uses a cross-compilation platform (e.g. --platforms=linux_aarch64), this causes bazel to resolve twine for the target platform rather than the host, which fails because rules_python_publish_deps only provides wheels for the host platform.
Twine is a developer tool that uploads wheels to PyPI; it only ever runs on the developer's workstation and should always be built for the exec platform regardless of the target.
Replace the native_binary call with a minimal inline rule, _py_wheel_publish_binary, that is identical except its src attribute uses cfg = "exec".