Compare commits
1 Commits
62b31af5bd
...
d9be89c339
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
d9be89c339 |
@@ -774,38 +774,11 @@ style conventions are in separate documents:
|
|||||||
# was closed. That case is not hypothetical here: two runs in the same
|
# was closed. That case is not hypothetical here: two runs in the same
|
||||||
# checkout is exactly what --allow-serial-runners below exists to support,
|
# checkout is exactly what --allow-serial-runners below exists to support,
|
||||||
# and serialising the linter does not serialise the shell's redirections
|
# and serialising the linter does not serialise the shell's redirections
|
||||||
# or the grep and cat that read them. mktemp rather than $$: two
|
# or the grep and cat that read them. $$ is the same idiom CHECK_EPOCH
|
||||||
# containerised runs over one bind-mounted checkout are in separate PID
|
# uses, for the same reason.
|
||||||
# namespaces and can both be PID 7, which puts the collision back.
|
LINT_OUT="$LINT_STATE/run.$$.stdout"
|
||||||
LINT_OUT="$(mktemp "$LINT_STATE/run.out.XXXXXX")"
|
LINT_ERR="$LINT_STATE/run.$$.stderr"
|
||||||
LINT_ERR="$(mktemp "$LINT_STATE/run.err.XXXXXX")"
|
trap 'rm -f "$LINT_OUT" "$LINT_ERR"' EXIT HUP INT TERM
|
||||||
|
|
||||||
# Print whatever the linter had written before the interruption, then exit
|
|
||||||
# 128+signal. The exit is what makes this handler TERMINATING, and that is
|
|
||||||
# the point: a signal-trap handler that returns RESUMES the script. With
|
|
||||||
# the capture files already deleted, execution would fall into the grep
|
|
||||||
# below against a missing file, take the not-a-collision branch, and report
|
|
||||||
# the FINDINGS exit status with empty output for a run that was killed —
|
|
||||||
# after deleting the findings it was about to print. That is why cleanup
|
|
||||||
# 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
|
|
||||||
# it is what Ctrl-C on a make check does.
|
|
||||||
lint_interrupted() {
|
|
||||||
if [ -f "$LINT_ERR" ]; then cat "$LINT_ERR" >&2; fi
|
|
||||||
if [ -f "$LINT_OUT" ]; then cat "$LINT_OUT"; fi
|
|
||||||
echo "lint: interrupted by a signal, so nothing was completed." \
|
|
||||||
"This is NOT a lint result." >&2
|
|
||||||
exit "$1"
|
|
||||||
}
|
|
||||||
|
|
||||||
# The EXIT trap still fires on the way out of a terminating handler, so
|
|
||||||
# cleanup happens exactly once on every path. `|| :` because a failing rm
|
|
||||||
# — an unwritable state directory is enough — would otherwise change the
|
|
||||||
# exit status of an otherwise clean run under set -e.
|
|
||||||
trap 'rm -f "$LINT_OUT" "$LINT_ERR" || :' EXIT
|
|
||||||
trap 'lint_interrupted 129' HUP
|
|
||||||
trap 'lint_interrupted 130' INT
|
|
||||||
trap 'lint_interrupted 143' TERM
|
|
||||||
|
|
||||||
# Backstop for a caller that reached the linter without the environment
|
# Backstop for a caller that reached the linter without the environment
|
||||||
# above. With it set, this should never fire.
|
# above. With it set, this should never fire.
|
||||||
@@ -833,12 +806,10 @@ style conventions are in separate documents:
|
|||||||
# this failure only on stderr, so a finding that quotes the string
|
# this failure only on stderr, so a finding that quotes the string
|
||||||
# from source cannot be mistaken for a collision and retried away —
|
# from source cannot be mistaken for a collision and retried away —
|
||||||
# that direction would be a false green. The exit status is not a
|
# that direction would be a false green. The exit status is not a
|
||||||
# usable discriminator: the collision exits 3 (exitcodes.Failure),
|
# usable discriminator: the collision exits 3 (exitcodes.Failure)
|
||||||
# which does separate it from findings at 1, but run.go returns it
|
# while findings exit 1, and the other codes in pkg/exitcodes
|
||||||
# as a plain error that Execute maps to Failure like every other
|
# carry meanings of their own, so no exit status tells a collision
|
||||||
# error at that level, so 3 cannot separate a collision from a
|
# apart from a result.
|
||||||
# genuine linter failure — a broken config, an unparseable file.
|
|
||||||
# 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
|
||||||
cat "$LINT_OUT"
|
cat "$LINT_OUT"
|
||||||
@@ -911,27 +882,10 @@ style conventions are in separate documents:
|
|||||||
found something, and a lock error erased before `grep` reads it, so the
|
found something, and a lock error erased before `grep` reads it, so the
|
||||||
retry never fires and the void run returns as a result. Both are a run
|
retry never fires and the void run returns as a result. Both are a run
|
||||||
reporting a result that is not its own, which is this bullet's entire
|
reporting a result that is not its own, which is this bullet's entire
|
||||||
subject reintroduced one layer above where it was fixed. Use `mktemp`
|
subject reintroduced one layer above where it was fixed. `$$` is
|
||||||
under the state directory rather than `$$`: two containerised runs over
|
sufficient and is the same idiom the `CHECK_EPOCH` rule uses; `mktemp`
|
||||||
one bind-mounted checkout sit in separate PID namespaces and can hold the
|
under the state directory is equally fine. The `trap` matters as much as
|
||||||
same low PID, which puts the collision back on exactly the fleet's
|
the paths, or the directory accumulates a pair per run forever.
|
||||||
arrangement. `$$` is acceptable only where `mktemp` is unavailable and
|
|
||||||
that arrangement is ruled out.
|
|
||||||
- **Clean up on `EXIT` only, and give each signal a TERMINATING handler.** A
|
|
||||||
signal-trap handler that does not exit **resumes** the script: with the
|
|
||||||
capture files already deleted, the run falls into the `grep` against a
|
|
||||||
missing file, takes the not-a-collision branch, and reports the
|
|
||||||
**findings** exit status with empty output for a run that was killed —
|
|
||||||
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
|
|
||||||
the same block without the handler. Exit `128+signal` instead, after
|
|
||||||
printing what the linter had already written, and let the `EXIT` trap do
|
|
||||||
the cleanup on the way out. Killing a run is not a corner case here — it
|
|
||||||
is the stated mitigation for the unbounded wait `--allow-serial-runners`
|
|
||||||
can produce, and it is what Ctrl-C on a `make check` does. Put `|| :` on
|
|
||||||
the `rm`: an unwritable state directory makes it fail, and a failing
|
|
||||||
`EXIT` trap under `set -e` changes the exit status of an otherwise clean
|
|
||||||
run to 1, which was measured in both `dash` and `bash`.
|
|
||||||
|
|
||||||
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