Skip to content

Commit

Permalink
fix: in use_exit_code mode, print reports to stdio (#63)
Browse files Browse the repository at this point in the history
Previously we always directed the tools output into a report file, but when you ask for the exit code to be used, this means the Bazel build fails without printing the result of the tool.
Instead, all the logic related to directing output into the report file should only be used when exit code is ignored.
  • Loading branch information
alexeagle authored Dec 1, 2023
1 parent 8ef674a commit 918fab8
Show file tree
Hide file tree
Showing 7 changed files with 100 additions and 60 deletions.
3 changes: 2 additions & 1 deletion example/lint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ buildevents=$(mktemp)
filter='.namedSetOfFiles | values | .files[] | ((.pathPrefix | join("/")) + "/" + .name)'

# Produce report files
# You can add --aspects_parameters=fail_on_violation=true to make this command fail instead.
# To make the command fail when there's a lint warning, you can add arguments:
# --aspects_parameters=fail_on_violation=true --keep_going
# NB: perhaps --remote_download_toplevel is needed as well with remote execution?
bazel build \
--aspects $(echo //tools:lint.bzl%{buf,eslint,flake8,pmd,ruff,shellcheck} | tr ' ' ',') \
Expand Down
10 changes: 6 additions & 4 deletions lint/buf.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -57,19 +57,21 @@ def buf_lint_action(ctx, buf_toolchain, target, report, use_exit_code = False):
args.add_joined(["--buf-plugin_out", "."], join_with = "=")
args.add_all(sources)

if use_exit_code:
command = "{protoc} $@ && touch {report}"
else:
command = "{protoc} $@ 2>{report} || true"

ctx.actions.run_shell(
inputs = depset([
ctx.file._config,
ctx.executable._protoc,
buf_toolchain.cli,
], transitive = [deps]),
outputs = [report],
command = """\
{protoc} $@ 2>{report} {exit_zero}
""".format(
command = command.format(
protoc = ctx.executable._protoc.path,
report = report.path,
exit_zero = "" if use_exit_code else "|| true",
),
arguments = [args],
mnemonic = _MNEMONIC,
Expand Down
41 changes: 24 additions & 17 deletions lint/eslint.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,6 @@ def eslint_action(ctx, executable, srcs, report, use_exit_code = False):

args = ctx.actions.args()

# Workaround: create an empty report file in case eslint doesn't write one
# Use `../../..` to return to the execroot?
args.add_joined(["--node_options", "--require", "../../../" + ctx.file._workaround_17660.path], join_with = "=")

# require explicit path to the eslintrc file, don't search for one
args.add("--no-eslintrc")

Expand All @@ -46,7 +42,6 @@ def eslint_action(ctx, executable, srcs, report, use_exit_code = False):

args.add_all(["--config", ctx.file._config_file.short_path])
args.add_all(["--format", "../../../" + ctx.file._formatter.path])
args.add_all(["--output-file", report.short_path])
args.add_all([s.short_path for s in srcs])

env = {"BAZEL_BINDIR": ctx.bin_dir.path}
Expand All @@ -61,21 +56,33 @@ def eslint_action(ctx, executable, srcs, report, use_exit_code = False):
include_npm_linked_packages = True,
).to_list())

outputs = [report]

if not use_exit_code:
if use_exit_code:
ctx.actions.run_shell(
inputs = inputs,
outputs = [report],
tools = [executable._eslint],
arguments = [args],
command = executable._eslint.path + " $@ && touch " + report.path,
env = env,
mnemonic = _MNEMONIC,
)
else:
# Workaround: create an empty report file in case eslint doesn't write one
# Use `../../..` to return to the execroot?
args.add_joined(["--node_options", "--require", "../../../" + ctx.file._workaround_17660.path], join_with = "=")

args.add_all(["--output-file", report.short_path])
exit_code_out = ctx.actions.declare_file("_{}.exit_code_out".format(ctx.label.name))
outputs.append(exit_code_out)
env["JS_BINARY__EXIT_CODE_OUTPUT_FILE"] = exit_code_out.path

ctx.actions.run(
inputs = inputs,
outputs = outputs,
executable = executable._eslint,
arguments = [args],
env = env,
mnemonic = _MNEMONIC,
)
ctx.actions.run(
inputs = inputs,
outputs = [report, exit_code_out],
executable = executable._eslint,
arguments = [args],
env = env,
mnemonic = _MNEMONIC,
)

# buildifier: disable=function-docstring
def _eslint_aspect_impl(target, ctx):
Expand Down
27 changes: 18 additions & 9 deletions lint/flake8.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -36,18 +36,27 @@ def flake8_action(ctx, executable, srcs, config, report, use_exit_code = False):
# https://flake8.pycqa.org/en/latest/user/options.html
args = ctx.actions.args()
args.add_all(srcs)
args.add(report, format = "--output-file=%s")
args.add(config, format = "--config=%s")
if not use_exit_code:
if use_exit_code:
ctx.actions.run_shell(
inputs = inputs,
outputs = outputs,
tools = [executable],
command = executable.path + " $@ && touch " + report.path,
arguments = [args],
mnemonic = _MNEMONIC,
)
else:
args.add(report, format = "--output-file=%s")
args.add("--exit-zero")

ctx.actions.run(
inputs = inputs,
outputs = outputs,
executable = executable,
arguments = [args],
mnemonic = _MNEMONIC,
)
ctx.actions.run(
inputs = inputs,
outputs = outputs,
executable = executable,
arguments = [args],
mnemonic = _MNEMONIC,
)

# buildifier: disable=function-docstring
def _flake8_aspect_impl(target, ctx):
Expand Down
32 changes: 21 additions & 11 deletions lint/pmd.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -34,24 +34,34 @@ def pmd_action(ctx, executable, srcs, rulesets, report, use_exit_code = False):
# Wire command-line options, see
# https://docs.pmd-code.org/latest/pmd_userdocs_cli_reference.html
args = ctx.actions.args()
args.add_all(["--report-file", report])
args.add("--rulesets")
args.add_joined(rulesets, join_with = ",")
if not use_exit_code:
# NB: this arg changes in PMD 7
args.add_all(["--fail-on-violation", "false"])

src_args = ctx.actions.args()
src_args.use_param_file("%s", use_always = True)
src_args.add_all(srcs)

ctx.actions.run(
inputs = inputs,
outputs = [report],
executable = executable,
arguments = [args, "--file-list", src_args],
mnemonic = _MNEMONIC,
)
if use_exit_code:
ctx.actions.run_shell(
inputs = inputs,
outputs = [report],
command = executable.path + " $@ && touch " + report.path,
arguments = [args, "--file-list", src_args],
mnemonic = _MNEMONIC,
tools = [executable],
)
else:
args.add_all(["--report-file", report])

# NB: this arg changes in PMD 7
args.add_all(["--fail-on-violation", "false"])
ctx.actions.run(
inputs = inputs,
outputs = [report],
executable = executable,
arguments = [args, "--file-list", src_args],
mnemonic = _MNEMONIC,
)

# buildifier: disable=function-docstring
def _pmd_aspect_impl(target, ctx):
Expand Down
31 changes: 20 additions & 11 deletions lint/ruff.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -50,19 +50,28 @@ def ruff_action(ctx, executable, srcs, config, report, use_exit_code = False):
# `ruff help check` to see available options
args = ctx.actions.args()
args.add("check")
args.add(report, format = "--output-file=%s")
if not use_exit_code:
args.add("--exit-zero")

args.add_all(srcs)

ctx.actions.run(
inputs = inputs,
outputs = outputs,
executable = executable,
arguments = [args],
mnemonic = _MNEMONIC,
)
if use_exit_code:
ctx.actions.run_shell(
inputs = inputs,
outputs = outputs,
command = executable.path + " $@ && touch " + report.path,
arguments = [args],
mnemonic = _MNEMONIC,
tools = [executable],
)
else:
args.add(report, format = "--output-file=%s")
args.add("--exit-zero")

ctx.actions.run(
inputs = inputs,
outputs = outputs,
executable = executable,
arguments = [args],
mnemonic = _MNEMONIC,
)

# buildifier: disable=function-docstring
def _ruff_aspect_impl(target, ctx):
Expand Down
16 changes: 9 additions & 7 deletions lint/shellcheck.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -53,25 +53,27 @@ def shellcheck_action(ctx, executable, srcs, config, report, use_exit_code = Fal
use_exit_code: whether to fail the build when a lint violation is reported
"""
inputs = srcs + [config]
outputs = [report]

# Wire command-line options, see
# https://github.com/koalaman/shellcheck/blob/master/shellcheck.1.md#options
args = ctx.actions.args()
args.add_all(srcs)

if use_exit_code:
command = "{shellcheck} $@ && touch {report}"
else:
command = "{shellcheck} $@ >{report} || true"

ctx.actions.run_shell(
inputs = inputs + [executable],
outputs = outputs,
command = """\
{shellcheck} $@ >{report} {exit_zero}
""".format(
inputs = inputs,
outputs = [report],
command = command.format(
shellcheck = executable.path,
report = report.path,
exit_zero = "" if use_exit_code else "|| true",
),
arguments = [args],
mnemonic = _MNEMONIC,
tools = [executable],
)

# buildifier: disable=function-docstring
Expand Down

0 comments on commit 918fab8

Please sign in to comment.