Skip to content

Commit

Permalink
refactor: use new toolchain for nodejs_binary
Browse files Browse the repository at this point in the history
  • Loading branch information
alexeagle committed Dec 23, 2021
1 parent 0a7d5c7 commit 07be662
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 12 deletions.
4 changes: 4 additions & 0 deletions e2e/symlinked_node_modules_npm/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ local_repository(
path = "../..",
)

load("@rules_nodejs//nodejs:repositories.bzl", "nodejs_register_toolchains")

nodejs_register_toolchains(name = "node")

# rules_nodejs_dev_dependencies() required since we're using local_repository for
# build_bazel_rules_nodejs above.
load("@build_bazel_rules_nodejs//:package.bzl", "rules_nodejs_dev_dependencies")
Expand Down
4 changes: 4 additions & 0 deletions e2e/symlinked_node_modules_yarn/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ local_repository(
path = "../..",
)

load("@rules_nodejs//nodejs:repositories.bzl", "nodejs_register_toolchains")

nodejs_register_toolchains(name = "node")

# rules_nodejs_dev_dependencies() required since we're using local_repository for
# build_bazel_rules_nodejs above.
load("@build_bazel_rules_nodejs//:package.bzl", "rules_nodejs_dev_dependencies")
Expand Down
8 changes: 4 additions & 4 deletions internal/node/node.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -241,18 +241,18 @@ fi

# Add both the node executable for the user's local machine which is in ctx.files._node and comes
# from @nodejs//:node_bin and the node executable from the selected node --platform which comes from
# ctx.toolchains["@build_bazel_rules_nodejs//toolchains/node:toolchain_type"].nodeinfo.
# ctx.toolchains["@rules_nodejs//nodejs:toolchain_type"].nodeinfo.
# In most cases these are the same files but for RBE and when explitely setting --platform for cross-compilation
# any given nodejs_binary should be able to run on both the user's local machine and on the RBE or selected
# platform.
#
# Rules such as nodejs_image should use only ctx.toolchains["@build_bazel_rules_nodejs//toolchains/node:toolchain_type"].nodeinfo
# Rules such as nodejs_image should use only ctx.toolchains["@rules_nodejs//nodejs:toolchain_type"].nodeinfo
# when building the image as that will reflect the selected --platform.

if ctx.attr.toolchain:
node_toolchain = ctx.attr.toolchain[platform_common.ToolchainInfo]
else:
node_toolchain = ctx.toolchains["@build_bazel_rules_nodejs//toolchains/node:toolchain_type"]
node_toolchain = ctx.toolchains["@rules_nodejs//nodejs:toolchain_type"]

node_tool_files = []
node_tool_files.extend(node_toolchain.nodeinfo.tool_files)
Expand Down Expand Up @@ -636,7 +636,7 @@ This will pass --preserve-symlinks and --no-warnings flags to nodejs. Available
"implementation": _nodejs_binary_impl,
"outputs": _NODEJS_EXECUTABLE_OUTPUTS,
"toolchains": [
"@build_bazel_rules_nodejs//toolchains/node:toolchain_type",
"@rules_nodejs//nodejs:toolchain_type",
"@bazel_tools//tools/sh:toolchain_type",
],
}
Expand Down
11 changes: 3 additions & 8 deletions internal/node/node_repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ See https://docs.bazel.build/versions/main/skylark/repository_rules.html

load("@bazel_tools//tools/build_defs/repo:utils.bzl", "maybe")
load("//internal/common:check_bazel_version.bzl", "check_bazel_version")
load("//nodejs/private:nodejs_repo_host_os_alias.bzl", "nodejs_repo_host_os_alias")
load("//nodejs/private:os_name.bzl", "OS_ARCH_NAMES", "node_exists_for_os", "os_name")
load("//nodejs:repositories.bzl", "DEFAULT_NODE_VERSION", node_repositories_rule = "node_repositories")
load("//toolchains/node:node_toolchain_configure.bzl", "node_toolchain_configure")
load("@rules_nodejs//nodejs:repositories.bzl", "nodejs_register_toolchains")

def node_repositories(**kwargs):
"""
Expand Down Expand Up @@ -64,10 +64,5 @@ def node_repositories(**kwargs):
target_tool = target_tool,
)

# This "nodejs" repo is just for convenience so one does not have to target @nodejs_<os_name>//...
# All it does is create aliases to the @nodejs_<host_os>_<host_arch> repository
maybe(
nodejs_repo_host_os_alias,
name = "nodejs",
node_version = node_version,
)
# Install new toolchain under "nodejs" repository name prefix
nodejs_register_toolchains(name = "nodejs")

0 comments on commit 07be662

Please sign in to comment.