build: disable bazel nodejs linker to improve stability on windows (angular#45872)

The NodeJS Bazel linker does not work well on Windows because there
is no sandboxing and linker processes from different tests will attempt
to modify the same `node_modules`, causing concurrency race conditions
and resulting in flakiness.

PR Close angular#45872

Author: devversion

BUILD.bazel

@@ -1,4 +1,4 @@
-load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_binary")
+load("//tools:defaults.bzl", "nodejs_binary")
 
 package(default_visibility = ["//visibility:public"])
 

aio/BUILD.bazel

@@ -1,5 +1,5 @@
 load("@aio_npm//@angular-devkit/architect-cli:index.bzl", "architect", "architect_test")
-load("@build_bazel_rules_nodejs//:index.bzl", "npm_package_bin")
+load("//tools:defaults.bzl", "npm_package_bin")
 
 # The write_source_files macro is used to write bazel outputs to the source tree and test that they are up to date.
 # See: https://docs.aspect.build/aspect-build/bazel-lib/v0.5.0/docs/docs/write_source_files-docgen.html

aio/scripts/BUILD.bazel

@@ -1,4 +1,4 @@
-load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_binary")
+load("//tools:defaults.bzl", "nodejs_binary")
 
 package(default_visibility = ["//visibility:public"])
 

integration/bazel_workspace_tests/bazel_ngtsc_plugin/tools/BUILD.bazel

@@ -13,6 +13,9 @@ nodejs_binary(
         "@npm//@bazel/concatjs",
     ],
     entry_point = "@npm//:node_modules/@bazel/concatjs/internal/tsc_wrapped/tsc_wrapped.js",
+    # Disable the linker and rely on patched resolution which works better on Windows
+    # and is less prone to race conditions when targets build concurrently.
+    templated_args = ["--nobazel_run_linker"],
     visibility = ["//:__subpackages__"],
 )
 

packages/bazel/package.json

@@ -17,7 +17,7 @@
   "bazelBin": {
     "ngc-wrapped": {
       "additionalAttributes": {
-        "templated_args": "[\"--bazel_patch_module_resolver\"]"
+        "templated_args": "[\"--nobazel_run_linker\"]"
       }
     }
   },

packages/bazel/src/ng_package/BUILD.bazel

@@ -36,9 +36,9 @@ nodejs_binary(
         # END-DEV-ONLY
         "//:node_modules/rollup/dist/bin/rollup"
     ),
-    # TODO(josephperrott): update dependency usages to no longer need bazel patch module resolver
-    # See: https://github.com/bazelbuild/rules_nodejs/wiki#--bazel_patch_module_resolver-now-defaults-to-false-2324
-    templated_args = ["--bazel_patch_module_resolver"],
+    # Disable the linker and rely on patched resolution which works better on Windows
+    # and is less prone to race conditions when targets build concurrently.
+    templated_args = ["--nobazel_run_linker"],
 )
 
 exports_files([
@@ -72,8 +72,8 @@ nodejs_binary(
         "@npm//shelljs",
     ],
     entry_point = ":packager.ts",
-    # TODO(josephperrott): update dependency usages to no longer need bazel patch module resolver
-    # See: https://github.com/bazelbuild/rules_nodejs/wiki#--bazel_patch_module_resolver-now-defaults-to-false-2324
-    templated_args = ["--bazel_patch_module_resolver"],
+    # Disable the linker and rely on patched resolution which works better on Windows
+    # and is less prone to race conditions when targets build concurrently.
+    templated_args = ["--nobazel_run_linker"],
 )
 # END-DEV-ONLY

packages/bazel/src/ngc-wrapped/BUILD.bazel

@@ -49,9 +49,8 @@ nodejs_binary(
         "@npm//source-map-support",
     ],
     entry_point = ":extract_i18n.ts",
-    # TODO(josephperrott): update dependency usages to no longer need bazel patch module resolver
-    # See: https://github.com/bazelbuild/rules_nodejs/wiki#--bazel_patch_module_resolver-now-defaults-to-false-2324
-    templated_args = ["--bazel_patch_module_resolver"],
+    # Follows the same reasoning as for the actual `ngc-wrapped` target.
+    templated_args = ["--nobazel_run_linker"],
     visibility = ["//visibility:public"],
 )
 

