From 4c4844946b1214096687783b8df3bfeec21547bd Mon Sep 17 00:00:00 2001 From: Paolo Tranquilli Date: Wed, 18 Sep 2024 19:01:12 +0200 Subject: [PATCH] feat: add `node_urls` parameter to bzlmod `toolchain` in the `node` extension (#3763) --------- Co-authored-by: Paolo Tranquilli --- .github/workflows/ci.yaml | 4 +- e2e/conflicting_toolchains/.bazelversion | 1 + e2e/conflicting_toolchains/.gitignore | 1 + e2e/conflicting_toolchains/BUILD.bazel | 13 +++ e2e/conflicting_toolchains/MODULE.bazel | 2 + e2e/conflicting_toolchains/test.sh | 22 +++++ .../test_include_headers/.bazelversion | 1 + .../test_include_headers/MODULE.bazel | 14 +++ .../test_node_urls/.bazelversion | 1 + .../test_node_urls/MODULE.bazel | 14 +++ .../test_node_version/.bazelversion | 1 + .../test_node_version/MODULE.bazel | 14 +++ .../.bazelversion | 1 + .../test_node_version_from_nvmrc/.nvmrc | 0 .../test_node_version_from_nvmrc/BUILD.bazel | 0 .../test_node_version_from_nvmrc/MODULE.bazel | 14 +++ e2e/smoke/BUILD.bazel | 25 +++++ e2e/smoke/MODULE.bazel | 11 +++ nodejs/extensions.bzl | 91 ++++++++++++------- 19 files changed, 197 insertions(+), 33 deletions(-) create mode 120000 e2e/conflicting_toolchains/.bazelversion create mode 100644 e2e/conflicting_toolchains/.gitignore create mode 100644 e2e/conflicting_toolchains/BUILD.bazel create mode 100644 e2e/conflicting_toolchains/MODULE.bazel create mode 100755 e2e/conflicting_toolchains/test.sh create mode 120000 e2e/conflicting_toolchains/test_include_headers/.bazelversion create mode 100644 e2e/conflicting_toolchains/test_include_headers/MODULE.bazel create mode 120000 e2e/conflicting_toolchains/test_node_urls/.bazelversion create mode 100644 e2e/conflicting_toolchains/test_node_urls/MODULE.bazel create mode 120000 e2e/conflicting_toolchains/test_node_version/.bazelversion create mode 100644 e2e/conflicting_toolchains/test_node_version/MODULE.bazel create mode 120000 e2e/conflicting_toolchains/test_node_version_from_nvmrc/.bazelversion create mode 100644 e2e/conflicting_toolchains/test_node_version_from_nvmrc/.nvmrc create mode 100644 e2e/conflicting_toolchains/test_node_version_from_nvmrc/BUILD.bazel create mode 100644 e2e/conflicting_toolchains/test_node_version_from_nvmrc/MODULE.bazel diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 13c3acce73..6cde22d0be 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -19,7 +19,7 @@ jobs: test: uses: bazel-contrib/.github/.github/workflows/bazel.yaml@v6 with: - folders: '[".", "e2e/headers", "e2e/smoke", "e2e/nodejs_host"]' + folders: '[".", "e2e/headers", "e2e/smoke", "e2e/nodejs_host", "e2e/conflicting_toolchains"]' # stardoc generated docs fail on diff_test with Bazel 6.4.0 so don't test against it in root repository exclude: | [ @@ -27,4 +27,6 @@ jobs: {"bazelversion": "6.4.0", "os": "windows-latest"}, {"bazelversion": "6.4.0", folder: "."}, {"bazelversion": "6.4.0", bzlmodEnabled: true} + # this test is for bzlmod only + {folder: "e2e/conflicting_toolchains", bzlmodEnabled: false}, ] diff --git a/e2e/conflicting_toolchains/.bazelversion b/e2e/conflicting_toolchains/.bazelversion new file mode 120000 index 0000000000..96cf94962b --- /dev/null +++ b/e2e/conflicting_toolchains/.bazelversion @@ -0,0 +1 @@ +../../.bazelversion \ No newline at end of file diff --git a/e2e/conflicting_toolchains/.gitignore b/e2e/conflicting_toolchains/.gitignore new file mode 100644 index 0000000000..5d0bfe2294 --- /dev/null +++ b/e2e/conflicting_toolchains/.gitignore @@ -0,0 +1 @@ +error.txt diff --git a/e2e/conflicting_toolchains/BUILD.bazel b/e2e/conflicting_toolchains/BUILD.bazel new file mode 100644 index 0000000000..48c1d3386d --- /dev/null +++ b/e2e/conflicting_toolchains/BUILD.bazel @@ -0,0 +1,13 @@ +# this is set up to make bazel test //... pass +load("@bazel_skylib//rules:write_file.bzl", "write_file") + +write_file( + name = "empty", + out = "empty.sh", + content = [], +) + +sh_test( + name = "dummy", + srcs = [":empty"], +) diff --git a/e2e/conflicting_toolchains/MODULE.bazel b/e2e/conflicting_toolchains/MODULE.bazel new file mode 100644 index 0000000000..3ef14c7486 --- /dev/null +++ b/e2e/conflicting_toolchains/MODULE.bazel @@ -0,0 +1,2 @@ +# this is set up to make bazel test //... pass +bazel_dep(name = "bazel_skylib", version = "1.7.1", dev_dependency = True) diff --git a/e2e/conflicting_toolchains/test.sh b/e2e/conflicting_toolchains/test.sh new file mode 100755 index 0000000000..7ce57694ee --- /dev/null +++ b/e2e/conflicting_toolchains/test.sh @@ -0,0 +1,22 @@ +#!/bin/bash +# this is the integration test checking various combinations of conflicting nodejs toolchain definitions + +set -eu + +for test_attr in test_*; do + pushd $test_attr > /dev/null + attr=${test_attr#test_} + echo -n "testing conflict on $attr... " + if bazel mod tidy &> error.txt; then + echo "ERROR: bazel mod tidy should have failed with following MODULE.bazel:" + cat MODULE.bazel + exit 1 + elif ! grep "conflicting toolchains" error.txt > /dev/null; then + echo "ERROR: expected bazel mod tidy to mention conflicting toolchains, found:" + cat error.txt + exit 1 + else + echo "PASS" + fi + popd > /dev/null +done diff --git a/e2e/conflicting_toolchains/test_include_headers/.bazelversion b/e2e/conflicting_toolchains/test_include_headers/.bazelversion new file mode 120000 index 0000000000..b332604979 --- /dev/null +++ b/e2e/conflicting_toolchains/test_include_headers/.bazelversion @@ -0,0 +1 @@ +../.bazelversion \ No newline at end of file diff --git a/e2e/conflicting_toolchains/test_include_headers/MODULE.bazel b/e2e/conflicting_toolchains/test_include_headers/MODULE.bazel new file mode 100644 index 0000000000..572f3e14db --- /dev/null +++ b/e2e/conflicting_toolchains/test_include_headers/MODULE.bazel @@ -0,0 +1,14 @@ +bazel_dep(name = "rules_nodejs", version = "0.0.0", dev_dependency = True) +local_path_override( + module_name = "rules_nodejs", + path = "../../..", +) + +node = use_extension("@rules_nodejs//nodejs:extensions.bzl", "node", dev_dependency = True) +node.toolchain( + name = "mynode", +) +node.toolchain( + name = "mynode", + node_urls = ["file://whatever"], +) diff --git a/e2e/conflicting_toolchains/test_node_urls/.bazelversion b/e2e/conflicting_toolchains/test_node_urls/.bazelversion new file mode 120000 index 0000000000..b332604979 --- /dev/null +++ b/e2e/conflicting_toolchains/test_node_urls/.bazelversion @@ -0,0 +1 @@ +../.bazelversion \ No newline at end of file diff --git a/e2e/conflicting_toolchains/test_node_urls/MODULE.bazel b/e2e/conflicting_toolchains/test_node_urls/MODULE.bazel new file mode 100644 index 0000000000..966e383092 --- /dev/null +++ b/e2e/conflicting_toolchains/test_node_urls/MODULE.bazel @@ -0,0 +1,14 @@ +bazel_dep(name = "rules_nodejs", version = "0.0.0", dev_dependency = True) +local_path_override( + module_name = "rules_nodejs", + path = "../../..", +) + +node = use_extension("@rules_nodejs//nodejs:extensions.bzl", "node", dev_dependency = True) +node.toolchain( + name = "mynode", +) +node.toolchain( + name = "mynode", + include_headers = True, +) diff --git a/e2e/conflicting_toolchains/test_node_version/.bazelversion b/e2e/conflicting_toolchains/test_node_version/.bazelversion new file mode 120000 index 0000000000..b332604979 --- /dev/null +++ b/e2e/conflicting_toolchains/test_node_version/.bazelversion @@ -0,0 +1 @@ +../.bazelversion \ No newline at end of file diff --git a/e2e/conflicting_toolchains/test_node_version/MODULE.bazel b/e2e/conflicting_toolchains/test_node_version/MODULE.bazel new file mode 100644 index 0000000000..90f1a8871d --- /dev/null +++ b/e2e/conflicting_toolchains/test_node_version/MODULE.bazel @@ -0,0 +1,14 @@ +bazel_dep(name = "rules_nodejs", version = "0.0.0", dev_dependency = True) +local_path_override( + module_name = "rules_nodejs", + path = "../../..", +) + +node = use_extension("@rules_nodejs//nodejs:extensions.bzl", "node", dev_dependency = True) +node.toolchain( + name = "mynode", +) +node.toolchain( + name = "mynode", + node_version = "15.14.0", +) diff --git a/e2e/conflicting_toolchains/test_node_version_from_nvmrc/.bazelversion b/e2e/conflicting_toolchains/test_node_version_from_nvmrc/.bazelversion new file mode 120000 index 0000000000..b332604979 --- /dev/null +++ b/e2e/conflicting_toolchains/test_node_version_from_nvmrc/.bazelversion @@ -0,0 +1 @@ +../.bazelversion \ No newline at end of file diff --git a/e2e/conflicting_toolchains/test_node_version_from_nvmrc/.nvmrc b/e2e/conflicting_toolchains/test_node_version_from_nvmrc/.nvmrc new file mode 100644 index 0000000000..e69de29bb2 diff --git a/e2e/conflicting_toolchains/test_node_version_from_nvmrc/BUILD.bazel b/e2e/conflicting_toolchains/test_node_version_from_nvmrc/BUILD.bazel new file mode 100644 index 0000000000..e69de29bb2 diff --git a/e2e/conflicting_toolchains/test_node_version_from_nvmrc/MODULE.bazel b/e2e/conflicting_toolchains/test_node_version_from_nvmrc/MODULE.bazel new file mode 100644 index 0000000000..703cc24b10 --- /dev/null +++ b/e2e/conflicting_toolchains/test_node_version_from_nvmrc/MODULE.bazel @@ -0,0 +1,14 @@ +bazel_dep(name = "rules_nodejs", version = "0.0.0", dev_dependency = True) +local_path_override( + module_name = "rules_nodejs", + path = "../../..", +) + +node = use_extension("@rules_nodejs//nodejs:extensions.bzl", "node", dev_dependency = True) +node.toolchain( + name = "mynode", +) +node.toolchain( + name = "mynode", + node_version_from_nvmrc = "//:.nvmrc", +) diff --git a/e2e/smoke/BUILD.bazel b/e2e/smoke/BUILD.bazel index 9eaa324d99..4b4fe10018 100644 --- a/e2e/smoke/BUILD.bazel +++ b/e2e/smoke/BUILD.bazel @@ -202,6 +202,12 @@ write_file( # Files used in test cases later that contain the correct nodejs version # that is imported into the workspace. +write_file( + name = "write_node_version_15", + out = "expected_node_15", + content = ["v15.14.0"], +) + write_file( name = "write_node_version_17", out = "expected_node_17", @@ -268,3 +274,22 @@ diff_test( file1 = "write_node_version_16", file2 = "thing_toolchain_16", ) + +my_nodejs( + name = "run_15", + out = "thing_toolchain_15", + entry_point = "version.js", + # using the select statement will download toolchains for all three platforms + # you can also just provide an individual toolchain if you don't want to download them all + toolchain = select({ + "@bazel_tools//src/conditions:linux_x86_64": "@node15_linux_amd64//:toolchain", + "@bazel_tools//src/conditions:darwin": "@node15_darwin_amd64//:toolchain", + "@bazel_tools//src/conditions:windows": "@node15_windows_amd64//:toolchain", + }), +) + +diff_test( + name = "test_node_version_15", + file1 = "write_node_version_15", + file2 = "thing_toolchain_15", +) diff --git a/e2e/smoke/MODULE.bazel b/e2e/smoke/MODULE.bazel index bee007f953..ee6b2091b9 100644 --- a/e2e/smoke/MODULE.bazel +++ b/e2e/smoke/MODULE.bazel @@ -14,10 +14,21 @@ node.toolchain( name = "node17", node_version = "17.9.1", ) +node.toolchain( + name = "node15", + node_urls = [ + "https://nodejs.org/dist/v{version}/{filename}", + "https://mirrors.dotsrc.org/nodejs/release/v{version}/{filename}", + ], + node_version = "15.14.0", +) # FIXME(6.0): a repo rule with name=foo should create a repo named @foo, not @foo_toolchains use_repo( node, + "node15_darwin_amd64", + "node15_linux_amd64", + "node15_windows_amd64", "node17_darwin_amd64", "node17_linux_amd64", "node17_windows_amd64", diff --git a/nodejs/extensions.bzl b/nodejs/extensions.bzl index 1de6f3639a..10dd5c7b45 100644 --- a/nodejs/extensions.bzl +++ b/nodejs/extensions.bzl @@ -1,6 +1,24 @@ "extensions for bzlmod" -load(":repositories.bzl", "DEFAULT_NODE_REPOSITORY", "DEFAULT_NODE_VERSION", "nodejs_register_toolchains") +load( + ":repositories.bzl", + "DEFAULT_NODE_REPOSITORY", + "DEFAULT_NODE_URL", + "DEFAULT_NODE_VERSION", + "nodejs_register_toolchains", +) + +def _toolchain_repr(toolchain): + """ Return a `toolchain` tag object representation useful for diagnostics """ + key_values = [(attr, getattr(toolchain, attr)) for attr in _ATTRS] + return ", ".join(["%s = %r" % (attr, value) for attr, value in key_values if value]) + +def _toolchains_equal(lhs, rhs): + """ Compare two `toolchain` tag objects """ + for attr in _ATTRS: + if getattr(lhs, attr) != getattr(rhs, attr): + return False + return True def _toolchain_extension(module_ctx): registrations = {} @@ -14,54 +32,63 @@ def _toolchain_extension(module_ctx): # Prioritize the root-most registration of the default node toolchain version and # ignore any further registrations (modules are processed breadth-first) continue - if toolchain.node_version == registrations[toolchain.name].node_version and toolchain.node_version_from_nvmrc == registrations[toolchain.name].node_version_from_nvmrc: + if not _toolchains_equal(toolchain, registrations[toolchain.name]): + fail("Multiple conflicting toolchains declared:\n* {}\n* {}".format( + _toolchain_repr(toolchain), + _toolchain_repr(registrations[toolchain.name]), + )) + else: # No problem to register a matching toolchain twice continue - fail("Multiple conflicting toolchains declared for name {} ({} and {})".format( - toolchain.name, - toolchain.node_version, - registrations[toolchain.name], - )) else: - registrations[toolchain.name] = struct( - node_version = toolchain.node_version, - node_version_from_nvmrc = toolchain.node_version_from_nvmrc, - include_headers = toolchain.include_headers, - ) + registrations[toolchain.name] = toolchain for k, v in registrations.items(): nodejs_register_toolchains( name = k, node_version = v.node_version, node_version_from_nvmrc = v.node_version_from_nvmrc, + node_urls = v.node_urls, include_headers = v.include_headers, register = False, ) -node = module_extension( - implementation = _toolchain_extension, - tag_classes = { - "toolchain": tag_class(attrs = { - "name": attr.string( - doc = "Base name for generated repositories", - default = DEFAULT_NODE_REPOSITORY, - ), - "node_version": attr.string( - doc = "Version of the Node.js interpreter", - default = DEFAULT_NODE_VERSION, - ), - "node_version_from_nvmrc": attr.label( - allow_single_file = True, - doc = """The .nvmrc file containing the version of Node.js to use. +_ATTRS = { + "name": attr.string( + doc = "Base name for generated repositories", + default = DEFAULT_NODE_REPOSITORY, + ), + "node_version": attr.string( + doc = "Version of the Node.js interpreter", + default = DEFAULT_NODE_VERSION, + ), + "node_version_from_nvmrc": attr.label( + allow_single_file = True, + doc = """The .nvmrc file containing the version of Node.js to use. If set then the version found in the .nvmrc file is used instead of the one specified by node_version.""", - ), - "include_headers": attr.bool( - doc = """Set headers field in NodeInfo provided by this toolchain. + ), + "include_headers": attr.bool( + doc = """Set headers field in NodeInfo provided by this toolchain. This setting creates a dependency on a c++ toolchain. """, - ), - }), + ), + "node_urls": attr.string_list( + doc = """List of URLs to use to download Node.js. + + Each entry is a template for downloading a node distribution. + + The `{version}` parameter is substituted with the `node_version` attribute, + and `{filename}` with the matching entry from the `node_repositories` attribute. + """, + default = [DEFAULT_NODE_URL], + ), +} + +node = module_extension( + implementation = _toolchain_extension, + tag_classes = { + "toolchain": tag_class(attrs = _ATTRS), }, )