Skip to content

Split :toolchain_type to correctly cover all use cases #3800

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ use_repo(node, "nodejs_toolchains")
# For each platform, `:<PLATFORM>_toolchain_target` should be registered before `:<PLATFORM>_toolchain`,
# https://github.com/bazel-contrib/rules_nodejs/blob/4c373209b058d46f2a5f9ab9f8abf11b161ae459/nodejs/repositories.bzl#L461/.
# See https://github.com/bazelbuild/bazel/issues/19645 and https://github.com/bazel-contrib/rules_nodejs/pull/3750 for more context.
# TODO This can be simplified to `register_toolchains("@nodejs_toolchains//:all")` in the next
# major version, or sooner if INCOMPATIBLE_RULES_NODEJS_DO_NOT_OVERLOAD_TOOLCHAIN_TYPE=1.
register_toolchains("@nodejs_toolchains//:linux_amd64_toolchain_target")

register_toolchains("@nodejs_toolchains//:linux_amd64_toolchain")
Expand Down
4 changes: 3 additions & 1 deletion docs/Core.md
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,8 @@ Additional parameters

## nodejs_toolchain

TODO Update to reflect toolchain splitting.

**USAGE**

<pre>
Expand Down Expand Up @@ -193,7 +195,7 @@ toolchain(
"@platforms//cpu:x86_64",
],
toolchain = ":toolchain",
toolchain_type = "@rules_nodejs//nodejs:toolchain_type",
toolchain_type = "@rules_nodejs//nodejs:exec_runtime_toolchain_type",
)
```

Expand Down
4 changes: 3 additions & 1 deletion docs/Toolchains.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@ If you have an advanced use-case and want to use a version of node not supported

## Registering a custom toolchain

TODO Update to reflect toolchain splitting

To run a custom toolchain (i.e., to run a node binary not supported by the built-in toolchains), you'll need four things:

1) A rule which can build or load a node binary from your repository
(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).
2) A [`nodejs_toolchain` rule](Core.html#nodejs_toolchain) which depends on your binary defined in step 1 as its `node`.
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`
and on `@rules_nodejs//nodejs:toolchain_type` as its `toolchain_type`. Make sure to define appropriate platform restrictions as described in the
and on `@rules_nodejs//nodejs:runtime_toolchain_type` as its `toolchain_type`. Make sure to define appropriate platform restrictions as described in the
documentation for the `toolchain` rule.
4) A call to [the `register_toolchains` function](https://bazel.build/rules/lib/globals#register_toolchains) in your `WORKSPACE`
that refers to the `toolchain` rule defined in step 3.
Expand Down
2 changes: 2 additions & 0 deletions e2e/smoke/MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ use_repo(
"nodejs_windows_amd64",
)

register_toolchains("@nodejs_toolchains//:all")

http_archive = use_repo_rule("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")

http_archive(
Expand Down
11 changes: 7 additions & 4 deletions e2e/smoke/defs.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ def _my_nodejs_impl(ctx):
if ctx.attr.toolchain:
nodeinfo = ctx.attr.toolchain[platform_common.ToolchainInfo].nodeinfo
else:
nodeinfo = ctx.toolchains["@rules_nodejs//nodejs:toolchain_type"].nodeinfo
nodeinfo = ctx.toolchains["@rules_nodejs//nodejs:exec_runtime_toolchain_type"].nodeinfo
ctx.actions.run(
inputs = [ctx.file.entry_point],
executable = nodeinfo.node,
Expand All @@ -16,9 +16,12 @@ def _my_nodejs_impl(ctx):
my_nodejs = rule(
implementation = _my_nodejs_impl,
attrs = {
"entry_point": attr.label(allow_single_file = True),
"entry_point": attr.label(
allow_single_file = True,
cfg = "exec",
),
"out": attr.output(),
"toolchain": attr.label(),
"toolchain": attr.label(cfg = "exec"),
},
toolchains = ["@rules_nodejs//nodejs:toolchain_type"],
toolchains = ["@rules_nodejs//nodejs:exec_runtime_toolchain_type"],
)
58 changes: 53 additions & 5 deletions nodejs/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
load("@bazel_skylib//:bzl_library.bzl", "bzl_library")
load("//nodejs/private:nodejs_toolchains_repo.bzl", "PLATFORMS")
load("//nodejs/private:user_build_settings.bzl", "user_args")
load("@rules_nodejs//nodejs:toolchain.bzl", "resolved_toolchain")

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

Expand All @@ -19,11 +20,58 @@ bzl_library(
],
)

# This is the target rule authors should put in their "toolchains"
# attribute in order to get a node interpreter for the correct
# platform.
# See https://docs.bazel.build/versions/main/toolchains.html#writing-rules-that-use-toolchains
toolchain_type(name = "toolchain_type")
# Appropriate for;
# - Providing a runtime for *_binary rules.
# - Compiling against the Node-API.
# See also exec_runtime_toolchain_type.
# See also :resolved_toolchain (e.g. for genrule)
toolchain_type(name = "runtime_toolchain_type")

# Helper for rules such as `genrule`.
# Appropriate for (under target configuration);
# - Providing a runtime for executable outputs.
# - Compiling against the Node-API.
# Appropriate for (under exec configuration);
# - Providing a runtime for rule actions.
# Appropriate for (under exec configuration with matching target constraints);
# - Snapshot generation (--build-snapshot, --experimental-sea-config with `useSnapshot` or
# `useCodeCache`). Sensitive to OS, architecture NodeJS version and other factors constraints
# alone cannot capture such as;
# - V8 flags
# - CPU features
resolved_toolchain(
name = "resolved_toolchain",
visibility = ["//visibility:public"],
)

alias(
name = "toolchain_type",
actual = "runtime_toolchain_type",
deprecation = """
Use one of the following instead;
- @rules_nodejs//nodejs:runtime_toolchain_type
- @rules_nodejs//nodejs:exec_runtime_toolchain_type
- @rules_nodejs//nodejs:compilation_toolchain_type
See https://github.com/bazel-contrib/rules_nodejs/issues/3795.
"""
)

# Inverse of :runtime_toolchain_type,
# Appropriate for;
# - Providing a runtime for rule actions.
# See also :resolved_toolchain (e.g. for genrule).
toolchain_type(name = "exec_runtime_toolchain_type")

# Appropriate for;
# - Snapshot generation (--build-snapshot, --experimental-sea-config with `useSnapshot` or
# `useCodeCache`). Sensitive to OS, architecture NodeJS version and other factors constraints
# alone cannot capture such as;
# - V8 flags
# - CPU features
# This toolchain exists to cover niche scenarios where the runtime needs to be compatible across
# target _and_ execution platforms. It should not be used without good reason.
# See also resolved_toolchain (e.g. for genrule).
toolchain_type(name = "compilation_toolchain_type")

[
platform(
Expand Down
9 changes: 9 additions & 0 deletions nodejs/extensions.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ def _toolchain_extension(module_ctx):
node_urls = v.node_urls,
include_headers = v.include_headers,
register = False,
incompatible_split_toolchains = v.incompatible_split_toolchains,
)

_ATTRS = {
Expand Down Expand Up @@ -84,6 +85,14 @@ This setting creates a dependency on a c++ toolchain.
""",
default = [DEFAULT_NODE_URL],
),
"incompatible_split_toolchains": attr.bool(
default = False,
doc = """
When true, the the `@rules_nodejs//nodejs:runtime_toolchain_type` (formerly
`@rules_nodejs//nodejs:toolchain_type`) toolchain type will only contain toolchains suitable for
*_binary rule outputs. See [#3795](https://github.com/bazel-contrib/rules_nodejs/issues/3795).
""",
),
}

node = module_extension(
Expand Down
4 changes: 2 additions & 2 deletions nodejs/private/current_node_cc_headers.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@
"""Implementation of current_node_cc_headers rule."""

def _current_node_cc_headers_impl(ctx):
return ctx.toolchains["//nodejs:toolchain_type"].nodeinfo.headers.providers_map.values()
return ctx.toolchains["//nodejs:runtime_toolchain_type"].nodeinfo.headers.providers_map.values()

current_node_cc_headers = rule(
implementation = _current_node_cc_headers_impl,
toolchains = ["//nodejs:toolchain_type"],
toolchains = ["//nodejs:runtime_toolchain_type"],
provides = [CcInfo],
doc = """\
Provides the currently active Node toolchain's C++ headers.
Expand Down
90 changes: 60 additions & 30 deletions nodejs/private/nodejs_toolchains_repo.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -68,51 +68,71 @@ def _nodejs_toolchains_repo_impl(repository_ctx):
# Workaround for https://github.com/bazelbuild/bazel/issues/14009
starlark_content = """# Generated by nodejs_toolchains_repo.bzl

# Forward all the providers
def _resolved_toolchain_impl(ctx):
toolchain_info = ctx.toolchains["@rules_nodejs//nodejs:toolchain_type"]
return [
toolchain_info,
toolchain_info.default,
toolchain_info.nodeinfo,
toolchain_info.template_variables,
]
load("@rules_nodejs//nodejs:toolchain.bzl", _resolved_toolchain = "resolved_toolchain")

# Copied from java_toolchain_alias
# https://cs.opensource.google/bazel/bazel/+/master:tools/jdk/java_toolchain_alias.bzl
resolved_toolchain = rule(
implementation = _resolved_toolchain_impl,
toolchains = ["@rules_nodejs//nodejs:toolchain_type"],
incompatible_use_toolchain_transition = True,
)
# DEPRECATED
resolved_toolchain = _resolved_toolchain
"""
repository_ctx.file("defs.bzl", starlark_content)

# TODO Remove in next major version
build_content = """# Generated by nodejs_toolchains_repo.bzl
#
# These can be registered in the workspace file or passed to --extra_toolchains flag.
# By default all these toolchains are registered by the nodejs_register_toolchains macro
# so you don't normally need to interact with these targets.

load(":defs.bzl", "resolved_toolchain")

resolved_toolchain(name = "resolved_toolchain", visibility = ["//visibility:public"])

alias(
name = "resolved_toolchain",
actual = "@rules_nodejs//nodejs:resolved_toolchain",
visibility = ["//visibility:public"],
deprecation = "Use @rules_nodejs//nodejs:resolved_toolchain instead. See https://github.com/bazel-contrib/rules_nodejs/issues/3795.",
)
"""

for [platform, meta] in PLATFORMS.items():
build_content += """
toolchain(
name = "{platform}_toolchain",
name = "{platform}_runtime_toolchain",
target_compatible_with = {compatible_with},
toolchain = "@{user_node_repository_name}_{platform}//:toolchain",
toolchain_type = "@rules_nodejs//nodejs:runtime_toolchain_type",
)
toolchain(
name = "{platform}_exec_runtime_toolchain",
exec_compatible_with = {compatible_with},
toolchain = "@{user_node_repository_name}_{platform}//:toolchain",
toolchain_type = "@rules_nodejs//nodejs:toolchain_type",
toolchain_type = "@rules_nodejs//nodejs:exec_runtime_toolchain_type",
)
toolchain(
name = "{platform}_toolchain_target",
name = "{platform}_compilation_toolchain",
exec_compatible_with = {compatible_with},
target_compatible_with = {compatible_with},
toolchain = "@{user_node_repository_name}_{platform}//:toolchain",
toolchain_type = "@rules_nodejs//nodejs:toolchain_type",
toolchain_type = "@rules_nodejs//nodejs:compilation_toolchain_type",
)
""".format(
platform = platform,
name = repository_ctx.attr.name,
user_node_repository_name = repository_ctx.attr.user_node_repository_name,
compatible_with = meta.compatible_with,
)

# TODO Remove in next major version
if repository_ctx.attr.incompatible_split_toolchains == False:
build_content += """
toolchain(
name = "{platform}_runtime_toolchain_exec_overload",
exec_compatible_with = {compatible_with},
toolchain = "@{user_node_repository_name}_{platform}//:toolchain",
toolchain_type = "@rules_nodejs//nodejs:runtime_toolchain_type",
deprecation = "Will be removed in next major version. See https://github.com/bazel-contrib/rules_nodejs/issues/3795.",
)
alias(
name = "{platform}_toolchain",
actual = "{platform}_runtime_toolchain_exec_overload",
deprecation = "Will be removed in next major version. See https://github.com/bazel-contrib/rules_nodejs/issues/3795.",
)
alias(
name = "{platform}_toolchain_target",
actual = "{platform}_runtime_toolchain",
deprecation = "Use :{platform}_runtime_toolchain instead. See https://github.com/bazel-contrib/rules_nodejs/issues/3795.",
)
""".format(
platform = platform,
Expand All @@ -126,9 +146,19 @@ toolchain(

nodejs_toolchains_repo = repository_rule(
_nodejs_toolchains_repo_impl,
doc = """Creates a repository with toolchain definitions for all known platforms
which can be registered or selected.""",
doc = """
Creates a repository with toolchain definitions for all known platforms which can be
registered or selected.
""",
attrs = {
"user_node_repository_name": attr.string(doc = "what the user chose for the base name, eg. node16"),
"incompatible_split_toolchains": attr.bool(
default = False,
doc = """
When true, the the `@rules_nodejs//nodejs:runtime_toolchain_type` (formerly
`@rules_nodejs//nodejs:toolchain_type`) toolchain type will only contain toolchains suitable for
*_binary rule outputs. See [#3795](https://github.com/bazel-contrib/rules_nodejs/issues/3795).
""",
),
},
)
18 changes: 15 additions & 3 deletions nodejs/repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,12 @@ If your are not calling node_repositories directly you may need to upgrade to ru
nodejs_repositories(**kwargs)

# Wrapper macro around everything above, this is the primary API
def nodejs_register_toolchains(name = DEFAULT_NODE_REPOSITORY, register = True, **kwargs):
def nodejs_register_toolchains(
name = DEFAULT_NODE_REPOSITORY,
register = True,
incompatible_split_toolchains = False,
**kwargs
):
"""Convenience macro for users which does typical setup.

- create a repository for each built-in platform like "node16_linux_amd64" -
Expand All @@ -465,6 +470,9 @@ def nodejs_register_toolchains(name = DEFAULT_NODE_REPOSITORY, register = True,
register: whether to call Bazel register_toolchains on the created toolchains.
Should be True when used from a WORKSPACE file, and False used from bzlmod
which has its own toolchain registration syntax.
incompatible_split_toolchains: when true, the `:runtime_toolchain_type` (formerly
`:toolchain_type`) toolchain type will only contain toolchains suitable for *_binary
rule outputs. See [#3795](https://github.com/bazel-contrib/rules_nodejs/issues/3795).
**kwargs: passed to each nodejs_repositories call
"""
for platform in BUILT_IN_NODE_PLATFORMS:
Expand All @@ -475,9 +483,12 @@ def nodejs_register_toolchains(name = DEFAULT_NODE_REPOSITORY, register = True,
)
if register:
native.register_toolchains(
"@%s_toolchains//:%s_toolchain_target" % (name, platform),
"@%s_toolchains//:%s_toolchain" % (name, platform),
"@%s_toolchains//:%s_runtime_toolchain" % (name, platform),
"@%s_toolchains//:%s_exec_runtime_toolchain" % (name, platform),
"@%s_toolchains//:%s_compilation_toolchain" % (name, platform),
)
if incompatible_split_toolchains:
native.register_toolchains("@%s_toolchains//:%s_runtime_toolchain_exec_overload" % (name, platform))

nodejs_repo_host_os_alias(
name = name,
Expand All @@ -493,6 +504,7 @@ def nodejs_register_toolchains(name = DEFAULT_NODE_REPOSITORY, register = True,
nodejs_toolchains_repo(
name = name + "_toolchains",
user_node_repository_name = name,
incompatible_split_toolchains = incompatible_split_toolchains,
)

def rules_nodejs_dependencies():
Expand Down
18 changes: 17 additions & 1 deletion nodejs/toolchain.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ def nodejs_toolchain(
"@platforms//cpu:x86_64",
],
toolchain = ":toolchain",
toolchain_type = "@rules_nodejs//nodejs:toolchain_type",
toolchain_type = "@rules_nodejs//nodejs:runtime_toolchain_type",
)
```

Expand Down Expand Up @@ -261,3 +261,19 @@ WARNING: node_toolchain is deprecated; use nodejs_toolchain instead.
If your are not calling node_toolchain directly you may need to upgrade to rules_js 2.x to suppress this warning.
""")
nodejs_toolchain(**kwargs)

# Forward all the providers
def _resolved_toolchain_impl(ctx):
toolchain_info = ctx.toolchains["@rules_nodejs//nodejs:runtime_toolchain_type"]
return [
toolchain_info,
toolchain_info.default,
toolchain_info.nodeinfo,
toolchain_info.template_variables,
]

# Based on https://github.com/bazelbuild/rules_java/blob/6a34389003a6bed549858bb8f4673dd521ad8a54/toolchains/java_toolchain_alias.bzl#L19-L40
resolved_toolchain = rule(
implementation = _resolved_toolchain_impl,
toolchains = ["@rules_nodejs//nodejs:runtime_toolchain_type"],
)
Loading