Skip to content

Commit

Permalink
feat: add stdout capture to npm_package_bin
Browse files Browse the repository at this point in the history
  • Loading branch information
mattem authored and alexeagle committed Jun 16, 2020
1 parent 080b460 commit 3f182f0
Show file tree
Hide file tree
Showing 8 changed files with 113 additions and 5 deletions.
17 changes: 16 additions & 1 deletion internal/node/launcher.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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" )
Expand Down Expand Up @@ -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
Expand Down
23 changes: 20 additions & 3 deletions internal/node/npm_package_bin.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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:
Expand All @@ -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,
Expand All @@ -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.
Expand Down
48 changes: 48 additions & 0 deletions internal/node/test/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions internal/node/test/stderr_output_test.golden
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
WARN: Dropping unreachable code [internal/node/test/terser_input_with_diagnostics.js:6,4]
1 change: 1 addition & 0 deletions internal/node/test/stdout_output_test.golden
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
class A{doThing(){console.error("thing")}}
6 changes: 6 additions & 0 deletions internal/node/test/terser_diagnostics_stats.js
Original file line number Diff line number Diff line change
@@ -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' : ''}`);
8 changes: 8 additions & 0 deletions internal/node/test/terser_input_with_diagnostics.js
Original file line number Diff line number Diff line change
@@ -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}`);
}
}
14 changes: 13 additions & 1 deletion internal/providers/node_runtime_deps_info.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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
Expand All @@ -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).
Expand All @@ -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,
Expand Down

0 comments on commit 3f182f0

Please sign in to comment.