feat: add stdout capture to npm_package_bin

Author: mattem

internal/node/launcher.sh

@@ -194,9 +194,14 @@ ARGS=()
 LAUNCHER_NODE_OPTIONS=( "--require" "$require_patch_script" )
 USER_NODE_OPTIONS=()
 ALL_ARGS=(TEMPLATED_args $NODE_REPOSITORY_ARGS "$@")
+STDOUT_CAPTURE=""
+STDERR_CAPTURE=""
+
 for ARG in "${ALL_ARGS[@]:-}"; do
   case "$ARG" in
     --bazel_node_modules_manifest=*) MODULES_MANIFEST="${ARG#--bazel_node_modules_manifest=}" ;;
+    --bazel_capture_stdout=*) STDOUT_CAPTURE="${ARG#--bazel_capture_stdout=}" ;;
+    --bazel_capture_stderr=*) STDERR_CAPTURE="${ARG#--bazel_capture_stderr=}" ;;
     --nobazel_patch_module_resolver)
       MAIN=TEMPLATED_entry_point_execroot_path
       LAUNCHER_NODE_OPTIONS=( "--require" "$node_patches_script" )
@@ -283,7 +288,17 @@ _int() {
 
 # Execute the main program
 set +e
-"${node}" "${LAUNCHER_NODE_OPTIONS[@]:-}" "${USER_NODE_OPTIONS[@]:-}" "${MAIN}" ${ARGS[@]+"${ARGS[@]}"} <&0 &
+
+if [[ -n "${STDOUT_CAPTURE}" ]] && [[ -n "${STDERR_CAPTURE}" ]]; then
+  "${node}" "${LAUNCHER_NODE_OPTIONS[@]:-}" "${USER_NODE_OPTIONS[@]:-}" "${MAIN}" ${ARGS[@]+"${ARGS[@]}"} <&0 >$STDOUT_CAPTURE 2>$STDERR_CAPTURE &
+elif [[ -n "${STDOUT_CAPTURE}" ]]; then
+  "${node}" "${LAUNCHER_NODE_OPTIONS[@]:-}" "${USER_NODE_OPTIONS[@]:-}" "${MAIN}" ${ARGS[@]+"${ARGS[@]}"} <&0 >$STDOUT_CAPTURE &
+elif [[ -n "${STDERR_CAPTURE}" ]]; then
+  "${node}" "${LAUNCHER_NODE_OPTIONS[@]:-}" "${USER_NODE_OPTIONS[@]:-}" "${MAIN}" ${ARGS[@]+"${ARGS[@]}"} <&0 2>$STDERR_CAPTURE &
+else
+  "${node}" "${LAUNCHER_NODE_OPTIONS[@]:-}" "${USER_NODE_OPTIONS[@]:-}" "${MAIN}" ${ARGS[@]+"${ARGS[@]}"} <&0 &
+fi
+
 readonly child=$!
 trap _term SIGTERM
 trap _int SIGINT

internal/node/npm_package_bin.bzl

@@ -12,6 +12,8 @@ _ATTRS = {
     "data": attr.label_list(allow_files = True, aspects = [module_mappings_aspect, node_modules_aspect]),
     "output_dir": attr.bool(),
     "outs": attr.output_list(),
+    "stderr": attr.output(),
+    "stdout": attr.output(),
     "tool": attr.label(
         executable = True,
         cfg = "host",
@@ -38,12 +40,13 @@ def _inputs(ctx):
 def _impl(ctx):
     if ctx.attr.output_dir and ctx.outputs.outs:
         fail("Only one of output_dir and outs may be specified")
-    if not ctx.attr.output_dir and not ctx.outputs.outs:
-        fail("One of output_dir and outs must be specified")
+    if not ctx.attr.output_dir and not ctx.outputs.outs and not ctx.attr.stdout:
+        fail("One of output_dir, outs or stdout must be specified")
 
     args = ctx.actions.args()
     inputs = _inputs(ctx)
     outputs = []
+
     if ctx.attr.output_dir:
         outputs = [ctx.actions.declare_directory(ctx.attr.name)]
     else:
@@ -52,15 +55,25 @@ def _impl(ctx):
     for a in ctx.attr.args:
         args.add_all([expand_variables(ctx, e, outs = ctx.outputs.outs, output_dir = ctx.attr.output_dir) for e in _expand_locations(ctx, a)])
 
+    tool_outputs = []
+    if ctx.outputs.stdout:
+        tool_outputs.append(ctx.outputs.stdout)
+
+    if ctx.outputs.stderr:
+        tool_outputs.append(ctx.outputs.stderr)
+
     run_node(
         ctx,
         executable = "tool",
         inputs = inputs,
         outputs = outputs,
         arguments = [args],
         configuration_env_vars = ctx.attr.configuration_env_vars,
+        stdout = ctx.outputs.stdout,
+        stderr = ctx.outputs.stderr,
     )
-    return [DefaultInfo(files = depset(outputs))]
+
+    return [DefaultInfo(files = depset(outputs + tool_outputs))]
 
 _npm_package_bin = rule(
     _impl,
@@ -85,6 +98,10 @@ def npm_package_bin(tool = None, package = None, package_bin = None, data = [],
         output_dir: set to True if you want the output to be a directory
                  Exactly one of `outs`, `output_dir` may be used.
                  If you output a directory, there can only be one output, which will be a directory named the same as the target.
+        stderr: set to capture the stderr of the binary to a file, which can later be used as an input to another target
+                subject to the same semantics as `outs`
+        stdout: set to capture the stdout of the binary to a file, which can later be used as an input to another target
+                subject to the same semantics as `outs`
 
         args: Command-line arguments to the tool.
 

internal/node/test/BUILD.bazel

@@ -241,6 +241,54 @@ npm_package_bin(
     package = "terser",
 )
 
+# captures stdout of terser_input.js to minified.stdout.js (note args do not include --output flag)
+npm_package_bin(
+    name = "run_terser_stdout",
+    args = [
+        "$(execpath terser_input.js)",
+    ],
+    data = ["terser_input.js"],
+    package = "terser",
+    stdout = "minified.stdout.js",
+)
+
+generated_file_test(
+    name = "stdout_output_test",
+    src = "stdout_output_test.golden",
+    generated = ":minified.stdout.js",
+)
+
+# capture stderr to diagnostics.out, then analyze it in terser_diagnostics_stats printing some stats
+# stdout is captured into greeter.min.js (again, without the --output $@ flags)
+npm_package_bin(
+    name = "diagnostics_producing_bin",
+    args = [
+        "$(execpath terser_input_with_diagnostics.js)",
+        "--compress",
+        "--verbose",
+        "--warn",
+    ],
+    data = ["terser_input_with_diagnostics.js"],
+    package = "terser",
+    stderr = "diagnostics.out",
+    stdout = "greeter.min.js",
+)
+
+nodejs_binary(
+    name = "terser_diagnostics_stats",
+    data = [
+        "diagnostics_producing_bin",
+        "terser_diagnostics_stats.js",
+    ],
+    entry_point = "terser_diagnostics_stats.js",
+)
+
+generated_file_test(
+    name = "stderr_output_test",
+    src = "stderr_output_test.golden",
+    generated = ":diagnostics.out",
+)
+
 nodejs_binary(
     name = "copy_to_directory",
     entry_point = "copy_to_directory.js",

internal/node/test/stderr_output_test.golden

@@ -0,0 +1 @@
+WARN: Dropping unreachable code [internal/node/test/terser_input_with_diagnostics.js:6,4]

internal/node/test/stdout_output_test.golden

@@ -0,0 +1 @@
+class A{doThing(){console.error("thing")}}

internal/node/test/terser_diagnostics_stats.js

@@ -0,0 +1,6 @@
+const fs = require('fs');
+
+const diagnostics = fs.readFileSync('internal/node/test/diagnostics.out', {encoding: 'utf8'});
+const count = diagnostics.trim().split('\n').length;
+
+console.log(`Terser produced ${count} diagnostic${count > 1 ? 's' : ''}`);

internal/node/test/terser_input_with_diagnostics.js

@@ -0,0 +1,8 @@
+class Greeter {
+  greet(name) {
+    return name;
+    // return statement above is intentional to force terser to produce a diagnostic
+    // about dropping now unreachable code below
+    console.log(`Hello ${name}`);
+  }
+}

internal/providers/node_runtime_deps_info.bzl

@@ -66,6 +66,7 @@ def run_node(ctx, inputs, arguments, executable, **kwargs):
     exec_attr = getattr(ctx.attr, executable)
     exec_exec = getattr(ctx.executable, executable)
 
+    outputs = kwargs.pop("outputs", [])
     extra_inputs = []
     link_data = []
     if (NodeRuntimeDepsInfo in exec_attr):
@@ -75,6 +76,16 @@ def run_node(ctx, inputs, arguments, executable, **kwargs):
     modules_manifest = write_node_modules_manifest(ctx, link_data)
     add_arg(arguments, "--bazel_node_modules_manifest=%s" % modules_manifest.path)
 
+    stdout_file = kwargs.pop("stdout", None)
+    if stdout_file:
+        add_arg(arguments, "--bazel_capture_stdout=%s" % stdout_file.path)
+        outputs = outputs + [stdout_file]
+
+    stderr_file = kwargs.pop("stderr", None)
+    if stderr_file:
+        add_arg(arguments, "--bazel_capture_stderr=%s" % stderr_file.path)
+        outputs = outputs + [stderr_file]
+
     # By using the run_node helper, you suggest that your program
     # doesn't implicitly use runfiles to require() things
     # To access runfiles, you must use a runfiles helper in the program instead
@@ -86,7 +97,7 @@ def run_node(ctx, inputs, arguments, executable, **kwargs):
     configuration_env_vars = kwargs.pop("configuration_env_vars", []) + ["COMPILATION_MODE"]
     for var in configuration_env_vars:
         if var not in env.keys():
-            # If env is not explicitely specified, check ctx.var first & if env var not in there
+            # If env is not explicitly specified, check ctx.var first & if env var not in there
             # then check ctx.configuration.default_shell_env. The former will contain values from
             # --define=FOO=BAR and latter will contain values from --action_env=FOO=BAR
             # (but not from --action_env=FOO).
@@ -97,6 +108,7 @@ def run_node(ctx, inputs, arguments, executable, **kwargs):
     env["BAZEL_NODE_MODULES_ROOT"] = _compute_node_modules_root(ctx)
 
     ctx.actions.run(
+        outputs = outputs,
         inputs = inputs + extra_inputs + [modules_manifest],
         arguments = arguments,
         executable = exec_exec,