fix: fix nodejs_binary cross-platform RBE issue bazel-contrib#1305

* node_launcher.sh now picks the node binary at runtime instead of it being a templated arg evaluated at build time
* node_binary now adds the correct node tool_files to the nodejs_binary runfiles

Fixes bazel-contrib#1305

internal/node/node.bzl

@@ -139,6 +139,9 @@ def _to_execroot_path(ctx, file):
 
     return ("<ERROR> _to_execroot_path not yet implemented for " + file.path)
 
+def _strip_external(path):
+    return path[len("external/"):] if path.startswith("external/") or path.startswith("external\\") else path
+
 def _nodejs_binary_impl(ctx):
     node_modules = depset(ctx.files.node_modules)
 
@@ -173,16 +176,7 @@ def _nodejs_binary_impl(ctx):
     if hasattr(ctx.attr, "expected_exit_code"):
         expected_exit_code = ctx.attr.expected_exit_code
 
-    node_tool_info = ctx.toolchains["@build_bazel_rules_nodejs//toolchains/node:toolchain_type"].nodeinfo
-
-    # Make a copy so we don't try to mutate a frozen object
-    node_tool_files = node_tool_info.tool_files[:]
-    if node_tool_info.target_tool_path == "":
-        # If tool_path is empty and tool_target is None then there is no local
-        # node tool, we will just print a nice error message if the user
-        # attempts to do bazel run
-        fail("The node toolchain was not properly configured so %s cannot be executed. Make sure that target_tool_path or target_tool is set." % ctx.attr.name)
-
+    node_tool_files = ctx.files._node[:]
     node_tool_files.append(ctx.file._link_modules_script)
     node_tool_files.append(ctx.file._bazel_require_script)
 
@@ -211,6 +205,8 @@ def _nodejs_binary_impl(ctx):
         # also be sure to include the params file in the program inputs
         node_tool_files.append(ctx.outputs.templated_args_file)
 
+    is_vendored_node = (ctx.attr._node.label.workspace_name != "nodejs_darwin_amd64" and ctx.attr._node.label.workspace_name != "nodenodejs_linux_amd64js_darwin_amd64" and ctx.attr._node.label.workspace_name != "nodejs_windows_amd64")
+
     substitutions = {
         "TEMPLATED_args": " ".join([
             expand_location_into_runfiles(ctx, a)
@@ -221,9 +217,9 @@ def _nodejs_binary_impl(ctx):
         "TEMPLATED_expected_exit_code": str(expected_exit_code),
         "TEMPLATED_link_modules_script": _to_manifest_path(ctx, ctx.file._link_modules_script),
         "TEMPLATED_loader_path": script_path,
-        "TEMPLATED_node": node_tool_info.target_tool_path,
         "TEMPLATED_repository_args": _to_manifest_path(ctx, ctx.file._repository_args),
         "TEMPLATED_script_path": _to_execroot_path(ctx, ctx.file.entry_point),
+        "TEMPLATED_vendored_node": _strip_external(ctx.file._node.path) if is_vendored_node else "",
     }
     ctx.actions.expand_template(
         template = ctx.file._launcher_template,
@@ -466,6 +462,10 @@ jasmine_node_test(
         default = Label("//internal/node:node_loader.js"),
         allow_single_file = True,
     ),
+    "_node": attr.label(
+        default = Label("@nodejs//:node_bin"),
+        allow_single_file = True,
+    ),
     "_repository_args": attr.label(
         default = Label("@nodejs//:bin/node_repo_args.sh"),
         allow_single_file = True,

internal/node/node_launcher.sh

@@ -114,7 +114,32 @@ TEMPLATED_env_vars
 # This redirects to stderr so it doesn't interfere with Bazel's worker protocol
 # find . -name thingImLookingFor 1>&2
 
-readonly node=$(rlocation "TEMPLATED_node")
+readonly vendored_node="TEMPLATED_vendored_node"
+
+if [ -n "$vendored_node" ]; then
+  # Use the vendored node path
+  readonly node=$(rlocation "$vendored_node")
+else
+  # Check environment for which node path to use
+  unameOut="$(uname -s)"
+  case "${unameOut}" in
+      Linux*)     machine=linux;;
+      Darwin*)    machine=darwin;;
+      CYGWIN*)    machine=windows;;
+      MINGW*)     machine=windows;;
+      MSYS_NT*)   machine=windows;;
+      *)          machine=linux;;
+  esac
+
+  case "${machine}" in
+    # The following paths must match up with _download_node in node_repositories
+    linux) readonly node=$(rlocation "nodejs_linux_amd64/bin/nodejs/bin/node");;
+    darwin) readonly node=$(rlocation "nodejs_darwin_amd64/bin/nodejs/bin/node");;
+    windows) readonly node=$(rlocation "nodejs_windows_amd64/bin/nodejs/node.exe");;
+    *) readonly node=$(rlocation "nodejs_linux_amd64/bin/nodejs/bin/node");;
+  esac
+fi
+
 readonly repository_args=$(rlocation "TEMPLATED_repository_args")
 MAIN=$(rlocation "TEMPLATED_loader_path")
 readonly link_modules_script=$(rlocation "TEMPLATED_link_modules_script")

