diff --git a/MODULE.bazel b/MODULE.bazel index 2b47c33b2..16e891975 100644 --- a/MODULE.bazel +++ b/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") diff --git a/e2e/path_mapping/js_run_binary_path_mapping_check/BUILD.bazel b/e2e/path_mapping/js_run_binary_path_mapping_check/BUILD.bazel new file mode 100644 index 000000000..183c116e4 --- /dev/null +++ b/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"], +) diff --git a/e2e/path_mapping/js_run_binary_path_mapping_check/check.mjs b/e2e/path_mapping/js_run_binary_path_mapping_check/check.mjs new file mode 100644 index 000000000..076912720 --- /dev/null +++ b/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 \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') diff --git a/e2e/path_mapping/test.sh b/e2e/path_mapping/test.sh index a7f6e782f..ed18fdd8e 100755 --- a/e2e/path_mapping/test.sh +++ b/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" diff --git a/js/private/js_run_binary.bzl b/js/private/js_run_binary.bzl index 538c5a121..40ae93b4f 100644 --- a/js/private/js_run_binary.bzl +++ b/js/private/js_run_binary.bzl @@ -45,19 +45,17 @@ 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. This rule does not require Bash `native.genrule`. - 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): + The following environment variables are made available to the Node.js runtime if set_legacy_environment_variables is enabled. They are deprecated and will be removed in a future release: * 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 - - 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 @@ -253,6 +251,14 @@ 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_BINDIR`, + `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 +302,19 @@ 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_BINDIR"] = "$(BINDIR)" + 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 +430,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,