Skip to content
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

refactor: use new toolchain for nodejs_binary #3167

Merged
merged 1 commit into from
Dec 23, 2021
Merged
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
16 changes: 6 additions & 10 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -39,16 +39,6 @@ local_repository(
path = "internal/pkg_npm/test/vendored_external",
)

#
# Setup node repositories
#

load("//:index.bzl", "BAZEL_VERSION", "SUPPORTED_BAZEL_VERSIONS", "node_repositories")

node_repositories(
node_version = "16.5.0",
)

#
# Install rules_nodejs dev dependencies
#
Expand All @@ -75,6 +65,12 @@ nodejs_register_toolchains(
node_version = "15.14.0",
)

load("//:index.bzl", "BAZEL_VERSION", "SUPPORTED_BAZEL_VERSIONS", "node_repositories")

node_repositories(
node_version = "16.5.0",
)

load("@build_bazel_rules_nodejs//:npm_deps.bzl", "npm_deps")

npm_deps()
Expand Down
6 changes: 6 additions & 0 deletions e2e/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,12 @@ e2e_integration_test(

e2e_integration_test(
name = "e2e_nodejs_repository",
# Only run on buildkite linux as we hard-coded the platform in the test
tags = [
"no-bazelci-mac",
"no-bazelci-windows",
"no-circleci",
],
)

e2e_integration_test(
Expand Down
1 change: 1 addition & 0 deletions e2e/nodejs_image/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ nodejs_image(
name = "nodejs_image",
binary = ":main",
include_node_repo_args = False,
node_repository_name = "nodejs_linux_amd64",
)

container_test(
Expand Down
24 changes: 12 additions & 12 deletions e2e/nodejs_repository/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,18 @@ nodejs_test(
name = "test",
data = [
"index.spec.js",
"@nodejs//:node",
"@nodejs//:node_bin",
"@nodejs//:node_files",
"@nodejs//:npm",
"@nodejs//:npm_bin",
"@nodejs//:npm_files",
"@nodejs//:npm_node_repositories",
"@nodejs//:npx_bin",
"@nodejs//:yarn",
"@nodejs//:yarn_bin",
"@nodejs//:yarn_files",
"@nodejs//:yarn_node_repositories",
"@nodejs_linux_amd64//:node",
"@nodejs_linux_amd64//:node_bin",
"@nodejs_linux_amd64//:node_files",
"@nodejs_linux_amd64//:npm",
"@nodejs_linux_amd64//:npm_bin",
"@nodejs_linux_amd64//:npm_files",
"@nodejs_linux_amd64//:npm_node_repositories",
"@nodejs_linux_amd64//:npx_bin",
"@nodejs_linux_amd64//:yarn",
"@nodejs_linux_amd64//:yarn_bin",
"@nodejs_linux_amd64//:yarn_files",
"@nodejs_linux_amd64//:yarn_node_repositories",
],
entry_point = ":index.spec.js",
)
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
13 changes: 7 additions & 6 deletions examples/angular/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,15 @@ http_archive(
urls = ["https://github.com/bazelbuild/rules_nodejs/releases/download/4.4.0/rules_nodejs-4.4.0.tar.gz"],
)

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

rules_nodejs_dependencies()

nodejs_register_toolchains(
name = "nodejs",
node_version = "12.14.1",
)

load("@bazel_skylib//:workspace.bzl", "bazel_skylib_workspace")

bazel_skylib_workspace()
Expand All @@ -44,11 +49,7 @@ http_archive(
)

# Check the bazel version and download npm dependencies
load("@build_bazel_rules_nodejs//:index.bzl", "node_repositories", "yarn_install")

node_repositories(
node_version = "12.14.1",
)
load("@build_bazel_rules_nodejs//:index.bzl", "yarn_install")

# Setup the Node.js toolchain & install our npm dependencies into @npm
yarn_install(
Expand Down
1 change: 1 addition & 0 deletions examples/angular/src/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ nodejs_image(
name = "nodejs_image",
binary = ":prodserver",
include_node_repo_args = False,
node_repository_name = "nodejs_linux_amd64",
# Actions created by this rule are I/O-bound,
# so there is no benefit to running them remotely
tags = ["local"],
Expand Down
1 change: 1 addition & 0 deletions examples/nestjs/src/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -78,4 +78,5 @@ nodejs_image(
name = "docker",
binary = ":server",
include_node_repo_args = False,
node_repository_name = "nodejs_linux_amd64",
)
9 changes: 8 additions & 1 deletion examples/parcel/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
load("@build_bazel_rules_nodejs//:index.bzl", "generated_file_test")
load("@build_bazel_rules_nodejs//:index.bzl", "generated_file_test", "nodejs_test")
load(":parcel.bzl", "parcel")

# Double-check that we're on the right nodejs version
# since CircleCI fails with a python error if we are too new
nodejs_test(
name = "test_version",
entry_point = "node_version.js",
)

parcel(
name = "bundle",
srcs = [
Expand Down
9 changes: 5 additions & 4 deletions examples/parcel/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,12 @@ http_archive(
urls = ["https://github.com/bazelbuild/rules_nodejs/releases/download/4.4.0/rules_nodejs-4.4.0.tar.gz"],
)

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

rules_nodejs_dependencies()

load("@build_bazel_rules_nodejs//:index.bzl", "node_repositories", "npm_install")

node_repositories(
nodejs_register_toolchains(
name = "nodejs",
# with node 16, this example fails on circleCI, because
# parcel has a gyp build on install that fails due to wrong
# version of the python interpreter:
Expand All @@ -53,6 +52,8 @@ node_repositories(
node_version = "14.17.5",
)

load("@build_bazel_rules_nodejs//:index.bzl", "npm_install")

npm_install(
name = "npm",
exports_directories_only = True,
Expand Down
1 change: 1 addition & 0 deletions examples/parcel/node_version.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
require('assert').equal(process.version, 'v14.17.5')
16 changes: 16 additions & 0 deletions examples/vendored_node/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,4 +1,20 @@
load("@npm//@bazel/jasmine:index.bzl", "jasmine_node_test")
load("@rules_nodejs//nodejs:toolchain.bzl", "node_toolchain")

node_toolchain(
name = "node_toolchain",
target_tool = "@vendored_node_15_0_1//:node-v15.0.1-linux-x64/bin/node",
)

toolchain(
name = "node15_toolchain",
exec_compatible_with = [
"@platforms//os:linux",
"@platforms//cpu:x86_64",
],
toolchain = ":node_toolchain",
toolchain_type = "@rules_nodejs//nodejs:toolchain_type",
)

jasmine_node_test(
name = "test",
Expand Down
7 changes: 2 additions & 5 deletions examples/vendored_node/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,11 @@ http_archive(
urls = ["https://nodejs.org/dist/v15.0.1/node-v15.0.1-linux-x64.tar.xz"],
)

load("@build_bazel_rules_nodejs//:index.bzl", "node_repositories", "npm_install")
load("@build_bazel_rules_nodejs//:index.bzl", "npm_install")

# This example only works on linux as it downloads the linux node distribution
# TODO(gregmagolan): make node_repositories acccept different archives for different platforms
node_repositories(
node_version = "15.0.1",
vendored_node = "@vendored_node_15_0_1//:node-v15.0.1-linux-x64",
)
register_toolchains("//:node15_toolchain")

npm_install(
name = "npm",
Expand Down
16 changes: 16 additions & 0 deletions examples/vendored_node_and_yarn/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,4 +1,20 @@
load("@npm//@bazel/jasmine:index.bzl", "jasmine_node_test")
load("@rules_nodejs//nodejs:toolchain.bzl", "node_toolchain")

node_toolchain(
name = "node_toolchain",
target_tool = "@vendored_node_10_12_0//:node-v10.12.0-linux-x64/bin/node",
)

toolchain(
name = "node10_toolchain",
exec_compatible_with = [
"@platforms//os:linux",
"@platforms//cpu:x86_64",
],
toolchain = ":node_toolchain",
toolchain_type = "@rules_nodejs//nodejs:toolchain_type",
)

jasmine_node_test(
name = "test",
Expand Down
2 changes: 2 additions & 0 deletions examples/vendored_node_and_yarn/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ http_archive(
urls = ["https://github.com/yarnpkg/yarn/releases/download/v1.10.0/yarn-v1.10.0.tar.gz"],
)

register_toolchains("//:node10_toolchain")

load("@build_bazel_rules_nodejs//:index.bzl", "node_repositories", "yarn_install")

# This example only works on linux as it downloads the linux node distribution
Expand Down
1 change: 1 addition & 0 deletions internal/node/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ bzl_library(
"//nodejs:bzl",
"//third_party/github.com/bazelbuild/bazel-skylib:bzl",
"//toolchains/node:bzl",
"@rules_nodejs//nodejs:bzl",
],
)

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")
6 changes: 1 addition & 5 deletions packages/concatjs/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,7 @@ js_library(
bzl_library(
name = "bzl",
testonly = True,
srcs = glob(["*.bzl"]) + [
# Work-around since we don't have and don't wnat a bzl_library in the generated
# @nodejs//:BUILD.bazel file
"@nodejs//:index.bzl",
],
srcs = glob(["*.bzl"]),
deps = [
"//packages/concatjs/devserver:bzl",
"//packages/concatjs/internal:bzl",
Expand Down
6 changes: 1 addition & 5 deletions packages/typescript/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,7 @@ package(default_visibility = ["//visibility:public"])

bzl_library(
name = "bzl",
srcs = glob(["*.bzl"]) + [
# Work-around since we don't have and don't want a bzl_library in the generated
# @nodejs//:BUILD.bazel file
"@nodejs//:index.bzl",
],
srcs = glob(["*.bzl"]),
deps = [
"//packages/typescript/internal:bzl",
"@build_bazel_rules_nodejs//:bzl",
Expand Down