fix(): remove _repository_args from nodejs_binary

docs/Built-ins.md

@@ -19,8 +19,8 @@ This is necessary to bootstrap Bazel to run the package manager to download othe
 
 <pre>
 node_repositories(<a href="#node_repositories-name">name</a>, <a href="#node_repositories-node_download_auth">node_download_auth</a>, <a href="#node_repositories-node_repositories">node_repositories</a>, <a href="#node_repositories-node_urls">node_urls</a>, <a href="#node_repositories-node_version">node_version</a>,
-                  <a href="#node_repositories-package_json">package_json</a>, <a href="#node_repositories-platform">platform</a>, <a href="#node_repositories-preserve_symlinks">preserve_symlinks</a>, <a href="#node_repositories-repo_mapping">repo_mapping</a>, <a href="#node_repositories-use_nvmrc">use_nvmrc</a>, <a href="#node_repositories-vendored_node">vendored_node</a>,
-                  <a href="#node_repositories-vendored_yarn">vendored_yarn</a>, <a href="#node_repositories-yarn_download_auth">yarn_download_auth</a>, <a href="#node_repositories-yarn_repositories">yarn_repositories</a>, <a href="#node_repositories-yarn_urls">yarn_urls</a>, <a href="#node_repositories-yarn_version">yarn_version</a>)
+                  <a href="#node_repositories-package_json">package_json</a>, <a href="#node_repositories-platform">platform</a>, <a href="#node_repositories-repo_mapping">repo_mapping</a>, <a href="#node_repositories-use_nvmrc">use_nvmrc</a>, <a href="#node_repositories-vendored_node">vendored_node</a>, <a href="#node_repositories-vendored_yarn">vendored_yarn</a>,
+                  <a href="#node_repositories-yarn_download_auth">yarn_download_auth</a>, <a href="#node_repositories-yarn_repositories">yarn_repositories</a>, <a href="#node_repositories-yarn_urls">yarn_urls</a>, <a href="#node_repositories-yarn_version">yarn_version</a>)
 </pre>
 
 To be run in user's WORKSPACE to install rules_nodejs dependencies.
@@ -184,18 +184,6 @@ Defaults to `[]`
 
 Defaults to `""`
 
