fix: allow js_run_binary to hoist runfiles in the exec configuration

.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
@@ -123,7 +125,17 @@ workspaces:
         tasks: *e2e_tasks
     e2e/repo_mapping:
         icon: js
-        tasks: *e2e_tasks
+        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:
+                  without: true
     e2e/output_paths:
         icon: js
         tasks: *e2e_tasks
@@ -173,6 +185,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: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
+              USE_BAZEL_VERSION: '9.0.2'
     - format:
           queue: aspect-medium
     - buildifier:

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}",
+        use_execroot_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}",
+        use_execroot_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,20 @@
 "Public API"
 
 load("@bazel_skylib//:bzl_library.bzl", "bzl_library")
+load("@bazel_skylib//rules:common_settings.bzl", "bool_flag")
+
+# This flag controls the default value of the use_execroot_exec_cfg flag on
+# js_run_binary.
+bool_flag(
+    name = "use_execroot_exec_cfg",
+    build_setting_default = False,
+    visibility = ["//visibility:public"],
+)
+
+config_setting(
+    name = "_use_execroot_exec_cfg_true",
+    flag_values = {"use_execroot_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,
+        use_execroot_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.
 
+        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 `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: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.
 
         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 use_execroot_exec_cfg:
+            tools.append(runfiles_label)
+        elif use_execroot_exec_cfg == False:
+            extra_srcs.append(runfiles_label)
+        else:
+            # use_execroot_exec_cfg is None, so we will defer to the flag.
+            tools = tools + select({
+                "@aspect_rules_js//js:_use_execroot_exec_cfg_true": [runfiles_label],
+                "//conditions:default": [],
+            })
+            extra_srcs = extra_srcs + select({
+                "@aspect_rules_js//js:_use_execroot_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,6 +117,7 @@ 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",
@@ -142,6 +143,7 @@ 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",
@@ -174,6 +176,7 @@ js_binary(
 
 js_run_binary(
     name = "run-write-js_library-data",
+    srcs = ["data.json"],
     outs = ["rb2.json"],
     args = [
         "rb2.json",
@@ -199,6 +202,7 @@ 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",
@@ -224,6 +228,7 @@ 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",

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,

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 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(
+    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",
+    stdout = "tools_cfg_location.txt",
+    tool = ":find_cfg_probe",
+    use_execroot_exec_cfg = True,
+)
+
+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 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",
+    stdout = "target_cfg_location.txt",
+    tool = ":find_cfg_probe",
+    use_execroot_exec_cfg = False,
+)
+
+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)"],
+    stdout = "target_os.txt",
+    tags = ["manual"],
+    tool = ":read_file",
+    use_execroot_exec_cfg = True,
+)
+
+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",
+)