internal/node/node_repositories.bzl

@@ -609,18 +609,30 @@ node_repositories_rule = repository_rule(
 )
 
 def _nodejs_host_os_alias_impl(repository_ctx):
-    host_os = os_name(repository_ctx)
-    node_repository = "@nodejs_%s" % host_os
     is_windows_host = is_windows_os(repository_ctx)
     file_ending = ".cmd" if is_windows_host else ""
-    actual_node_bin = "bin/nodejs/node.exe" if is_windows_host else "bin/nodejs/bin/node"
+    node_repository = "@nodejs_%s" % os_name(repository_ctx)
+    if repository_ctx.attr.vendored_node:
+        node_bin_repository = "@%s" % repository_ctx.attr.vendored_node.workspace_name
+        actual_node_bin = "/".join([f for f in [
+            repository_ctx.attr.vendored_node.package,
+            repository_ctx.attr.vendored_node.name,
+            "bin/node" if not is_windows_host else "node.exe",
+        ] if f])
+    else:
+        node_bin_repository = node_repository
+        actual_node_bin = "%s/%s" % (
+            NODE_DIR,
+            "node.exe" if is_windows_host else "bin/node",
+        )
+    print("%s//:%s" % (node_bin_repository, actual_node_bin))
     repository_ctx.template(
         "BUILD.bazel",
         Label("@build_bazel_rules_nodejs//internal/node:BUILD.nodejs_host_os_alias.tpl"),
         substitutions = {
             "TEMPLATE__npm_node_repositories": "%s//:bin/npm_node_repositories%s" % (node_repository, file_ending),
             "TEMPLATE__yarn_node_repositories": "%s//:bin/yarn_node_repositories%s" % (node_repository, file_ending),
-            "TEMPLATE_actual_node_bin": "%s//:%s" % (node_repository, actual_node_bin),
+            "TEMPLATE_actual_node_bin": "%s//:%s" % (node_bin_repository, actual_node_bin),
             "TEMPLATE_node_repo_args": "%s//:bin/node_repo_args.sh" % node_repository,
             "TEMPLATE_npm": "%s//:bin/npm%s" % (node_repository, file_ending),
             "TEMPLATE_run_npm": "%s//:run_npm.sh.template" % node_repository,
@@ -632,9 +644,12 @@ def _nodejs_host_os_alias_impl(repository_ctx):
 
 _nodejs_repo_host_os_alias = repository_rule(
     _nodejs_host_os_alias_impl,
+    attrs = {
+        "vendored_node": attr.label(allow_single_file = True),
+    },
 )
 
-def node_repositories(package_json = [], **kwargs):
+def node_repositories(**kwargs):
     """
     Wrapper macro around node_repositories_rule to call it for each platform, register bazel toolchains,
     and make other convenience repositories.
@@ -652,14 +667,16 @@ def node_repositories(package_json = [], **kwargs):
         minimum_bazel_version = "0.21.0",
     )
 
+    vendored_node = kwargs.pop("vendored_node", None)
+
     # This needs to be setup so toolchains can access nodejs for all different versions
     for os_arch_name in OS_ARCH_NAMES:
         os_name = "_".join(os_arch_name)
         node_repository_name = "nodejs_%s" % os_name
         _maybe(
             node_repositories_rule,
             name = node_repository_name,
-            package_json = package_json,
+            vendored_node = vendored_node,
             **kwargs
         )
         native.register_toolchains("@build_bazel_rules_nodejs//toolchains/node:node_%s_toolchain" % os_arch_name[0])
@@ -673,6 +690,7 @@ def node_repositories(package_json = [], **kwargs):
     _maybe(
         _nodejs_repo_host_os_alias,
         name = "nodejs",
+        vendored_node = vendored_node,
     )
 
     _maybe(