Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add support for capturing and overriding the exit code within run_node #1990

Merged
merged 1 commit into from
Jul 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion internal/node/launcher.sh
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ USER_NODE_OPTIONS=()
ALL_ARGS=(TEMPLATED_args $NODE_REPOSITORY_ARGS "$@")
STDOUT_CAPTURE=""
STDERR_CAPTURE=""
EXIT_CODE_CAPTURE=""

RUN_LINKER=true
NODE_PATCHES=true
Expand All @@ -179,8 +180,12 @@ for ARG in "${ALL_ARGS[@]:-}"; do
case "$ARG" in
# Supply custom linker arguments for first-party dependencies
--bazel_node_modules_manifest=*) MODULES_MANIFEST="${ARG#--bazel_node_modules_manifest=}" ;;
# Captures stdout of the node process to the file specified
--bazel_capture_stdout=*) STDOUT_CAPTURE="${ARG#--bazel_capture_stdout=}" ;;
# Captures stderr of the node process to the file specified
--bazel_capture_stderr=*) STDERR_CAPTURE="${ARG#--bazel_capture_stderr=}" ;;
# Captures the exit code of the node process to the file specified
--bazel_capture_exit_code=*) EXIT_CODE_CAPTURE="${ARG#--bazel_capture_exit_code=}" ;;
# Disable the node_loader.js monkey patches for require()
# Note that this means you need an explicit runfiles helper library
--nobazel_patch_module_resolver) PATCH_REQUIRE=false ;;
Expand Down Expand Up @@ -327,6 +332,10 @@ wait "${child}"
RESULT="$?"
set -e

if [[ -n "${EXIT_CODE_CAPTURE}" ]]; then
echo "${RESULT}" > "${EXIT_CODE_CAPTURE}"
fi

if [ "${EXPECTED_EXIT_CODE}" != "0" ]; then
if [ ${RESULT} != ${EXPECTED_EXIT_CODE} ]; then
echo "Expected exit code to be ${EXPECTED_EXIT_CODE}, but got ${RESULT}" >&2
Expand Down Expand Up @@ -360,4 +369,8 @@ if [[ -n "${COVERAGE_DIR:-}" ]]; then
fi
fi

exit ${RESULT}
if [[ -n "${EXIT_CODE_CAPTURE}" ]]; then
exit 0
else
exit ${RESULT}
fi
8 changes: 8 additions & 0 deletions internal/node/npm_package_bin.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ _ATTRS = {
"args": attr.string_list(mandatory = True),
"configuration_env_vars": attr.string_list(default = []),
"data": attr.label_list(allow_files = True, aspects = [module_mappings_aspect, node_modules_aspect]),
"exit_code_out": attr.output(),
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exit_code on it's own seemed not descriptive enough, but can change these attr names to something better

"output_dir": attr.bool(),
"outs": attr.output_list(),
"stderr": attr.output(),
Expand Down Expand Up @@ -64,6 +65,9 @@ def _impl(ctx):
if ctx.outputs.stderr:
tool_outputs.append(ctx.outputs.stderr)

if ctx.outputs.exit_code_out:
tool_outputs.append(ctx.outputs.exit_code_out)

run_node(
ctx,
executable = "tool",
Expand All @@ -73,6 +77,7 @@ def _impl(ctx):
configuration_env_vars = ctx.attr.configuration_env_vars,
stdout = ctx.outputs.stdout,
stderr = ctx.outputs.stderr,
exit_code_out = ctx.outputs.exit_code_out,
)

return [DefaultInfo(files = depset(outputs + tool_outputs))]
Expand Down Expand Up @@ -104,6 +109,9 @@ def npm_package_bin(tool = None, package = None, package_bin = None, data = [],
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`
exit_code_out: set to capture the exit code of the binary to a file, which can later be used as an input to another target
subject to the same semantics as `outs`. Note that setting this will force the binary to exit 0.
If the binary creates outputs and these are declared, they must still be created

args: Command-line arguments to the tool.

Expand Down
8 changes: 8 additions & 0 deletions internal/node/test/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,7 @@ generated_file_test(

# 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)
# the exit code is also capture to a file with exit_code_out, allowing downstream actions to read it
npm_package_bin(
name = "diagnostics_producing_bin",
args = [
Expand All @@ -269,6 +270,7 @@ npm_package_bin(
"--warn",
],
data = ["terser_input_with_diagnostics.js"],
exit_code_out = "exit.code",
package = "terser",
stderr = "diagnostics.out",
stdout = "greeter.min.js",
Expand Down Expand Up @@ -299,6 +301,12 @@ generated_file_test(
generated = ":diagnostics.scrubbed.out",
)

generated_file_test(
name = "exit_code_test",
src = "exit_code_test.golden",
generated = ":exit.code",
)

nodejs_binary(
name = "copy_to_directory",
entry_point = "copy_to_directory.js",
Expand Down
1 change: 1 addition & 0 deletions internal/node/test/exit_code_test.golden
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
0
6 changes: 6 additions & 0 deletions internal/providers/node_runtime_deps_info.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,12 @@ def run_node(ctx, inputs, arguments, executable, **kwargs):
add_arg(arguments, "--bazel_capture_stderr=%s" % stderr_file.path)
outputs = outputs + [stderr_file]

exit_code_file = kwargs.pop("exit_code_out", None)
if exit_code_file:
# this will force the script to exit 0, all declared outputs must still be created
add_arg(arguments, "--bazel_capture_exit_code=%s" % exit_code_file.path)
outputs = outputs + [exit_code_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 Down