Compare commits
1 Commits
62b31af5bd
...
33fb5dde98
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
33fb5dde98 |
@@ -790,11 +790,20 @@ style conventions are in separate documents:
|
|||||||
# is on EXIT only. Killing a run is not hypothetical here: it is the stated
|
# is on EXIT only. Killing a run is not hypothetical here: it is the stated
|
||||||
# mitigation for the unbounded wait --allow-serial-runners can produce, and
|
# mitigation for the unbounded wait --allow-serial-runners can produce, and
|
||||||
# it is what Ctrl-C on a make check does.
|
# it is what Ctrl-C on a make check does.
|
||||||
|
#
|
||||||
|
# All three writes are best-effort, and the `|| :` on each is load-bearing
|
||||||
|
# rather than defensive habit. Under set -e a failed write aborts the
|
||||||
|
# function BEFORE exit "$1", and the shell then exits 1 — the findings
|
||||||
|
# status, on a run that analysed nothing. SIGHUP is precisely the case
|
||||||
|
# where writing fails: once the controlling terminal is gone the writes
|
||||||
|
# return EIO. Guarding only the two cats is not enough, because with the
|
||||||
|
# capture files empty the cats write nothing and succeed, and the echo is
|
||||||
|
# what fails.
|
||||||
lint_interrupted() {
|
lint_interrupted() {
|
||||||
if [ -f "$LINT_ERR" ]; then cat "$LINT_ERR" >&2; fi
|
if [ -f "$LINT_ERR" ]; then cat "$LINT_ERR" >&2 || :; fi
|
||||||
if [ -f "$LINT_OUT" ]; then cat "$LINT_OUT"; fi
|
if [ -f "$LINT_OUT" ]; then cat "$LINT_OUT" || :; fi
|
||||||
echo "lint: interrupted by a signal, so nothing was completed." \
|
echo "lint: interrupted by a signal, so nothing was completed." \
|
||||||
"This is NOT a lint result." >&2
|
"This is NOT a lint result." >&2 || :
|
||||||
exit "$1"
|
exit "$1"
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -837,7 +846,9 @@ style conventions are in separate documents:
|
|||||||
# which does separate it from findings at 1, but run.go returns it
|
# which does separate it from findings at 1, but run.go returns it
|
||||||
# as a plain error that Execute maps to Failure like every other
|
# as a plain error that Execute maps to Failure like every other
|
||||||
# error at that level, so 3 cannot separate a collision from a
|
# error at that level, so 3 cannot separate a collision from a
|
||||||
# genuine linter failure — a broken config, an unparseable file.
|
# genuine linter failure — an unknown linter name, an unknown flag,
|
||||||
|
# malformed config YAML all exit 3 too. (An unparseable Go source
|
||||||
|
# file does not: that is reported as typecheck issues and exits 1.)
|
||||||
# Retrying on 3 would retry real failures into a void.
|
# Retrying on 3 would retry real failures into a void.
|
||||||
if ! grep -q 'parallel golangci-lint is running' "$LINT_ERR"; then
|
if ! grep -q 'parallel golangci-lint is running' "$LINT_ERR"; then
|
||||||
cat "$LINT_ERR" >&2
|
cat "$LINT_ERR" >&2
|
||||||
@@ -924,14 +935,34 @@ style conventions are in separate documents:
|
|||||||
**findings** exit status with empty output for a run that was killed —
|
**findings** exit status with empty output for a run that was killed —
|
||||||
having deleted the findings it was about to print. Measured: `TERM`, `INT`
|
having deleted the findings it was about to print. Measured: `TERM`, `INT`
|
||||||
and `HUP` all returning 1 with empty stdout, against 143, 130 and 129 for
|
and `HUP` all returning 1 with empty stdout, against 143, 130 and 129 for
|
||||||
the same block without the handler. Exit `128+signal` instead, after
|
the same block without the handler. Exit `128+signal` instead, and let the
|
||||||
printing what the linter had already written, and let the `EXIT` trap do
|
`EXIT` trap do the cleanup on the way out. The handler also makes a
|
||||||
the cleanup on the way out. Killing a run is not a corner case here — it
|
**best-effort** attempt to print what the linter had already written —
|
||||||
is the stated mitigation for the unbounded wait `--allow-serial-runners`
|
best-effort because under `SIGHUP` the terminal is typically gone and
|
||||||
can produce, and it is what Ctrl-C on a `make check` does. Put `|| :` on
|
every write returns `EIO`, in which case nothing is printed and only the
|
||||||
the `rm`: an unwritable state directory makes it fail, and a failing
|
status carries the message. Each of those writes needs its own `|| :`:
|
||||||
`EXIT` trap under `set -e` changes the exit status of an otherwise clean
|
under `set -e` a failed write aborts the handler before it reaches `exit`,
|
||||||
run to 1, which was measured in both `dash` and `bash`.
|
and the shell then exits 1, which is the findings status on a run that
|
||||||
|
analysed nothing. Guarding only the `cat`s is not enough — with the
|
||||||
|
capture files empty they write nothing and succeed, and the `echo` is what
|
||||||
|
fails. Put `|| :` on the `rm` in the `EXIT` trap for the same reason: an
|
||||||
|
unwritable state directory makes it fail, and a failing `EXIT` trap under
|
||||||
|
`set -e` turns an otherwise clean run into exit 1, measured in both `dash`
|
||||||
|
and `bash`.
|
||||||
|
- **A signal must reach the LINTER, not just the wrapper.** POSIX defers a
|
||||||
|
trap until the running foreground command completes, so
|
||||||
|
`kill -TERM <wrapper pid>` does nothing at all while `golangci-lint` is
|
||||||
|
running — measured still alive three seconds later, where the same block
|
||||||
|
without a handler dies immediately at 143. Ctrl-C is unaffected because
|
||||||
|
the terminal signals the whole process group. This matters exactly where
|
||||||
|
the handler is supposed to help: the unbounded `--allow-serial-runners`
|
||||||
|
wait, where the process holding things up is the linter itself. Kill the
|
||||||
|
group (`kill -- -<pgid>`) or use Ctrl-C.
|
||||||
|
- **Known, accepted gap:** a signal arriving between the `mktemp` calls and
|
||||||
|
the `trap ... EXIT` line leaves the two capture files behind. Closing it
|
||||||
|
needs a trap installed before the files have names and rewritten after,
|
||||||
|
which is more moving parts than a couple of stray files in a gitignored
|
||||||
|
directory is worth. Stated rather than silently left.
|
||||||
|
|
||||||
Adopting repos must add `.lint-cache/` to both `.gitignore` and
|
Adopting repos must add `.lint-cache/` to both `.gitignore` and
|
||||||
`.dockerignore`. The second matters as much as the first: the directory
|
`.dockerignore`. The second matters as much as the first: the directory
|
||||||
|
|||||||
Reference in New Issue
Block a user