Skip to content

Commit

Permalink
fix(builtin): don't expose any darwin_arm64 repo or toolchains if not…
Browse files Browse the repository at this point in the history
… supported by the node version

Fixes #2779
  • Loading branch information
alexeagle committed Jul 2, 2021
1 parent 6e2f660 commit 6748383
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 36 deletions.
4 changes: 2 additions & 2 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ workspace(

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

# Node version used in the repository. Needs to be at least `v12.14.1` to satisfy
# the minimum version required by the Angular packages.
node_repositories(
node_version = "12.14.1",
# TODO(alexeagle): upgrade to Node 16 to get native Mac M1 support
# node_version = "16.3.0",
)

#
Expand Down
16 changes: 16 additions & 0 deletions internal/common/os_name.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
"""Helper function for repository rules
"""

load(":check_version.bzl", "check_version")

OS_ARCH_NAMES = [
("windows", "amd64"),
("darwin", "amd64"),
Expand Down Expand Up @@ -66,3 +68,17 @@ def is_darwin_os(rctx):
def is_linux_os(rctx):
name = os_name(rctx)
return name == OS_NAMES[3] or name == OS_NAMES[4] or name == OS_NAMES[5]

def node_exists_for_os(node_version, os_name):
"Whether a node binary is available for this platform"
is_16_or_greater = check_version(node_version, "16.0.0")

# There is no Apple Silicon native version of node before 16
return is_16_or_greater or os_name != "darwin_arm64"

def assert_node_exists_for_host(rctx):
node_version = rctx.attr.node_version
if not node_exists_for_os(node_version, os_name(rctx)):
fail("No nodejs is available for {} at version {}".format(os_name(rctx), node_version) +
"\n Consider upgrading by setting node_version in a call to node_repositories in WORKSPACE." +
"\n Note that Node 16.x is the minimum published for Apple Silicon (M1 Macs)")
24 changes: 8 additions & 16 deletions internal/node/node_labels.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -21,28 +21,20 @@ load("//internal/common:os_name.bzl", "is_windows_os", "os_name")

def get_node_label(rctx):
if is_windows_os(rctx):
label = Label("@nodejs_%s//:bin/node.cmd" % os_name(rctx))
else:
label = Label("@nodejs_%s//:bin/node" % os_name(rctx))
return label
return Label("@nodejs_%s//:bin/node.cmd" % os_name(rctx))
return Label("@nodejs_%s//:bin/node" % os_name(rctx))

def get_npm_label(rctx):
if is_windows_os(rctx):
label = Label("@nodejs_%s//:bin/npm.cmd" % os_name(rctx))
else:
label = Label("@nodejs_%s//:bin/npm" % os_name(rctx))
return label
return Label("@nodejs_%s//:bin/npm.cmd" % os_name(rctx))
return Label("@nodejs_%s//:bin/npm" % os_name(rctx))

def get_npm_node_repositories_label(rctx):
if is_windows_os(rctx):
label = Label("@nodejs_%s//:bin/npm_node_repositories.cmd" % os_name(rctx))
else:
label = Label("@nodejs_%s//:bin/npm_node_repositories" % os_name(rctx))
return label
return Label("@nodejs_%s//:bin/npm_node_repositories.cmd" % os_name(rctx))
return Label("@nodejs_%s//:bin/npm_node_repositories" % os_name(rctx))

def get_yarn_label(rctx):
if is_windows_os(rctx):
label = Label("@nodejs_%s//:bin/yarn.cmd" % os_name(rctx))
else:
label = Label("@nodejs_%s//:bin/yarn" % os_name(rctx))
return label
return Label("@nodejs_%s//:bin/yarn.cmd" % os_name(rctx))
return Label("@nodejs_%s//:bin/yarn" % os_name(rctx))
39 changes: 22 additions & 17 deletions internal/node/node_repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ See https://docs.bazel.build/versions/master/skylark/repository_rules.html
"""

load("//internal/common:check_bazel_version.bzl", "check_bazel_version")
load("//internal/common:os_name.bzl", "OS_ARCH_NAMES", "os_name")
load("//internal/common:os_name.bzl", "OS_ARCH_NAMES", "assert_node_exists_for_host", "node_exists_for_os", "os_name")
load("//internal/node:node_versions.bzl", "NODE_VERSIONS")
load("//third_party/github.com/bazelbuild/bazel-skylib:lib/paths.bzl", "paths")
load("//toolchains/node:node_toolchain_configure.bzl", "node_toolchain_configure")
Expand Down Expand Up @@ -261,15 +261,6 @@ done
SCRIPT_DIR="$(cd -P "$( dirname "$SOURCE" )" >/dev/null && pwd)"
"""

def _node_exists_for_platform(node_version, os_name):
"Whether a node binary is available for this platform"
if not node_version:
node_version = _DEFAULT_NODE_VERSION
node_major_version = int(node_version.split(".")[0])

# There is no Apple Silicon native version of node before 16
return node_major_version >= 16 or os_name != "darwin_arm64"

def _download_node(repository_ctx):
"""Used to download a NodeJS runtime package.
Expand All @@ -288,7 +279,9 @@ def _download_node(repository_ctx):
host_os = repository_ctx.name.split("nodejs_", 1)[1]

node_version = repository_ctx.attr.node_version
if not _node_exists_for_platform(node_version, host_os):

# Skip the download if we know it will fail
if not node_exists_for_os(node_version, host_os):
return
node_repositories = repository_ctx.attr.node_repositories

Expand Down Expand Up @@ -434,16 +427,16 @@ def _prepare_node(repository_ctx):
node_bin = ("%s/bin/node" % node_path) if not is_windows else ("%s/node.exe" % node_path)
node_bin_label = ("%s/bin/node" % node_package) if not is_windows else ("%s/node.exe" % node_package)

# Use the npm-cli.js script as the bin for oxs & linux so there are no symlink issues with `%s/bin/npm`
# Use the npm-cli.js script as the bin for osx & linux so there are no symlink issues with `%s/bin/npm`
npm_bin = ("%s/lib/node_modules/npm/bin/npm-cli.js" % node_path) if not is_windows else ("%s/npm.cmd" % node_path)
npm_bin_label = ("%s/lib/node_modules/npm/bin/npm-cli.js" % node_package) if not is_windows else ("%s/npm.cmd" % node_package)
npm_script = ("%s/lib/node_modules/npm/bin/npm-cli.js" % node_path) if not is_windows else ("%s/node_modules/npm/bin/npm-cli.js" % node_path)

# Use the npx-cli.js script as the bin for oxs & linux so there are no symlink issues with `%s/bin/npx`
# Use the npx-cli.js script as the bin for osx & linux so there are no symlink issues with `%s/bin/npx`
npx_bin = ("%s/lib/node_modules/npm/bin/npx-cli.js" % node_path) if not is_windows else ("%s/npx.cmd" % node_path)
npx_bin_label = ("%s/lib/node_modules/npm/bin/npx-cli.js" % node_package) if not is_windows else ("%s/npx.cmd" % node_package)

# Use the yarn.js script as the bin for oxs & linux so there are no symlink issues with `%s/bin/npm`
# Use the yarn.js script as the bin for osx & linux so there are no symlink issues with `%s/bin/npm`
yarn_bin = ("%s/bin/yarn.js" % yarn_path) if not is_windows else ("%s/bin/yarn.cmd" % yarn_path)
yarn_bin_label = ("%s/bin/yarn.js" % yarn_package) if not is_windows else ("%s/bin/yarn.cmd" % yarn_package)
yarn_script = "%s/bin/yarn.js" % yarn_path
Expand Down Expand Up @@ -720,6 +713,7 @@ filegroup(
))

def _nodejs_repo_impl(repository_ctx):
assert_node_exists_for_host(repository_ctx)
_download_node(repository_ctx)
_download_yarn(repository_ctx)
_prepare_node(repository_ctx)
Expand All @@ -733,6 +727,8 @@ node_repositories_rule = repository_rule(
)

def _nodejs_host_os_alias_impl(repository_ctx):
assert_node_exists_for_host(repository_ctx)

# Base BUILD file for this repository
repository_ctx.file("BUILD.bazel", content = """# Generated by node_repositories.bzl
package(default_visibility = ["//visibility:public"])
Expand Down Expand Up @@ -761,7 +757,10 @@ exports_files(["index.bzl"])
host_platform="{host_platform}"
""".format(host_platform = os_name(repository_ctx)))

_nodejs_repo_host_os_alias = repository_rule(_nodejs_host_os_alias_impl)
_nodejs_repo_host_os_alias = repository_rule(
_nodejs_host_os_alias_impl,
attrs = {"node_version": attr.string()},
)

def node_repositories(**kwargs):
"""
Expand All @@ -782,26 +781,32 @@ def node_repositories(**kwargs):
)

# This needs to be setup so toolchains can access nodejs for all different versions
node_version = kwargs.get("node_version", _DEFAULT_NODE_VERSION)
for os_arch_name in OS_ARCH_NAMES:
os_name = "_".join(os_arch_name)

# If we couldn't download node, don't make an external repo for it either
if not node_exists_for_os(node_version, os_name):
continue
node_repository_name = "nodejs_%s" % os_name
_maybe(
node_repositories_rule,
name = node_repository_name,
**kwargs
)
target_tool = "@%s//:node_bin" % node_repository_name if _node_exists_for_platform(kwargs.get("node_version"), os_name) else "node"
target_tool = "@%s//:node_bin" % node_repository_name
native.register_toolchains("@build_bazel_rules_nodejs//toolchains/node:node_%s_toolchain" % os_name)
node_toolchain_configure(
name = "%s_config" % node_repository_name,
target_tool = target_tool,
)

# This "nodejs" repo is just for convinience so one does not have to target @nodejs_<os_name>//...
# 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,
)

def _maybe(repo_rule, name, **kwargs):
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"repository": "https://github.com/bazelbuild/rules_nodejs",
"license": "Apache-2.0",
"engines": {
"node": ">=12 <=14",
"node": ">=12 <=18",
"yarn": ">=1.13.0"
},
"devDependencies": {
Expand Down

0 comments on commit 6748383

Please sign in to comment.