fix(lint): check the syntax of every shell source #69

Merged
claudiomaradonna merged 3 commits from fix/lint-checks into master 2026-11-11 15:03:05 +01:00
Collaborator

Why

make lint is the check every contributor and agent runs before each
commit: a Bash syntax check of the project's 53 shell sources, then
ShellCheck where it is installed. On a host without ShellCheck it
passed a source with a syntax error, because the syntax check read
bin/azkaban alone: bash -n FILE FILE... parses its first file and
takes the others as that script's arguments. ShellCheck reports the
same error where it runs, which kept the defect out of sight in the
project's images.

The ShellCheck pass was also the slowest step of every commit: one
process for all the sources, about 11 s on a 20-CPU host, against 6 s
for the whole test suite.

Set aside: dropping bash -n and relying on ShellCheck, since the
recipe skips ShellCheck where it is missing; and keeping ShellCheck's
default multi-line reports in the parallel run, which interleave on a
terminal when several files have findings.

What changed

Before, a syntax error in any source but the first passed the lint on a
host without ShellCheck. Now the lint parses every source, reports each
error with its file and line, and fails.

Before, ShellCheck checked the sources one after another. Now one
process per CPU checks them, with the same findings and exit status in
about a third of the time, and each finding is one line in ShellCheck's
gcc format, file:line:column: note: text [SC2086]. The snippet and
the suggested fix are a shellcheck <file> away.

The change is the lint recipe of the Makefile and two tests in
tests/unit/test_containerfile.sh. Not in this change: the Verify
command still runs ShellCheck a second time through
.agents/hooks/shellcheck.sh repo, which comes from Agent Foundry.

How to verify

Have Git, GNU Make, Bash 4.4 or newer, and ShellCheck, on Linux or
macOS. No container engine is needed.

The syntax check, on a PATH without ShellCheck:

git clone https://git.unitoo.it/claudiomaradonna/azkaban.git
cd azkaban
scratch="$(mktemp -d)"
mkdir "$scratch/bin"
for tool in make bash sh; do ln -s "$(command -v "$tool")" "$scratch/bin/$tool"; done
printf 'true\n' >"$scratch/good.sh"
printf 'if then fi (\n' >"$scratch/broken.sh"
PATH="$scratch/bin" make lint SHELL_SOURCES="$scratch/good.sh $scratch/broken.sh"; echo "exit $?"

