feat(js_run_binary): add support for path mapping

MODULE.bazel

@@ -19,7 +19,8 @@ bazel_dep(name = "rules_nodejs", version = "6.7.3")
 
 # Changes ensured by rules_js:
 # 3.2.2: https://github.com/bazel-contrib/bazel-lib/commit/cac2d7855949d1b222fa26888892fbbe1d31015d
-bazel_dep(name = "bazel_lib", version = "3.2.2")
+# 3.7.0: https://github.com/bazel-contrib/bazel-lib/commit/e5c0630270f70dd2c7b50471bb9c872781a55282
+bazel_dep(name = "bazel_lib", version = "3.7.0")
 
 # NB: LOWER BOUND on earliest BCR release of protobuf module, to avoid upgrading the root module by accident
 bazel_dep(name = "protobuf", version = "3.19.6")

e2e/path_mapping/js_run_binary_path_mapping_check/BUILD.bazel

@@ -0,0 +1,26 @@
+load("@aspect_rules_js//js:defs.bzl", "js_binary", "js_run_binary")
+load("@bazel_skylib//rules:build_test.bzl", "build_test")
+
+# Verifies that a js_run_binary target supports path mapping. This requires
+# use_execroot_entry_point and set_legacy_environment_variables to both be
+# disabled, and args must avoid Make variable and location expansion.
+js_binary(
+    name = "check",
+    entry_point = "check.mjs",
+)
+
+js_run_binary(
+    name = "js_run_binary_path_mapping_check",
+    outs = ["out.txt"],
+    args = ["out.txt"],
+    chdir = package_name(),
+    mnemonic = "JsRunBinaryPathMappingCheck",
+    set_legacy_environment_variables = False,
+    tool = ":check",
+    use_execroot_entry_point = False,
+)
+
+build_test(
+    name = "js_run_binary_path_mapping_check_test",
+    targets = [":js_run_binary_path_mapping_check"],
+)

e2e/path_mapping/js_run_binary_path_mapping_check/check.mjs

@@ -0,0 +1,23 @@
+import { writeFileSync } from 'fs'
+
+const outFile = process.argv[2]
+if (!outFile) {
+    process.stderr.write('Usage: check.mjs <output-file>\n')
+    process.exit(1)
+}
+
+const bindir = process.env.BAZEL_BINDIR
+if (bindir !== 'bazel-out/cfg/bin') {
+    process.stderr.write(
+        `Expected BAZEL_BINDIR to be "bazel-out/cfg/bin", got "${bindir}"\n`
+    )
+    process.exit(1)
+}
+
+const leaked = process.argv.filter((arg) => arg === '--bazel-bindir')
+if (leaked.length > 0) {
+    process.stderr.write(`--bazel-bindir flag leaked into argv: ${leaked}\n`)
+    process.exit(1)
+}
+
+writeFileSync(outFile, 'OK\n')

e2e/path_mapping/test.sh

@@ -47,3 +47,31 @@ if [ "$cache_hit" != "true" ]; then
 fi
 
 echo "PASS: action was cache-shared across -c fastbuild and -c opt"
+
+# Same as above, but exercising a js_run_binary target instead of a custom rule
+# built on js_run_binary_action.
+exec_log2="$scratch/exec_log2.json"
+
+bazel build -c fastbuild //js_run_binary_path_mapping_check \
+    --disk_cache="$disk_cache" \
+    --action_env="JS_RUN_BINARY_PATH_MAPPING_CHECK_INVALIDATE=$invalidate"
+
+bazel build -c opt //js_run_binary_path_mapping_check \
+    --disk_cache="$disk_cache" \
+    --action_env="JS_RUN_BINARY_PATH_MAPPING_CHECK_INVALIDATE=$invalidate" \
+    --execution_log_json_file="$exec_log2"
+
+matches2="$(jq -s '[.[] | select(.mnemonic == "JsRunBinaryPathMappingCheck")]' "$exec_log2")"
+count2="$(echo "$matches2" | jq 'length')"
+if [ "$count2" -eq 0 ]; then
+    echo "FAIL: no JsRunBinaryPathMappingCheck entry found in the -c opt execution log" >&2
+    exit 1
+fi
+
+cache_hit2="$(echo "$matches2" | jq -r '.[0].cacheHit')"
+if [ "$cache_hit2" != "true" ]; then
+    echo "FAIL: js_run_binary action was re-executed under -c opt (cacheHit=$cache_hit2); path mapping did not share the cache entry from -c fastbuild" >&2
+    exit 1
+fi
+
+echo "PASS: js_run_binary action was cache-shared across -c fastbuild and -c opt"

js/private/js_run_binary.bzl

