diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml index 04261acc27..9e49c0ccb0 100644 --- a/.bazelci/presubmit.yml +++ b/.bazelci/presubmit.yml @@ -16,6 +16,16 @@ tasks: - "--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 @@ tasks: - "--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 @@ tasks: - "--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 9391386c85..695df6d744 100644 --- 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 index c274428dc5..13c3acce73 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/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 41bceabae2..e2202cc798 100644 --- a/docs/Core.md +++ b/docs/Core.md @@ -34,7 +34,7 @@ UserBuildSettingInfo(value)
 nodejs_repositories(name, node_download_auth, node_repositories, node_urls, node_version,
-                    node_version_from_nvmrc, kwargs)
+                    node_version_from_nvmrc, include_headers, kwargs)
 
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` +

include_headers

+ +Set headers field in NodeInfo provided by this toolchain. + +This setting creates a dependency on a c++ toolchain. + +Defaults to `False` +

kwargs

Additional parameters diff --git a/e2e/headers/.bazelrc b/e2e/headers/.bazelrc new file mode 100644 index 0000000000..507f3d822d --- /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 0000000000..96cf94962b --- /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 0000000000..160858e22d --- /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 0000000000..181c507543 --- /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 0000000000..a4acd07fa1 --- /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 0000000000..e69de29bb2 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 27a0534fad..9eaa324d99 100644 --- 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 index 452b037be3..c0a08eb8f5 100644 --- a/e2e/smoke/WORKSPACE.bazel +++ b/e2e/smoke/WORKSPACE.bazel @@ -44,23 +44,3 @@ load("@aspect_bazel_lib//lib:repositories.bzl", "aspect_bazel_lib_dependencies", 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 e69de29bb2..cb8c8b7f9b 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 e612bb0fb4..e66eda2d1b 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 @@ user_args( 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 45d589be3c..1de6f3639a 100644 --- a/nodejs/extensions.bzl +++ b/nodejs/extensions.bzl @@ -26,6 +26,7 @@ def _toolchain_extension(module_ctx): 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 @@ def _toolchain_extension(module_ctx): 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 @@ node = module_extension( 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 0000000000..9cc097f557 --- /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 01bf064fc6..af3901f849 100644 --- a/nodejs/private/current_node_cc_headers.bzl +++ b/nodejs/private/current_node_cc_headers.bzl @@ -43,7 +43,7 @@ cc_library( 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 b7226182fd..a1af284208 100644 --- a/nodejs/repositories.bzl +++ b/nodejs/repositories.bzl @@ -31,6 +31,7 @@ _ATTRS = { "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 @@ filegroup( 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 @@ cc_library( ), 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 @@ nodejs_toolchain( 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 @@ def nodejs_repositories( 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 @@ def nodejs_repositories( 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 @@ WARNING: use_nvmrc attribute of node_repositories is deprecated; use node_versio 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 62dd4c2348..20e2c455f9 100644 --- a/nodejs/toolchain.bzl +++ b/nodejs/toolchain.bzl @@ -33,7 +33,8 @@ For backward compability, if set then npm_path will be set to the runfiles path 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