On master it prints shellcheck not installed; skipped and exit 0.
After git switch fix/lint-checks the same last line prints
broken.sh: line 1: syntax error near unexpected token `then' and
exit 2.

The parallel pass, with ShellCheck on the PATH:

time make lint
printf '#!/usr/bin/env bash\necho $1\n' >"$scratch/finding.sh"
make lint SHELL_SOURCES="$scratch/finding.sh"; echo "exit $?"

On a 20-CPU host time make lint took 11.3 s on master and 3.5 s on
the branch. The second command prints, on the branch,
finding.sh:2:6: note: Double quote to prevent globbing and word splitting. [SC2086]
and exit 2.

Checks run on the tip: make lint && .agents/hooks/shellcheck.sh repo && make test
passes, 354 tests. The new test
test_make_lint_fails_on_a_syntax_error_in_any_source_without_shellcheck
fails on master; the pin
test_make_lint_fails_on_a_shellcheck_finding_in_any_source passes on
both. Not tried: BSD xargs on macOS, which documents the -P the
recipe uses.

Considerations

A passing make lint now prints nothing, where it echoed the bash -n
command. xargs -P is outside POSIX; GNU and BSD xargs both take it.

## Why `make lint` is the check every contributor and agent runs before each commit: a Bash syntax check of the project's 53 shell sources, then ShellCheck where it is installed. On a host without ShellCheck it passed a source with a syntax error, because the syntax check read `bin/azkaban` alone: `bash -n FILE FILE...` parses its first file and takes the others as that script's arguments. ShellCheck reports the same error where it runs, which kept the defect out of sight in the project's images. The ShellCheck pass was also the slowest step of every commit: one process for all the sources, about 11 s on a 20-CPU host, against 6 s for the whole test suite. Set aside: dropping `bash -n` and relying on ShellCheck, since the recipe skips ShellCheck where it is missing; and keeping ShellCheck's default multi-line reports in the parallel run, which interleave on a terminal when several files have findings. ## What changed Before, a syntax error in any source but the first passed the lint on a host without ShellCheck. Now the lint parses every source, reports each error with its file and line, and fails. Before, ShellCheck checked the sources one after another. Now one process per CPU checks them, with the same findings and exit status in about a third of the time, and each finding is one line in ShellCheck's gcc format, `file:line:column: note: text [SC2086]`. The snippet and the suggested fix are a `shellcheck <file>` away. The change is the `lint` recipe of the `Makefile` and two tests in `tests/unit/test_containerfile.sh`. Not in this change: the Verify command still runs ShellCheck a second time through `.agents/hooks/shellcheck.sh repo`, which comes from Agent Foundry. ## How to verify Have Git, GNU Make, Bash 4.4 or newer, and ShellCheck, on Linux or macOS. No container engine is needed. The syntax check, on a `PATH` without ShellCheck: ```bash git clone https://git.unitoo.it/claudiomaradonna/azkaban.git cd azkaban scratch="$(mktemp -d)" mkdir "$scratch/bin" for tool in make bash sh; do ln -s "$(command -v "$tool")" "$scratch/bin/$tool"; done printf 'true\n' >"$scratch/good.sh" printf 'if then fi (\n' >"$scratch/broken.sh" PATH="$scratch/bin" make lint SHELL_SOURCES="$scratch/good.sh $scratch/broken.sh"; echo "exit $?" ``` On `master` it prints `shellcheck not installed; skipped` and `exit 0`. After `git switch fix/lint-checks` the same last line prints ``broken.sh: line 1: syntax error near unexpected token `then'`` and `exit 2`. The parallel pass, with ShellCheck on the `PATH`: ```bash time make lint printf '#!/usr/bin/env bash\necho $1\n' >"$scratch/finding.sh" make lint SHELL_SOURCES="$scratch/finding.sh"; echo "exit $?" ``` On a 20-CPU host `time make lint` took 11.3 s on `master` and 3.5 s on the branch. The second command prints, on the branch, `finding.sh:2:6: note: Double quote to prevent globbing and word splitting. [SC2086]` and `exit 2`. Checks run on the tip: `make lint && .agents/hooks/shellcheck.sh repo && make test` passes, 354 tests. The new test `test_make_lint_fails_on_a_syntax_error_in_any_source_without_shellcheck` fails on `master`; the pin `test_make_lint_fails_on_a_shellcheck_finding_in_any_source` passes on both. Not tried: BSD `xargs` on macOS, which documents the `-P` the recipe uses. ## Considerations A passing `make lint` now prints nothing, where it echoed the `bash -n` command. `xargs -P` is outside POSIX; GNU and BSD `xargs` both take it.
No test showed that make lint fails, and names the finding, when
ShellCheck objects to a source that is not the first in the list. The
recipe's ShellCheck pass is about to run in parallel and must keep
that.
bash -n reads its first file and takes the rest as that script's
arguments, so make lint parsed bin/azkaban alone. ShellCheck hid it
where it is installed; where it is not, the recipe skips it and a
syntax error in any other source passed.
One ShellCheck process checked the 53 sources in turn, about 11 s on a
20-CPU host against 6 s for the whole test suite, at every commit.
ShellCheck checks each file on its own, so one process per CPU gives
the same findings in about a third of the time.

The processes share the terminal, where the default multi-line reports
of two files could interleave, so each finding is one line in the gcc
format; ShellCheck run on a file gives its snippet and suggestion.
claudiomaradonna deleted branch fix/lint-checks 2026-11-11 15:03:11 +01:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
claudiomaradonna/azkaban!69
No description provided.