packages/bazel/src/types_bundle/BUILD.bazel

@@ -1,7 +1,6 @@
 # BEGIN-DEV-ONLY
 
-load("//tools:defaults.bzl", "ts_library")
-load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_binary")
+load("//tools:defaults.bzl", "nodejs_binary", "ts_library")
 
 package(default_visibility = ["//packages:__subpackages__"])
 
@@ -24,9 +23,9 @@ nodejs_binary(
         "@npm//@microsoft/api-extractor",
     ],
     entry_point = ":index.ts",
-    # TODO(josephperrott): update dependency usages to no longer need bazel patch module resolver
-    # See: https://github.com/bazelbuild/rules_nodejs/wiki#--bazel_patch_module_resolver-now-defaults-to-false-2324
-    templated_args = ["--bazel_patch_module_resolver"],
+    # Disable the linker and rely on patched resolution which works better on Windows
+    # and is less prone to race conditions when targets build concurrently.
+    templated_args = ["--nobazel_run_linker"],
     visibility = ["//visibility:public"],
 )
 

packages/common/locales/generate-locales-tool/bin/BUILD.bazel

@@ -1,5 +1,4 @@
-load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_binary")
-load("//tools:defaults.bzl", "ts_library")
+load("//tools:defaults.bzl", "nodejs_binary", "ts_library")
 
 package(default_visibility = ["//visibility:public"])
 
@@ -30,5 +29,5 @@ ts_library(
     # We need to patch the NodeJS module resolution as this binary runs as
     # part of a genrule where the linker does not work as expected.
     # See: https://github.com/bazelbuild/rules_nodejs/issues/2600.
-    templated_args = ["--bazel_patch_module_resolver"],
+    templated_args = ["--nobazel_run_linker"],
 ) for entrypoint in BIN_ENTRYPOINTS]

packages/compiler-cli/ngcc/test/BUILD.bazel

@@ -99,11 +99,6 @@ jasmine_node_test(
     ],
     shard_count = 4,
     tags = [],
