Skip to content

Commit

Permalink
refactor: npm_install and yarn_install no longer create node reposito…
Browse files Browse the repository at this point in the history
…ries

This convenience is now too magical and confusing.
If the user registers nodejs toolchains, this will create an extra toolchain they don't use,
but can be fetched by rules that reference the default.
It also makes a @Yarn workspace available even for users who only need npm.

BREAKING CHANGE:
you now need to explicitly register nodejs toolchains or use the node_repositories helper
  • Loading branch information
alexeagle committed Jan 16, 2022
1 parent 5ad9753 commit fd31243
Show file tree
Hide file tree
Showing 14 changed files with 101 additions and 55 deletions.
11 changes: 10 additions & 1 deletion WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,13 @@ nodejs_register_toolchains(
node_version = "15.14.0",
)

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

yarn_repositories(
name = "yarn",
node_repository = "node16",
)

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

npm_deps()
Expand Down Expand Up @@ -131,7 +138,9 @@ browser_repositories(
# Setup esbuild dependencies
load("//toolchains/esbuild:esbuild_repositories.bzl", "esbuild_repositories")

esbuild_repositories()
esbuild_repositories(
node_repository = "node16",
)

#
# Dependencies to run stardoc & generating documentation
Expand Down
8 changes: 7 additions & 1 deletion docs/Toolchains.md
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ Defaults to `""`
**USAGE**

<pre>
esbuild_repositories(<a href="#esbuild_repositories-name">name</a>, <a href="#esbuild_repositories-npm_repository">npm_repository</a>, <a href="#esbuild_repositories-npm_args">npm_args</a>)
esbuild_repositories(<a href="#esbuild_repositories-name">name</a>, <a href="#esbuild_repositories-npm_repository">npm_repository</a>, <a href="#esbuild_repositories-npm_args">npm_args</a>, <a href="#esbuild_repositories-kwargs">kwargs</a>)
</pre>

Helper for fetching and setting up the esbuild versions and toolchains
Expand Down Expand Up @@ -228,4 +228,10 @@ additional args to pass to the npm install rule

Defaults to `[]`

<h4 id="esbuild_repositories-kwargs">kwargs</h4>

additional named parameters to the npm_install rule




8 changes: 7 additions & 1 deletion docs/esbuild.md
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ Any other common attributes
**USAGE**

<pre>
esbuild_repositories(<a href="#esbuild_repositories-name">name</a>, <a href="#esbuild_repositories-npm_repository">npm_repository</a>, <a href="#esbuild_repositories-npm_args">npm_args</a>)
esbuild_repositories(<a href="#esbuild_repositories-name">name</a>, <a href="#esbuild_repositories-npm_repository">npm_repository</a>, <a href="#esbuild_repositories-npm_args">npm_args</a>, <a href="#esbuild_repositories-kwargs">kwargs</a>)
</pre>

Helper for fetching and setting up the esbuild versions and toolchains
Expand Down Expand Up @@ -476,4 +476,10 @@ additional args to pass to the npm install rule

Defaults to `[]`

<h4 id="esbuild_repositories-kwargs">kwargs</h4>

additional named parameters to the npm_install rule




2 changes: 1 addition & 1 deletion e2e/nodejs_image/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ nodejs_image(
name = "nodejs_image",
binary = ":main",
include_node_repo_args = False,
node_repository_name = "nodejs_linux_amd64",
node_repository_name = "node16_linux_amd64",
)

container_test(
Expand Down
1 change: 1 addition & 0 deletions e2e/nodejs_image/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ load("@build_bazel_rules_nodejs//:index.bzl", "yarn_install")

yarn_install(
name = "npm",
node_repository = "node16",
package_json = "//:package.json",
yarn_lock = "//:yarn.lock",
)
Expand Down
5 changes: 4 additions & 1 deletion e2e/symlinked_node_modules_npm/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ local_repository(

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

nodejs_register_toolchains(name = "node")
_NODE_REPO = "node"

nodejs_register_toolchains(name = _NODE_REPO)

# build_bazel_rules_nodejs_dev_dependencies() required since we're using local_repository for
# build_bazel_rules_nodejs above.
Expand All @@ -27,6 +29,7 @@ load("@build_bazel_rules_nodejs//:index.bzl", "npm_install")

npm_install(
name = "npm",
node_repository = _NODE_REPO,
package_json = "//:package.json",
package_lock_json = "//:package-lock.json",
quiet = False,
Expand Down
5 changes: 4 additions & 1 deletion e2e/symlinked_node_modules_yarn/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ local_repository(

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

nodejs_register_toolchains(name = "node")
_NODE_REPO = "node"

nodejs_register_toolchains(name = _NODE_REPO)

# build_bazel_rules_nodejs_dev_dependencies() required since we're using local_repository for
# build_bazel_rules_nodejs above.
Expand All @@ -27,6 +29,7 @@ load("@build_bazel_rules_nodejs//:index.bzl", "yarn_install")

yarn_install(
name = "npm",
node_repository = _NODE_REPO,
package_json = "//:package.json",
quiet = False,
yarn_lock = "//:yarn.lock",
Expand Down
16 changes: 10 additions & 6 deletions examples/vendored_node_and_yarn/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -28,23 +28,26 @@ build_bazel_rules_nodejs_dependencies()

# See comment in README about these fetches
http_archive(
name = "vendored_node_linux",
build_file_content = """exports_files(["node-v15.0.1-linux-x64/bin/node"])""",
name = "vendored_node_linux_amd64",
build_file_content = """exports_files(["bin/node"])""",
sha256 = "cc9c3eed21755b490e5333ccab208ce15b539c35f64a764eeeae77c58746a7ff",
strip_prefix = "node-v15.0.1-linux-x64",
urls = ["https://nodejs.org/dist/v15.0.1/node-v15.0.1-linux-x64.tar.xz"],
)

http_archive(
name = "vendored_node_mac",
build_file_content = """exports_files(["node-v15.0.1-darwin-x64/bin/node"])""",
name = "vendored_node_darwin_amd64",
build_file_content = """exports_files(["bin/node"])""",
sha256 = "78571df5b35d3ec73d7543332776bcb8cab3bc0e3abd12b1440fbcd01c74c055",
strip_prefix = "node-v15.0.1-darwin-x64",
urls = ["https://nodejs.org/dist/v15.0.1/node-v15.0.1-darwin-x64.tar.xz"],
)

http_archive(
name = "vendored_node_windows",
build_file_content = """exports_files(["node-v15.0.1-win-x64/node.exe"])""",
name = "vendored_node_windows_amd64",
build_file_content = """exports_files(["node.exe"])""",
sha256 = "efa7a74d91789a6e9f068f375e49f108ff87578fd88ff4b4e7fefd930c04db6c",
strip_prefix = "node-v15.0.1-win-x64",
urls = ["https://nodejs.org/dist/v15.0.1/node-v15.0.1-win-x64.zip"],
)

Expand All @@ -62,6 +65,7 @@ load("@build_bazel_rules_nodejs//:index.bzl", "yarn_install")
yarn_install(
name = "npm",
exports_directories_only = False,
node_repository = "vendored_node",
package_json = "//:package.json",
yarn = "@vendored_yarn_1_10_0//:yarn-v1.10.0/bin/yarn.js",
yarn_lock = "//:yarn.lock",
Expand Down
6 changes: 3 additions & 3 deletions examples/vendored_node_and_yarn/toolchains/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,15 @@ load("@rules_nodejs//nodejs:toolchain.bzl", "node_toolchain")

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

node_toolchain(
name = "node_macos",
target_tool = "@vendored_node_mac//:node-v15.0.1-darwin-x64/bin/node",
target_tool = "@vendored_node_darwin_amd64//:bin/node",
)

node_toolchain(
name = "node_windows",
target_tool = "@vendored_node_windows//:node-v15.0.1-win-x64/node.exe",
target_tool = "@vendored_node_windows_amd64//:node.exe",
)
5 changes: 4 additions & 1 deletion examples/vue/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,18 @@ build_bazel_rules_nodejs_dependencies()

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

_NODE_REPO = "node16"

nodejs_register_toolchains(
name = "node16",
name = _NODE_REPO,
node_version = "16.0.0",
)

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

npm_install(
name = "npm",
node_repository = _NODE_REPO,
package_json = "//:package.json",
package_lock_json = "//:package-lock.json",
)
25 changes: 21 additions & 4 deletions index.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
Users should not load files under "/internal"
"""

load("@rules_nodejs//nodejs:repositories.bzl", _nodejs_register_toolchains = "nodejs_register_toolchains")
load("@rules_nodejs//nodejs:yarn_repositories.bzl", _yarn_repositories = "yarn_repositories")
load("//:version.bzl", "VERSION")
load("//internal/common:check_version.bzl", "check_version")
load("//internal/common:copy_to_bin.bzl", _copy_to_bin = "copy_to_bin")
Expand Down Expand Up @@ -55,14 +57,29 @@ COMMON_REPLACEMENTS = {
"\"@bazel/([a-zA-Z_-]+)\":\\s+\"(file|bazel)[^\"]+\"": "\"@bazel/$1\": \"0.0.0-PLACEHOLDER\"",
}

def _maybe_install_node(**kwargs):
# Just in case the user didn't install node, and if they didn't supply the name of the node repo,
# then install a default one "@nodejs" for them here.
if "node_repository" not in kwargs.keys() and "nodejs_toolchains" not in native.existing_rules():
# buildifier: disable=print
print("node_repository attribute not set and no repository named 'nodejs_toolchains' exists; installing default node")
_nodejs_register_toolchains(name = "nodejs")

def _maybe_install_yarn(**kwargs):
# Just in case the user didn't install yarn, and if they aren't using a custom yarn binary,
# install the default "@yarn" one for them here.
if "yarn" not in kwargs.keys() and "yarn" not in native.existing_rules():
# buildifier: disable=print
print("yarn_install#yarn attribute not set and no repository named 'yarn' exists; installing default yarn")
_yarn_repositories(name = "yarn", node_repository = kwargs.get("node_repository"))

def npm_install(**kwargs):
# Just in case the user didn't install nodejs, do it now
_node_repositories()
_maybe_install_node(**kwargs)
_npm_install(**kwargs)

def yarn_install(**kwargs):
# Just in case the user didn't install nodejs, do it now
_node_repositories()
_maybe_install_node(**kwargs)
_maybe_install_yarn(**kwargs)
_yarn_install(**kwargs)

# Currently used Bazel version. This version is what the rules here are tested
Expand Down
21 changes: 2 additions & 19 deletions internal/node/node_repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@ See https://docs.bazel.build/versions/main/skylark/repository_rules.html
"""

load("@bazel_skylib//lib:versions.bzl", "versions")
load("@bazel_tools//tools/build_defs/repo:utils.bzl", "maybe")
load("@rules_nodejs//nodejs/private:os_name.bzl", "OS_ARCH_NAMES", "node_exists_for_os", "os_name")
load("@rules_nodejs//nodejs:repositories.bzl", "DEFAULT_NODE_VERSION", "nodejs_register_toolchains", node_repositories_rule = "node_repositories")
load("@rules_nodejs//nodejs:repositories.bzl", "nodejs_register_toolchains")
load("@rules_nodejs//nodejs:yarn_repositories.bzl", "yarn_repositories")

def node_repositories(**kwargs):
Expand All @@ -41,27 +39,12 @@ def node_repositories(**kwargs):
yarn_args = {}
yarn_name = kwargs.pop("yarn_repository_name", "yarn")
for k, v in kwargs.items():
if k.startswith("yarn_"):
if k.startswith("yarn"):
yarn_args[k] = kwargs.pop(k)
yarn_repositories(
name = yarn_name,
**yarn_args
)

# 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
)

# Install new toolchain under "nodejs" repository name prefix
nodejs_register_toolchains(name = "nodejs")
39 changes: 24 additions & 15 deletions internal/npm_install/npm_install.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -611,16 +611,34 @@ def _copy_data_dependencies(repository_ctx):
# files as npm file:// packages
_copy_file(repository_ctx, f)

def _repository_contains_file(rctx, repo, file):
"""Detect whether the file exists relative to the repo.
Surprisingly rctx.path() throws when the argument doesn't exist,
so you can't get a path object directly to check exists on.
As a workaround, make a path object from the WORKSPACE file first
and then go relative to it.
"""
wksp = Label("@%s//:WORKSPACE" % repo)
child = rctx.path(wksp).dirname
for sub in file.split("/"):
child = child.get_child(sub)
return child.exists

def _add_node_repositories_info_deps(repository_ctx, yarn = None):
# Add a dep to the node_info & yarn_info files from node_repositories
# so that if the node or yarn versions change we re-run the repository rule
repository_ctx.symlink(
Label("@{}_{}//:node_info".format(repository_ctx.attr.node_repository, os_name(repository_ctx))),
repository_ctx.path("_node_info"),
)
# But in case they are vendored, our info file may not be present, so check first.
# A vendored node may have no info file in the repo.
node_repo = "_".join([repository_ctx.attr.node_repository, os_name(repository_ctx)])
if _repository_contains_file(repository_ctx, node_repo, "node_info"):
repository_ctx.symlink(
Label("@{}//:node_info".format(node_repo)),
repository_ctx.path("_node_info"),
)

# A custom yarn might be vendored, and not have a yarn_info file in the repo.
if _yarn_from_yarn_repositories(yarn):
if yarn and _repository_contains_file(repository_ctx, yarn.workspace_name, "yarn_info"):
repository_ctx.symlink(
Label("@{}//:yarn_info".format(yarn.workspace_name)),
repository_ctx.path("_yarn_info"),
Expand Down Expand Up @@ -808,7 +826,7 @@ def _yarn_install_impl(repository_ctx):
yarn_label = repository_ctx.attr.yarn

# A custom yarn won't have our special wrapper batch script
if _yarn_from_yarn_repositories(yarn_label) and is_windows_host:
if is_windows_host and _repository_contains_file(repository_ctx, yarn_label.workspace_name, "bin/yarn.cmd"):
yarn_label = yarn_label.relative(":bin/yarn.cmd")
yarn = repository_ctx.path(yarn_label)
yarn_version = _detect_yarn_version(repository_ctx, yarn)
Expand Down Expand Up @@ -944,15 +962,6 @@ cd /D "{root}" && "{yarn}" {yarn_args}

_DEFAULT_YARN = Label("@yarn//:bin/yarn")

def _yarn_from_yarn_repositories(yarn_label):
"""Detect if yarn appears to come from an install we performed in yarn_repositories.bzl
If it does, then it has our wrapper scripts and the info file.
If the user vendors their own yarn.js, this won't exist."""
if not yarn_label:
return False
return yarn_label.package == _DEFAULT_YARN.package and yarn_label.name == _DEFAULT_YARN.name

yarn_install = repository_rule(
attrs = dict(COMMON_ATTRIBUTES, **{
"args": attr.string_list(
Expand Down
4 changes: 3 additions & 1 deletion toolchains/esbuild/esbuild_repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ load("@bazel_tools//tools/build_defs/repo:utils.bzl", "maybe")
load("@build_bazel_rules_nodejs//:index.bzl", "npm_install")
load(":esbuild_packages.bzl", "ESBUILD_PACKAGES")

def esbuild_repositories(name = "", npm_repository = "npm", npm_args = []):
def esbuild_repositories(name = "", npm_repository = "npm", npm_args = [], **kwargs):
"""Helper for fetching and setting up the esbuild versions and toolchains
This uses Bazel's downloader (via `http_archive`) to fetch the esbuild package
Expand All @@ -29,6 +29,7 @@ def esbuild_repositories(name = "", npm_repository = "npm", npm_args = []):
npm_repository: the name of the repository where the @bazel/esbuild package is installed
by npm_install or yarn_install.
npm_args: additional args to pass to the npm install rule
**kwargs: additional named parameters to the npm_install rule
"""

maybe(
Expand Down Expand Up @@ -78,4 +79,5 @@ def esbuild_repositories(name = "", npm_repository = "npm", npm_args = []):
"--ignore-scripts",
] + npm_args,
package_path = package_path,
**kwargs
)

0 comments on commit fd31243

Please sign in to comment.