Make the pinned golangci-lint actually reach the host (closes #28)
All checks were successful
check / check (push) Successful in 9s
All checks were successful
check / check (push) Successful in 9s
REPO_POLICIES.md now carries the canonical script/bootstrap snippet for Go repos alongside the .golangci.yml bullet, where the pinned linter version already lives. The guard it replaces, `if missing golangci-lint; then go install ...; fi`, tests PATH presence and never version, so on any already-provisioned machine the pin is inert and a version bump is a no-op. The Dockerfile installs unconditionally into a clean image, so CI and local then disagree about what the linter is: a local `make check` green while `make docker` rejects the same commit, and a container run surfacing findings the host run cannot see. Comparing versions alone is not enough. `go install` writes to GOBIN (or GOPATH/bin) while callers resolve through PATH, so a shadowing binary earlier in PATH lets the install succeed and change nothing a caller ever sees, while bootstrap prints success. The canonical form therefore compares the installed version against the pin, re-resolves through PATH after installing and asserts the pin, and treats any unparseable --version output as a mismatch so the failure direction is a redundant install rather than a skipped one. The comparison is exact over the whole version token. A parser that stops at the first `-` reports 2.12.2 for a host running 2.12.2-rc1, which compares equal to a 2.12.2 pin and skips the install -- the original defect, reachable through the comparison meant to close it, and not caught by requiring the pin to be tagged, since a pre-release is tagged too. When the assert fails the diagnosis is derived from the resolved path rather than asserted: a path outside the install directory is shadowing and the operator is told to remove it or reorder PATH; a path inside it is not, and saying so would send them after a fault that does not exist; no resolution at all means the install directory is simply absent from PATH. A trailing slash on GOBIN is normalised away, since it would otherwise make the inside-the- directory test miss and misreport shadowing. The snippet ends with a call site, and both success paths print a confirmation naming the version. Two definitions with no invocation are a silent no-op with exactly the shape this change exists to close, and a success path that prints nothing is byte-identical to that no-op: same exit status, same empty output. The version helper ends in `|| true` so a --version that exits non-zero cannot kill the script through `set -e` under `set -o pipefail` before the diagnostic is printed, which the styleguide's bash form would otherwise do. The policy text states each of those as a requirement rather than leaving them implicit in the code, and records why the commit-pinned `go install` ref satisfies the hash-pinning rule: a commit hash is not a mutable tag, and the checksum database verifies the fetch, with no repo go.sum consulted, since `go install pkg@version` ignores the go.mod in the current directory or any parent. Whether a go.mod tool dependency should replace that is an open decision and is linked rather than settled here. The two version strings must be kept in sync and must match exactly what --version prints; tagged pins are preferred because the expected string is then derivable from the ref, rather than because the comparison cannot handle a pseudo-version. Verification runs the controls against the block as a consuming repo would adopt it, pasted into a script/bootstrap-shaped file and executed, rather than sourcing it and calling the function directly. The node and yarn handling described earlier in the document is untouched.
This commit is contained in:
8
TODO.md
8
TODO.md
@@ -21,6 +21,14 @@ fmt-check, and commit.
|
||||
|
||||
# Completed Steps
|
||||
|
||||
- 2026-08-09: Made the pinned golangci-lint actually propagate: REPO_POLICIES.md
|
||||
now carries the canonical `script/bootstrap` snippet for Go repos, which
|
||||
installs when the installed version does not match the pin (the old
|
||||
`if missing` guard tested PATH presence only, so pins were inert on any
|
||||
provisioned machine and CI silently disagreed with local) and then re-resolves
|
||||
the binary through `PATH` and fails loudly, naming the shadowing path, when
|
||||
the install did not take effect — the failure mode the naive
|
||||
compare-then-install fix leaves behind while reporting success.
|
||||
- 2026-08-09: Fixed the false green in the canonical CI gate: `script/cibuild`
|
||||
and `script/docker` now pass a per-invocation `CHECK_EPOCH` nonce, and the
|
||||
`Dockerfile` (plus the Go multistage template in REPO_POLICIES.md, in both its
|
||||
|
||||
@@ -354,6 +354,195 @@ style conventions are in separate documents:
|
||||
commit-pinned via
|
||||
`go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@c0d3ddc9cf3faa61a4e378e879ece580256d76e5`.
|
||||
|
||||
- **`script/bootstrap` in Go repos must install the pinned golangci-lint
|
||||
whenever the installed version does not match the pin — not merely when the
|
||||
binary is absent — and must then verify the install took effect by
|
||||
re-resolving the binary through `PATH`.** The presence test
|
||||
`if missing golangci-lint; then go install "$GOLANGCI_LINT_REF"; fi` is wrong:
|
||||
it tests `PATH` presence and never version, so on any already-provisioned
|
||||
machine the pin is inert and a version bump is a no-op. Meanwhile the
|
||||
Dockerfile installs unconditionally into a clean image, so CI and local
|
||||
silently disagree about what the linter even is. Observed consequences: a
|
||||
local `make check` green while `make docker` rejected the same commit with six
|
||||
`goconst` findings, and a container linter surfacing thirteen findings the
|
||||
host run missed. A stale host linter does not merely fail to prove the tree is
|
||||
clean — it hides findings only the container can see. This is a deliberate
|
||||
departure from the node handling described above, which uses whatever node is
|
||||
installed: the linter version is the specific thing being held equal between
|
||||
host and container, so for it, presence is not enough.
|
||||
|
||||
Comparing versions is necessary but **not sufficient**, because the obvious
|
||||
fix also fails green. `go install` writes to `GOBIN` (or `GOPATH/bin`) while
|
||||
callers resolve `golangci-lint` through `PATH`. If a different binary
|
||||
shadows it earlier in `PATH`, the install genuinely succeeds and changes
|
||||
nothing any caller will ever see: bootstrap prints success and the next
|
||||
`make lint` still runs the stale linter. That is worse than no fix, because
|
||||
it converts a known-stale toolchain into one everyone believes is pinned.
|
||||
The canonical form, placed in `script/bootstrap` after Go itself is present:
|
||||
|
||||
```sh
|
||||
# golangci-lint v2.12.2, 2026-05-06. GOLANGCI_LINT_VERSION must be exactly
|
||||
# what `golangci-lint --version` prints for this ref; update both together.
|
||||
GOLANGCI_LINT_VERSION="2.12.2"
|
||||
GOLANGCI_LINT_REF="github.com/golangci/golangci-lint/v2/cmd/golangci-lint@c0d3ddc9cf3faa61a4e378e879ece580256d76e5"
|
||||
|
||||
# The version golangci-lint reports, resolved the way callers resolve it.
|
||||
# Prints nothing when the binary is absent, exits non-zero, or prints
|
||||
# something unparseable: all of those must read as "does not match".
|
||||
# The capture is the whole version token, not just its numeric prefix.
|
||||
# Stopping at the first `-` would make 2.12.2-rc1 compare equal to 2.12.2
|
||||
# and skip the install, which is the defect this whole rule exists to close.
|
||||
# The trailing `|| true` is required, not tidiness. Under `set -o pipefail`
|
||||
# a non-zero --version would otherwise propagate out of the pipeline and
|
||||
# kill the script through `set -e` before the diagnostic below is printed.
|
||||
golangci_lint_version() {
|
||||
command -v golangci-lint >/dev/null 2>&1 || return 0
|
||||
golangci-lint --version 2>/dev/null | head -n 1 |
|
||||
sed -n 's/.*has version v\{0,1\}\([0-9][^ ]*\).*/\1/p' || true
|
||||
}
|
||||
|
||||
ensure_golangci_lint() {
|
||||
if [ "$(golangci_lint_version)" = "$GOLANGCI_LINT_VERSION" ]; then
|
||||
echo "bootstrap: golangci-lint $GOLANGCI_LINT_VERSION already installed"
|
||||
return 0
|
||||
fi
|
||||
echo "bootstrap: installing golangci-lint $GOLANGCI_LINT_VERSION"
|
||||
go install "$GOLANGCI_LINT_REF"
|
||||
|
||||
# go install writes to GOBIN (or GOPATH/bin); callers resolve through
|
||||
# PATH. Re-resolve through PATH and assert the install took effect.
|
||||
# `hash -r` is load-bearing: without it a shell that already resolved
|
||||
# a stale golangci-lint answers from its own lookup cache, and this
|
||||
# check false-fails with the shadowing message below.
|
||||
hash -r 2>/dev/null || true
|
||||
gcl_got="$(golangci_lint_version)"
|
||||
if [ "$gcl_got" = "$GOLANGCI_LINT_VERSION" ]; then
|
||||
echo "bootstrap: golangci-lint $GOLANGCI_LINT_VERSION installed," \
|
||||
"and PATH resolves it"
|
||||
return 0
|
||||
fi
|
||||
gcl_bin="$(go env GOBIN)"
|
||||
[ -n "$gcl_bin" ] || gcl_bin="$(go env GOPATH)/bin"
|
||||
# Strip a trailing slash: GOBIN=/x/ would otherwise make the
|
||||
# "$gcl_bin"/* test below miss and misreport shadowing.
|
||||
while :; do
|
||||
case "$gcl_bin" in
|
||||
*/) gcl_bin="${gcl_bin%/}" ;;
|
||||
*) break ;;
|
||||
esac
|
||||
done
|
||||
gcl_found="$(command -v golangci-lint 2>/dev/null || true)"
|
||||
echo "bootstrap: installed golangci-lint $GOLANGCI_LINT_VERSION into" \
|
||||
"$gcl_bin, but that is not what callers will get." >&2
|
||||
case "$gcl_found" in
|
||||
"")
|
||||
echo "bootstrap: PATH resolves no golangci-lint at all." \
|
||||
"Add $gcl_bin to PATH, then re-run bootstrap." >&2
|
||||
;;
|
||||
"$gcl_bin"/*)
|
||||
echo "bootstrap: PATH resolves $gcl_found, inside that same" \
|
||||
"directory, reporting version ${gcl_got:-unparseable}." \
|
||||
"Nothing is shadowing it, so the install itself did not" \
|
||||
"produce the pinned version: check that" \
|
||||
"GOLANGCI_LINT_VERSION matches GOLANGCI_LINT_REF." >&2
|
||||
;;
|
||||
*)
|
||||
echo "bootstrap: PATH resolves $gcl_found instead, reporting" \
|
||||
"version ${gcl_got:-unparseable}. Remove that binary or" \
|
||||
"put $gcl_bin earlier in PATH, then re-run bootstrap." >&2
|
||||
;;
|
||||
esac
|
||||
exit 1
|
||||
}
|
||||
|
||||
# The definitions above are inert on their own; the call site is part of
|
||||
# the canonical form. In a script/bootstrap that follows the "define all
|
||||
# functions, then call main" convention, this line belongs inside main()
|
||||
# next to the other ensure_* steps.
|
||||
ensure_golangci_lint
|
||||
```
|
||||
|
||||
Four properties are load-bearing; each guards a failure mode that otherwise
|
||||
fails green:
|
||||
- **Compare the installed version against the pin**, never test presence.
|
||||
This is what makes a version bump propagate to machines that already have
|
||||
some golangci-lint. Compare the **whole** version token, exactly: a parser
|
||||
that stops at the first `-` reports `2.12.2` for a host running
|
||||
`2.12.2-rc1`, which compares equal to a `2.12.2` pin and skips the install
|
||||
— the original defect, reintroduced through the comparison meant to fix
|
||||
it.
|
||||
- **After installing, re-resolve the binary the way callers resolve it** —
|
||||
through `PATH`, not the path `go install` wrote to — and assert
|
||||
`--version` reports the pin. When it does not, fail non-zero and name the
|
||||
path `command -v` actually found, the version it reports, and the
|
||||
directory the install wrote to. That is a condition a human has to fix by
|
||||
hand, so bootstrap must not print success in it. Use `hash -r` first so
|
||||
the shell does not answer from its own lookup cache. Diagnose the cause
|
||||
from the resolved path rather than asserting one: only a path **outside**
|
||||
the install directory is shadowing. When the resolved path is inside it,
|
||||
nothing is shadowing and telling the operator to delete that binary or
|
||||
reorder `PATH` sends them after a fault that does not exist.
|
||||
- **A mis-parse must fall through to reinstall, never to a false match.**
|
||||
Absent binary, non-zero exit, empty output, and unrecognised output all
|
||||
yield an empty string, which compares unequal to the pin. The failure
|
||||
direction is always a redundant install, never a skipped one.
|
||||
- **Call it, and say so on success.** Two function definitions with no call
|
||||
site are a silent no-op that reproduces the original defect exactly: exit
|
||||
0, nothing installed, no output, stale linter still resolved. A success
|
||||
path that prints nothing is byte-identical to that no-op — same exit
|
||||
status, same empty output — so both success branches must print a
|
||||
confirmation naming the version. In a change about undetectable no-ops,
|
||||
"it printed nothing and exited 0" must not be the healthy signal.
|
||||
|
||||
Keep it POSIX sh: no bashisms, no arrays, no `[[`, no `grep -P`.
|
||||
|
||||
**On the hash-pinning rule.** `@c0d3ddc9cf3faa61a4e378e879ece580256d76e5` is
|
||||
a commit hash, not a server-mutable version tag, and the go command verifies
|
||||
the fetched module against the checksum database — the mechanism the
|
||||
hash-pinning rule at the top of this document already names as acceptable
|
||||
for Go modules. Note that `go install pkg@version` runs in module-aware mode
|
||||
ignoring the `go.mod` in the current directory or any parent, so no repo
|
||||
`go.sum` is consulted for this install; the checksum database is what
|
||||
verifies it. The linter is a bootstrap prerequisite rather than part of any
|
||||
repo's module graph, which is why the canonical form installs it by
|
||||
commit-pinned ref instead of declaring it in `go.mod`. Whether a `go.mod`
|
||||
tool dependency — which would pin the hash in a committed, reviewable file
|
||||
instead — should replace this is an open decision, tracked at
|
||||
[prompts#37](https://git.eeqj.de/sneak/prompts/issues/37).
|
||||
|
||||
**Keep `GOLANGCI_LINT_VERSION` and the ref in sync.** The ref is a hash and
|
||||
carries no readable version, so the expected version is a separate string,
|
||||
and it must be exactly what `--version` prints for that ref — the comparison
|
||||
is an exact match on the whole version token. When the pinned commit carries
|
||||
a release tag the go command resolves the hash to that tag, so the string is
|
||||
simply the release number, `2.12.2` here. When it does not, the go command
|
||||
falls back to a pseudo-version and the binary reports something like
|
||||
`2.12.3-0.20260506110758-c0d3ddc9cf3f`; that compares exactly like any other
|
||||
string, so it works, but it cannot be known without building the binary once
|
||||
and reading `--version` off it. Prefer pins on tagged releases for that
|
||||
reason — the expected string is then derivable from the ref — not because
|
||||
the comparison cannot handle the alternative.
|
||||
|
||||
Because the comparison covers the whole token, a pre-release is never
|
||||
confused with its release: a host carrying `2.12.2-rc1` against a `2.12.2`
|
||||
pin compares unequal and gets reinstalled. This matters more than it looks,
|
||||
because a pre-release tag is still a tag, so a rule requiring merely that
|
||||
the pin be tagged would not catch it.
|
||||
|
||||
**Verifying a change to this logic requires a negative control run in an
|
||||
environment where a shadowing binary exists earlier in `PATH` than the
|
||||
install target.** Without that, the control passes against the naive
|
||||
compare-then-install form as well and therefore proves nothing. Also check
|
||||
the mis-parse direction by feeding it unparseable `--version` output and
|
||||
confirming it reinstalls rather than reporting a match.
|
||||
|
||||
**Run those controls against the block as a consuming repo would adopt it**
|
||||
— pasted into a `script/bootstrap`-shaped file that is then executed — not
|
||||
by sourcing it and invoking the function yourself. Driving the function
|
||||
directly tests something the artifact does not do, and it is exactly how a
|
||||
missing call site passes every control while the adopted snippet does
|
||||
nothing.
|
||||
|
||||
- When pinning images or packages by hash, add a comment above the reference
|
||||
with the version and date (YYYY-MM-DD).
|
||||
|
||||
|
||||
Reference in New Issue
Block a user