-    templated_args = [
-        # TODO(josephperrott): update dependency usages to no longer need bazel patch module resolver
-        # See: https://github.com/bazelbuild/rules_nodejs/wiki#--bazel_patch_module_resolver-now-defaults-to-false-2324
-        "--bazel_patch_module_resolver",
-    ],
     deps = [
         ":integration_lib",
         "@npm//convert-source-map",

packages/compiler-cli/test/compliance/partial/partial_compliance_goldens.bzl

@@ -1,4 +1,5 @@
-load("@build_bazel_rules_nodejs//:index.bzl", "generated_file_test", "nodejs_binary", "npm_package_bin")
+load("@build_bazel_rules_nodejs//:index.bzl", "generated_file_test")
+load("//tools:defaults.bzl", "nodejs_binary", "npm_package_bin")
 
 def partial_compliance_golden(filePath):
     """Creates the generate and testing targets for partial compile results.
@@ -19,10 +20,7 @@ def partial_compliance_golden(filePath):
         data = data + [filePath],
         visibility = [":__pkg__"],
         entry_point = "//packages/compiler-cli/test/compliance/partial:cli.ts",
-        templated_args = [
-            "--bazel_patch_module_resolver",
-            "$(execpath %s)" % filePath,
-        ],
+        templated_args = ["$(execpath %s)" % filePath],
     )
 
     nodejs_binary(
@@ -31,13 +29,7 @@ def partial_compliance_golden(filePath):
         data = data,
         visibility = [":__pkg__"],
         entry_point = "//packages/compiler-cli/test/compliance/partial:cli.ts",
-        templated_args = [
-            # TODO(josephperrott): update dependency usages to no longer need bazel patch module resolver
-            # See: https://github.com/bazelbuild/rules_nodejs/wiki#--bazel_patch_module_resolver-now-defaults-to-false-2324
-            "--bazel_patch_module_resolver",
-            "--node_options=--inspect-brk",
-            filePath,
-        ],
+        templated_args = ["--node_options=--inspect-brk", filePath],
     )
 
     npm_package_bin(
@@ -46,6 +38,9 @@ def partial_compliance_golden(filePath):
         testonly = True,
         stdout = "%s/_generated.js" % path,
         link_workspace_root = True,
+        # Disable the linker and rely on patched resolution which works better on Windows
+        # and is less prone to race conditions when targets build concurrently.
+        args = ["--nobazel_run_linker"],
         visibility = [":__pkg__"],
         data = [],
     )

packages/core/test/render3/perf/BUILD.bazel

@@ -1,4 +1,5 @@
-load("//tools:defaults.bzl", "app_bundle", "jasmine_node_test", "ng_benchmark", "ts_library")
+load("//tools:defaults.bzl", "app_bundle", "jasmine_node_test", "ts_library")
+load("//tools:ng_benchmark.bzl", "ng_benchmark")
 
 package(default_visibility = ["//visibility:private"])
 

tools/circular_dependency_test/index.bzl

@@ -3,7 +3,7 @@
 # Use of this source code is governed by an MIT-style license that can be
 # found in the LICENSE file at https://angular.io/license
 
-load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_test")
+load("//tools:defaults.bzl", "nodejs_test")
 
 MADGE_CONFIG_LABEL = "//tools/circular_dependency_test:madge-resolve.config.js"
 

tools/defaults.bzl

@@ -1,7 +1,7 @@
 """Re-export of some bazel rules with repository-wide defaults."""
 
 load("@rules_pkg//:pkg.bzl", "pkg_tar")
-load("@build_bazel_rules_nodejs//:index.bzl", _nodejs_binary = "nodejs_binary", _pkg_npm = "pkg_npm")
+load("@build_bazel_rules_nodejs//:index.bzl", _nodejs_binary = "nodejs_binary", _nodejs_test = "nodejs_test", _npm_package_bin = "npm_package_bin", _pkg_npm = "pkg_npm")
 load("@npm//@bazel/jasmine:index.bzl", _jasmine_node_test = "jasmine_node_test")
 load("@npm//@bazel/concatjs:index.bzl", _concatjs_devserver = "concatjs_devserver", _ts_config = "ts_config", _ts_library = "ts_library")
 load("@npm//@bazel/rollup:index.bzl", _rollup_bundle = "rollup_bundle")
@@ -10,7 +10,6 @@ load("@npm//@bazel/protractor:index.bzl", _protractor_web_test_suite = "protract
 load("@npm//typescript:index.bzl", "tsc")
 load("//packages/bazel:index.bzl", _ng_module = "ng_module", _ng_package = "ng_package")
 load("@npm//@angular/dev-infra-private/bazel/benchmark/app_bundling:index.bzl", _app_bundle = "app_bundle")
-load("//tools:ng_benchmark.bzl", _ng_benchmark = "ng_benchmark")
 load("@npm//@angular/dev-infra-private/bazel/http-server:index.bzl", _http_server = "http_server")
 load("@npm//@angular/dev-infra-private/bazel/karma:index.bzl", _karma_web_test = "karma_web_test", _karma_web_test_suite = "karma_web_test_suite")
 load("@npm//@angular/dev-infra-private/bazel/api-golden:index.bzl", _api_golden_test = "api_golden_test", _api_golden_test_npm_package = "api_golden_test_npm_package")
@@ -391,14 +390,28 @@ def protractor_web_test_suite(**kwargs):
         **kwargs
     )
 
-def ng_benchmark(**kwargs):
-    """Default values for ng_benchmark"""
-    _ng_benchmark(**kwargs)
-
-def nodejs_binary(data = [], **kwargs):
-    """Default values for nodejs_binary"""
+def nodejs_binary(data = [], templated_args = [], **kwargs):
     _nodejs_binary(
         data = data + ["@npm//source-map-support"],
+        # Disable the linker and rely on patched resolution which works better on Windows
+        # and is less prone to race conditions when targets build concurrently.
+        templated_args = ["--nobazel_run_linker"] + templated_args,
+        **kwargs
+    )
+
+def nodejs_test(templated_args = [], **kwargs):
+    _nodejs_test(
+        # Disable the linker and rely on patched resolution which works better on Windows
+        # and is less prone to race conditions when targets build concurrently.
+        templated_args = ["--nobazel_run_linker"] + templated_args,
+        **kwargs
+    )
+
+def npm_package_bin(args = [], **kwargs):
+    _npm_package_bin(
+        # Disable the linker and rely on patched resolution which works better on Windows
+        # and is less prone to race conditions when targets build concurrently.
+        args = ["--nobazel_run_linker"] + args,
         **kwargs
     )
 
@@ -440,9 +453,10 @@ def jasmine_node_test(bootstrap = [], **kwargs):
     ]
     configuration_env_vars = kwargs.pop("configuration_env_vars", [])
 
-    # TODO(josephperrott): update dependency usages to no longer need bazel patch module resolver
-    # See: https://github.com/bazelbuild/rules_nodejs/wiki#--bazel_patch_module_resolver-now-defaults-to-false-2324
-    templated_args = ["--bazel_patch_module_resolver"] + kwargs.pop("templated_args", [])
+    # Disable the linker and rely on patched resolution which works better on Windows
+    # and is less prone to race conditions when targets build concurrently.
+    templated_args = ["--nobazel_run_linker"] + kwargs.pop("templated_args", [])
+
     for label in bootstrap:
         deps += [label]
         templated_args += ["--node_options=--require=$$(rlocation $(rootpath %s))" % label]

tools/ng_benchmark.bzl

@@ -4,7 +4,7 @@
 # found in the LICENSE file at https://angular.io/license
 """Bazel macro for running Angular benchmarks"""
 
-load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_binary")
+load("//tools:defaults.bzl", "nodejs_binary")
 
 def ng_benchmark(name, bundle):
     """

tools/saucelabs/BUILD.bazel

@@ -1,4 +1,4 @@
-load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_binary")
+load("//tools:defaults.bzl", "nodejs_binary")
 
 package(default_visibility = ["//visibility:public"])
 

tools/size-tracking/index.bzl

@@ -3,7 +3,7 @@
 # Use of this source code is governed by an MIT-style license that can be
 # found in the LICENSE file at https://angular.io/license
 
-load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_binary", "nodejs_test")
+load("//tools:defaults.bzl", "nodejs_binary", "nodejs_test")
 
 """
   Macro that can be used to track the size of a given input file by inspecting

tools/symbol-extractor/index.bzl

@@ -3,7 +3,7 @@
 # Use of this source code is governed by an MIT-style license that can be
 # found in the LICENSE file at https://angular.io/license
 
-load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_binary", "nodejs_test")
+load("//tools:defaults.bzl", "nodejs_binary", "nodejs_test")
 
 """
   This test verifies that a set of top level symbols from a javascript file match a gold file.
@@ -25,9 +25,9 @@ def js_expected_symbol_test(name, src, golden, data = [], **kwargs):
         data = all_data,
         entry_point = entry_point,
         tags = kwargs.pop("tags", []) + ["symbol_extractor"],
-        # TODO(josephperrott): update dependency usages to no longer need bazel patch module resolver
-        # See: https://github.com/bazelbuild/rules_nodejs/wiki#--bazel_patch_module_resolver-now-defaults-to-false-2324
-        templated_args = ["--bazel_patch_module_resolver", "$(rootpath %s)" % src, "$(rootpath %s)" % golden],
+        # Disable the linker and rely on patched resolution which works better on Windows
+        # and is less prone to race conditions when targets build concurrently.
+        templated_args = ["--nobazel_run_linker", "$(rootpath %s)" % src, "$(rootpath %s)" % golden],
         **kwargs
     )
 
@@ -36,8 +36,8 @@ def js_expected_symbol_test(name, src, golden, data = [], **kwargs):
         testonly = True,
         data = all_data,
         entry_point = entry_point,
-        # TODO(josephperrott): update dependency usages to no longer need bazel patch module resolver
-        # See: https://github.com/bazelbuild/rules_nodejs/wiki#--bazel_patch_module_resolver-now-defaults-to-false-2324
-        templated_args = ["--bazel_patch_module_resolver", "$(rootpath %s)" % src, "$(rootpath %s)" % golden, "--accept"],
+        # Disable the linker and rely on patched resolution which works better on Windows
+        # and is less prone to race conditions when targets build concurrently.
+        templated_args = ["--nobazel_run_linker", "$(rootpath %s)" % src, "$(rootpath %s)" % golden, "--accept"],
         **kwargs
     )