fix: de-couple CcInfo/cc toolchain from nodejs toolchain
diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml
index 953f3f6..59230f3 100644
--- a/.bazelci/presubmit.yml
+++ b/.bazelci/presubmit.yml
@@ -14,67 +14,68 @@
name: ubuntu1804
platform: ubuntu1804
build_targets:
- - "//..."
+ - "//..."
test_flags:
- - "--test_tag_filters=-skip-on-bazelci-ubuntu"
+ - "--test_tag_filters=-skip-on-bazelci-ubuntu"
test_targets:
- - "//..."
+ - "//..."
ubuntu1804-smoke:
name: ubuntu1804-smoke
platform: ubuntu1804
working_directory: "e2e/smoke"
build_targets:
- - "//..."
+ - "//..."
test_flags:
- - "--test_tag_filters=-skip-on-bazelci-ubuntu"
+ - "--test_tag_filters=-skip-on-bazelci-ubuntu"
test_targets:
- - "//..."
+ - "//..."
ubuntu1804-nodejs_host:
name: ubuntu1804-nodejs_host
platform: ubuntu1804
working_directory: "e2e/nodejs_host"
build_targets:
- - "//..."
+ - "//..."
test_flags:
- - "--test_tag_filters=-skip-on-bazelci-ubuntu"
+ - "--test_tag_filters=-skip-on-bazelci-ubuntu"
test_targets:
- - "//..."
+ - "//..."
macos-smoke:
name: macos
platform: macos
working_directory: "e2e/smoke"
build_targets:
- - "//..."
+ - "//..."
test_flags:
- - "--test_tag_filters=-skip-on-bazelci-macos"
+ - "--test_tag_filters=-skip-on-bazelci-macos"
test_targets:
- - "//..."
+ - "//..."
windows-smoke:
name: windows-smoke
platform: windows
working_directory: "e2e/smoke"
build_targets:
- - "//..."
+ - "//..."
test_flags:
- - "--test_tag_filters=-skip-on-bazelci-windows"
+ - "--test_tag_filters=-skip-on-bazelci-windows"
test_targets:
- - "//..."
+ - "//..."
windows-nodejs_host:
name: windows-nodejs_host
platform: windows
working_directory: "e2e/nodejs_host"
build_targets:
- - "//..."
+ - "//..."
test_flags:
- - "--test_tag_filters=-skip-on-bazelci-windows"
+ - "--test_tag_filters=-skip-on-bazelci-windows"
test_targets:
- - "//..."
- # Temporarily disabled RBE CI until cc toolchain failures are resolved
- # rbe_ubuntu1604-smoke:
- # name: rbe_ubuntu1604-smoke
- # platform: rbe_ubuntu1604
- # working_directory: "e2e/smoke"
- # build_targets:
- # - "//..."
- # test_targets:
- # - "//..."
+ - "//..."
+ rbe_ubuntu1604-smoke:
+ name: rbe_ubuntu1604-smoke
+ platform: rbe_ubuntu1604
+ working_directory: "e2e/smoke"
+ build_flags:
+ - "--build_tag_filters=-skip-on-rbe"
+ build_targets:
+ - "//..."
+ test_targets:
+ - "//..."
diff --git a/docs/Core.md b/docs/Core.md
index 41bceab..3afcc24 100644
--- a/docs/Core.md
+++ b/docs/Core.md
@@ -249,7 +249,7 @@
<h4 id="nodejs_toolchain-headers">headers</h4>
-cc_library that contains the Node/v8 header files
+filegroup that contains the Node/v8 header files
Defaults to `None`
diff --git a/e2e/smoke/BUILD.bazel b/e2e/smoke/BUILD.bazel
index 27a0534..619af58 100644
--- a/e2e/smoke/BUILD.bazel
+++ b/e2e/smoke/BUILD.bazel
@@ -276,10 +276,11 @@
"@platforms//os:windows": ["/std:c++14"],
"//conditions:default": ["-std=c++14"],
}),
+ tags = ["skip-on-rbe"],
target_compatible_with = select({
# Windows does not ship headers in the release artifact so this won't work yet.
"@platforms//os:windows": ["@platforms//:incompatible"],
"//conditions:default": [],
}),
- deps = ["@rules_nodejs//nodejs:current_node_cc_headers"],
+ deps = ["@rules_nodejs//nodejs/headers:current_node_cc_headers"],
)
diff --git a/e2e/smoke/WORKSPACE.bazel b/e2e/smoke/WORKSPACE.bazel
index b7b821b..3a71e7d 100644
--- a/e2e/smoke/WORKSPACE.bazel
+++ b/e2e/smoke/WORKSPACE.bazel
@@ -44,23 +44,3 @@
sha256 = "d8f9d40c4656537a60bf0c6daae6f0553f54df5ff2518f86464b7c785f20376b",
urls = ["https://registry.npmjs.org/acorn/-/acorn-8.5.0.tgz"],
)
-
-#
-# RBE configuration
-#
-# See https://github.com/bazelbuild/continuous-integration/releases/tag/rules-1.0.0
-http_archive(
- name = "bazelci_rules",
- sha256 = "eca21884e6f66a88c358e580fd67a6b148d30ab57b1680f62a96c00f9bc6a07e",
- strip_prefix = "bazelci_rules-1.0.0",
- url = "https://github.com/bazelbuild/continuous-integration/releases/download/rules-1.0.0/bazelci_rules-1.0.0.tar.gz",
-)
-
-load("@bazelci_rules//:rbe_repo.bzl", "rbe_preconfig")
-
-# Creates toolchain configuration for remote execution with BuildKite CI
-# for rbe_ubuntu1604
-rbe_preconfig(
- name = "buildkite_config",
- toolchain = "ubuntu1804-bazel-java11",
-)
diff --git a/e2e/smoke/WORKSPACE.bzlmod b/e2e/smoke/WORKSPACE.bzlmod
index e69de29..cb8c8b7 100644
--- a/e2e/smoke/WORKSPACE.bzlmod
+++ b/e2e/smoke/WORKSPACE.bzlmod
@@ -0,0 +1,21 @@
+load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
+
+#
+# RBE configuration
+#
+# See https://github.com/bazelbuild/continuous-integration/releases/tag/rules-1.0.0
+http_archive(
+ name = "bazelci_rules",
+ sha256 = "eca21884e6f66a88c358e580fd67a6b148d30ab57b1680f62a96c00f9bc6a07e",
+ strip_prefix = "bazelci_rules-1.0.0",
+ url = "https://github.com/bazelbuild/continuous-integration/releases/download/rules-1.0.0/bazelci_rules-1.0.0.tar.gz",
+)
+
+load("@bazelci_rules//:rbe_repo.bzl", "rbe_preconfig")
+
+# Creates toolchain configuration for remote execution with BuildKite CI
+# for rbe_ubuntu1604
+rbe_preconfig(
+ name = "buildkite_config",
+ toolchain = "ubuntu1804-bazel-java11",
+)
diff --git a/nodejs/BUILD.bazel b/nodejs/BUILD.bazel
index cc5b1c0..e66eda2 100644
--- a/nodejs/BUILD.bazel
+++ b/nodejs/BUILD.bazel
@@ -1,8 +1,9 @@
load("@bazel_skylib//:bzl_library.bzl", "bzl_library")
-load("//nodejs/private:current_node_cc_headers.bzl", "current_node_cc_headers")
load("//nodejs/private:nodejs_toolchains_repo.bzl", "PLATFORMS")
load("//nodejs/private:user_build_settings.bzl", "user_args")
+package(default_visibility = ["//visibility:public"])
+
exports_files([
"index.for_docs.bzl",
"providers.bzl",
@@ -11,7 +12,6 @@
bzl_library(
name = "index.for_docs",
srcs = glob(["*.bzl"]) + ["@bazel_tools//tools:bzl_srcs"],
- visibility = ["//visibility:public"],
deps = [
"//nodejs/private:bzl",
"//nodejs/private/providers:bzl",
@@ -23,10 +23,7 @@
# attribute in order to get a node interpreter for the correct
# platform.
# See https://docs.bazel.build/versions/main/toolchains.html#writing-rules-that-use-toolchains
-toolchain_type(
- name = "toolchain_type",
- visibility = ["//visibility:public"],
-)
+toolchain_type(name = "toolchain_type")
[
platform(
@@ -42,13 +39,4 @@
user_args(
name = "default_args",
build_setting_default = "--preserve-symlinks",
- visibility = ["//visibility:public"],
-)
-
-# This target provides the C headers for whatever the current toolchain is
-# for the consuming rule. It basically acts like a cc_library by forwarding
-# on the providers for the underlying cc_library that the toolchain is using.
-current_node_cc_headers(
- name = "current_node_cc_headers",
- visibility = ["//visibility:public"],
)
diff --git a/nodejs/headers/BUILD.bazel b/nodejs/headers/BUILD.bazel
new file mode 100644
index 0000000..1343e3f
--- /dev/null
+++ b/nodejs/headers/BUILD.bazel
@@ -0,0 +1,34 @@
+load("//nodejs/private:current_node_cc_headers.bzl", "current_node_cc_headers")
+
+package(default_visibility = ["//visibility:public"])
+
+# Provides the currently active Node toolchain's C++ headers.
+#
+# Note, "node.h" is only usable from C++, and you'll need to
+# ensure you are compiling with c++14 or later.
+#
+# Also, on Windows, NodeJS releases do not ship headers, so this rule is currently
+# not usable with the built-in toolchains. If you define your own toolchain on Windows,
+# you can include the headers and then this rule will work.
+#
+# To use, simply depend on this target where you would have wanted the
+# toolchain's underlying `:headers` target:
+#
+# ```starlark
+# cc_library(
+# name = "foo",
+# srcs = ["foo.cc"],
+# # If toolchain sets this already, you can omit.
+# copts = ["-std=c++14"],
+# deps = ["@rules_nodejs//:current_node_cc_headers"]
+# )
+# ```
+cc_library(
+ name = "current_node_cc_headers",
+ hdrs = [":current_node_cc_headers_filegroup"],
+ includes = ["../bin/nodejs/include/node"],
+)
+
+current_node_cc_headers(
+ name = "current_node_cc_headers_filegroup",
+)
diff --git a/nodejs/private/current_node_cc_headers.bzl b/nodejs/private/current_node_cc_headers.bzl
index 01bf064..e836787 100644
--- a/nodejs/private/current_node_cc_headers.bzl
+++ b/nodejs/private/current_node_cc_headers.bzl
@@ -15,36 +15,11 @@
"""Implementation of current_node_cc_headers rule."""
def _current_node_cc_headers_impl(ctx):
- return ctx.toolchains["//nodejs:toolchain_type"].nodeinfo.headers.providers_map.values()
+ return DefaultInfo(
+ files = ctx.toolchains["//nodejs:toolchain_type"].nodeinfo.headers.files,
+ )
current_node_cc_headers = rule(
implementation = _current_node_cc_headers_impl,
toolchains = ["//nodejs:toolchain_type"],
- provides = [CcInfo],
- doc = """\
-Provides the currently active Node toolchain's C++ headers.
-
-This is a wrapper around the underlying `cc_library()` for the
-C headers for the consuming target's currently active Node toolchain.
-
-Note, "node.h" is only usable from C++, and you'll need to
-ensure you are compiling with c++14 or later.
-
-Also, on Windows, Node.js releases do not ship headers, so this rule is currently
-not usable with the built-in toolchains. If you define your own toolchain on Windows,
-you can include the headers and then this rule will work.
-
-To use, simply depend on this target where you would have wanted the
-toolchain's underlying `:headers` target:
-
-```starlark
-cc_library(
- name = "foo",
- srcs = ["foo.cc"],
- # If toolchain sets this already, you can omit.
- copts = ["-std=c++14"],
- deps = ["@rules_nodejs//:current_node_cc_headers"]
-)
-```
-""",
)
diff --git a/nodejs/repositories.bzl b/nodejs/repositories.bzl
index b722618..e6ae7f5 100644
--- a/nodejs/repositories.bzl
+++ b/nodejs/repositories.bzl
@@ -241,9 +241,9 @@
name = "npm_files",
srcs = glob(["bin/nodejs/**"]) + [":node_files"],
)
-cc_library(
+filegroup(
name = "headers",
- hdrs = glob(
+ srcs = glob(
["bin/nodejs/include/node/**"],
# Apparently, node.js doesn't ship the headers in their Windows package.
# https://stackoverflow.com/questions/50745670/nodejs-headers-on-windows-are-not-installed-automatically
@@ -254,7 +254,6 @@
# -> no results ...
allow_empty = True,
),
- includes = ["bin/nodejs/include/node"],
)
""".format(
node_bin_export = "\n \"%s\"," % node_bin,
diff --git a/nodejs/toolchain.bzl b/nodejs/toolchain.bzl
index 62dd4c2..103d3bf 100644
--- a/nodejs/toolchain.bzl
+++ b/nodejs/toolchain.bzl
@@ -33,27 +33,7 @@
For backward compability, npm_path is set to the runfiles path of npm if npm is set.
""",
"npm_sources": """Additional source files required to run npm""",
- "headers": """\
-(struct) Information about the header files, with fields:
- * providers_map: a dict of string to provider instances. The key should be
- a fully qualified name (e.g. `@rules_foo//bar:baz.bzl#MyInfo`) of the
- provider to uniquely identify its type.
-
- The following keys are always present:
- * CcInfo: the CcInfo provider instance for the headers.
- * DefaultInfo: the DefaultInfo provider instance for the headers.
-
- A map is used to allow additional providers from the originating headers
- target (typically a `cc_library`) to be propagated to consumers (directly
- exposing a Target object can cause memory issues and is an anti-pattern).
-
- When consuming this map, it's suggested to use `providers_map.values()` to
- return all providers; or copy the map and filter out or replace keys as
- appropriate. Note that any keys begining with `_` (underscore) are
- considered private and should be forward along as-is (this better allows
- e.g. `:current_node_cc_headers` to act as the underlying headers target it
- represents).
-""",
+ "headers": "A filegroup containing the Node headers.",
# DEPRECATED
"target_tool_path": "(DEPRECATED) Path to Node.js executable for backward compatibility",
"tool_files": """(DEPRECATED) Alias for [node] for backward compatibility""",
@@ -93,12 +73,7 @@
npm = ctx.file.npm,
npm_path = ctx.attr.npm_path if ctx.attr.npm_path else (_to_manifest_path(ctx, ctx.file.npm) if ctx.file.npm else ""), # _to_manifest_path for backward compat
npm_sources = npm_sources,
- headers = struct(
- providers_map = {
- "CcInfo": ctx.attr.headers[CcInfo],
- "DefaultInfo": ctx.attr.headers[DefaultInfo],
- },
- ) if ctx.attr.headers else None,
+ headers = ctx.attr.headers,
# For backward compat
target_tool_path = _to_manifest_path(ctx, ctx.file.node) if ctx.attr.node else ctx.attr.node_path,
tool_files = [ctx.file.node] if ctx.attr.node else [],
@@ -205,7 +180,7 @@
Not necessary if specifying `npm_path` to a non-hermetic npm installation.
- headers: cc_library that contains the Node/v8 header files
+ headers: filegroup that contains the Node/v8 header files
**kwargs: Additional parameters
"""
