feat(js_run_binary): add support for path mapping - #2962
Merged
Conversation
acozzette
marked this pull request as ready for review
August 1, 2026 23:27
jbedard
reviewed
Aug 2, 2026
jbedard
reviewed
Aug 2, 2026
jbedard
reviewed
Aug 3, 2026
This change adds support for path mapping in `js_run_binary` by updating it to use the new `--bazel-bindir` flag instead of setting the `BAZEL_BINDIR` environment variable. The only mechanism Bazel provides for determining path-mapped paths is by adding inputs directly to an `Args` object from `ctx.actions.args()`. We can now compute the output Bazel bin directory that way using a special case added to `run_binary()` for this purpose in bazel-contrib/bazel-lib#1269. The `js_run_binary` macro currently sets several other problematic environment variables: BAZEL_BUILD_FILE_PATH, BAZEL_COMPILATION_MODE, BAZEL_TARGET_CPU, BAZEL_TARGET, and BAZEL_WORKSPACE. These are determined by evaluating Make variables, and `run_binary` conservatively assumes that path mapping is not safe if any location or Make variable expansion occurs. This change deprecates these variables and adds a `set_legacy_environment_variables` parameter (default `True`) so they can be disabled without a breaking change, ahead of flipping the default in a future release. To enable path mapping on a particular `js_run_binary` target, make sure to set this parameter to `False`. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Verify these js_run_binary env vars, now computed directly in the macro instead of via Make-variable substitution, are correct when the js_run_binary target lives in a module other than the main one.
The expected BAZEL_TARGET value hardcoded the canonical repo name's
separator ("workspace+"), which differs across Bazel versions
("workspace~" on Bazel 7, "workspace+" on Bazel 9+), causing the test
to fail under Bazel 7.7.1. Derive it via repo_name() instead.
…t_variables All seven js_run_binary-provided BAZEL_* env vars are now gated behind the same flag, for consistency.
jbedard
reviewed
Aug 3, 2026
jbedard
reviewed
Aug 3, 2026
jbedard
reviewed
Aug 4, 2026
jbedard
reviewed
Aug 4, 2026
thesayyn
reviewed
Aug 4, 2026
thesayyn
approved these changes
Aug 4, 2026
jbedard
approved these changes
Aug 4, 2026
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.
This change adds support for path mapping in
js_run_binaryby updating it to use the new--bazel-bindirflag instead of setting theBAZEL_BINDIRenvironment variable. The only mechanism Bazel provides for determining path-mapped paths is by adding inputs directly to anArgsobject fromctx.actions.args(). We can now compute the output Bazel bin directory that way using a special case added torun_binary()for this purpose in bazel-contrib/bazel-lib#1269.The
js_run_binarymacro currently sets several other problematic environment variables: BAZEL_BUILD_FILE_PATH, BAZEL_COMPILATION_MODE, BAZEL_TARGET_CPU, BAZEL_TARGET, BAZEL_WORKSPACE, BAZEL_PACKAGE, and BAZEL_TARGET_NAME. Most of these are determined by evaluating Make variables, andrun_binaryconservatively assumes that path mapping is not safe if any location or Make variable expansion occurs. This change guards those variables behind a newset_legacy_environment_variablesparameter (defaultTrue) so they can be disabled without a breaking change, ahead of flipping the default in a future release. To enable path mapping on a particularjs_run_binarytarget, make sure to set this parameter toFalse.Changes are visible to end-users: yes
Support for path mapping in
js_run_binaryTest plan