Skip to content

Commit e07d468

Browse files
acozzetteclaude
andcommitted
feat: add bool_flag to control hoist_runfiles_to_exec_cfg default
Introduces `//js:hoist_runfiles_to_exec_cfg` as a `bool_flag` (default `False`) to control the default value of `js_run_binary`'s `hoist_runfiles_to_exec_cfg` parameter. The parameter's default changes from `True` to `None`, deferring to the flag when not set explicitly. The launcher script now auto-detects which configuration the entry point was built in (exec vs target) by checking file existence, removing the need for the `JS_BINARY__HOIST_RUNFILES_TO_EXEC_CFG` env var entirely. CI is updated to run `e2e/bzlmod` with `--@aspect_rules_js//js:hoist_runfiles_to_exec_cfg=true` to verify the opt-in path continues to work. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
1 parent d19674f commit e07d468

5 files changed

Lines changed: 77 additions & 21 deletions

File tree

.github/workflows/ci.yaml

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,32 @@ jobs:
120120
echo "No test.sh in ${PWD}; skipping"
121121
fi
122122
123+
# Ubuntu smoke test for e2e/bzlmod with hoist_runfiles_to_exec_cfg=true to verify
124+
# that the flag-controlled opt-in path works correctly.
125+
test-hoist-runfiles:
126+
runs-on: ubuntu-22.04
127+
defaults:
128+
run:
129+
working-directory: e2e/bzlmod
130+
env:
131+
ASPECT_RULES_JS_FROZEN_PNPM_LOCK: 1
132+
steps:
133+
- uses: actions/checkout@v6
134+
135+
- uses: bazel-contrib/[email protected]
136+
with:
137+
bazelisk-cache: true
138+
disk-cache: hoist-runfiles-bzlmod
139+
repository-cache: true
140+
141+
- name: bazel test //... with hoist_runfiles_to_exec_cfg=true
142+
run: |
143+
bazel test \
144+
--test_tag_filters=-skip-on-bazel7 \
145+
--build_tag_filters=-skip-on-bazel7 \
146+
--@aspect_rules_js//js:hoist_runfiles_to_exec_cfg=true \
147+
//...
148+
123149
# Mac/Windows smoke tests - only e2e/bzlmod
124150
# Only run on main branch (not PRs) to minimize minutes (billed at 10X and 2X respectively)
125151
# https://docs.github.com/en/billing/managing-billing-for-github-actions/about-billing-for-github-actions#included-storage-and-minutes
@@ -155,12 +181,12 @@ jobs:
155181
# For branch protection settings, this job provides a "stable" name that can be used to gate PR merges
156182
# on "all matrix jobs were successful".
157183
conclusion:
158-
needs: [test, smoke]
184+
needs: [test, smoke, test-hoist-runfiles]
159185
runs-on: ubuntu-latest
160186
if: always()
161187
steps:
162188
- run: |
163-
if [[ "${{ needs.test.result }}" == "success" && ("${{ needs.smoke.result }}" == "success" || "${{ needs.smoke.result }}" == "skipped") ]]; then
189+
if [[ "${{ needs.test.result }}" == "success" && ("${{ needs.smoke.result }}" == "success" || "${{ needs.smoke.result }}" == "skipped") && "${{ needs.test-hoist-runfiles.result }}" == "success" ]]; then
164190
exit 0
165191
else
166192
exit 1

js/BUILD.bazel

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,18 @@
11
"Public API"
22

33
load("@bazel_skylib//:bzl_library.bzl", "bzl_library")
4+
load("@bazel_skylib//rules:common_settings.bzl", "bool_flag")
5+
6+
bool_flag(
7+
name = "hoist_runfiles_to_exec_cfg",
8+
build_setting_default = False,
9+
visibility = ["//visibility:public"],
10+
)
11+
12+
config_setting(
13+
name = "hoist_runfiles_to_exec_cfg_true",
14+
flag_values = {":hoist_runfiles_to_exec_cfg": "True"},
15+
)
416

