fix: make CcInfo/cc dep in nodejs toolchain opt-in via include_headers attribute (#3760)
diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml
index 04261ac..9e49c0c 100644
--- a/.bazelci/presubmit.yml
+++ b/.bazelci/presubmit.yml
@@ -16,6 +16,16 @@
- "--test_tag_filters=-skip-on-bazelci-ubuntu"
test_targets:
- "//..."
+ ubuntu1804-headers:
+ name: ubuntu1804-headers
+ platform: ubuntu1804
+ working_directory: "e2e/headers"
+ build_targets:
+ - "//..."
+ test_flags:
+ - "--test_tag_filters=-skip-on-bazelci-ubuntu"
+ test_targets:
+ - "//..."
ubuntu1804-smoke:
name: ubuntu1804-smoke
platform: ubuntu1804
@@ -36,9 +46,28 @@
- "--test_tag_filters=-skip-on-bazelci-ubuntu"
test_targets:
- "//..."
- macos-smoke:
+ macos:
name: macos
platform: macos
+ build_targets:
+ - "//..."
+ test_flags:
+ - "--test_tag_filters=-skip-on-bazelci-macos"
+ test_targets:
+ - "//..."
+ macos-headers:
+ name: macos-headers
+ platform: macos
+ working_directory: "e2e/headers"
+ build_targets:
+ - "//..."
+ test_flags:
+ - "--test_tag_filters=-skip-on-bazelci-macos"
+ test_targets:
+ - "//..."
+ macos-smoke:
+ name: macos-smoke
+ platform: macos
working_directory: "e2e/smoke"
build_targets:
- "//..."
@@ -66,12 +95,11 @@
- "--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_targets:
+ - "//..."
+ test_targets:
+ - "//..."
diff --git a/.bazelrc b/.bazelrc
index 9391386..695df6d 100644
--- a/.bazelrc
+++ b/.bazelrc
@@ -28,6 +28,11 @@
# https://bazelbuild.slack.com/archives/C014RARENH0/p1691158021917459?thread_ts=1691156601.420349&cid=C014RARENH0
common --check_direct_dependencies=off
+# In the root MODULE.bazel file we don't set include_headers on the nodejs toolchain
+# so the `//nodejs/headers:current_node_cc_headers`` target will not build. This target
+# is instead tested in `e2e/headers``
+common --deleted_packages=nodejs/headers
+
# Load any settings specific to the current user.
# .bazelrc.user should appear in .gitignore so that settings are not shared with team members
# This needs to be last statement in this
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
index c274428..13c3acc 100644
--- a/.github/workflows/ci.yaml
+++ b/.github/workflows/ci.yaml
@@ -19,7 +19,7 @@
test:
uses: bazel-contrib/.github/.github/workflows/bazel.yaml@v6
with:
- folders: '[".", "e2e/smoke", "e2e/nodejs_host"]'
+ folders: '[".", "e2e/headers", "e2e/smoke", "e2e/nodejs_host"]'
# stardoc generated docs fail on diff_test with Bazel 6.4.0 so don't test against it in root repository
exclude: |
[
diff --git a/docs/Core.md b/docs/Core.md
index 41bceab..e2202cc 100644
--- a/docs/Core.md
+++ b/docs/Core.md
@@ -34,7 +34,7 @@
<pre>
nodejs_repositories(<a href="#nodejs_repositories-name">name</a>, <a href="#nodejs_repositories-node_download_auth">node_download_auth</a>, <a href="#nodejs_repositories-node_repositories">node_repositories</a>, <a href="#nodejs_repositories-node_urls">node_urls</a>, <a href="#nodejs_repositories-node_version">node_version</a>,
- <a href="#nodejs_repositories-node_version_from_nvmrc">node_version_from_nvmrc</a>, <a href="#nodejs_repositories-kwargs">kwargs</a>)
+ <a href="#nodejs_repositories-node_version_from_nvmrc">node_version_from_nvmrc</a>, <a href="#nodejs_repositories-include_headers">include_headers</a>, <a href="#nodejs_repositories-kwargs">kwargs</a>)
</pre>
To be run in user's WORKSPACE to install rules_nodejs dependencies.
@@ -143,6 +143,14 @@
Defaults to `None`
+<h4 id="nodejs_repositories-include_headers">include_headers</h4>
+
+Set headers field in NodeInfo provided by this toolchain.
+
+This setting creates a dependency on a c++ toolchain.
+
+Defaults to `False`
+
<h4 id="nodejs_repositories-kwargs">kwargs</h4>
Additional parameters
diff --git a/e2e/headers/.bazelrc b/e2e/headers/.bazelrc
new file mode 100644
index 0000000..507f3d8
--- /dev/null
+++ b/e2e/headers/.bazelrc
@@ -0,0 +1,20 @@
+# Specifies desired output mode for running tests.
+# Valid values are
+# 'summary' to output only test status summary
+# 'errors' to also print test logs for failed tests
+# 'all' to print logs for all tests
+# 'streamed' to output logs for all tests in real time
+# (this will force tests to be executed locally one at a time regardless of --test_strategy value).
+common --test_output=errors
+
+# Turn on --incompatible_strict_action_env which was on by default
+# in Bazel 0.21.0 but turned off again in 0.22.0. Follow
+# https://github.com/bazelbuild/bazel/issues/7026 for more details.
+# This flag is needed to so that the bazel cache is not invalidated
+# when running bazel via `yarn bazel`.
+# See https://github.com/angular/angular/issues/27514.
+common --incompatible_strict_action_env
+
+# Turn off legacy external runfiles
+# This prevents accidentally depending on this feature, which Bazel will remove.
+common --nolegacy_external_runfiles
diff --git a/e2e/headers/.bazelversion b/e2e/headers/.bazelversion
new file mode 120000
index 0000000..96cf949
--- /dev/null
+++ b/e2e/headers/.bazelversion
@@ -0,0 +1 @@
+../../.bazelversion
\ No newline at end of file
diff --git a/e2e/headers/BUILD.bazel b/e2e/headers/BUILD.bazel
new file mode 100644
index 0000000..160858e
--- /dev/null
+++ b/e2e/headers/BUILD.bazel
@@ -0,0 +1,14 @@
+cc_test(
+ name = "using_headers_test",
+ srcs = ["using_headers.cc"],
+ copts = select({
+ "@platforms//os:windows": ["/std:c++14"],
+ "//conditions:default": ["-std=c++14"],
+ }),
+ 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/headers:current_node_cc_headers"],
+)
diff --git a/e2e/headers/MODULE.bazel b/e2e/headers/MODULE.bazel
new file mode 100644
index 0000000..181c507
--- /dev/null
+++ b/e2e/headers/MODULE.bazel
@@ -0,0 +1,10 @@
+bazel_dep(name = "rules_nodejs", version = "0.0.0", dev_dependency = True)
+local_path_override(
+ module_name = "rules_nodejs",
+ path = "../..",
+)
+
+bazel_dep(name = "platforms", version = "0.0.10", dev_dependency = True)
+
+node = use_extension("@rules_nodejs//nodejs:extensions.bzl", "node", dev_dependency = True)
+node.toolchain(include_headers = True)
diff --git a/e2e/headers/WORKSPACE.bazel b/e2e/headers/WORKSPACE.bazel
new file mode 100644
index 0000000..a4acd07
--- /dev/null
+++ b/e2e/headers/WORKSPACE.bazel
@@ -0,0 +1,16 @@
+load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
+
+local_repository(
+ name = "rules_nodejs",
+ path = "../..",
+)
+
+load("@rules_nodejs//nodejs:repositories.bzl", "nodejs_register_toolchains")
+
+nodejs_register_toolchains(include_headers = True)
+
+http_archive(
+ name = "bazel_skylib",
+ sha256 = "bc283cdfcd526a52c3201279cda4bc298652efa898b10b4db0837dc51652756f",
+ urls = ["https://github.com/bazelbuild/bazel-skylib/releases/download/1.7.1/bazel-skylib-1.7.1.tar.gz"],
+)
diff --git a/e2e/headers/WORKSPACE.bzlmod b/e2e/headers/WORKSPACE.bzlmod
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/e2e/headers/WORKSPACE.bzlmod
diff --git a/e2e/smoke/using_headers.cc b/e2e/headers/using_headers.cc
similarity index 100%
rename from e2e/smoke/using_headers.cc
rename to e2e/headers/using_headers.cc
diff --git a/e2e/smoke/BUILD.bazel b/e2e/smoke/BUILD.bazel
index 27a0534..9eaa324 100644
--- a/e2e/smoke/BUILD.bazel
+++ b/e2e/smoke/BUILD.bazel
@@ -268,18 +268,3 @@
file1 = "write_node_version_16",
file2 = "thing_toolchain_16",
)
-
-cc_binary(
- name = "using_headers_test",
- srcs = ["using_headers.cc"],
- copts = select({
- "@platforms//os:windows": ["/std:c++14"],
- "//conditions:default": ["-std=c++14"],
- }),
- 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"],
-)
diff --git a/e2e/smoke/WORKSPACE.bazel b/e2e/smoke/WORKSPACE.bazel
index 452b037..c0a08eb 100644
--- a/e2e/smoke/WORKSPACE.bazel
+++ b/e2e/smoke/WORKSPACE.bazel
@@ -44,23 +44,3 @@
aspect_bazel_lib_dependencies()
aspect_bazel_lib_register_toolchains()
-
-#
-# 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 e612bb0..e66eda2 100644
--- a/nodejs/BUILD.bazel
+++ b/nodejs/BUILD.bazel
@@ -1,5 +1,4 @@
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")
@@ -41,8 +40,3 @@
name = "default_args",
build_setting_default = "--preserve-symlinks",
)
-
-# 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")
diff --git a/nodejs/extensions.bzl b/nodejs/extensions.bzl
index 45d589b..1de6f36 100644
--- a/nodejs/extensions.bzl
+++ b/nodejs/extensions.bzl
@@ -26,6 +26,7 @@
registrations[toolchain.name] = struct(
node_version = toolchain.node_version,
node_version_from_nvmrc = toolchain.node_version_from_nvmrc,
+ include_headers = toolchain.include_headers,
)
for k, v in registrations.items():
@@ -33,6 +34,7 @@
name = k,
node_version = v.node_version,
node_version_from_nvmrc = v.node_version_from_nvmrc,
+ include_headers = v.include_headers,
register = False,
)
@@ -54,6 +56,12 @@
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.
+
+This setting creates a dependency on a c++ toolchain.
+""",
+ ),
}),
},
)
diff --git a/nodejs/headers/BUILD.bazel b/nodejs/headers/BUILD.bazel
new file mode 100644
index 0000000..9cc097f
--- /dev/null
+++ b/nodejs/headers/BUILD.bazel
@@ -0,0 +1,8 @@
+load("//nodejs/private:current_node_cc_headers.bzl", "current_node_cc_headers")
+
+package(default_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")
diff --git a/nodejs/private/current_node_cc_headers.bzl b/nodejs/private/current_node_cc_headers.bzl
index 01bf064..af3901f 100644
--- a/nodejs/private/current_node_cc_headers.bzl
+++ b/nodejs/private/current_node_cc_headers.bzl
@@ -43,7 +43,7 @@
srcs = ["foo.cc"],
# If toolchain sets this already, you can omit.
copts = ["-std=c++14"],
- deps = ["@rules_nodejs//:current_node_cc_headers"]
+ deps = ["@rules_nodejs//nodejs/headers:current_node_cc_headers"]
)
```
""",
diff --git a/nodejs/repositories.bzl b/nodejs/repositories.bzl
index b722618..a1af284 100644
--- a/nodejs/repositories.bzl
+++ b/nodejs/repositories.bzl
@@ -31,6 +31,7 @@
"node_urls": attr.string_list(),
"node_version": attr.string(),
"node_version_from_nvmrc": attr.label(allow_single_file = True),
+ "include_headers": attr.bool(),
"platform": attr.string(
doc = "Internal use only. Which platform to install as a toolchain. If unset, we assume the repository is named nodejs_[platform]",
values = BUILT_IN_NODE_PLATFORMS,
@@ -241,6 +242,20 @@
name = "npm_files",
srcs = glob(["bin/nodejs/**"]) + [":node_files"],
)
+""".format(
+ node_bin_export = "\n \"%s\"," % node_bin,
+ npm_bin_export = "\n \"%s\"," % npm_bin,
+ npx_bin_export = "\n \"%s\"," % npx_bin,
+ node_bin_label = node_bin_label,
+ npm_bin_label = npm_bin_label,
+ npx_bin_label = npx_bin_label,
+ node_entry = node_entry,
+ npm_entry = npm_entry,
+ npx_entry = npx_entry,
+ )
+
+ if repository_ctx.attr.include_headers:
+ build_content += """
cc_library(
name = "headers",
hdrs = glob(
@@ -256,17 +271,7 @@
),
includes = ["bin/nodejs/include/node"],
)
-""".format(
- node_bin_export = "\n \"%s\"," % node_bin,
- npm_bin_export = "\n \"%s\"," % npm_bin,
- npx_bin_export = "\n \"%s\"," % npx_bin,
- node_bin_label = node_bin_label,
- npm_bin_label = npm_bin_label,
- npx_bin_label = npx_bin_label,
- node_entry = node_entry,
- npm_entry = npm_entry,
- npx_entry = npx_entry,
- )
+"""
if repository_ctx.attr.platform:
build_content += """
@@ -276,14 +281,15 @@
node = ":node_bin",
npm = ":npm",
npm_srcs = [":npm_files"],
- headers = ":headers",
+ headers = {headers},
)
# alias for backward compat
alias(
name = "node_toolchain",
actual = ":toolchain",
)
-"""
+""".format(headers = "\":headers\"" if repository_ctx.attr.include_headers else "None")
+
repository_ctx.file("BUILD.bazel", content = build_content)
def _strip_bin(path):
@@ -314,6 +320,7 @@
node_urls = [DEFAULT_NODE_URL],
node_version = DEFAULT_NODE_VERSION,
node_version_from_nvmrc = None,
+ include_headers = False,
**kwargs):
"""To be run in user's WORKSPACE to install rules_nodejs dependencies.
@@ -394,6 +401,10 @@
If set then the version found in the .nvmrc file is used instead of the one specified by node_version.
+ include_headers: Set headers field in NodeInfo provided by this toolchain.
+
+ This setting creates a dependency on a c++ toolchain.
+
**kwargs: Additional parameters
"""
use_nvmrc = kwargs.pop("use_nvmrc", None)
@@ -411,6 +422,7 @@
node_urls = node_urls,
node_version = node_version,
node_version_from_nvmrc = node_version_from_nvmrc,
+ include_headers = include_headers,
**kwargs
)
diff --git a/nodejs/toolchain.bzl b/nodejs/toolchain.bzl
index 62dd4c2..20e2c45 100644
--- a/nodejs/toolchain.bzl
+++ b/nodejs/toolchain.bzl
@@ -33,7 +33,8 @@
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": """\
+ "headers": """Optional.\
+
(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