@@ -45,6 +45,7 @@ def js_run_binary(
         patch_node_fs = True,
         allow_execroot_entry_point_with_no_copy_data_to_bin = False,
         use_default_shell_env = False,
+        set_legacy_environment_variables = True,
         **kwargs):
     """Wrapper around @bazel_lib `run_binary` that adds convenience attributes for using a `js_binary` tool.
 
@@ -53,16 +54,16 @@ def js_run_binary(
     The following environment variables are made available to the Node.js runtime based on available Bazel [Make variables](https://bazel.build/reference/be/make-variables#predefined_variables):
 
     * BAZEL_BINDIR: the bazel bin directory; equivalent to the `$(BINDIR)` Make variable of the `js_run_binary` target
-    * BAZEL_COMPILATION_MODE: One of `fastbuild`, `dbg`, or `opt` as set by [`--compilation_mode`](https://bazel.build/docs/user-manual#compilation-mode); equivalent to `$(COMPILATION_MODE)` Make variable of the `js_run_binary` target
-    * BAZEL_TARGET_CPU: the target cpu architecture; equivalent to `$(TARGET_CPU)` Make variable of the `js_run_binary` target
+    * BAZEL_COMPILATION_MODE (legacy; see `set_legacy_environment_variables`): One of `fastbuild`, `dbg`, or `opt` as set by [`--compilation_mode`](https://bazel.build/docs/user-manual#compilation-mode); equivalent to `$(COMPILATION_MODE)` Make variable of the `js_run_binary` target
+    * BAZEL_TARGET_CPU (legacy; see `set_legacy_environment_variables`): the target cpu architecture; equivalent to `$(TARGET_CPU)` Make variable of the `js_run_binary` target
 
     The following environment variables are made available to the Node.js runtime based on the rule context:
 
-    * BAZEL_BUILD_FILE_PATH: the path to the BUILD file of the bazel target being run; equivalent to `ctx.build_file_path` of the `js_run_binary` target's rule context
-    * BAZEL_PACKAGE: the package of the bazel target being run; equivalent to `ctx.label.package` of the `js_run_binary` target's rule context
-    * BAZEL_TARGET_NAME: the full label of the bazel target being run; a stringified version of `ctx.label` of the `js_run_binary` target's rule context
-    * BAZEL_TARGET: the name of the bazel target being run; equivalent to `ctx.label.name` of the  `js_run_binary` target's rule context
-    * BAZEL_WORKSPACE: the bazel repository name; equivalent to `ctx.workspace_name` of the `js_run_binary` target's rule context
+    * BAZEL_BUILD_FILE_PATH (legacy; see `set_legacy_environment_variables`): the path to the BUILD file of the bazel target being run; equivalent to `ctx.build_file_path` of the `js_run_binary` target's rule context
+    * BAZEL_PACKAGE (legacy; see `set_legacy_environment_variables`): the package of the bazel target being run; equivalent to `ctx.label.package` of the `js_run_binary` target's rule context
+    * BAZEL_TARGET_NAME (legacy; see `set_legacy_environment_variables`): the full label of the bazel target being run; a stringified version of `ctx.label` of the `js_run_binary` target's rule context
+    * BAZEL_TARGET (legacy; see `set_legacy_environment_variables`): the name of the bazel target being run; equivalent to `ctx.label.name` of the  `js_run_binary` target's rule context
+    * BAZEL_WORKSPACE (legacy; see `set_legacy_environment_variables`): the bazel repository name; equivalent to `ctx.workspace_name` of the `js_run_binary` target's rule context
 
     Args:
         name: Target name
@@ -253,6 +254,13 @@ def js_run_binary(
 
             Refer to https://bazel.build/rules/lib/builtins/actions#run for more details.
 
+        set_legacy_environment_variables: Whether to set the legacy `BAZEL_BUILD_FILE_PATH`,
+            `BAZEL_COMPILATION_MODE`, `BAZEL_TARGET_CPU`, `BAZEL_TARGET`, `BAZEL_WORKSPACE`,
+            `BAZEL_PACKAGE` and `BAZEL_TARGET_NAME` environment variables.
+
+            These variables are deprecated and setting them will default to False in a future
+            release. Set this to False to opt out now.
+
         **kwargs: Additional arguments
     """
 
@@ -296,17 +304,18 @@ def js_run_binary(
         )
         extra_srcs.append(":{}".format(copy_to_bin_name))
 
-    # Automatically add common and useful make variables to the environment for js_run_binary build targets
-    fixed_env = {
-        "BAZEL_BINDIR": "$(BINDIR)",
-        "BAZEL_BUILD_FILE_PATH": "$(BUILD_FILE_PATH)",
-        "BAZEL_COMPILATION_MODE": "$(COMPILATION_MODE)",
-        "BAZEL_PACKAGE": native.package_name(),
-        "BAZEL_TARGET_CPU": "$(TARGET_CPU)",
-        "BAZEL_TARGET_NAME": name,
-        "BAZEL_TARGET": "$(TARGET)",
-        "BAZEL_WORKSPACE": "$(WORKSPACE)",
-    }
+    fixed_env = {}
+
+    # These environment variables are deprecated and will default to not being set in a future
+    # release; see the `set_legacy_environment_variables` docstring.
+    if set_legacy_environment_variables:
+        fixed_env["BAZEL_BUILD_FILE_PATH"] = "$(BUILD_FILE_PATH)"
+        fixed_env["BAZEL_COMPILATION_MODE"] = "$(COMPILATION_MODE)"
+        fixed_env["BAZEL_TARGET_CPU"] = "$(TARGET_CPU)"
+        fixed_env["BAZEL_TARGET"] = "$(TARGET)"
+        fixed_env["BAZEL_WORKSPACE"] = "$(WORKSPACE)"
+        fixed_env["BAZEL_PACKAGE"] = native.package_name()
+        fixed_env["BAZEL_TARGET_NAME"] = name
 
     # Configure working directory to `chdir` is set
     if chdir != None:
@@ -422,7 +431,7 @@ See https://github.com/aspect-build/rules_js/tree/main/docs#using-binaries-publi
         srcs = srcs + extra_srcs + execroot_extra_srcs,
         outs = outs + extra_outs,
         out_dirs = out_dirs,
-        args = args,
+        args = ["--bazel-bindir", "$(BINDIR)"] + args,
         mnemonic = mnemonic,
         progress_message = progress_message,
         execution_requirements = execution_requirements,