517
bzl_library(
618
name = "defs",

js/private/js_binary.sh.tpl

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -96,16 +96,19 @@ function logf_debug {
9696
}
9797

9898
function resolve_execroot_bin_path {
99-
if [ "${JS_BINARY__HOIST_RUNFILES_TO_EXEC_CFG:-}" ]; then
100-
local bindir="$JS_BINARY__EXECROOT/$JS_BINARY__BINDIR"
101-
else
102-
local bindir="$JS_BINARY__EXECROOT/$BAZEL_BINDIR"
103-
fi
10499
local short_path="$1"
100+
local suffix
105101
if [[ "$short_path" == ../* ]]; then
106-
echo "$bindir/external/${short_path:3}"
102+
suffix="external/${short_path:3}"
103+
else
104+
suffix="$short_path"
105+
fi
106+
local exec_path="$JS_BINARY__EXECROOT/$JS_BINARY__BINDIR/$suffix"
107+
local target_path="$JS_BINARY__EXECROOT/${BAZEL_BINDIR:-$JS_BINARY__BINDIR}/$suffix"
108+
if [ "$exec_path" = "$target_path" ] || [ -e "$exec_path" ]; then
109+
echo "$exec_path"
107110
else
108-
echo "$bindir/$short_path"
111+
echo "$target_path"
109112
fi
110113
}
111114

js/private/js_run_binary.bzl

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ def js_run_binary(
3131
exit_code_out = None,
3232
silent_on_success = True,
3333
use_execroot_entry_point = True,
34-
hoist_runfiles_to_exec_cfg = True,
34+
hoist_runfiles_to_exec_cfg = None,
3535
copy_srcs_to_bin = True,
3636
include_sources = True,
3737
include_types = False,
@@ -157,6 +157,9 @@ def js_run_binary(
157157
results in a more correct build by ensuring that code that is executed during a build action is built for the
158158
execution platform, which may not be the same as the target platform.
159159
160+
When set to `None` (the default), the value of the `//js:hoist_runfiles_to_exec_cfg` build flag is used.
161+
Pass `--//js:hoist_runfiles_to_exec_cfg=true` on the command line or in `.bazelrc` to enable globally.
162+
160163
copy_srcs_to_bin: When True, all srcs files are copied to the output tree that are not already there.
161164
162165
include_sources: see `js_info_files` documentation
@@ -389,11 +392,20 @@ See https://github.com/aspect-build/rules_js/tree/main/docs#using-binaries-publi
389392
# Always propagate the testonly attribute
390393
testonly = kwargs.get("testonly", False),
391394
)
392-
if hoist_runfiles_to_exec_cfg:
393-
fixed_env["JS_BINARY__HOIST_RUNFILES_TO_EXEC_CFG"] = "1"
394-
tools.append(":{}".format(js_runfiles_name))
395+
runfiles_label = ":{}".format(js_runfiles_name)
396+
if hoist_runfiles_to_exec_cfg == True:
397+
tools.append(runfiles_label)
398+
elif hoist_runfiles_to_exec_cfg == False:
399+
extra_srcs.append(runfiles_label)
395400
else:
396-
extra_srcs.append(":{}".format(js_runfiles_name))
401+
tools = tools + select({
402+
"//js:hoist_runfiles_to_exec_cfg_true": [runfiles_label],
403+
"//conditions:default": [],
404+
})
405+
extra_srcs = extra_srcs + select({
406+
"//js:hoist_runfiles_to_exec_cfg_true": [],
407+
"//conditions:default": [runfiles_label],
408+
})
397409

398410
if allow_execroot_entry_point_with_no_copy_data_to_bin:
399411
fixed_env["JS_BINARY__ALLOW_EXECROOT_ENTRY_POINT_WITH_NO_COPY_DATA_TO_BIN"] = "1"

js/private/test/snapshots/launcher.sh

Lines changed: 10 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)