|
|
|
@@ -691,6 +691,348 @@ style conventions are in separate documents:
|
|
|
|
missing call site passes every control while the adopted snippet does
|
|
|
|
missing call site passes every control while the adopted snippet does
|
|
|
|
nothing.
|
|
|
|
nothing.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
- **`script/lint` in Go repos must give golangci-lint per-checkout cache and
|
|
|
|
|
|
|
|
lock state, and must never report a lock collision as a lint result.**
|
|
|
|
|
|
|
|
golangci-lint shares two pieces of state across every process on the host, and
|
|
|
|
|
|
|
|
they are separate mechanisms with separate fixes. Isolating one and stopping
|
|
|
|
|
|
|
|
leaves the other fully live while reading as a fix. This is independent of the
|
|
|
|
|
|
|
|
pinned-install rule above and does not replace it: that one makes the host run
|
|
|
|
|
|
|
|
the right linter, this one makes the run's result belong to your own tree.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
**Mechanism 1, the result cache — produces false greens as well as false
|
|
|
|
|
|
|
|
reds.** golangci-lint keys cached results on file **content, not location**,
|
|
|
|
|
|
|
|
so two checkouts of the same commit hold byte-identical files, share cache
|
|
|
|
|
|
|
|
entries, and one tree's findings are served for the other — reported at the
|
|
|
|
|
|
|
|
_other_ tree's path. Observed across the org: 399 issues attributed to a
|
|
|
|
|
|
|
|
`/tmp` worktree that no longer existed, returned from a clean clone that
|
|
|
|
|
|
|
|
genuinely lints 0 issues; ten findings against a deleted worktree; findings
|
|
|
|
|
|
|
|
reported against `../wt82-lint/...`; and, in the dangerous direction, an
|
|
|
|
|
|
|
|
implementer reporting "lint 0 issues" on a branch that was genuinely red.
|
|
|
|
|
|
|
|
Note what content-keying implies: **moving agents from worktrees to their
|
|
|
|
|
|
|
|
own clones does not help.** Two clones of a repo are byte-identical exactly
|
|
|
|
|
|
|
|
as two worktrees were. What own-clones removes is the deleted-worktree path
|
|
|
|
|
|
|
|
artefact — the loud, obviously wrong symptom — while leaving the mechanism
|
|
|
|
|
|
|
|
live, which makes the defect quieter rather than rarer.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
**Mechanism 2, the concurrency lock — and it does not live in the cache
|
|
|
|
|
|
|
|
directory.** From `pkg/commands/run.go`, `acquireFileLock()`:
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
```go
|
|
|
|
|
|
|
|
lockFile := filepath.Join(os.TempDir(), "golangci-lint.lock")
|
|
|
|
|
|
|
|
```
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
That is `$TMPDIR/golangci-lint.lock` — host-global, keyed on the temp
|
|
|
|
|
|
|
|
directory, entirely independent of `GOLANGCI_LINT_CACHE`. It is an `flock`
|
|
|
|
|
|
|
|
retried every second under a **5-second total timeout**, so it fails
|
|
|
|
|
|
|
|
precisely when the host is busiest. On failure the run emits
|
|
|
|
|
|
|
|
`parallel golangci-lint is running` and analyzes nothing. **A private cache
|
|
|
|
|
|
|
|
directory does not prevent this**; that was established by controlled test,
|
|
|
|
|
|
|
|
with two concurrent runs under separate cache directories sharing no mounted
|
|
|
|
|
|
|
|
path, one of which still collided. Anyone who sets only
|
|
|
|
|
|
|
|
`GOLANGCI_LINT_CACHE` has closed the contamination half and left the
|
|
|
|
|
|
|
|
false-red half untouched.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
The canonical form for a Go repo's `script/lint`:
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
```sh
|
|
|
|
|
|
|
|
#!/bin/sh
|
|
|
|
|
|
|
|
# script/lint: run the linter.
|
|
|
|
|
|
|
|
set -eu
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
# Per-checkout golangci-lint state. Both variables are required and they
|
|
|
|
|
|
|
|
# fix different defects; neither is redundant with the other.
|
|
|
|
|
|
|
|
#
|
|
|
|
|
|
|
|
# GOLANGCI_LINT_CACHE: the result cache is keyed on file content, not
|
|
|
|
|
|
|
|
# location, so checkouts holding identical files serve each other's
|
|
|
|
|
|
|
|
# findings. A per-REPO cache directory does NOT fix this — every checkout
|
|
|
|
|
|
|
|
# of that repo still collides — so the path must be inside the invoking
|
|
|
|
|
|
|
|
# checkout.
|
|
|
|
|
|
|
|
#
|
|
|
|
|
|
|
|
# TMPDIR: golangci-lint flocks $TMPDIR/golangci-lint.lock
|
|
|
|
|
|
|
|
# (pkg/commands/run.go, acquireFileLock: filepath.Join(os.TempDir(),
|
|
|
|
|
|
|
|
# "golangci-lint.lock")). That lock is host-global and independent of
|
|
|
|
|
|
|
|
# GOLANGCI_LINT_CACHE, with a 5s acquire timeout. Scoping TMPDIR into the
|
|
|
|
|
|
|
|
# checkout is the only thing here that isolates it. Do not delete this as
|
|
|
|
|
|
|
|
# redundant with the cache variable; it is not.
|
|
|
|
|
|
|
|
#
|
|
|
|
|
|
|
|
# The leading dot in .lint-cache is load-bearing: the go tool skips
|
|
|
|
|
|
|
|
# dot-prefixed directories when expanding ./..., so the linter never reads
|
|
|
|
|
|
|
|
# its own cache and temp files back as source. Do not rename it.
|
|
|
|
|
|
|
|
LINT_STATE="$ROOT/.lint-cache"
|
|
|
|
|
|
|
|
GOLANGCI_LINT_CACHE="$LINT_STATE/cache"
|
|
|
|
|
|
|
|
TMPDIR="$LINT_STATE/tmp"
|
|
|
|
|
|
|
|
export GOLANGCI_LINT_CACHE TMPDIR
|
|
|
|
|
|
|
|
mkdir -p "$GOLANGCI_LINT_CACHE" "$TMPDIR"
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
# Capture files, per INVOCATION and not per checkout. Two runs in the same
|
|
|
|
|
|
|
|
# checkout would otherwise redirect into one pair of fixed paths, opened
|
|
|
|
|
|
|
|
# O_TRUNC before the linter even starts, and each would print and scan the
|
|
|
|
|
|
|
|
# other's output — a run reporting a result that is not its own, which is
|
|
|
|
|
|
|
|
# the whole defect this bullet exists to close, one layer up from where it
|
|
|
|
|
|
|
|
# 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.
|
|
|
|
|
|
|
|
#
|
|
|
|
|
|
|
|
# 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() {
|
|
|
|
|
|
|
|
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
|
|
|
|
|
|
|
|
# above. With it set, this should never fire.
|
|
|
|
|
|
|
|
LINT_MAX_ATTEMPTS=5
|
|
|
|
|
|
|
|
# EX_TEMPFAIL. Distinct from 1 (findings) and 3 (linter error) so a void
|
|
|
|
|
|
|
|
# run is never counted as either.
|
|
|
|
|
|
|
|
LINT_VOID_EXIT=75
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
golangci_lint_run() {
|
|
|
|
|
|
|
|
attempt=1
|
|
|
|
|
|
|
|
delay=2
|
|
|
|
|
|
|
|
while :; do
|
|
|
|
|
|
|
|
rc=0
|
|
|
|
|
|
|
|
# --allow-serial-runners KEEPS the mutual-exclusion guard and makes
|
|
|
|
|
|
|
|
# an overlapping run queue on the lock instead of aborting after
|
|
|
|
|
|
|
|
# 5s. It is NOT --allow-parallel-runners, which removes the guard
|
|
|
|
|
|
|
|
# entirely; never use that one. This is what covers two runs inside
|
|
|
|
|
|
|
|
# the SAME checkout, which TMPDIR scoping cannot — script/precommit
|
|
|
|
|
|
|
|
# overlapping a make check is the realistic trigger.
|
|
|
|
|
|
|
|
golangci-lint run --allow-serial-runners "$@" \
|
|
|
|
|
|
|
|
>"$LINT_OUT" 2>"$LINT_ERR" || rc=$?
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
# Detect the lock collision on the STDERR STREAM, never on the exit
|
|
|
|
|
|
|
|
# status. Findings are written to stdout and golangci-lint reports
|
|
|
|
|
|
|
|
# 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 — 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.
|
|
|
|
|
|
|
|
if ! grep -q 'parallel golangci-lint is running' "$LINT_ERR"; then
|
|
|
|
|
|
|
|
cat "$LINT_ERR" >&2
|
|
|
|
|
|
|
|
cat "$LINT_OUT"
|
|
|
|
|
|
|
|
return "$rc"
|
|
|
|
|
|
|
|
fi
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
if [ "$attempt" -ge "$LINT_MAX_ATTEMPTS" ]; then
|
|
|
|
|
|
|
|
cat "$LINT_ERR" >&2
|
|
|
|
|
|
|
|
echo "lint: VOID after $LINT_MAX_ATTEMPTS attempts:" \
|
|
|
|
|
|
|
|
"golangci-lint never acquired its lock, so nothing was" \
|
|
|
|
|
|
|
|
"analyzed. This is NOT a lint result and no verdict may" \
|
|
|
|
|
|
|
|
"be recorded from it. Re-run it." >&2
|
|
|
|
|
|
|
|
return "$LINT_VOID_EXIT"
|
|
|
|
|
|
|
|
fi
|
|
|
|
|
|
|
|
echo "lint: lock held by another golangci-lint; attempt" \
|
|
|
|
|
|
|
|
"$attempt of $LINT_MAX_ATTEMPTS, retrying in ${delay}s" >&2
|
|
|
|
|
|
|
|
sleep "$delay"
|
|
|
|
|
|
|
|
attempt=$((attempt + 1))
|
|
|
|
|
|
|
|
delay=$((delay * 2))
|
|
|
|
|
|
|
|
done
|
|
|
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
main() {
|
|
|
|
|
|
|
|
cd "$ROOT"
|
|
|
|
|
|
|
|
golangci_lint_run ./...
|
|
|
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
main "$@"
|
|
|
|
|
|
|
|
```
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
Load-bearing properties, each guarding a mode that otherwise reports a
|
|
|
|
|
|
|
|
verdict it did not earn:
|
|
|
|
|
|
|
|
- **Both variables, per checkout.** Cache alone leaves the false reds;
|
|
|
|
|
|
|
|
`TMPDIR` alone leaves the contamination that produced a confirmed false
|
|
|
|
|
|
|
|
green. A per-repo path for either is not isolation on a host where every
|
|
|
|
|
|
|
|
worker holds its own copy of the same repo.
|
|
|
|
|
|
|
|
- **Set them on every path that reaches the linter.** The export block goes
|
|
|
|
|
|
|
|
_above_ any container-versus-host branch, and a native escape hatch must
|
|
|
|
|
|
|
|
call `golangci_lint_run` rather than `exec golangci-lint` directly. One
|
|
|
|
|
|
|
|
repo in the org had exactly one such path with no cache environment at
|
|
|
|
|
|
|
|
all, inheriting the fleet-wide default, so the fix on the other path was
|
|
|
|
|
|
|
|
worth nothing there.
|
|
|
|
|
|
|
|
- **The lock collision is not a result, and must never be reported as one.**
|
|
|
|
|
|
|
|
Retry it, and on exhaustion exit a status that is neither the findings
|
|
|
|
|
|
|
|
status nor success, with a message that says VOID. Swallowing it into a
|
|
|
|
|
|
|
|
success is the worst available outcome; reporting it as findings sends a
|
|
|
|
|
|
|
|
correct branch back for rework against findings that do not exist.
|
|
|
|
|
|
|
|
- **Detect the collision by the message on stderr, not by exit status, and
|
|
|
|
|
|
|
|
never retry a genuine finding.** Distinguishing "exited non-zero because
|
|
|
|
|
|
|
|
of the lock" from "exited non-zero because of findings" is the whole crux,
|
|
|
|
|
|
|
|
and getting it wrong in the direction of treating findings as a lock error
|
|
|
|
|
|
|
|
retries a real failure into a void — or, if a later implementation decided
|
|
|
|
|
|
|
|
to treat exhaustion as success, into a green.
|
|
|
|
|
|
|
|
- **`--allow-serial-runners`, never `--allow-parallel-runners`.** The first
|
|
|
|
|
|
|
|
keeps the guard and queues; the second deletes it and lets two runs
|
|
|
|
|
|
|
|
corrupt shared state. With the flag set, an overlapping run inside the
|
|
|
|
|
|
|
|
same checkout waits rather than failing, which is what is actually wanted.
|
|
|
|
|
|
|
|
The honest cost is that it waits without bound, so a stale process holding
|
|
|
|
|
|
|
|
the lock hangs the run instead of failing it; the contending set is
|
|
|
|
|
|
|
|
bounded to the same checkout, and an eventual result is preferable to a
|
|
|
|
|
|
|
|
fabricated one.
|
|
|
|
|
|
|
|
- **Capture stdout and stderr to per-INVOCATION paths, and clean them up.**
|
|
|
|
|
|
|
|
Two fixed paths under the checkout are one pair for every run in it, and
|
|
|
|
|
|
|
|
the redirections truncate them before the linter starts, so two
|
|
|
|
|
|
|
|
overlapping runs print and scan each other's output.
|
|
|
|
|
|
|
|
`--allow-serial-runners` does not prevent this — it serialises the linter,
|
|
|
|
|
|
|
|
not the shell — and the overlap it exists to support is precisely
|
|
|
|
|
|
|
|
`script/precommit` against a `make check` in one checkout. The observed
|
|
|
|
|
|
|
|
shapes are a run printing the other's `0 issues.` while its own linter
|
|
|
|
|
|
|
|
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, and let the
|
|
|
|
|
|
|
|
`EXIT` trap do the cleanup on the way out. The handler also makes a
|
|
|
|
|
|
|
|
**best-effort** attempt to print what the linter had already written —
|
|
|
|
|
|
|
|
best-effort because under `SIGHUP` the terminal is typically gone and
|
|
|
|
|
|
|
|
every write returns `EIO`, in which case nothing is printed and only the
|
|
|
|
|
|
|
|
status carries the message. Each of those writes needs its own `|| :`:
|
|
|
|
|
|
|
|
under `set -e` a failed write aborts the handler before it reaches `exit`,
|
|
|
|
|
|
|
|
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
|
|
|
|
|
|
|
|
`.dockerignore`. The second matters as much as the first: the directory
|
|
|
|
|
|
|
|
reaches tens of megabytes, and without the exclusion it enters the build
|
|
|
|
|
|
|
|
context and invalidates `COPY . .` for reasons unrelated to the repo's
|
|
|
|
|
|
|
|
content.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
**`GOCACHE` does not need isolating, and this was measured rather than
|
|
|
|
|
|
|
|
assumed.** With `GOLANGCI_LINT_CACHE` and `TMPDIR` per checkout and
|
|
|
|
|
|
|
|
`GOCACHE` left at the host default and shared, two checkouts of identical
|
|
|
|
|
|
|
|
content each reported their own paths and neither reported the other's. The
|
|
|
|
|
|
|
|
Go build cache is content-addressed and its entries are compiled artifacts
|
|
|
|
|
|
|
|
rather than diagnostics carrying a foreign tree's paths, and it has no
|
|
|
|
|
|
|
|
equivalent global lock — the whole fleet compiles concurrently against one
|
|
|
|
|
|
|
|
`GOCACHE` all day without a contention error. Isolating it would cost a full
|
|
|
|
|
|
|
|
cold compile per checkout for no measured benefit. The earlier hypothesis
|
|
|
|
|
|
|
|
that Go build-cache contention might explain the lock error is superseded:
|
|
|
|
|
|
|
|
the lock is located in source at `$TMPDIR/golangci-lint.lock`.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
**Verifying a change to this logic requires a negative control, and the
|
|
|
|
|
|
|
|
control must be built out of checkouts with identical content.** Create two
|
|
|
|
|
|
|
|
checkouts of the same tree containing a deliberate lint finding, run the
|
|
|
|
|
|
|
|
linter in the second so it populates the cache, then run it in the first and
|
|
|
|
|
|
|
|
confirm the finding is reported at the first checkout's own path and never
|
|
|
|
|
|
|
|
at the second's. Run the same control against the unisolated form and
|
|
|
|
|
|
|
|
confirm the contamination appears there — a control that passes against the
|
|
|
|
|
|
|
|
broken implementation proves nothing. Note specifically that a control built
|
|
|
|
|
|
|
|
from checkouts whose **content differs** passes against the unisolated form
|
|
|
|
|
|
|
|
too, because differing content does not collide in a content-keyed cache, so
|
|
|
|
|
|
|
|
it is not a test of anything. For the lock half, hold
|
|
|
|
|
|
|
|
`$TMPDIR/golangci-lint.lock` with `flock` and confirm the run queues rather
|
|
|
|
|
|
|
|
than aborting, that a caller never sees the collision as findings, and that
|
|
|
|
|
|
|
|
exhaustion fails loudly and distinguishably.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
**Run those controls against the block as a consuming repo would adopt it**
|
|
|
|
|
|
|
|
— pasted into a `script/lint`-shaped file that is then executed, not sourced
|
|
|
|
|
|
|
|
with the functions driven by hand. The same warning as for the bootstrap
|
|
|
|
|
|
|
|
block above, for the same reason.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
- **Interim rule for reading a golangci-lint result on a shared host, until
|
|
|
|
|
|
|
|
every repo has adopted the isolation above.** A lint run is **VOID** unless
|
|
|
|
|
|
|
|
both hold:
|
|
|
|
|
|
|
|
- the output contains no `parallel golangci-lint is running`, and
|
|
|
|
|
|
|
|
- no reported file path begins with `../`, and none is an absolute path
|
|
|
|
|
|
|
|
outside the tree the run was launched from.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
Do not record a verdict from a void run, and do not "fix" findings in files
|
|
|
|
|
|
|
|
the change does not touch — chasing phantom findings across untouched files
|
|
|
|
|
|
|
|
puts unrelated edits into a reviewed diff, which is more expensive than the
|
|
|
|
|
|
|
|
wasted rework.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
The `../` clause is the one that actually bites, and it is why a filter
|
|
|
|
|
|
|
|
keyed on `/tmp` or on absolute prefixes is not enough: golangci-lint reports
|
|
|
|
|
|
|
|
paths relative to its own resolved root rather than yours, and three of the
|
|
|
|
|
|
|
|
org's reported sightings had relative paths and would have passed such a
|
|
|
|
|
|
|
|
filter. Both clauses are needed and neither alone is sufficient — one
|
|
|
|
|
|
|
|
reproduction exited non-zero with the lock error and no foreign paths at
|
|
|
|
|
|
|
|
all, and another reported 34 well-formed findings, every one of them against
|
|
|
|
|
|
|
|
another checkout.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
**State the limit of these tests rather than treating them as a guarantee.**
|
|
|
|
|
|
|
|
They catch contamination that **names** foreign files. They cannot catch
|
|
|
|
|
|
|
|
contamination that **suppresses** findings through a poisoned entry for
|
|
|
|
|
|
|
|
colliding content, which has no wall-clock tell either — **no evidence of
|
|
|
|
|
|
|
|
that mode has been observed, and nobody should go chasing it**; the point is
|
|
|
|
|
|
|
|
the reach of the tests, not a claim that the mode exists. They are a filter
|
|
|
|
|
|
|
|
for the loud mode, not a proof of soundness — which is the whole argument
|
|
|
|
|
|
|
|
for fixing this in the tooling instead of documenting a discipline that
|
|
|
|
|
|
|
|
depends on every agent remembering to apply it.
|
|
|
|
|
|
|
|
|
|
|
|
- When pinning images or packages by hash, add a comment above the reference
|
|
|
|
- When pinning images or packages by hash, add a comment above the reference
|
|
|
|
with the version and date (YYYY-MM-DD).
|
|
|
|
with the version and date (YYYY-MM-DD).
|
|
|
|
|
|
|
|
|
|
|
|
|