Skip to content

Commit 1bcf009

Browse files
guwalexeagle
authored andcommitted
Introduce runtime_toolchain_type
According to discussions in #3854 having two toolchains of the same type for different things is troublesome. It's better to have separate runtime as well as compile toolchains. This commit creates a new runtime_toolchain_type and registers toolchains without execution constraints for this type. Once merged, rules_ts can start consuming the new toolchain type in its js_binary rule to ensure the correct Node for the correct target environment is selected. Fixed [Bug]: Execution toolchain defined without `target_compatible_with` makes it a candidate to selection Fixes #3854 Work towards #3795
1 parent 7efc330 commit 1bcf009

File tree

11 files changed

+218
-62
lines changed

11 files changed

+218
-62
lines changed

MODULE.bazel

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,37 +19,37 @@ use_repo(node, "nodejs_toolchains")
1919

2020
# Toolchain registration under bzlmod should match the order of WORKSPACE registration
2121
# which is the order specified in the PLATFORMS dict https://github.com/bazel-contrib/rules_nodejs/blob/4c373209b058d46f2a5f9ab9f8abf11b161ae459/nodejs/private/nodejs_toolchains_repo.bzl#L20.
22-
# For each platform, `:<PLATFORM>_toolchain_target` should be registered before `:<PLATFORM>_toolchain`,
22+
# For each platform, `:<PLATFORM>_runtime_toolchain` should be registered before `:<PLATFORM>_toolchain`,
2323
# https://github.com/bazel-contrib/rules_nodejs/blob/4c373209b058d46f2a5f9ab9f8abf11b161ae459/nodejs/repositories.bzl#L461/.
2424
# See https://github.com/bazelbuild/bazel/issues/19645 and https://github.com/bazel-contrib/rules_nodejs/pull/3750 for more context.
25-
register_toolchains("@nodejs_toolchains//:linux_amd64_toolchain_target")
25+
register_toolchains("@nodejs_toolchains//:linux_amd64_runtime_toolchain")
2626

2727
register_toolchains("@nodejs_toolchains//:linux_amd64_toolchain")
2828

29-
register_toolchains("@nodejs_toolchains//:linux_arm64_toolchain_target")
29+
register_toolchains("@nodejs_toolchains//:linux_arm64_runtime_toolchain")
3030

3131
register_toolchains("@nodejs_toolchains//:linux_arm64_toolchain")
3232

33-
register_toolchains("@nodejs_toolchains//:linux_s390x_toolchain_target")
33+
register_toolchains("@nodejs_toolchains//:linux_s390x_runtime_toolchain")
3434

3535
register_toolchains("@nodejs_toolchains//:linux_s390x_toolchain")
3636

37-
register_toolchains("@nodejs_toolchains//:linux_ppc64le_toolchain_target")
37+
register_toolchains("@nodejs_toolchains//:linux_ppc64le_runtime_toolchain")
3838

3939
register_toolchains("@nodejs_toolchains//:linux_ppc64le_toolchain")
4040

41-
register_toolchains("@nodejs_toolchains//:darwin_amd64_toolchain_target")
41+
register_toolchains("@nodejs_toolchains//:darwin_amd64_runtime_toolchain")
4242

4343
register_toolchains("@nodejs_toolchains//:darwin_amd64_toolchain")
4444

45-
register_toolchains("@nodejs_toolchains//:darwin_arm64_toolchain_target")
45+
register_toolchains("@nodejs_toolchains//:darwin_arm64_runtime_toolchain")
4646

4747
register_toolchains("@nodejs_toolchains//:darwin_arm64_toolchain")
4848

49-
register_toolchains("@nodejs_toolchains//:windows_amd64_toolchain_target")
49+
register_toolchains("@nodejs_toolchains//:windows_amd64_runtime_toolchain")
5050

5151
register_toolchains("@nodejs_toolchains//:windows_amd64_toolchain")
5252

53-
register_toolchains("@nodejs_toolchains//:windows_arm64_toolchain_target")
53+
register_toolchains("@nodejs_toolchains//:windows_arm64_runtime_toolchain")
5454

5555
register_toolchains("@nodejs_toolchains//:windows_arm64_toolchain")

