Skip to content

Commit 2790f0d

Browse files
jbedardjbedard
committed
fix: js_run_binary exec vs target platform
1 parent 7316607 commit 2790f0d

9 files changed

Lines changed: 109 additions & 12 deletions

‎js/private/js_binary.sh.tpl‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,10 +97,14 @@ function logf_debug {
9797

9898
function resolve_execroot_bin_path {
9999
local short_path="$1"
100+
# When acting as a js_run_binary tool, the binary is in exec cfg, so use the baked-in
101+
# JS_BINARY__BINDIR (exec cfg) rather than BAZEL_BINDIR (target cfg). See #2121 and #2754
102+
local bindir="${JS_BINARY__USE_EXECROOT_ENTRY_POINT:+$JS_BINARY__BINDIR}"
103+
bindir="${bindir:-${BAZEL_BINDIR:-$JS_BINARY__BINDIR}}"
100104
if [[ "$short_path" == ../* ]]; then
101-
echo "$JS_BINARY__EXECROOT/${BAZEL_BINDIR:-$JS_BINARY__BINDIR}/external/${short_path:3}"
105+
echo "$JS_BINARY__EXECROOT/$bindir/external/${short_path:3}"
102106
else
103-
echo "$JS_BINARY__EXECROOT/${BAZEL_BINDIR:-$JS_BINARY__BINDIR}/$short_path"
107+
echo "$JS_BINARY__EXECROOT/$bindir/$short_path"
104108
fi
105109
}
106110

‎js/private/js_run_binary.bzl‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,17 @@ load(":js_helpers.bzl", _envs_for_log_level = "envs_for_log_level")
1717
load(":js_info_files.bzl", _js_info_files = "js_info_files")
1818
load(":js_library.bzl", _js_library = "js_library")
1919

20+
def _js_run_binary_runfiles_impl(ctx):
21+
# Expose tool runfiles in exec cfg so platform-specific select()s resolve for the exec
22+
# platform. See https://github.com/aspect-build/rules_js/issues/2121 and #2754
23+
runfiles = ctx.runfiles().merge(ctx.attr.tool[DefaultInfo].default_runfiles)
24+
return [DefaultInfo(files = runfiles.files)]
25+
26+
_js_run_binary_runfiles = rule(
27+
implementation = _js_run_binary_runfiles_impl,
28+
attrs = {"tool": attr.label(cfg = "exec")},
29+
)
30+
2031
def js_run_binary(
2132
name,
2233
tool,
@@ -382,6 +393,19 @@ See https://github.com/aspect-build/rules_js/tree/main/docs#using-binaries-publi
382393
)
383394
extra_srcs.append(":{}".format(js_runfiles_name))
384395

396+
# Also hoist exec-cfg runfiles so platform-specific select()s in the tool's data
397+
# resolve for the exec platform. See #2121 and #2754
398+
js_exec_runfiles_name = "{}_exec_runfiles".format(name)
399+
_js_run_binary_runfiles(
400+
name = js_exec_runfiles_name,
401+
tool = tool,
402+
# Always tag the target manual since we should only build it when the final target is built.
403+
tags = kwargs.get("tags", []) + ["manual"],
404+
# Always propagate the testonly attribute
405+
testonly = kwargs.get("testonly", False),
406+
)
407+
extra_srcs.append(":{}".format(js_exec_runfiles_name))
408+
385409
if allow_execroot_entry_point_with_no_copy_data_to_bin:
386410
fixed_env["JS_BINARY__ALLOW_EXECROOT_ENTRY_POINT_WITH_NO_COPY_DATA_TO_BIN"] = "1"
387411

‎js/private/test/BUILD.bazel‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ load("@bazel_lib_host//:defs.bzl", "host")
33
load("@bazel_skylib//rules:write_file.bzl", "write_file")
44
load("//js:defs.bzl", "js_binary", "js_library", "js_test")
55
load(":js_library_test.bzl", "js_library_test_suite")
6+
load(":js_run_binary_test.bzl", "js_run_binary_exec_cfg_test_suite")
67
load(":run_environment_info_test.bzl", "run_environment_info_test_suite")
78

89
####################################################################################################
@@ -43,6 +44,8 @@ write_source_files(
4344

4445
js_library_test_suite(name = "js_library_test")
4546

47+
js_run_binary_exec_cfg_test_suite(name = "js_run_binary_test")
48+
4649
run_environment_info_test_suite(name = "run_environment_info_tests")
4750

4851
# js_library(data) wrapper of the data

‎js/private/test/image/custom_owner_test_app.listing‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ drwxr-xr-x 0 100 0 0 Jan 1 1970 ./js/
22
drwxr-xr-x 0 100 0 0 Jan 1 1970 ./js/private/
33
drwxr-xr-x 0 100 0 0 Jan 1 1970 ./js/private/test/
44
drwxr-xr-x 0 100 0 0 Jan 1 1970 ./js/private/test/image/
5-
-r-xr-xr-x 0 100 0 xxxxx Jan 1 1970 ./js/private/test/image/bin
5+
-r-xr-xr-x 0 100 0 25102 Jan 1 1970 ./js/private/test/image/bin
66
drwxr-xr-x 0 100 0 0 Jan 1 1970 ./js/private/test/image/bin.runfiles/
77
drwxr-xr-x 0 100 0 0 Jan 1 1970 ./js/private/test/image/bin.runfiles/_main/
88
drwxr-xr-x 0 100 0 0 Jan 1 1970 ./js/private/test/image/bin.runfiles/_main/js/
@@ -14,7 +14,7 @@ drwxr-xr-x 0 100 0 0 Jan 1 1970 ./js/private/test/image/bin.runf
1414
drwxr-xr-x 0 100 0 0 Jan 1 1970 ./js/private/test/image/bin.runfiles/_main/js/private/test/image-fixture-d/
1515
-r-xr-xr-x 0 100 0 128 Jan 1 1970 ./js/private/test/image/bin.runfiles/_main/js/private/test/image-fixture-d/package.json
1616
drwxr-xr-x 0 100 0 0 Jan 1 1970 ./js/private/test/image/bin.runfiles/_main/js/private/test/image/bin_/
17-
-r-xr-xr-x 0 100 0 xxxxx Jan 1 1970 ./js/private/test/image/bin.runfiles/_main/js/private/test/image/bin_/bin
17+
-r-xr-xr-x 0 100 0 25102 Jan 1 1970 ./js/private/test/image/bin.runfiles/_main/js/private/test/image/bin_/bin
1818
drwxr-xr-x 0 100 0 0 Jan 1 1970 ./js/private/test/image/bin.runfiles/_main/js/private/test/image/bin_node_bin/
1919
-r-xr-xr-x 0 100 0 133 Jan 1 1970 ./js/private/test/image/bin.runfiles/_main/js/private/test/image/bin_node_bin/node
2020
-r-xr-xr-x 0 100 0 20 Jan 1 1970 ./js/private/test/image/bin.runfiles/_main/js/private/test/image/main.js

‎js/private/test/image/default_test_app.listing‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ drwxr-xr-x 0 0 0 0 Jan 1 1970 ./js/
22
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./js/private/
33
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./js/private/test/
44
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./js/private/test/image/
5-
-r-xr-xr-x 0 0 0 xxxxx Jan 1 1970 ./js/private/test/image/bin
5+
-r-xr-xr-x 0 0 0 25102 Jan 1 1970 ./js/private/test/image/bin
66
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./js/private/test/image/bin.runfiles/
77
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./js/private/test/image/bin.runfiles/_main/
88
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./js/private/test/image/bin.runfiles/_main/js/
@@ -14,7 +14,7 @@ drwxr-xr-x 0 0 0 0 Jan 1 1970 ./js/private/test/image/bin.runf
1414
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./js/private/test/image/bin.runfiles/_main/js/private/test/image-fixture-d/
1515
-r-xr-xr-x 0 0 0 128 Jan 1 1970 ./js/private/test/image/bin.runfiles/_main/js/private/test/image-fixture-d/package.json
1616
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./js/private/test/image/bin.runfiles/_main/js/private/test/image/bin_/
17-
-r-xr-xr-x 0 0 0 xxxxx Jan 1 1970 ./js/private/test/image/bin.runfiles/_main/js/private/test/image/bin_/bin
17+
-r-xr-xr-x 0 0 0 25102 Jan 1 1970 ./js/private/test/image/bin.runfiles/_main/js/private/test/image/bin_/bin
1818
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./js/private/test/image/bin.runfiles/_main/js/private/test/image/bin_node_bin/
1919
-r-xr-xr-x 0 0 0 133 Jan 1 1970 ./js/private/test/image/bin.runfiles/_main/js/private/test/image/bin_node_bin/node
2020
-r-xr-xr-x 0 0 0 20 Jan 1 1970 ./js/private/test/image/bin.runfiles/_main/js/private/test/image/main.js

‎js/private/test/image/non_ascii/custom_layer_groups_test_app.listing‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/
44
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/
55
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/
66
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/non_ascii/
7-
-r-xr-xr-x 0 0 0 xxxxx Jan 1 1970 ./app/js/private/test/image/non_ascii/bin2
7+
-r-xr-xr-x 0 0 0 25197 Jan 1 1970 ./app/js/private/test/image/non_ascii/bin2
88
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/non_ascii/bin2.runfiles/
99
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/non_ascii/bin2.runfiles/_main/
1010
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/non_ascii/bin2.runfiles/_main/js/
@@ -14,7 +14,7 @@ drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/non_
1414
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/non_ascii/bin2.runfiles/_main/js/private/test/image/non_ascii/
1515
-r-xr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/non_ascii/bin2.runfiles/_main/js/private/test/image/non_ascii/ㅑㅕㅣㅇ.ㄴㅅ
1616
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/non_ascii/bin2.runfiles/_main/js/private/test/image/non_ascii/bin2_/
17-
-r-xr-xr-x 0 0 0 xxxxx Jan 1 1970 ./app/js/private/test/image/non_ascii/bin2.runfiles/_main/js/private/test/image/non_ascii/bin2_/bin2
17+
-r-xr-xr-x 0 0 0 25197 Jan 1 1970 ./app/js/private/test/image/non_ascii/bin2.runfiles/_main/js/private/test/image/non_ascii/bin2_/bin2
1818
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/non_ascii/bin2.runfiles/_main/js/private/test/image/non_ascii/bin2_node_bin/
1919
-r-xr-xr-x 0 0 0 133 Jan 1 1970 ./app/js/private/test/image/non_ascii/bin2.runfiles/_main/js/private/test/image/non_ascii/bin2_node_bin/node
2020
-r-xr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/non_ascii/bin2.runfiles/_main/js/private/test/image/non_ascii/empty empty.ㄴㅅ

‎js/private/test/image/regex_edge_cases_test_app.listing‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/
33
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/
44
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/
55
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/
6-
-r-xr-xr-x 0 0 0 xxxxx Jan 1 1970 ./app/js/private/test/image/bin
6+
-r-xr-xr-x 0 0 0 25102 Jan 1 1970 ./app/js/private/test/image/bin
77
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/bin.runfiles/
88
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/bin.runfiles/_main/
99
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/bin.runfiles/_main/js/
@@ -15,7 +15,7 @@ drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/bin.
1515
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/bin.runfiles/_main/js/private/test/image-fixture-d/
1616
-r-xr-xr-x 0 0 0 128 Jan 1 1970 ./app/js/private/test/image/bin.runfiles/_main/js/private/test/image-fixture-d/package.json
1717
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/bin.runfiles/_main/js/private/test/image/bin_/
18-
-r-xr-xr-x 0 0 0 xxxxx Jan 1 1970 ./app/js/private/test/image/bin.runfiles/_main/js/private/test/image/bin_/bin
18+
-r-xr-xr-x 0 0 0 25102 Jan 1 1970 ./app/js/private/test/image/bin.runfiles/_main/js/private/test/image/bin_/bin
1919
drwxr-xr-x 0 0 0 0 Jan 1 1970 ./app/js/private/test/image/bin.runfiles/_main/js/private/test/image/bin_node_bin/
2020
-r-xr-xr-x 0 0 0 133 Jan 1 1970 ./app/js/private/test/image/bin.runfiles/_main/js/private/test/image/bin_node_bin/node
2121
-r-xr-xr-x 0 0 0 20 Jan 1 1970 ./app/js/private/test/image/bin.runfiles/_main/js/private/test/image/main.js

‎js/private/test/js_run_binary_test.bzl‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
"""Analysis tests for js_run_binary exec cfg handling.
2+
3+
Reproduces:
4+
https://github.com/aspect-build/rules_js/issues/2121
5+
https://github.com/aspect-build/rules_js/issues/2754
6+
"""
7+
8+
load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts")
9+
load("@bazel_skylib//rules:write_file.bzl", "write_file")
10+
load("//js:defs.bzl", "js_binary", "js_run_binary")
11+
12+
def _exec_cfg_runfiles_test_impl(ctx):
13+
env = analysistest.begin(ctx)
14+
target = analysistest.target_under_test(env)
15+
16+
# Files built for exec cfg have "-exec-" in their output path; target-cfg files don't.
17+
# Source files (not in bazel-out) are cfg-neutral and are excluded from this check.
18+
non_exec = [
19+
f.short_path
20+
for f in target[DefaultInfo].files.to_list()
21+
if not f.is_source and "-exec-" not in f.path
22+
]
23+
asserts.equals(
24+
env,
25+
[],
26+
non_exec,
27+
"runfiles must use exec cfg, not target cfg (#2121, #2754)",
28+
)
29+
return analysistest.end(env)
30+
31+
_exec_cfg_runfiles_test = analysistest.make(_exec_cfg_runfiles_test_impl)
32+
33+
def js_run_binary_exec_cfg_test_suite(name):
34+
"""Regression test for js_run_binary using exec cfg for tool runfiles (#2121, #2754).
35+
36+
Args:
37+
name: Name of the test_suite target.
38+
"""
39+
write_file(name = "tool_entry_point", out = "js_run_binary_test_tool.js", content = ["process.exit(0);"], tags = ["manual"])
40+
41+
js_binary(
42+
name = "js_run_binary_test_tool",
43+
entry_point = "js_run_binary_test_tool.js",
44+
tags = ["manual"],
45+
)
46+
47+
js_run_binary(
48+
name = "js_run_binary_test_subject",
49+
tool = ":js_run_binary_test_tool",
50+
outs = ["js_run_binary_test_out.txt"],
51+
tags = ["manual"],
52+
)
53+
54+
_exec_cfg_runfiles_test(
55+
name = name + "_exec_cfg_test",
56+
target_under_test = ":js_run_binary_test_subject_exec_runfiles",
57+
)
58+
59+
native.test_suite(
60+
name = name,
61+
tests = [":" + name + "_exec_cfg_test"],
62+
)

‎js/private/test/snapshots/launcher.sh‎

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

0 commit comments

Comments
 (0)