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
|
||||
# checkout is exactly what --allow-serial-runners below exists to support,
|
||||
# and serialising the linter does not serialise the shell's redirections
|
||||
# or the grep and cat that read them. mktemp rather than $$: two
|
||||
# containerised runs over one bind-mounted checkout are in separate PID
|
||||
# namespaces and can both be PID 7, which puts the collision back.
|
||||
LINT_OUT="$(mktemp "$LINT_STATE/run.out.XXXXXX")"
|
||||
LINT_ERR="$(mktemp "$LINT_STATE/run.err.XXXXXX")"
|
||||
|
||||
# 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
|
||||
# or the grep and cat that read them. $$ is the same idiom CHECK_EPOCH
|
||||
# uses, for the same reason.
|
||||
LINT_OUT="$LINT_STATE/run.$$.stdout"
|
||||
LINT_ERR="$LINT_STATE/run.$$.stderr"
|
||||
trap 'rm -f "$LINT_OUT" "$LINT_ERR"' EXIT HUP INT TERM
|
||||
|
||||
# Backstop for a caller that reached the linter without the environment
|
||||
# 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
|
||||
# from source cannot be mistaken for a collision and retried away —
|
||||
# that direction would be a false green. The exit status is not a
|
||||
# usable discriminator: the collision exits 3 (exitcodes.Failure),
|
||||
# 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
|
||||
# error at that level, so 3 cannot separate a collision from a
|
||||
# genuine linter failure — a broken config, an unparseable file.
|
||||
# Retrying on 3 would retry real failures into a void.
|
||||
# usable discriminator: the collision exits 3 (exitcodes.Failure)
|
||||
# while findings exit 1, and the other codes in pkg/exitcodes
|
||||
# carry meanings of their own, so no exit status tells a collision
|
||||
# apart from a result.
|
||||
if ! grep -q 'parallel golangci-lint is running' "$LINT_ERR"; then
|
||||
cat "$LINT_ERR" >&2
|
||||
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
|
||||
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
|
||||
subject reintroduced one layer above where it was fixed. Use `mktemp`
|
||||
under the state directory rather than `$$`: two containerised runs over
|
||||
one bind-mounted checkout sit in separate PID namespaces and can hold the
|
||||
same low PID, which puts the collision back on exactly the fleet's
|
||||
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`.
|
||||
subject reintroduced one layer above where it was fixed. `$$` is
|
||||
sufficient and is the same idiom the `CHECK_EPOCH` rule uses; `mktemp`
|
||||
under the state directory is equally fine. The `trap` matters as much as
|
||||
the paths, or the directory accumulates a pair per run forever.
|
||||
|
||||
Adopting repos must add `.lint-cache/` to both `.gitignore` and
|
||||
`.dockerignore`. The second matters as much as the first: the directory
|
||||
|
||||
Reference in New Issue
Block a user