From bb975a4474a97337380724fee9d2c71850b5fae5 Mon Sep 17 00:00:00 2001 From: Adam Cozzette Date: Thu, 30 Apr 2026 09:55:50 -0700 Subject: [PATCH 1/7] fix: allow js_run_binary to hoist runfiles in the exec configuration The `js_run_binary` macro has a `use_execroot_entry_point` option which is enabled by default and which hoists out the runfiles into the execroot. Currently this hoisting effectively rebuilds the runfiles in the target platform config, but they should really be in the exec config since they consist of code that is going to run during the build action. This change fixes that problem by making sure we hoist the runfiles in the exec config. I also updated the launcher script, since it needs to be prepared to look for the entry point in a different directory. This is all guarded by the `hoist_runfiles_to_exec_cfg` option on `js_run_binary`, which effectively defaults to False for now. The default is actually `None` but only for technical reasons. The default is controlled by a new flag `//js:_hoist_runfiles_to_exec_cfg`, but this is intended only for our own CI purposes, allowing us to run most tests against both behaviors. There are a handful of `js_run_binary` targets that require one behavior or the other, so I updated those ones to explicitly specify the flag they need. This addresses #2754, #2121, and #2073, but does not automatically fix them yet since the fix is opt-in for now. Co-Authored-By: Claude Sonnet 4.6 --- .aspect/workflows/config.yaml | 12 +- MODULE.bazel | 2 +- contrib/nextjs/defs.bzl | 2 + .../npm_translate_lock_LTE4Nzc1MDcwNjU= | 2 +- js/BUILD.bazel | 15 ++ js/private/js_binary.sh.tpl | 12 +- js/private/js_run_binary.bzl | 30 +++- js/private/test/data/BUILD.bazel | 30 ++-- js/private/test/image/asserts.bzl | 2 +- js/private/test/js_run_binary/BUILD.bazel | 145 ++++++++++++++++++ .../test/js_run_binary/find_cfg_probe.mjs | 32 ++++ js/private/test/js_run_binary/read_file.mjs | 4 + js/private/test/snapshots/launcher.sh | 12 +- 13 files changed, 275 insertions(+), 25 deletions(-) create mode 100644 js/private/test/js_run_binary/BUILD.bazel create mode 100644 js/private/test/js_run_binary/find_cfg_probe.mjs create mode 100644 js/private/test/js_run_binary/read_file.mjs diff --git a/.aspect/workflows/config.yaml b/.aspect/workflows/config.yaml index ffb5c70b24..9fbfb262f4 100644 --- a/.aspect/workflows/config.yaml +++ b/.aspect/workflows/config.yaml @@ -123,7 +123,15 @@ workspaces: tasks: *e2e_tasks e2e/repo_mapping: icon: js - tasks: *e2e_tasks + tasks: + - test: + queue: aspect-medium + - format: + without: true + - buildifier: + without: true + - bazel-7: + without: true e2e/output_paths: icon: js tasks: *e2e_tasks @@ -154,7 +162,7 @@ tasks: name: Bazel 7 id: bazel-7 bazel: - flags: ['--test_tag_filters=-skip-on-bazel7'] + flags: ['--test_tag_filters=-skip-on-bazel7', '--@aspect_rules_js//js:_hoist_runfiles_to_exec_cfg=true'] env: USE_BAZEL_VERSION: '7.x' - test: diff --git a/MODULE.bazel b/MODULE.bazel index 493e46ff0b..3aaf1f18c1 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -19,7 +19,7 @@ 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") +bazel_dep(name = "bazel_lib", version = "3.3.1") # 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/contrib/nextjs/defs.bzl b/contrib/nextjs/defs.bzl index e6e1e216ae..f7b21a33c5 100644 --- a/contrib/nextjs/defs.bzl +++ b/contrib/nextjs/defs.bzl @@ -202,6 +202,7 @@ def nextjs_build(name, config, srcs, next_js_binary, data = [], **kwargs): chdir = native.package_name(), mnemonic = "NextJs", progress_message = "Compile Next.js app %{label}", + hoist_runfiles_to_exec_cfg = False, **kwargs ) @@ -327,6 +328,7 @@ def nextjs_standalone_build(name, config, srcs, next_js_binary, data = [], **kwa chdir = native.package_name(), mnemonic = "NextJs", progress_message = "Compile Next.js standalone app %{label}", + hoist_runfiles_to_exec_cfg = False, **kwargs ) diff --git a/examples/.aspect/rules/external_repository_action_cache/npm_translate_lock_LTE4Nzc1MDcwNjU= b/examples/.aspect/rules/external_repository_action_cache/npm_translate_lock_LTE4Nzc1MDcwNjU= index cf3c2845e0..432032fa13 100755 --- a/examples/.aspect/rules/external_repository_action_cache/npm_translate_lock_LTE4Nzc1MDcwNjU= +++ b/examples/.aspect/rules/external_repository_action_cache/npm_translate_lock_LTE4Nzc1MDcwNjU= @@ -20,7 +20,7 @@ npm_package/packages/pkg_d/package.json=1146506398 npm_package/packages/pkg_e/package.json=2046864123 package.json=-2075121688 package_json_module/package.json=-1167380556 -pnpm-lock.yaml=-657827575 +pnpm-lock.yaml=559561144 pnpm-workspace.yaml=-1653994035 runfiles/package.json=-1545884645 stack_traces/package.json=2011229626 diff --git a/js/BUILD.bazel b/js/BUILD.bazel index 7fab92fbbe..cd3accf894 100644 --- a/js/BUILD.bazel +++ b/js/BUILD.bazel @@ -1,6 +1,21 @@ "Public API" load("@bazel_skylib//:bzl_library.bzl", "bzl_library") +load("@bazel_skylib//rules:common_settings.bzl", "bool_flag") + +# Underscore prefix signals this is internal — do not rely on it. +# This controls the default value of the hoist_runfiles_to_exec_cfg flag on +# js_run_binary, and is for CI testing only. +bool_flag( + name = "_hoist_runfiles_to_exec_cfg", + build_setting_default = False, + visibility = ["//visibility:public"], +) + +config_setting( + name = "_hoist_runfiles_to_exec_cfg_true", + flag_values = {"_hoist_runfiles_to_exec_cfg": "True"}, +) bzl_library( name = "defs", diff --git a/js/private/js_binary.sh.tpl b/js/private/js_binary.sh.tpl index ad99a00047..4a8f3d6da1 100644 --- a/js/private/js_binary.sh.tpl +++ b/js/private/js_binary.sh.tpl @@ -97,10 +97,18 @@ function logf_debug { function resolve_execroot_bin_path { local short_path="$1" + local suffix if [[ "$short_path" == ../* ]]; then - echo "$JS_BINARY__EXECROOT/${BAZEL_BINDIR:-$JS_BINARY__BINDIR}/external/${short_path:3}" + suffix="external/${short_path:3}" else - echo "$JS_BINARY__EXECROOT/${BAZEL_BINDIR:-$JS_BINARY__BINDIR}/$short_path" + suffix="$short_path" + fi + local exec_path="$JS_BINARY__EXECROOT/$JS_BINARY__BINDIR/$suffix" + local target_path="$JS_BINARY__EXECROOT/${BAZEL_BINDIR:-$JS_BINARY__BINDIR}/$suffix" + if [ -e "$exec_path" ]; then + echo "$exec_path" + else + echo "$target_path" fi } diff --git a/js/private/js_run_binary.bzl b/js/private/js_run_binary.bzl index 844da7f4c1..ce5995f86a 100644 --- a/js/private/js_run_binary.bzl +++ b/js/private/js_run_binary.bzl @@ -31,6 +31,7 @@ def js_run_binary( exit_code_out = None, silent_on_success = True, use_execroot_entry_point = True, + hoist_runfiles_to_exec_cfg = None, copy_srcs_to_bin = True, include_sources = True, include_types = False, @@ -149,6 +150,17 @@ def js_run_binary( needed by the binary are available in the execroot output tree. This requirement can be turned off with by setting `allow_execroot_entry_point_with_no_copy_data_to_bin` to True. + hoist_runfiles_to_exec_cfg: Hoist runfiles to the execution platform configuration instead of the target platform + configuration. + + This only has an effect when `use_execroot_entry_point` is True. Setting `hoist_runfiles_to_exec_cfg` to True + results in a more correct build by ensuring that code that is executed during a build action is built for the + execution platform, which may not be the same as the target platform. + + When set to `None` (the default), the value of the internal `//js:_hoist_runfiles_to_exec_cfg` build flag + is used. That flag is for internal testing only and should not be used. It is temporary and will be removed once + the default is changed to `True`. + copy_srcs_to_bin: When True, all srcs files are copied to the output tree that are not already there. include_sources: see `js_info_files` documentation @@ -246,6 +258,7 @@ def js_run_binary( fail("Use srcs instead of deps in js_run_binary: https://docs.aspect.build/rules/aspect_rules_js/docs/js_run_binary#srcs") extra_srcs = [] + tools = [] # Hoist js provider files to DefaultInfo make_js_info_files_target = (include_transitive_sources or @@ -380,7 +393,21 @@ See https://github.com/aspect-build/rules_js/tree/main/docs#using-binaries-publi # Always propagate the testonly attribute testonly = kwargs.get("testonly", False), ) - extra_srcs.append(":{}".format(js_runfiles_name)) + runfiles_label = ":{}".format(js_runfiles_name) + if hoist_runfiles_to_exec_cfg: + tools.append(runfiles_label) + elif hoist_runfiles_to_exec_cfg == False: + extra_srcs.append(runfiles_label) + else: + # hoist_runfiles_to_exec_cfg is None, so we will defer to the flag. + tools = tools + select({ + "@aspect_rules_js//js:_hoist_runfiles_to_exec_cfg_true": [runfiles_label], + "//conditions:default": [], + }) + extra_srcs = extra_srcs + select({ + "@aspect_rules_js//js:_hoist_runfiles_to_exec_cfg_true": [], + "//conditions:default": [runfiles_label], + }) if allow_execroot_entry_point_with_no_copy_data_to_bin: fixed_env["JS_BINARY__ALLOW_EXECROOT_ENTRY_POINT_WITH_NO_COPY_DATA_TO_BIN"] = "1" @@ -388,6 +415,7 @@ See https://github.com/aspect-build/rules_js/tree/main/docs#using-binaries-publi _run_binary( name = name, tool = tool, + tools = tools, env = fixed_env | env, srcs = srcs + extra_srcs, outs = outs + extra_outs, diff --git a/js/private/test/data/BUILD.bazel b/js/private/test/data/BUILD.bazel index 814afb8ed9..1c49cefbea 100644 --- a/js/private/test/data/BUILD.bazel +++ b/js/private/test/data/BUILD.bazel @@ -117,12 +117,12 @@ js_test( # TEST: js_test(data = js_run_binary(srcs)) --------------- js_run_binary( name = "run-write-srcs", + srcs = ["data.json"], outs = ["rb0.json"], args = [ - "rb0.json", - "data.json", + "$(rootpath rb0.json)", + "$(rootpath data.json)", ], - chdir = package_name(), tool = ":write-js_library-srcs", ) @@ -142,12 +142,12 @@ js_test( # TEST: js_test(data = js_run_binary(srcs = js_library(srcs))) ---------------- js_run_binary( name = "run-write-js_library-srcs", + srcs = ["data.json"], outs = ["rb1.json"], args = [ - "rb1.json", - "data.json", + "$(rootpath rb1.json)", + "$(rootpath data.json)", ], - chdir = package_name(), tool = ":write-js_library-srcs", ) @@ -174,12 +174,12 @@ js_binary( js_run_binary( name = "run-write-js_library-data", + srcs = ["data.json"], outs = ["rb2.json"], args = [ - "rb2.json", - "data.json", + "$(rootpath rb2.json)", + "$(rootpath data.json)", ], - chdir = package_name(), tool = ":write-js_library-data", ) @@ -199,12 +199,12 @@ js_test( # TEST: js_test(data = js_run_binary(srcs = genrule())) ----------------------- js_run_binary( name = "run-write-generated", + srcs = ["data-generated.json"], outs = ["rb3.json"], args = [ - "rb3.json", - "data-generated.json", + "$(rootpath rb3.json)", + "$(rootpath data-generated.json)", ], - chdir = package_name(), tool = ":write-genrule", ) @@ -224,12 +224,12 @@ js_test( # TEST: js_test(data = js_run_binary(srcs = genrule(srcs = filegroup))) ------- js_run_binary( name = "run-write-generated-copied", + srcs = ["data-copied.json"], outs = ["rb4.json"], args = [ - "rb4.json", - "data-copied.json", + "$(rootpath rb4.json)", + "$(rootpath data-copied.json)", ], - chdir = package_name(), tool = ":write-genrule-filegroup-srcs", ) diff --git a/js/private/test/image/asserts.bzl b/js/private/test/image/asserts.bzl index 87e42207cc..7b988f29ae 100644 --- a/js/private/test/image/asserts.bzl +++ b/js/private/test/image/asserts.bzl @@ -6,7 +6,7 @@ load("//js:defs.bzl", "js_image_layer") # buildifier: disable=function-docstring def assert_tar_listing(name, actual, expected): # Either of these two file sizes may be observed on a file like /js/private/test/image/bin - sanitize_cmd = "sed -E 's/239[0-9]{2}|24[0-9]{3}/xxxxx/g'" + sanitize_cmd = "sed -E 's/250[0-9]{2}|24[0-9]{3}/xxxxx/g'" actual_listing = "_{}_listing".format(name) native.genrule( name = actual_listing, diff --git a/js/private/test/js_run_binary/BUILD.bazel b/js/private/test/js_run_binary/BUILD.bazel new file mode 100644 index 0000000000..c4d699af3e --- /dev/null +++ b/js/private/test/js_run_binary/BUILD.bazel @@ -0,0 +1,145 @@ +load("@bazel_lib//lib:diff_test.bzl", "diff_test") +load("@bazel_lib//lib:testing.bzl", "assert_contains") +load("@bazel_lib//lib:transitions.bzl", "platform_transition_filegroup") +load("@bazel_skylib//rules:write_file.bzl", "write_file") +load("//js:defs.bzl", "js_binary", "js_run_binary") + +# Verify that the js_binary tool's data dependencies are built in the exec configuration +# when hoist_runfiles_to_exec_cfg is True. +# To do this we check that js_run_binary agrees with genrule on the location of a +# tool dependency. +write_file( + name = "gen_cfg_probe", + out = "cfg_probe.txt", + content = ["probe"], +) + +js_binary( + name = "find_cfg_probe", + data = [":cfg_probe.txt"], + entry_point = "find_cfg_probe.mjs", +) + +js_run_binary( + name = "tools_cfg_location", + hoist_runfiles_to_exec_cfg = True, + stdout = "tools_cfg_location.txt", + tool = ":find_cfg_probe", +) + +genrule( + name = "expected_tools_cfg_location", + outs = ["expected_tools_cfg_location.txt"], + cmd = "echo $(execpath :cfg_probe.txt) > $@", + tools = [":cfg_probe.txt"], +) + +diff_test( + name = "tools_exec_cfg_test", + file1 = ":tools_cfg_location.txt", + file2 = ":expected_tools_cfg_location.txt", +) + +# Verify the opposite: with hoist_runfiles_to_exec_cfg = False, the tool's data +# dependencies are built in the target configuration, matching a genrule srcs path. +js_run_binary( + name = "target_cfg_location", + hoist_runfiles_to_exec_cfg = False, + stdout = "target_cfg_location.txt", + tool = ":find_cfg_probe", +) + +genrule( + name = "expected_target_cfg_location", + srcs = [":cfg_probe.txt"], + outs = ["expected_target_cfg_location.txt"], + cmd = "echo $(execpath :cfg_probe.txt) > $@", +) + +diff_test( + name = "target_cfg_test", + file1 = ":target_cfg_location.txt", + file2 = ":expected_target_cfg_location.txt", +) + +# Verify that dependencies passed via the srcs parameter are always built for +# the target platform, unlike the tool parameter and its dependencies. +# We use a select() on the OS to produce a platform-specific file, then +# transition to three different target platforms and assert the contents match. +write_file( + name = "gen_os_name", + out = "os_name.txt", + content = select({ + "@platforms//os:linux": ["linux"], + "@platforms//os:macos": ["macos"], + "@platforms//os:windows": ["windows"], + "//conditions:default": ["other"], + }), + tags = ["manual"], +) + +platform( + name = "linux", + constraint_values = ["@platforms//os:linux"], +) + +platform( + name = "macos", + constraint_values = ["@platforms//os:macos"], +) + +platform( + name = "windows", + constraint_values = ["@platforms//os:windows"], +) + +js_binary( + name = "read_file", + entry_point = "read_file.mjs", +) + +js_run_binary( + name = "gen_target_os", + srcs = [":os_name.txt"], + args = ["$(rootpath :os_name.txt)"], + hoist_runfiles_to_exec_cfg = True, + stdout = "target_os.txt", + tags = ["manual"], + tool = ":read_file", +) + +platform_transition_filegroup( + name = "target_os_linux", + srcs = [":gen_target_os"], + target_platform = ":linux", +) + +platform_transition_filegroup( + name = "target_os_macos", + srcs = [":gen_target_os"], + target_platform = ":macos", +) + +platform_transition_filegroup( + name = "target_os_windows", + srcs = [":gen_target_os"], + target_platform = ":windows", +) + +assert_contains( + name = "linux_target_platform_test", + actual = ":target_os_linux", + expected = "linux", +) + +assert_contains( + name = "macos_target_platform_test", + actual = ":target_os_macos", + expected = "macos", +) + +assert_contains( + name = "windows_target_platform_test", + actual = ":target_os_windows", + expected = "windows", +) diff --git a/js/private/test/js_run_binary/find_cfg_probe.mjs b/js/private/test/js_run_binary/find_cfg_probe.mjs new file mode 100644 index 0000000000..5acea9a483 --- /dev/null +++ b/js/private/test/js_run_binary/find_cfg_probe.mjs @@ -0,0 +1,32 @@ +// The purpose of this binary is to determine where the generated file +// cfg_probe.txt ended up. We look for it in both the exec and target bin +// directories and output the path where it appears. +import fs from 'fs' +import path from 'path' + +const suffix = 'js/private/test/js_run_binary/cfg_probe.txt' +const execCfgRelPath = path.join(process.env.JS_BINARY__BINDIR, suffix) +const targetCfgRelPath = path.join(process.env.BAZEL_BINDIR, suffix) + +function fileExists(absPath) { + try { + fs.lstatSync(absPath) + return true + } catch (_) { + return false + } +} + +const foundInExecCfg = fileExists(path.join(process.env.JS_BINARY__EXECROOT, execCfgRelPath)) +const foundInTargetCfg = fileExists(path.join(process.env.JS_BINARY__EXECROOT, targetCfgRelPath)) + +if (foundInExecCfg && foundInTargetCfg) { + process.stderr.write('cfg_probe.txt unexpectedly found in both exec cfg and target cfg paths\n') + process.exit(1) +} +if (!foundInExecCfg && !foundInTargetCfg) { + process.stderr.write('cfg_probe.txt not found in either exec cfg or target cfg path\n') + process.exit(1) +} + +process.stdout.write((foundInExecCfg ? execCfgRelPath : targetCfgRelPath) + '\n') diff --git a/js/private/test/js_run_binary/read_file.mjs b/js/private/test/js_run_binary/read_file.mjs new file mode 100644 index 0000000000..46b7ecae70 --- /dev/null +++ b/js/private/test/js_run_binary/read_file.mjs @@ -0,0 +1,4 @@ +import fs from 'fs' + +const content = fs.readFileSync(process.argv[2], 'utf8').trim() +process.stdout.write(content + '\n') diff --git a/js/private/test/snapshots/launcher.sh b/js/private/test/snapshots/launcher.sh index 7fdf331a76..5386f31b2f 100644 --- a/js/private/test/snapshots/launcher.sh +++ b/js/private/test/snapshots/launcher.sh @@ -108,10 +108,18 @@ function logf_debug { function resolve_execroot_bin_path { local short_path="$1" + local suffix if [[ "$short_path" == ../* ]]; then - echo "$JS_BINARY__EXECROOT/${BAZEL_BINDIR:-$JS_BINARY__BINDIR}/external/${short_path:3}" + suffix="external/${short_path:3}" else - echo "$JS_BINARY__EXECROOT/${BAZEL_BINDIR:-$JS_BINARY__BINDIR}/$short_path" + suffix="$short_path" + fi + local exec_path="$JS_BINARY__EXECROOT/$JS_BINARY__BINDIR/$suffix" + local target_path="$JS_BINARY__EXECROOT/${BAZEL_BINDIR:-$JS_BINARY__BINDIR}/$suffix" + if [ -e "$exec_path" ]; then + echo "$exec_path" + else + echo "$target_path" fi } From 34f9e68a53570da8b37e5f9c606ed898f0114378 Mon Sep 17 00:00:00 2001 From: Adam Cozzette Date: Thu, 30 Apr 2026 11:23:27 -0700 Subject: [PATCH 2/7] Simplify test fixes --- js/private/test/data/BUILD.bazel | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/js/private/test/data/BUILD.bazel b/js/private/test/data/BUILD.bazel index 1c49cefbea..845e04f714 100644 --- a/js/private/test/data/BUILD.bazel +++ b/js/private/test/data/BUILD.bazel @@ -120,9 +120,10 @@ js_run_binary( srcs = ["data.json"], outs = ["rb0.json"], args = [ - "$(rootpath rb0.json)", - "$(rootpath data.json)", + "rb0.json", + "data.json", ], + chdir = package_name(), tool = ":write-js_library-srcs", ) @@ -145,9 +146,10 @@ js_run_binary( srcs = ["data.json"], outs = ["rb1.json"], args = [ - "$(rootpath rb1.json)", - "$(rootpath data.json)", + "rb1.json", + "data.json", ], + chdir = package_name(), tool = ":write-js_library-srcs", ) @@ -177,9 +179,10 @@ js_run_binary( srcs = ["data.json"], outs = ["rb2.json"], args = [ - "$(rootpath rb2.json)", - "$(rootpath data.json)", + "rb2.json", + "data.json", ], + chdir = package_name(), tool = ":write-js_library-data", ) @@ -202,9 +205,10 @@ js_run_binary( srcs = ["data-generated.json"], outs = ["rb3.json"], args = [ - "$(rootpath rb3.json)", - "$(rootpath data-generated.json)", + "rb3.json", + "data-generated.json", ], + chdir = package_name(), tool = ":write-genrule", ) @@ -227,9 +231,10 @@ js_run_binary( srcs = ["data-copied.json"], outs = ["rb4.json"], args = [ - "$(rootpath rb4.json)", - "$(rootpath data-copied.json)", + "rb4.json", + "data-copied.json", ], + chdir = package_name(), tool = ":write-genrule-filegroup-srcs", ) From 3873c45829f861606ff6b5b7a943af5b5054d05f Mon Sep 17 00:00:00 2001 From: Adam Cozzette Date: Thu, 30 Apr 2026 13:25:23 -0700 Subject: [PATCH 3/7] Tweak CI setup --- .aspect/workflows/config.yaml | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/.aspect/workflows/config.yaml b/.aspect/workflows/config.yaml index 9fbfb262f4..8c7ecf01cd 100644 --- a/.aspect/workflows/config.yaml +++ b/.aspect/workflows/config.yaml @@ -124,13 +124,9 @@ workspaces: e2e/repo_mapping: icon: js tasks: - - test: - queue: aspect-medium - - format: - without: true - - buildifier: - without: true - - bazel-7: + # We need to pass a flag that begins with @aspect_rules_js//, but + # that repo is given a different name in e2e/repo_mapping. + - bazel-9-exec: without: true e2e/output_paths: icon: js @@ -162,7 +158,7 @@ tasks: name: Bazel 7 id: bazel-7 bazel: - flags: ['--test_tag_filters=-skip-on-bazel7', '--@aspect_rules_js//js:_hoist_runfiles_to_exec_cfg=true'] + flags: ['--test_tag_filters=-skip-on-bazel7'] env: USE_BAZEL_VERSION: '7.x' - test: @@ -181,6 +177,15 @@ tasks: # TODO: change this back to 9.x once this bug is fixed: # https://github.com/bazelbuild/bazel/issues/29393 USE_BAZEL_VERSION: '9.0.2' + - test: + name: Bazel 9 with exec cfg + id: bazel-9-exec + bazel: + flags: ['--test_tag_filters=-skip-on-bazel9', '--@aspect_rules_js//js:_hoist_runfiles_to_exec_cfg=true'] + env: + # TODO: change this back to 9.x once this bug is fixed: + # https://github.com/bazelbuild/bazel/issues/29393 + USE_BAZEL_VERSION: '9.0.2' - format: queue: aspect-medium - buildifier: From adac4ad2ee23ad305ce577f2db989fb5c34e966d Mon Sep 17 00:00:00 2001 From: Adam Cozzette Date: Thu, 30 Apr 2026 14:54:21 -0700 Subject: [PATCH 4/7] Rename flag to use_execroot_exec_cfg --- .aspect/workflows/config.yaml | 2 +- contrib/nextjs/defs.bzl | 4 ++-- js/BUILD.bazel | 11 +++++----- js/private/js_run_binary.bzl | 26 +++++++++++------------ js/private/test/js_run_binary/BUILD.bazel | 10 ++++----- 5 files changed, 26 insertions(+), 27 deletions(-) diff --git a/.aspect/workflows/config.yaml b/.aspect/workflows/config.yaml index 8c7ecf01cd..3f47dd9085 100644 --- a/.aspect/workflows/config.yaml +++ b/.aspect/workflows/config.yaml @@ -181,7 +181,7 @@ tasks: name: Bazel 9 with exec cfg id: bazel-9-exec bazel: - flags: ['--test_tag_filters=-skip-on-bazel9', '--@aspect_rules_js//js:_hoist_runfiles_to_exec_cfg=true'] + flags: ['--test_tag_filters=-skip-on-bazel9', '--@aspect_rules_js//js:use_execroot_exec_cfg=true'] env: # TODO: change this back to 9.x once this bug is fixed: # https://github.com/bazelbuild/bazel/issues/29393 diff --git a/contrib/nextjs/defs.bzl b/contrib/nextjs/defs.bzl index f7b21a33c5..732b61d38c 100644 --- a/contrib/nextjs/defs.bzl +++ b/contrib/nextjs/defs.bzl @@ -202,7 +202,7 @@ def nextjs_build(name, config, srcs, next_js_binary, data = [], **kwargs): chdir = native.package_name(), mnemonic = "NextJs", progress_message = "Compile Next.js app %{label}", - hoist_runfiles_to_exec_cfg = False, + use_execroot_exec_cfg = False, **kwargs ) @@ -328,7 +328,7 @@ def nextjs_standalone_build(name, config, srcs, next_js_binary, data = [], **kwa chdir = native.package_name(), mnemonic = "NextJs", progress_message = "Compile Next.js standalone app %{label}", - hoist_runfiles_to_exec_cfg = False, + use_execroot_exec_cfg = False, **kwargs ) diff --git a/js/BUILD.bazel b/js/BUILD.bazel index cd3accf894..043bed5014 100644 --- a/js/BUILD.bazel +++ b/js/BUILD.bazel @@ -3,18 +3,17 @@ load("@bazel_skylib//:bzl_library.bzl", "bzl_library") load("@bazel_skylib//rules:common_settings.bzl", "bool_flag") -# Underscore prefix signals this is internal — do not rely on it. -# This controls the default value of the hoist_runfiles_to_exec_cfg flag on -# js_run_binary, and is for CI testing only. +# This flag controls the default value of the use_execroot_exec_cfg flag on +# js_run_binary. bool_flag( - name = "_hoist_runfiles_to_exec_cfg", + name = "use_execroot_exec_cfg", build_setting_default = False, visibility = ["//visibility:public"], ) config_setting( - name = "_hoist_runfiles_to_exec_cfg_true", - flag_values = {"_hoist_runfiles_to_exec_cfg": "True"}, + name = "_use_execroot_exec_cfg_true", + flag_values = {"use_execroot_exec_cfg": "True"}, ) bzl_library( diff --git a/js/private/js_run_binary.bzl b/js/private/js_run_binary.bzl index ce5995f86a..b356e97524 100644 --- a/js/private/js_run_binary.bzl +++ b/js/private/js_run_binary.bzl @@ -31,7 +31,7 @@ def js_run_binary( exit_code_out = None, silent_on_success = True, use_execroot_entry_point = True, - hoist_runfiles_to_exec_cfg = None, + use_execroot_exec_cfg = None, copy_srcs_to_bin = True, include_sources = True, include_types = False, @@ -150,16 +150,16 @@ def js_run_binary( needed by the binary are available in the execroot output tree. This requirement can be turned off with by setting `allow_execroot_entry_point_with_no_copy_data_to_bin` to True. - hoist_runfiles_to_exec_cfg: Hoist runfiles to the execution platform configuration instead of the target platform + use_execroot_exec_cfg: Hoist runfiles to the execution platform configuration instead of the target platform configuration. - This only has an effect when `use_execroot_entry_point` is True. Setting `hoist_runfiles_to_exec_cfg` to True - results in a more correct build by ensuring that code that is executed during a build action is built for the - execution platform, which may not be the same as the target platform. + This only has an effect when `use_execroot_entry_point` is True. Setting `use_execroot_exec_cfg` to True results + in a more correct build by ensuring that code that is executed during a build action is built for the execution + platform, which may not be the same as the target platform. - When set to `None` (the default), the value of the internal `//js:_hoist_runfiles_to_exec_cfg` build flag - is used. That flag is for internal testing only and should not be used. It is temporary and will be removed once - the default is changed to `True`. + When set to `None` (the default), the value of the internal `//js:use_execroot_exec_cfg` build flag is used. That + flag is for internal testing only and should not be used. It is temporary and will be removed once the default is + changed to `True`. copy_srcs_to_bin: When True, all srcs files are copied to the output tree that are not already there. @@ -394,18 +394,18 @@ See https://github.com/aspect-build/rules_js/tree/main/docs#using-binaries-publi testonly = kwargs.get("testonly", False), ) runfiles_label = ":{}".format(js_runfiles_name) - if hoist_runfiles_to_exec_cfg: + if use_execroot_exec_cfg: tools.append(runfiles_label) - elif hoist_runfiles_to_exec_cfg == False: + elif use_execroot_exec_cfg == False: extra_srcs.append(runfiles_label) else: - # hoist_runfiles_to_exec_cfg is None, so we will defer to the flag. + # use_execroot_exec_cfg is None, so we will defer to the flag. tools = tools + select({ - "@aspect_rules_js//js:_hoist_runfiles_to_exec_cfg_true": [runfiles_label], + "@aspect_rules_js//js:_use_execroot_exec_cfg_true": [runfiles_label], "//conditions:default": [], }) extra_srcs = extra_srcs + select({ - "@aspect_rules_js//js:_hoist_runfiles_to_exec_cfg_true": [], + "@aspect_rules_js//js:_use_execroot_exec_cfg_true": [], "//conditions:default": [runfiles_label], }) diff --git a/js/private/test/js_run_binary/BUILD.bazel b/js/private/test/js_run_binary/BUILD.bazel index c4d699af3e..3cda436d4c 100644 --- a/js/private/test/js_run_binary/BUILD.bazel +++ b/js/private/test/js_run_binary/BUILD.bazel @@ -5,7 +5,7 @@ load("@bazel_skylib//rules:write_file.bzl", "write_file") load("//js:defs.bzl", "js_binary", "js_run_binary") # Verify that the js_binary tool's data dependencies are built in the exec configuration -# when hoist_runfiles_to_exec_cfg is True. +# when use_execroot_exec_cfg is True. # To do this we check that js_run_binary agrees with genrule on the location of a # tool dependency. write_file( @@ -22,7 +22,7 @@ js_binary( js_run_binary( name = "tools_cfg_location", - hoist_runfiles_to_exec_cfg = True, + use_execroot_exec_cfg = True, stdout = "tools_cfg_location.txt", tool = ":find_cfg_probe", ) @@ -40,11 +40,11 @@ diff_test( file2 = ":expected_tools_cfg_location.txt", ) -# Verify the opposite: with hoist_runfiles_to_exec_cfg = False, the tool's data +# Verify the opposite: with use_execroot_exec_cfg = False, the tool's data # dependencies are built in the target configuration, matching a genrule srcs path. js_run_binary( name = "target_cfg_location", - hoist_runfiles_to_exec_cfg = False, + use_execroot_exec_cfg = False, stdout = "target_cfg_location.txt", tool = ":find_cfg_probe", ) @@ -102,7 +102,7 @@ js_run_binary( name = "gen_target_os", srcs = [":os_name.txt"], args = ["$(rootpath :os_name.txt)"], - hoist_runfiles_to_exec_cfg = True, + use_execroot_exec_cfg = True, stdout = "target_os.txt", tags = ["manual"], tool = ":read_file", From 4d6f900003cf52b030b72a79986baa35300fe2c3 Mon Sep 17 00:00:00 2001 From: Adam Cozzette Date: Thu, 30 Apr 2026 15:03:34 -0700 Subject: [PATCH 5/7] Fix CI config --- .aspect/workflows/config.yaml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.aspect/workflows/config.yaml b/.aspect/workflows/config.yaml index 3f47dd9085..6237f03647 100644 --- a/.aspect/workflows/config.yaml +++ b/.aspect/workflows/config.yaml @@ -124,6 +124,12 @@ workspaces: e2e/repo_mapping: icon: js tasks: + - test: + queue: aspect-medium + - format: + without: true + - buildifier: + without: true # We need to pass a flag that begins with @aspect_rules_js//, but # that repo is given a different name in e2e/repo_mapping. - bazel-9-exec: From 2e0deeee960f33cdaf7c7c2c4f2423731d0aa14c Mon Sep 17 00:00:00 2001 From: Adam Cozzette Date: Thu, 30 Apr 2026 15:04:02 -0700 Subject: [PATCH 6/7] buildifier --- js/private/test/js_run_binary/BUILD.bazel | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/js/private/test/js_run_binary/BUILD.bazel b/js/private/test/js_run_binary/BUILD.bazel index 3cda436d4c..d163d3977d 100644 --- a/js/private/test/js_run_binary/BUILD.bazel +++ b/js/private/test/js_run_binary/BUILD.bazel @@ -22,9 +22,9 @@ js_binary( js_run_binary( name = "tools_cfg_location", - use_execroot_exec_cfg = True, stdout = "tools_cfg_location.txt", tool = ":find_cfg_probe", + use_execroot_exec_cfg = True, ) genrule( @@ -44,9 +44,9 @@ diff_test( # dependencies are built in the target configuration, matching a genrule srcs path. js_run_binary( name = "target_cfg_location", - use_execroot_exec_cfg = False, stdout = "target_cfg_location.txt", tool = ":find_cfg_probe", + use_execroot_exec_cfg = False, ) genrule( @@ -102,10 +102,10 @@ js_run_binary( name = "gen_target_os", srcs = [":os_name.txt"], args = ["$(rootpath :os_name.txt)"], - use_execroot_exec_cfg = True, stdout = "target_os.txt", tags = ["manual"], tool = ":read_file", + use_execroot_exec_cfg = True, ) platform_transition_filegroup( From 49a6e788c7a0eda9e53058699759657411a2d3e5 Mon Sep 17 00:00:00 2001 From: Adam Cozzette Date: Thu, 30 Apr 2026 15:18:59 -0700 Subject: [PATCH 7/7] Exclude e2e/js_image_oci --- .aspect/workflows/config.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.aspect/workflows/config.yaml b/.aspect/workflows/config.yaml index 6237f03647..2643a10caa 100644 --- a/.aspect/workflows/config.yaml +++ b/.aspect/workflows/config.yaml @@ -36,6 +36,8 @@ workspaces: # Still depends on some WORKSPACE setup - bazel-9: without: true + - bazel-9-exec: + without: true e2e/nextjs: icon: nextjs tasks: *e2e_tasks