-<h4 id="node_repositories-preserve_symlinks">preserve_symlinks</h4>
-
-(*Boolean*): Turn on --node_options=--preserve-symlinks for nodejs_binary and nodejs_test rules.
-
-When this option is turned on, node will preserve the symlinked path for resolves instead of the default
-behavior of resolving to the real path. This means that all required files must be in be included in your
-runfiles as it prevents the default behavior of potentially resolving outside of the runfiles. For example,
-all required files need to be included in your node_modules filegroup. This option is desirable as it gives
-a stronger guarantee of hermeticity which is required for remote execution.
-
-Defaults to `True`
-
 <h4 id="node_repositories-repo_mapping">repo_mapping</h4>
 
 (*<a href="https://bazel.build/docs/skylark/lib/dict.html">Dictionary: String -> String</a>, mandatory*): A dictionary from local repository name to global repository name. This allows controls over workspace dependency resolution for dependencies of this repository.<p>For example, an entry `"@foo": "@bar"` declares that, for any time this repository depends on `@foo` (such as a dependency on `@foo//some:target`, it should actually resolve that dependency within globally-declared `@bar` (`@bar//some:target`).

e2e/bazel_managed_deps/BUILD.bazel

@@ -7,6 +7,7 @@ jasmine_node_test(
     name = "fine_grained_test",
     srcs = glob(["*.spec.js"]),
     data = ["@npm//:bin_files"],
+    templated_args = ["--node_options=--preserve-symlinks"],
     deps = [
         "@npm//jasmine",
         "@npm//typescript",

e2e/fine_grained_symlinks/BUILD.bazel

@@ -9,4 +9,5 @@ nodejs_test(
         "@npm//webpack",
     ],
     entry_point = ":index.spec.js",
+    templated_args = ["--node_options=--preserve-symlinks"],
 )

e2e/jasmine/BUILD.bazel

@@ -3,6 +3,7 @@ load("@npm//@bazel/jasmine:index.bzl", "jasmine_node_test")
 jasmine_node_test(
     name = "test",
     srcs = ["test.spec.js"],
+    templated_args = ["--node_options=--preserve-symlinks"],
 )
 
 jasmine_node_test(
@@ -11,6 +12,7 @@ jasmine_node_test(
     data = ["jasmine_shared_env_bootstrap.js"],
     templated_args = [
         "--node_options=--require=$$(rlocation $(rootpath :jasmine_shared_env_bootstrap.js))",
+        "--node_options=--preserve-symlinks",
     ],
     deps = [
         "@npm//jasmine",

e2e/node_loader_no_preserve_symlinks/WORKSPACE

@@ -19,7 +19,7 @@ rules_nodejs_dependencies()
 
 load("@build_bazel_rules_nodejs//:index.bzl", "node_repositories", "yarn_install")
 
-node_repositories(preserve_symlinks = False)
+node_repositories()
 
 yarn_install(
     name = "npm",

e2e/node_loader_preserve_symlinks/BUILD.bazel

@@ -9,4 +9,5 @@ nodejs_test(
         "@npm//:node_modules",
     ],
     entry_point = ":node_loader_test.spec.js",
+    templated_args = ["--node_options=--preserve-symlinks"],
 )

e2e/nodejs_image/BUILD.bazel

@@ -1,6 +1,7 @@
 load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_binary")
 load("@io_bazel_rules_docker//contrib:test.bzl", "container_test")
 load("@io_bazel_rules_docker//nodejs:image.bzl", "nodejs_image")
+load("@bazel_skylib//rules:write_file.bzl", "write_file")
 
 nodejs_binary(
     name = "main",
@@ -11,14 +12,69 @@ nodejs_binary(
     entry_point = "main.js",
 )
 
-# bazel run --platforms=@rules_nodejs//nodejs:linux_amd64 //:nodejs_image
+# bazel run --platforms=@build_bazel_rules_nodejs//toolchains/node:linux_amd64 //:nodejs_image
 nodejs_image(
     name = "nodejs_image",
     binary = ":main",
+    include_node_repo_args = False,
 )
 
 container_test(
     name = "nodejs_image_test",
     configs = [":nodejs_image.yaml"],
     image = ":nodejs_image",
 )
+
+# this creates:
+# bazel run main_node15_linux_amd64
+# bazel run main_node16_linux_amd64
+# bazel run nodejs_image_node15_linux_amd64
+# bazel run nodejs_image_node16_linux_amd64
+# bazel test nodejs_image_test_node15_linux_amd64
+# bazel test nodejs_image_test_node16_linux_amd64
+[
+    [
+        # Trivial test fixture: a nodejs program that writes to a file
+        write_file(
+            name = "test_" + id,
+            out = "image_%s.yaml" % id,
+            content = [
+                "schemaVersion: 2.0.0",
+                "metadataTest:",
+                "  entrypoint: ['/app//main_%s']" % id,
+                """  workdir: "/app//main_%s.runfiles/e2e_nodejs_image" """ % id,
+            ],
+        ),
+        nodejs_binary(
+            name = "main_" + id,
+            data = [
+                "//foolib",
+                "@npm//date-fns",
+            ],
+            entry_point = "binary_version.js",
+            node = toolchain,
+            toolchain = toolchain,
+        ),
+        nodejs_image(
+            name = "nodejs_image_" + id,
+            binary = ":main_" + id,
+            include_node_repo_args = False,
+            node_repository_name = id,
+        ),
+        container_test(
+            name = "nodejs_image_test_" + id,
+            configs = ["test_" + id],
+            image = ":nodejs_image_" + id,
+        ),
+    ]
+    for id, toolchain in zip(
+        [
+            "node15_linux_amd64",
+            "node16_linux_amd64",
+        ],
+        [
+            "@node15_linux_amd64//:node_toolchain",
+            "@node16_linux_amd64//:node_toolchain",
+        ],
+    )
+]

e2e/nodejs_image/WORKSPACE

@@ -43,9 +43,11 @@ yarn_install(
 
 http_archive(
     name = "io_bazel_rules_docker",
-    sha256 = "20dfc1d9bc6a251455c4f306d3dea030b323ea3f22e0600698fab9665eb273b8",
-    strip_prefix = "rules_docker-0086a29d19b124b04a864e3913cfa7a2cba46349",
-    urls = ["https://github.com/bazelbuild/rules_docker/archive/0086a29d19b124b04a864e3913cfa7a2cba46349.tar.gz"],
+    patch_args = ["-p1"],
+    patches = ["//:rules_docker.patch"],
+    sha256 = "9f4948332e5fa34255c71e605ed49cc6fb684315f713cdff18ecb1fac21b67f2",
+    strip_prefix = "rules_docker-12de489abc70d7bb1ed00ab4b2cf431d46eccd00",
+    urls = ["https://github.com/bazelbuild/rules_docker/archive/12de489abc70d7bb1ed00ab4b2cf431d46eccd00.tar.gz"],
 )
 
 load("@io_bazel_rules_docker//repositories:repositories.bzl", container_repositories = "repositories")
@@ -59,3 +61,17 @@ container_deps()
 load("@io_bazel_rules_docker//nodejs:image.bzl", nodejs_image_repositories = "repositories")
 
 nodejs_image_repositories()
+
+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
+# This applies to the resolved_toolchain
+nodejs_register_toolchains(
+    name = "node16",
+    node_version = "16.5.0",
+)
+
+nodejs_register_toolchains(
+    name = "node15",
+    node_version = "15.14.0",
+)

e2e/nodejs_image/binary_version.js

@@ -0,0 +1 @@
+console.log(process.version)

e2e/nodejs_image/rules_docker.patch

@@ -0,0 +1,30 @@
+diff --git a/nodejs/image.bzl b/nodejs/image.bzl
+index 7c72596..0dac854 100644
+--- a/nodejs/image.bzl
++++ b/nodejs/image.bzl
+@@ -117,6 +117,8 @@ def nodejs_image(
+         binary = None,
+         launcher = None,
+         launcher_args = None,
++        node_repository_name = "nodejs",
++        include_node_repo_args = True,
+         **kwargs):
+     """Constructs a container image wrapping a nodejs_binary target.
+
+@@ -142,11 +144,13 @@ def nodejs_image(
+
+     nodejs_layers = [
+         # Put the Node binary into its own layers.
+-        "@nodejs//:node",
+-        "@nodejs//:node_files",
+-        "@nodejs//:bin/node_repo_args.sh",
++        "@%s//:node" % node_repository_name,
++        "@%s//:node_files" % node_repository_name,
+     ]
+
++    if include_node_repo_args:
++        nodejs_layers.append("@%s//:bin/node_repo_args.sh" % node_repository_name)
++
+     all_layers = nodejs_layers + layers
+
+     visibility = kwargs.get("visibility", None)

e2e/nodejs_repository/BUILD.bazel

@@ -21,4 +21,5 @@ nodejs_test(
         "@nodejs//:yarn_node_repositories",
     ],
     entry_point = ":index.spec.js",
+    templated_args = ["--node_options=--preserve-symlinks"],
 )

e2e/packages/BUILD.bazel

@@ -27,7 +27,10 @@ VARIANTS = [
     # syscall: 'mkdir',
     # path: 'C:\\users\\b\\_bazel_b\\p4se2lwa\\execroot\\e2e_packages\\node_modules'
     # }
-    templated_args = ["--nobazel_run_linker"],
+    templated_args = [
+        "--nobazel_run_linker",
+        "--node_options=--preserve-symlinks",
+    ],
 ) for variant in VARIANTS]
 
 nodejs_test(
@@ -41,7 +44,10 @@ nodejs_test(
     # TODO(gregmagolan): fix this test on windows
     tags = ["fix-windows"],
     # TODO: use runfiles
-    templated_args = ["--bazel_patch_module_resolver"],
+    templated_args = [
+        "--bazel_patch_module_resolver",
+        "--node_options=--preserve-symlinks",
+    ],
 )
 
 nodejs_test(
@@ -55,5 +61,8 @@ nodejs_test(
     # TODO(gregmagolan): fix this test on windows
     tags = ["fix-windows"],
     # TODO: use runfiles
-    templated_args = ["--bazel_patch_module_resolver"],
+    templated_args = [
+        "--bazel_patch_module_resolver",
+        "--node_options=--preserve-symlinks",
+    ],
 )

e2e/symlinked_node_modules_npm/BUILD

@@ -6,4 +6,5 @@ nodejs_test(
         "@npm//typescript",
     ],
     entry_point = ":main.js",
+    templated_args = ["--node_options=--preserve-symlinks"],
 )

e2e/symlinked_node_modules_yarn/BUILD

@@ -6,4 +6,5 @@ nodejs_test(
         "@npm//typescript",
     ],
     entry_point = ":main.js",
+    templated_args = ["--node_options=--preserve-symlinks"],
 )

e2e/webapp/BUILD.bazel

@@ -21,4 +21,5 @@ nodejs_test(
     name = "test",
     data = ["out.min"],
     entry_point = ":test.js",
+    templated_args = ["--node_options=--preserve-symlinks"],
 )

examples/angular/WORKSPACE

@@ -117,9 +117,11 @@ http_archive(
 
 http_archive(
     name = "io_bazel_rules_docker",
-    sha256 = "c27ab432594e793eb864604ec0e4cfd708285218da663b805eefdd479378da93",
-    strip_prefix = "rules_docker-2b35b2dd56f0be6cc6b8df957332a31435f6b3ce",
-    urls = ["https://github.com/bazelbuild/rules_docker/archive/2b35b2dd56f0be6cc6b8df957332a31435f6b3ce.tar.gz"],
+    patch_args = ["-p1"],
+    patches = ["//:rules_docker.patch"],
+    sha256 = "9f4948332e5fa34255c71e605ed49cc6fb684315f713cdff18ecb1fac21b67f2",
+    strip_prefix = "rules_docker-12de489abc70d7bb1ed00ab4b2cf431d46eccd00",
+    urls = ["https://github.com/bazelbuild/rules_docker/archive/12de489abc70d7bb1ed00ab4b2cf431d46eccd00.tar.gz"],
 )
 
 load("@io_bazel_rules_docker//repositories:repositories.bzl", container_repositories = "repositories")

examples/angular/rules_docker.patch

@@ -0,0 +1,30 @@
+diff --git a/nodejs/image.bzl b/nodejs/image.bzl
+index 7c72596..0dac854 100644
+--- a/nodejs/image.bzl
++++ b/nodejs/image.bzl
+@@ -117,6 +117,8 @@ def nodejs_image(
+         binary = None,
+         launcher = None,
+         launcher_args = None,
++        node_repository_name = "nodejs",
++        include_node_repo_args = True,
+         **kwargs):
+     """Constructs a container image wrapping a nodejs_binary target.
+
+@@ -142,11 +144,13 @@ def nodejs_image(
+
+     nodejs_layers = [
+         # Put the Node binary into its own layers.
+-        "@nodejs//:node",
+-        "@nodejs//:node_files",
+-        "@nodejs//:bin/node_repo_args.sh",
++        "@%s//:node" % node_repository_name,
++        "@%s//:node_files" % node_repository_name,
+     ]
+
++    if include_node_repo_args:
++        nodejs_layers.append("@%s//:bin/node_repo_args.sh" % node_repository_name)
++
+     all_layers = nodejs_layers + layers
+
+     visibility = kwargs.get("visibility", None)

examples/angular/src/BUILD.bazel

@@ -223,6 +223,7 @@ history_server(
 nodejs_image(
     name = "nodejs_image",
     binary = ":prodserver",
+    include_node_repo_args = False,
     # Actions created by this rule are I/O-bound,
     # so there is no benefit to running them remotely
     tags = ["local"],

examples/nestjs/WORKSPACE

@@ -43,9 +43,11 @@ yarn_install(
 
 http_archive(
     name = "io_bazel_rules_docker",
-    sha256 = "c27ab432594e793eb864604ec0e4cfd708285218da663b805eefdd479378da93",
-    strip_prefix = "rules_docker-2b35b2dd56f0be6cc6b8df957332a31435f6b3ce",
-    urls = ["https://github.com/bazelbuild/rules_docker/archive/2b35b2dd56f0be6cc6b8df957332a31435f6b3ce.tar.gz"],
+    patch_args = ["-p1"],
+    patches = ["//:rules_docker.patch"],
+    sha256 = "9f4948332e5fa34255c71e605ed49cc6fb684315f713cdff18ecb1fac21b67f2",
+    strip_prefix = "rules_docker-12de489abc70d7bb1ed00ab4b2cf431d46eccd00",
+    urls = ["https://github.com/bazelbuild/rules_docker/archive/12de489abc70d7bb1ed00ab4b2cf431d46eccd00.tar.gz"],
 )
 
 load(

examples/nestjs/rules_docker.patch

@@ -0,0 +1,30 @@
+diff --git a/nodejs/image.bzl b/nodejs/image.bzl
+index 7c72596..0dac854 100644
+--- a/nodejs/image.bzl
++++ b/nodejs/image.bzl
+@@ -117,6 +117,8 @@ def nodejs_image(
+         binary = None,
+         launcher = None,
+         launcher_args = None,
++        node_repository_name = "nodejs",
++        include_node_repo_args = True,
+         **kwargs):
+     """Constructs a container image wrapping a nodejs_binary target.
+
+@@ -142,11 +144,13 @@ def nodejs_image(
+
+     nodejs_layers = [
+         # Put the Node binary into its own layers.
+-        "@nodejs//:node",
+-        "@nodejs//:node_files",
+-        "@nodejs//:bin/node_repo_args.sh",
++        "@%s//:node" % node_repository_name,
++        "@%s//:node_files" % node_repository_name,
+     ]
+
++    if include_node_repo_args:
++        nodejs_layers.append("@%s//:bin/node_repo_args.sh" % node_repository_name)
++
+     all_layers = nodejs_layers + layers
+
+     visibility = kwargs.get("visibility", None)

examples/nestjs/src/BUILD.bazel

@@ -77,4 +77,5 @@ jasmine_node_test(
 nodejs_image(
     name = "docker",
     binary = ":server",
+    include_node_repo_args = False,
 )

internal/bazel_integration_test/test_runner.js

@@ -384,7 +384,6 @@ for (const bazelCommand of config.bazelCommands) {
     'GTEST_TMP_DIR',
     'INIT_CWD',
     'JAVA_RUNFILES',
-    'NODE_REPOSITORY_ARGS',
     'OLDPWD',
     'PYTHON_RUNFILES',
     'RUN_UNDER_RUNFILES',