refactor(builtin): improve node toolchain

- Support the toolchains attribute of rules like genrule, so they can get the NODE_PATH
- Expose files and a path to node, rather than whether the node binary is locally installed or came from Bazel fetching it

Author: alexeagle

internal/node/node.bzl

@@ -119,11 +119,12 @@ def _write_loader_script(ctx):
         is_executable = True,
     )
 
-def _short_path_to_manifest_path(ctx, short_path):
-    if short_path.startswith("../"):
-        return short_path[3:]
+# Avoid writing non-normalized paths (workspace/../other_workspace/path)
+def _to_manifest_path(ctx, file):
+    if file.short_path.startswith("../"):
+        return file.short_path[3:]
     else:
-        return ctx.workspace_name + "/" + short_path
+        return ctx.workspace_name + "/" + file.short_path
 
 def _nodejs_binary_impl(ctx):
     node_modules = depset(ctx.files.node_modules)
@@ -147,14 +148,8 @@ def _nodejs_binary_impl(ctx):
 
     _write_loader_script(ctx)
 
-    # Avoid writing non-normalized paths (workspace/../other_workspace/path)
-    if ctx.outputs.loader.short_path.startswith("../"):
-        script_path = ctx.outputs.loader.short_path[len("../"):]
-    else:
-        script_path = "/".join([
-            ctx.workspace_name,
-            ctx.outputs.loader.short_path,
-        ])
+    script_path = _to_manifest_path(ctx, ctx.outputs.loader)
+
     env_vars = "export BAZEL_TARGET=%s\n" % ctx.label
     for k in ctx.attr.configuration_env_vars + ctx.attr.default_env_vars:
         if k in ctx.var.keys():
@@ -165,61 +160,58 @@ def _nodejs_binary_impl(ctx):
         expected_exit_code = ctx.attr.expected_exit_code
 
     node_tool_info = ctx.toolchains["@build_bazel_rules_nodejs//toolchains/node:toolchain_type"].nodeinfo
-    node_tool_files = []
-    if node_tool_info.target_tool_path == "" and not node_tool_info.target_tool:
+
+    # 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)
-    else:
-        node_tool = node_tool_info.target_tool_path
-        if node_tool_info.target_tool:
-            node_tool_files += node_tool_info.target_tool.files.to_list()
-            node_tool = _short_path_to_manifest_path(ctx, node_tool_files[0].short_path)
-
-        if not ctx.outputs.templated_args_file:
-            templated_args = ctx.attr.templated_args
-        else:
-            # Distribute the templated_args between the params file and the node options
-            params = []
-            templated_args = []
-            for a in ctx.attr.templated_args:
-                if a.startswith("--node_options="):
-                    templated_args.append(a)
-                else:
-                    params.append(a)
-
-            # Put the params into the params file
-            ctx.actions.write(
-                output = ctx.outputs.templated_args_file,
-                content = "\n".join([expand_location_into_runfiles(ctx, p) for p in params]),
-                is_executable = False,
-            )
-
-            # after the node_options args, pass the params file arg
-            templated_args.append(ctx.outputs.templated_args_file.short_path)
-
-            # also be sure to include the params file in the program inputs
-            node_tool_files += [ctx.outputs.templated_args_file]
 
