fix: allow multiple run_node calls to be made from the same rule context

Author: mattem

internal/linker/link_node_modules.bzl

@@ -56,12 +56,13 @@ def _link_mapping(label, mappings, k, v):
     else:
         return True
 
-def write_node_modules_manifest(ctx, extra_data = []):
+def write_node_modules_manifest(ctx, extra_data = [], mnemonic = None):
     """Writes a manifest file read by the linker, containing info about resolving runtime dependencies
 
     Args:
         ctx: starlark rule execution context
         extra_data: labels to search for npm packages that need to be linked (ctx.attr.deps and ctx.attr.data will always be searched)
+        mnemonic: optional action mnemonic, used to differentiate module mapping files from the same rule context
     """
 
     mappings = {}
@@ -81,7 +82,7 @@ def write_node_modules_manifest(ctx, extra_data = []):
         for k, v in getattr(dep, _ASPECT_RESULT_NAME, {}).items():
             if _link_mapping(dep.label, mappings, k, v):
                 # Special case for ts_library module_name for legacy behavior and for AMD name
-                # work-around. Do not propogate tslibrary root type to runtime as it is not
+                # work-around. Do not propagate tslibrary root type to runtime as it is not
                 # needed at runtime. See comment below in _get_module_mappings for more info.
                 if v[0] == "_tslibrary":
                     v = ["execroot", v[1]]
@@ -90,7 +91,10 @@ def write_node_modules_manifest(ctx, extra_data = []):
 
     # Write the result to a file, and use the magic node option --bazel_node_modules_manifest
     # The launcher.sh will peel off this argument and pass it to the linker rather than the program.
-    modules_manifest = ctx.actions.declare_file("_%s.module_mappings.json" % ctx.label.name)
+    prefix = ctx.label.name
+    if mnemonic != None:
+        prefix += "_%s" % mnemonic
+    modules_manifest = ctx.actions.declare_file("_%s.module_mappings.json" % prefix)
     content = {
         "bin": ctx.bin_dir.path,
         "modules": mappings,

internal/providers/node_runtime_deps_info.bzl

@@ -73,7 +73,8 @@ def run_node(ctx, inputs, arguments, executable, **kwargs):
         extra_inputs = exec_attr[NodeRuntimeDepsInfo].deps.to_list()
         link_data = exec_attr[NodeRuntimeDepsInfo].pkgs
 
-    modules_manifest = write_node_modules_manifest(ctx, link_data)
+    mnemonic = kwargs.get("mnemonic")
+    modules_manifest = write_node_modules_manifest(ctx, link_data, mnemonic)
     add_arg(arguments, "--bazel_node_modules_manifest=%s" % modules_manifest.path)
 
     stdout_file = kwargs.pop("stdout", None)

internal/providers/test/BUILD.bazel

@@ -0,0 +1,36 @@
+load("@build_bazel_rules_nodejs//:index.bzl", "generated_file_test", "nodejs_binary")
+load("//internal/providers/test:run_node_test.bzl", "js_write_file")
+
+# two nodejs_binary targets provide two separate executables in the run_node test
+nodejs_binary(
+    name = "writer_bin",
+    entry_point = "js-write-file.js",
+)
+
+nodejs_binary(
+    name = "writer_bin2",
+    data = [
+        # the JS files doesn't actually consume this dependency, but it causes a different modules
+        # manifest file to be written in for both run_node actions in the js_write_file rule below
+        "@npm//shelljs",
+    ],
+    entry_point = "js-write-file.js",
+)
+
+js_write_file(
+    name = "write_file",
+    content = "test file content",
+)
+
+# tests that the rule above generated two files for each run_node action
+[
+    generated_file_test(
+        name = "run_node_test_%s" % file,
+        src = "_run_node_out.txt",
+        generated = "%s.txt" % file,
+    )
+    for file in [
+        "out",
+        "out2",
+    ]
+]

internal/providers/test/_run_node_out.txt

@@ -0,0 +1 @@
+test file content

internal/providers/test/js-write-file.js

@@ -0,0 +1,6 @@
+const writeFileSync =  require('fs').writeFileSync;
+
+const content = process.argv[2];
+const outputPath = process.argv[3];
+
+writeFileSync(outputPath, content, {encoding: 'utf8'});

internal/providers/test/run_node_test.bzl

@@ -0,0 +1,53 @@
+"""
+Contrived rule for writing some content to a file using run_node
+Tests that two run_node calls can be made from the same rule context, where each run_node call
+uses a different executable (and therefore the content of the modules manifest file is different)
+"""
+
+load("//:providers.bzl", "run_node")
+
+def _js_write_file_impl(ctx):
+    run_node(
+        ctx = ctx,
+        executable = "_writer",
+        mnemonic = "writer",
+        inputs = [],
+        arguments = [
+            ctx.attr.content,
+            ctx.outputs.out.path,
+        ],
+        outputs = [ctx.outputs.out],
+    )
+
+    run_node(
+        ctx = ctx,
+        executable = "_writer2",
+        # mnemonic is left as None here to test the node_modules manifest writer handling None
+        inputs = [],
+        arguments = [
+            ctx.attr.content,
+            ctx.outputs.out2.path,
+        ],
+        outputs = [ctx.outputs.out2],
+    )
+
+js_write_file = rule(
+    implementation = _js_write_file_impl,
+    outputs = {
+        "out": "out.txt",
+        "out2": "out2.txt",
+    },
+    attrs = {
+        "content": attr.string(),
+        "_writer": attr.label(
+            default = Label("//internal/providers/test:writer_bin"),
+            cfg = "host",
+            executable = True,
+        ),
+        "_writer2": attr.label(
+            default = Label("//internal/providers/test:writer_bin2"),
+            cfg = "host",
+            executable = True,
+        ),
+    },
+)