Conversation
PR bazel-contrib#4151 added a Rust implementation of `exe_zip_maker`, but nothing used it. This wires it in for rules_python development only, leaving downstream users on the Python implementation. A new optional toolchain type, `//python/private/toolchain_types:exe_zip_maker`, is consulted by `py_binary`/`py_test` and `py_zipapp_*` when creating self-executable zips. If no toolchain is resolved (or it provides no tool), the rules fall back to the existing `_exe_zip_maker` attribute, so users and WORKSPACE mode need no new registration and see no behavior change. When the toolchain's tool is used, the action is bound to that toolchain type so it runs on the exec platform the tool was built for. A dev-only toolchain in `dev/dev_only_toolchains/` points at `//crates/exe_zip_maker` and is registered with `dev_dependency = True`. The `toolchain()` and its implementation live in separate packages so registration doesn't load the implementation. It is gated behind `--//dev/dev_only_toolchains:use_rust_exe_zip_maker`, which defaults to off. Analysis tests verify the Rust tool is selected when the flag is on and the Python fallback is used when off. Work towards bazel-contrib#4151
Use a string flag so `auto` can later let rules_python decide; for now `auto` behaves as `no`. Also wrap a long load() line.
The `--build_python_zip` path in py_executable is deprecated, so there's no need to wire the toolchain into py_binary/py_test. Restrict the toolchain lookup to the py_zipapp rules and adjust the tests to cover both flag states through py_zipapp instead.
`//python/private:distribution` auto-discovers subpackages and expects each to declare a `:distribution` filegroup. The new `toolchain_types` package lacked one, causing loading-phase errors in every CI job that builds `//...`.
On Windows, py_zipapp emits the Bazel launcher instead of a self-executable zip, so the PyZipAppCreateExecutableZip action the tests assert on never exists there.
Collaborator
Author
|
Ready for review. This is just the basics to use it via a toolchain and lay the groundwork for productionization. |
This branch has not been deployed
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
1 participant
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.
PR #4151 added a Rust implementation of
exe_zip_maker, but nothing usedit. This wires it in as dev-only, from-source toolchain so it can be
used and tested. For now, usage is gated by a private flag.
A new optional toolchain type,
//python/private/toolchain_types:exe_zip_maker, is consulted by thepy_zipapp_*rules. If not found, the Python based tool is used.Analysis tests verify the Rust tool is selected when the flag is on and
the Python fallback is used when off.
Work towards #4216