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 <[email protected]>

Author: acozzette

.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:

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")

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
     )
 

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

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",

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
 }
 

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,14 +393,29 @@ 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"
 
     _run_binary(
         name = name,
         tool = tool,
+        tools = tools,
         env = fixed_env | env,
         srcs = srcs + extra_srcs,
         outs = outs + extra_outs,

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",
 )
 

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,