-        substitutions = {
-            "TEMPLATED_args": " ".join([
-                expand_location_into_runfiles(ctx, a)
-                for a in templated_args
-            ]),
-            "TEMPLATED_env_vars": env_vars,
-            "TEMPLATED_expected_exit_code": str(expected_exit_code),
-            "TEMPLATED_node": node_tool,
-            "TEMPLATED_repository_args": _short_path_to_manifest_path(ctx, ctx.file._repository_args.short_path),
-            "TEMPLATED_script_path": script_path,
-        }
-        ctx.actions.expand_template(
-            template = ctx.file._launcher_template,
-            output = ctx.outputs.script,
-            substitutions = substitutions,
-            is_executable = True,
+    if not ctx.outputs.templated_args_file:
+        templated_args = ctx.attr.templated_args
+    else:
+        # Distribute the templated_args between the params file and the node options
+        params = []
+        templated_args = []
+        for a in ctx.attr.templated_args:
+            if a.startswith("--node_options="):
+                templated_args.append(a)
+            else:
+                params.append(a)
+
+        # Put the params into the params file
+        ctx.actions.write(
+            output = ctx.outputs.templated_args_file,
+            content = "\n".join([expand_location_into_runfiles(ctx, p) for p in params]),
+            is_executable = False,
         )
 
+        # after the node_options args, pass the params file arg
+        templated_args.append(ctx.outputs.templated_args_file.short_path)
+
+        # also be sure to include the params file in the program inputs
+        node_tool_files.append(ctx.outputs.templated_args_file)
+
+    substitutions = {
+        "TEMPLATED_args": " ".join([
+            expand_location_into_runfiles(ctx, a)
+            for a in templated_args
+        ]),
+        "TEMPLATED_env_vars": env_vars,
+        "TEMPLATED_expected_exit_code": str(expected_exit_code),
+        "TEMPLATED_node": node_tool_info.target_tool_path,
+        "TEMPLATED_repository_args": _to_manifest_path(ctx, ctx.file._repository_args),
+        "TEMPLATED_script_path": script_path,
+    }
+    ctx.actions.expand_template(
+        template = ctx.file._launcher_template,
+        output = ctx.outputs.script,
+        substitutions = substitutions,
+        is_executable = True,
+    )
+
     runfiles = depset(node_tool_files + [ctx.outputs.loader, ctx.file._repository_args], transitive = [sources, node_modules])
 
     if is_windows(ctx):

toolchains/node/BUILD.bazel

@@ -56,6 +56,33 @@ filegroup(
 # node toolchain type
 toolchain_type(name = "toolchain_type")
 
+# Allow targets to use a toolchains attribute, such as sh_binary and genrule
+# This way they can reference the NODE_PATH make variable.
+alias(
+    name = "toolchain",
+    actual = select({
+        "@bazel_tools//src/conditions:darwin": "@nodejs_darwin_amd64_config//:toolchain",
+        "@bazel_tools//src/conditions:darwin_x86_64": "@nodejs_darwin_amd64_config//:toolchain",
+        "@bazel_tools//src/conditions:linux_x86_64": "@nodejs_linux_amd64_config//:toolchain",
+        "@bazel_tools//src/conditions:windows": "@nodejs_windows_amd64_config//:toolchain",
+        "//conditions:default": "@nodejs_linux_amd64_config//:toolchain",
+    }),
+    visibility = ["//visibility:public"],
+)
+
+# Allow targets to declare a dependency on the node binary for the current host platform
+alias(
+    name = "node_bin",
+    actual = select({
+        "@bazel_tools//src/conditions:darwin": "@nodejs_darwin_amd64//:node_bin",
+        "@bazel_tools//src/conditions:darwin_x86_64": "@nodejs_darwin_amd64//:node_bin",
+        "@bazel_tools//src/conditions:linux_x86_64": "@nodejs_linux_amd64//:node_bin",
+        "@bazel_tools//src/conditions:windows": "@nodejs_windows_amd64//:node_bin",
+        "//conditions:default": "@nodejs_linux_amd64//:node_bin",
+    }),
+    visibility = ["//visibility:public"],
+)
+
 toolchain(
     name = "node_linux_toolchain",
     target_compatible_with = [

toolchains/node/node_toolchain.bzl

@@ -18,35 +18,56 @@ This module implements the node toolchain rule.
 NodeInfo = provider(
     doc = "Information about how to invoke the node executable.",
     fields = {
-        "target_tool": "A hermetically downloaded nodejs executable target for the target platform.",
-        "target_tool_path": "Path to an existing nodejs executable for the target platform..",
+        "target_tool_path": "Path to the nodejs executable for the target platform.",
+        "tool_files": """Files required in runfiles to make the nodejs executable available.
+
+May be empty if the target_tool_path points to a locally installed node binary.""",
     },
 )
 
+def _to_manifest_path(ctx, file):
+    if file.short_path.startswith("../"):
+        return file.short_path[3:]
+    else:
+        return ctx.workspace_name + "/" + file.short_path
+
 def _node_toolchain_impl(ctx):
     if ctx.attr.target_tool and ctx.attr.target_tool_path:
-        fail("Can only set one of target_tool or target_tool_path but both where set.")
+        fail("Can only set one of target_tool or target_tool_path but both were set.")
     if not ctx.attr.target_tool and not ctx.attr.target_tool_path:
         fail("Must set one of target_tool or target_tool_path.")
 
-    toolchain_info = platform_common.ToolchainInfo(
-        nodeinfo = NodeInfo(
-            target_tool_path = ctx.attr.target_tool_path,
-            target_tool = ctx.attr.target_tool,
+    tool_files = []
+    target_tool_path = ctx.attr.target_tool_path
+
+    if ctx.attr.target_tool:
+        tool_files = ctx.attr.target_tool.files.to_list()
+        target_tool_path = _to_manifest_path(ctx, tool_files[0])
+
+    return [
+        platform_common.ToolchainInfo(
+            nodeinfo = NodeInfo(
+                target_tool_path = target_tool_path,
+                tool_files = tool_files,
+            ),
         ),
-    )
-    return [toolchain_info]
+        # Make the $(NODE_PATH) variable available in places like genrules.
+        # See https://docs.bazel.build/versions/master/be/make-variables.html#custom_variables
+        platform_common.TemplateVariableInfo({
+            "NODE_PATH": target_tool_path,
+        }),
+    ]
 
 node_toolchain = rule(
     implementation = _node_toolchain_impl,
     attrs = {
         "target_tool": attr.label(
-            doc = "A hermetically downloaded nodejs executable target for the target platform..",
+            doc = "A hermetically downloaded nodejs executable target for the target platform.",
             mandatory = False,
             allow_single_file = True,
         ),
         "target_tool_path": attr.string(
-            doc = "Path to an existing nodejs executable for the target platform..",
+            doc = "Path to an existing nodejs executable for the target platform.",
             mandatory = False,
         ),
     },