Skip to content

Commit a3aa85b

Browse files
authored
refactor: adopt new NodeJS runtime toolchain (#2330)
`js_binary` should use the new runtime toolchain to avoid execution toolchain being leaked into target environments (eg., `js_image_oci`) See bazel-contrib/rules_nodejs#3854 Depends on: - bazel-contrib/rules_nodejs#3859 ### Changes are visible to end-users: yes - Searched for relevant documentation and updated as needed: yes - Breaking change (forces users to change their own code or config): no - Suggested release notes appear below: yes > Switched `js_binary` (and `js_test`) to start using the new runtime toolchain type introduced by `rules_nodejs` to better support cross-platform builds (eg., building `arm64` container from `amd64`). ### Test plan - Covered by existing test cases
1 parent bb17273 commit a3aa85b

File tree

8 files changed

+31
-17
lines changed

8 files changed

+31
-17
lines changed

docs/js_binary.md

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

e2e/pnpm_workspace/BUILD.bazel

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ sh_test(
1919
"@nodejs_linux_amd64//:node_files",
2020
"@nodejs_linux_arm64//:node_files",
2121
],
22-
toolchains = ["@nodejs_toolchains//:resolved_toolchain"],
22+
toolchains = ["@rules_nodejs//nodejs:current_node_runtime"],
2323
)
2424

2525
build_test(

e2e/pnpm_workspace_rerooted/BUILD.bazel

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ sh_test(
1919
"@nodejs_linux_amd64//:node_files",
2020
"@nodejs_linux_arm64//:node_files",
2121
],
22-
toolchains = ["@nodejs_toolchains//:resolved_toolchain"],
22+
toolchains = ["@rules_nodejs//nodejs:current_node_runtime"],
2323
)
2424

2525
build_test(

e2e/vendored_node/toolchains/BUILD.bazel

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,23 @@ load("@rules_nodejs//nodejs:toolchain.bzl", "nodejs_toolchain")
2626
]
2727
]
2828

29+
[
30+
toolchain(
31+
name = "node_vendored_%s_runtime" % os,
32+
target_compatible_with = [
33+
"@platforms//os:" + os,
34+
"@platforms//cpu:x86_64",
35+
],
36+
toolchain = ":node_" + os,
37+
toolchain_type = "@rules_nodejs//nodejs:runtime_toolchain_type",
38+
)
39+
for os in [
40+
"linux",
41+
"macos",
42+
"windows",
43+
]
44+
]
45+
2946
nodejs_toolchain(
3047
name = "node_linux",
3148
node = "@vendored_node_linux_amd64//:bin/node",

examples/genrule/BUILD.bazel

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,8 @@ genrule(
4343
# $@ is bazel shorthand for the path of the output file
4444
">$@",
4545
]),
46-
toolchains = ["@nodejs_toolchains//:resolved_toolchain"],
47-
tools = ["@nodejs_toolchains//:resolved_toolchain"],
46+
toolchains = ["@rules_nodejs//nodejs:current_node_toolchain"],
47+
tools = ["@rules_nodejs//nodejs:current_node_toolchain"],
4848
)
4949

5050
diff_test(
@@ -74,8 +74,8 @@ genrule(
7474
$(NODE_PATH) \\
7575
./$(execpath :require_acorn_js) \\
7676
$@""",
77-
toolchains = ["@nodejs_toolchains//:resolved_toolchain"],
78-
tools = ["@nodejs_toolchains//:resolved_toolchain"],
77+
toolchains = ["@rules_nodejs//nodejs:current_node_toolchain"],
78+
tools = ["@rules_nodejs//nodejs:current_node_toolchain"],
7979
)
8080

8181
diff_test(

js/private/js_binary.bzl

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ _ATTRS = {
288288
""",
289289
),
290290
"node_toolchain": attr.label(
291-
doc = """The Node.js toolchain to use for this target.
291+
doc = """The Node.js runtime toolchain to use for this target.
292292
293293
See https://bazel-contrib.github.io/rules_nodejs/Toolchains.html
294294
@@ -490,7 +490,7 @@ def _create_launcher(ctx, log_prefix_rule_set, log_prefix_rule, fixed_args = [],
490490
if ctx.attr.node_toolchain:
491491
nodeinfo = ctx.attr.node_toolchain[platform_common.ToolchainInfo].nodeinfo
492492
else:
493-
nodeinfo = ctx.toolchains["@rules_nodejs//nodejs:toolchain_type"].nodeinfo
493+
nodeinfo = ctx.toolchains["@rules_nodejs//nodejs:runtime_toolchain_type"].nodeinfo
494494

495495
directory_path_provider = DirectoryPathInfo if DirectoryPathInfo in ctx.attr.entry_point else _LegacyDirectoryPathInfo
496496
if directory_path_provider in ctx.attr.entry_point:
@@ -612,7 +612,7 @@ js_binary_lib = struct(
612612
toolchains = [
613613
# TODO: on Windows this toolchain is never referenced
614614
"@bazel_tools//tools/sh:toolchain_type",
615-
"@rules_nodejs//nodejs:toolchain_type",
615+
"@rules_nodejs//nodejs:runtime_toolchain_type",
616616
] + COPY_FILE_TO_BIN_TOOLCHAINS,
617617
)
618618

js/private/js_image_layer.bzl

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,7 @@ else {
408408
transitive = [files],
409409
)
410410

411-
nodeinfo = ctx.attr._current_node[platform_common.ToolchainInfo].nodeinfo
411+
nodeinfo = ctx.toolchains["@rules_nodejs//nodejs:toolchain_type"].nodeinfo
412412
node_exec = nodeinfo.node
413413
ctx.actions.run(
414414
inputs = inputs,
@@ -418,6 +418,7 @@ else {
418418
executable = node_exec,
419419
progress_message = "Computing Layer Groups %{label}",
420420
mnemonic = "JsImageLayerGroups",
421+
toolchain = "@rules_nodejs//nodejs:toolchain_type",
421422
)
422423

423424
return expected_layer_groups
@@ -620,10 +621,6 @@ js_image_layer_lib = struct(
620621
default = "//js/private:js_image_layer.mjs",
621622
allow_single_file = True,
622623
),
623-
"_current_node": attr.label(
624-
default = "@nodejs_toolchains//:resolved_toolchain",
625-
cfg = "exec",
626-
),
627624
"binary": attr.label(
628625
mandatory = True,
629626
cfg = _js_image_layer_transition,

npm/private/test/BUILD.bazel

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,5 +85,5 @@ sh_test(
8585
"@nodejs_linux_amd64//:node_files",
8686
"@nodejs_linux_arm64//:node_files",
8787
],
88-
toolchains = ["@nodejs_toolchains//:resolved_toolchain"],
88+
toolchains = ["@rules_nodejs//nodejs:current_node_runtime"],
8989
)

0 commit comments

Comments
 (0)