docs/Toolchains.md

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,16 @@
33
API docs for [Toolchain](https://docs.bazel.build/versions/main/toolchains.html) support.
44

55
When you call `nodejs_register_toolchains()` in your `WORKSPACE` file it will setup a node toolchain for executing tools on all currently supported platforms.
6+
In `bzlmod` default toolchains are registered automatically when you depend on `rules_nodejs`.
7+
8+
There are two toolchain types:
9+
10+
1) The transpilation toolchain, which provides the Node runtime used to execute the transpiler (and type checker), as well as various helper tools and settings.
11+
(`@rules_nodejs//nodejs:toolchain_type`)
12+
2) The Node runtime that executable Node outputs (e.g., js_binary) will run on.
13+
(`@rules_nodejs//nodejs:runtime_toolchain_type`)
14+
15+
See [//nodjes/BUILD.bazel](https://github.com/bazel-contrib/rules_nodejs/blob/main/nodejs/BUILD.bazel) for details.
616

717
If you have an advanced use-case and want to use a version of node not supported by this repository, you can also register your own toolchains.
818

@@ -13,9 +23,9 @@ To run a custom toolchain (i.e., to run a node binary not supported by the built
1323
1) A rule which can build or load a node binary from your repository
1424
(a checked-in binary or a build using a relevant [`rules_foreign_cc` build rule](https://bazelbuild.github.io/rules_foreign_cc/) will do nicely).
1525
2) A [`nodejs_toolchain` rule](Core.html#nodejs_toolchain) which depends on your binary defined in step 1 as its `node`.
16-
3) A [`toolchain` rule](https://bazel.build/reference/be/platform#toolchain) that depends on your `nodejs_toolchain` rule defined in step 2 as its `toolchain`
17-
and on `@rules_nodejs//nodejs:toolchain_type` as its `toolchain_type`. Make sure to define appropriate platform restrictions as described in the
18-
documentation for the `toolchain` rule.
26+
3) Two [`toolchain` rules](https://bazel.build/reference/be/platform#toolchain) each depends on your `nodejs_toolchain` rule defined in step 2 as its `toolchain`
27+
and one on `@rules_nodejs//nodejs:toolchain_type` as its `toolchain_type` and the other one `@rules_nodejs//nodejs:runtime_toolchain_type`.
28+
Make sure to define appropriate platform restrictions as described in the documentation for the `toolchain` rule.
1929
4) A call to [the `register_toolchains` function](https://bazel.build/rules/lib/globals#register_toolchains) in your `WORKSPACE`
2030
that refers to the `toolchain` rule defined in step 3.
2131

e2e/smoke/BUILD.bazel

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,12 @@ not_windows = select({
99
"//conditions:default": [],
1010
})
1111

12+
not_silicon = select({
13+
# There isn't a published arm64 binary for Node 15
14+
"@platforms//cpu:arm64": ["@platforms//:incompatible"],
15+
"//conditions:default": [],
16+
})
17+
1218
# Trivial test fixture: a nodejs program that writes to a file
1319
write_file(
1420
name = "js",
@@ -67,7 +73,6 @@ genrule(
6773
outs = ["actual1"],
6874
cmd = "$(NODE_PATH) $(execpath some.js) $@",
6975
toolchains = ["@nodejs_toolchains//:resolved_toolchain"],
70-
tools = ["@nodejs_toolchains//:resolved_toolchain"],
7176
)
7277

7378
diff_test(
@@ -253,7 +258,9 @@ my_nodejs(
253258
# you can also just provide an individual toolchain if you don't want to download them all
254259
toolchain = select({
255260
"@bazel_tools//src/conditions:linux_x86_64": "@node17_linux_amd64//:toolchain",
256-
"@bazel_tools//src/conditions:darwin": "@node17_darwin_amd64//:toolchain",
261+
"@bazel_tools//src/conditions:linux_aarch64": "@node17_linux_arm64//:toolchain",
262+
"@bazel_tools//src/conditions:darwin_x86_64": "@node17_darwin_amd64//:toolchain",
263+
"@bazel_tools//src/conditions:darwin_arm64": "@node17_darwin_arm64//:toolchain",
257264
"@bazel_tools//src/conditions:windows": "@node17_windows_amd64//:toolchain",
258265
}),
259266
)
@@ -273,7 +280,9 @@ my_nodejs(
273280
# you can also just provide an individual toolchain if you don't want to download them all
274281
toolchain = select({
275282
"@bazel_tools//src/conditions:linux_x86_64": "@nodejs_linux_amd64//:toolchain",
276-
"@bazel_tools//src/conditions:darwin": "@nodejs_darwin_amd64//:toolchain",
283+
"@bazel_tools//src/conditions:linux_aarch64": "@nodejs_linux_arm64//:toolchain",
284+
"@bazel_tools//src/conditions:darwin_x86_64": "@nodejs_darwin_amd64//:toolchain",
285+
"@bazel_tools//src/conditions:darwin_arm64": "@nodejs_darwin_arm64//:toolchain",
277286
"@bazel_tools//src/conditions:windows": "@nodejs_windows_amd64//:toolchain",
278287
}),
279288
)
@@ -288,11 +297,12 @@ my_nodejs(
288297
name = "run_15",
289298
out = "thing_toolchain_15",
290299
entry_point = "version.js",
300+
target_compatible_with = not_silicon,
291301
# using the select statement will download toolchains for all three platforms
292302
# you can also just provide an individual toolchain if you don't want to download them all
293303
toolchain = select({
294304
"@bazel_tools//src/conditions:linux_x86_64": "@node15_linux_amd64//:toolchain",
295-
"@bazel_tools//src/conditions:darwin": "@node15_darwin_amd64//:toolchain",
305+
"@bazel_tools//src/conditions:darwin_x86_64": "@node15_darwin_amd64//:toolchain",
296306
"@bazel_tools//src/conditions:windows": "@node15_windows_amd64//:toolchain",
297307
}),
298308
)

e2e/smoke/MODULE.bazel

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,14 @@ use_repo(
3030
"node15_linux_amd64",
3131
"node15_windows_amd64",
3232
"node17_darwin_amd64",
33+
"node17_darwin_arm64",
3334
"node17_linux_amd64",
35+
"node17_linux_arm64",
3436
"node17_windows_amd64",
3537
"nodejs_darwin_amd64",
38+
"nodejs_darwin_arm64",
3639
"nodejs_linux_amd64",
40+
"nodejs_linux_arm64",
3741
"nodejs_toolchains",
3842
"nodejs_windows_amd64",
3943
)

e2e/smoke/defs.bzl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ def _my_nodejs_impl(ctx):
44
if ctx.attr.toolchain:
55
nodeinfo = ctx.attr.toolchain[platform_common.ToolchainInfo].nodeinfo
66
else:
7-
nodeinfo = ctx.toolchains["@rules_nodejs//nodejs:toolchain_type"].nodeinfo
7+
nodeinfo = ctx.toolchains["@rules_nodejs//nodejs:runtime_toolchain_type"].nodeinfo
88
ctx.actions.run(
99
inputs = [ctx.file.entry_point],
1010
executable = nodeinfo.node,
@@ -20,5 +20,5 @@ my_nodejs = rule(
2020
"out": attr.output(),
2121
"toolchain": attr.label(),
2222
},
23-
toolchains = ["@rules_nodejs//nodejs:toolchain_type"],
23+
toolchains = ["@rules_nodejs//nodejs:runtime_toolchain_type"],
2424
)

nodejs/BUILD.bazel

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
load("@bazel_lib//:bzl_library.bzl", "bzl_library")
33
load("//nodejs/private:nodejs_toolchains_repo.bzl", "PLATFORMS")
44
load("//nodejs/private:user_build_settings.bzl", "user_args")
5+
load(":node_toolchain_alias.bzl", "node_host_runtime_alias", "node_runtime_alias", "node_toolchain_alias")
56

67
package(default_visibility = ["//visibility:public"])
78

@@ -25,23 +26,44 @@ bzl_library(
2526
],
2627
)
2728

28-
bzl_library(
29-
name = "toolchain",
30-
srcs = ["toolchain.bzl"],
31-
)
32-
33-
bzl_library(
34-
name = "extensions",
35-
srcs = ["extensions.bzl"],
36-
deps = ["repositories"],
37-
)
38-
3929
# This is the target rule authors should put in their "toolchains"
4030
# attribute in order to get a node interpreter for the correct
4131
# platform.
4232
# See https://docs.bazel.build/versions/main/toolchains.html#writing-rules-that-use-toolchains
33+
# A single binary distribution of a Node provides two different types of toolchains from the
34+
# perspective of Bazel:
35+
36+
# (1) The transpilation toolchain, which provides the Node runtime used to execute the transpiler
37+
# (and type checker), as well as various helper tools and settings.
38+
#
39+
# Toolchains of this type typically have constraints on the execution platform so that their Node
40+
# runtime can run the transpiler, but not on the target platform as Node transpilation outputs are
41+
# platform independent.
42+
#
43+
# Obtain the associated NodeInfo via:
44+
# ctx.toolchains["@bazel_tools//tools/jdk:toolchain_type"].nodeinfo
4345
toolchain_type(name = "toolchain_type")
4446

47+
# (2) The Node runtime that executable Node outputs (e.g., js_binary) will run on.
48+
#
49+
# Toolchains of this type typically have constraints on the target platform so that the runtime's
50+
# native 'node' binary can be run there, but not on the execution platform as building an executable
51+
# Node target only requires copying or symlinking the runtime, which can be done on any platform.
52+
#
53+
# Obtain the associated NodeRuntimeInfo via:
54+
# ctx.toolchains["@bazel_tools//tools/jdk:runtime_toolchain_type"].nodeinfo
55+
toolchain_type(name = "runtime_toolchain_type")
56+
57+
# Points to toolchain[":runtime_toolchain_type"]
58+
node_runtime_alias(name = "current_node_runtime")
59+
60+
# Host configuration of ":current_node_runtime"
61+
node_host_runtime_alias(name = "current_host_node_runtime")
62+
63+
# Points to toolchain[":toolchain_type"]
64+
node_toolchain_alias(name = "current_node_toolchain")
65+
66+
# The platforms that are supported by the Node toolchains.
4567
[
4668
platform(
4769
name = key,

nodejs/node_toolchain_alias.bzl

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
# Copyright 2019 The Bazel Authors. All rights reserved.
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
"""Node toolchain aliases using toolchain resolution."""
16+
17+
load(":semantics.bzl", "semantics")
18+
load(":toolchain.bzl", "NodeInfo")
19+
20+
def _node_runtime_alias(ctx):
21+
"""Implementation of node_runtime_alias using toolchain resolution."""
22+
toolchain_info = ctx.toolchains[semantics.NODE_RUNTIME_TOOLCHAIN_TYPE]
23+
toolchain = toolchain_info.nodeinfo
24+
template_variable_info = toolchain_info.template_variables
25+
default_info = toolchain_info.default
26+
return [
27+
toolchain_info,
28+
toolchain,
29+
template_variable_info,
30+
default_info,
31+
]
32+
33+
node_runtime_alias = rule(
34+
implementation = _node_runtime_alias,
35+
toolchains = [semantics.NODE_RUNTIME_TOOLCHAIN],
36+
)
37+
38+
def _node_host_runtime_alias(ctx):
39+
"""Implementation of node_host_runtime_alias using toolchain resolution."""
40+
runtime = ctx.attr._runtime
41+
toolchain = runtime[NodeInfo]
42+
template_variable_info = runtime[platform_common.TemplateVariableInfo]
43+
default_info = runtime[DefaultInfo]
44+
toolchain_info = platform_common.ToolchainInfo(nodeinfo = toolchain)
45+
return [
46+
toolchain,
47+
template_variable_info,
48+
toolchain_info,
49+
default_info,
50+
]
51+
52+
node_host_runtime_alias = rule(
53+
implementation = _node_host_runtime_alias,
54+
attrs = {
55+
"_runtime": attr.label(
56+
default = Label("//nodejs:current_node_runtime"),
57+
providers = [
58+
NodeInfo,
59+
platform_common.TemplateVariableInfo,
60+
],
61+
cfg = "exec",
62+
),
63+
},
64+
provides = [
65+
NodeInfo,
66+
platform_common.TemplateVariableInfo,
67+
platform_common.ToolchainInfo,
68+
],
69+
)
70+
71+
def _node_toolchain_alias(ctx):
72+
"""An implementation of node_toolchain_alias using toolchain resolution."""
73+
toolchain_info = ctx.toolchains[semantics.NODE_TOOLCHAIN_TYPE]
74+
toolchain = toolchain_info.nodeinfo
75+
76+
return [
77+
toolchain_info,
78+
toolchain,
79+
]
80+
81+
node_toolchain_alias = rule(
82+
implementation = _node_toolchain_alias,
83+
toolchains = [semantics.NODE_TOOLCHAIN],
84+
)

nodejs/private/nodejs_toolchains_repo.bzl

Lines changed: 18 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -69,55 +69,42 @@ PLATFORMS = {
6969
}
7070

7171
def _nodejs_toolchains_repo_impl(repository_ctx):
72-
# Expose a concrete toolchain which is the result of Bazel resolving the toolchain
73-
# for the execution or target platform.
74-
# Workaround for https://github.com/bazelbuild/bazel/issues/14009
75-
starlark_content = """# Generated by nodejs_toolchains_repo.bzl
76-
77-
# Forward all the providers
78-
def _resolved_toolchain_impl(ctx):
79-
toolchain_info = ctx.toolchains["@rules_nodejs//nodejs:toolchain_type"]
80-
return [
81-
toolchain_info,
82-
toolchain_info.default,
83-
toolchain_info.nodeinfo,
84-
toolchain_info.template_variables,
85-
]
86-
87-
# Copied from java_toolchain_alias
88-
# https://cs.opensource.google/bazel/bazel/+/master:tools/jdk/java_toolchain_alias.bzl
89-
resolved_toolchain = rule(
90-
implementation = _resolved_toolchain_impl,
91-
toolchains = ["@rules_nodejs//nodejs:toolchain_type"],
92-
)
93-
"""
94-
repository_ctx.file("defs.bzl", starlark_content)
95-
96-
build_content = """# Generated by nodejs_toolchains_repo.bzl
72+
# TODO(7.0) Drop support for deprecated alias
73+
build_content = '''# Generated by nodejs_toolchains_repo.bzl
9774
#
9875
# These can be registered in the workspace file or passed to --extra_toolchains flag.
9976
# By default all these toolchains are registered by the nodejs_register_toolchains macro
10077
# so you don't normally need to interact with these targets.
10178
102-
load(":defs.bzl", "resolved_toolchain")
103-
104-
resolved_toolchain(name = "resolved_toolchain", visibility = ["//visibility:public"])
79+
alias(
80+
name = "resolved_toolchain",
81+
actual = "@rules_nodejs//nodejs:current_node_runtime",
82+
deprecation = """
83+
Use one of the following instead:
84+
- @rules_nodejs//nodejs:current_node_runtime
85+
- @rules_nodejs//nodejs:current_host_node_runtime
86+
- @rules_nodejs//nodejs:current_node_toolchain
87+
See https://github.com/bazel-contrib/rules_nodejs/issues/3795.
88+
""",
89+
visibility = ["//visibility:public"],
90+
)
10591
106-
"""
92+
'''
10793

10894
for [platform, meta] in PLATFORMS.items():
10995
build_content += """
11096
toolchain(
11197
name = "{platform}_toolchain",
11298
exec_compatible_with = {compatible_with},
99+
target_compatible_with = {compatible_with}, # https://github.com/bazel-contrib/rules_nodejs/issues/3854
113100
toolchain = "@{user_node_repository_name}_{platform}//:toolchain",
114101
toolchain_type = "@rules_nodejs//nodejs:toolchain_type",
115102
)
116103
toolchain(
117-
name = "{platform}_toolchain_target",
104+
name = "{platform}_runtime_toolchain",
118105
target_compatible_with = {compatible_with},
119106
toolchain = "@{user_node_repository_name}_{platform}//:toolchain",
120-
toolchain_type = "@rules_nodejs//nodejs:toolchain_type",
107+
toolchain_type = "@rules_nodejs//nodejs:runtime_toolchain_type",
121108
)
122109
""".format(
123110
platform = platform,

nodejs/repositories.bzl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -476,7 +476,7 @@ def nodejs_register_toolchains(name = DEFAULT_NODE_REPOSITORY, register = True,
476476
)
477477
if register:
478478
native.register_toolchains(
479-
"@%s_toolchains//:%s_toolchain_target" % (name, platform),
479+
"@%s_toolchains//:%s_runtime_toolchain" % (name, platform),
480480
"@%s_toolchains//:%s_toolchain" % (name, platform),
481481
)
482482

0 commit comments

Comments
 (0)