fix: make CcInfo/cc dep in nodejs toolchain opt-in via include_header…

…s attribute
bazel-contrib · Jun 8, 2024 · 6d8c697 · 6d8c697
1 parent 96343c9
commit 6d8c697
Show file tree

Hide file tree

Showing 24 changed files with 219 additions and 233 deletions.
diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml
@@ -3,78 +3,103 @@ bazel: 7.1.1
 # Note, this will tell the user to do the wrong thing (manually run buildifer)
 # See https://github.com/bazelbuild/continuous-integration/issues/1161
 buildifier:
-  version: 4.0.1
+  version: 6.4.0
   # Keep this in sync with the list in .pre-commit-config.yaml
   # https://github.com/bazelbuild/buildtools/issues/479 should fix this by giving us a config file
-  warnings: "-bzl-visibility,-function-docstring-args,-function-docstring-return,-print,-unnamed-macro,-provider-params,-function-docstring-header,-no-effect,-uninitialized,-rule-impl-return"
-  # Note, we ought to be able to exclude third_party from this check, but currently cannot:
-  # https://github.com/bazelbuild/continuous-integration/issues/1162
 tasks:
   ubuntu1804:
     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-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
     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:
+      - "//..."
+  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:
-    - "//..."
+      - "//..."
     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:
+      - "//..."
+  rbe_ubuntu1604-smoke:
+    name: rbe_ubuntu1604-smoke
+    platform: rbe_ubuntu1604
+    working_directory: "e2e/smoke"
+    build_targets:
+      - "//..."
     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:
-  #   - "//..."
+      - "//..."
diff --git a/.bazelrc b/.bazelrc
@@ -28,6 +28,11 @@ common --nolegacy_external_runfiles
 # 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
@@ -19,7 +19,7 @@ jobs:
   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
@@ -34,7 +34,7 @@ UserBuildSettingInfo(<a href="#UserBuildSettingInfo-value">value</a>)
 
 <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 @@ If set then the version found in the .nvmrc file is used instead of the one spec
 
 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
@@ -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
@@ -0,0 +1 @@
+../../.bazelversion
diff --git a/e2e/headers/BUILD.bazel 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
@@ -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
@@ -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
diff --git a/e2e/smoke/using_headers.cc → e2e/headers/using_headers.cc b/e2e/smoke/using_headers.cc → e2e/headers/using_headers.cc
diff --git a/e2e/nodejs_host/WORKSPACE.bazel b/e2e/nodejs_host/WORKSPACE.bazel
@@ -1,11 +1,5 @@
 load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
 
-http_archive(
-    name = "bazel_skylib",
-    sha256 = "cd55a062e763b9349921f0f5db8c3933288dc8ba4f76dd9416aac68acee3cb94",
-    urls = ["https://github.com/bazelbuild/bazel-skylib/releases/download/1.5.0/bazel-skylib-1.5.0.tar.gz"],
-)
-
 local_repository(
     name = "rules_nodejs",
     path = "../..",
@@ -26,3 +20,9 @@ nodejs_register_toolchains(
     name = "node16_nvmrc",
     node_version_from_nvmrc = "//:.nvmrc",
 )
+
+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/smoke/BUILD.bazel b/e2e/smoke/BUILD.bazel
@@ -268,18 +268,3 @@ diff_test(
     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
@@ -5,19 +5,6 @@ local_repository(
     path = "../..",
 )
 
-http_archive(
-    name = "aspect_bazel_lib",
-    sha256 = "6d758a8f646ecee7a3e294fbe4386daafbe0e5966723009c290d493f227c390b",
-    strip_prefix = "bazel-lib-2.7.7",
-    url = "https://github.com/aspect-build/bazel-lib/releases/download/v2.7.7/bazel-lib-v2.7.7.tar.gz",
-)
-
-load("@aspect_bazel_lib//lib:repositories.bzl", "aspect_bazel_lib_dependencies", "aspect_bazel_lib_register_toolchains")
-
-aspect_bazel_lib_dependencies()
-
-aspect_bazel_lib_register_toolchains()
-
 load("@rules_nodejs//nodejs:repositories.bzl", "nodejs_register_toolchains")
 
 # The order matters because Bazel will provide the first registered toolchain when a rule asks Bazel to select it
@@ -45,22 +32,15 @@ copy_directory(
     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",
+    name = "aspect_bazel_lib",
+    sha256 = "6d758a8f646ecee7a3e294fbe4386daafbe0e5966723009c290d493f227c390b",
+    strip_prefix = "bazel-lib-2.7.7",
+    url = "https://github.com/aspect-build/bazel-lib/releases/download/v2.7.7/bazel-lib-v2.7.7.tar.gz",
 )
 
-load("@bazelci_rules//:rbe_repo.bzl", "rbe_preconfig")
+load("@aspect_bazel_lib//lib:repositories.bzl", "aspect_bazel_lib_dependencies", "aspect_bazel_lib_register_toolchains")
 
-# Creates toolchain configuration for remote execution with BuildKite CI
-# for rbe_ubuntu1604
-rbe_preconfig(
-    name = "buildkite_config",
-    toolchain = "ubuntu1804-bazel-java11",
-)
+aspect_bazel_lib_dependencies()
+
+aspect_bazel_lib_register_toolchains()
diff --git 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",
+)