Compare commits
1 Commits
d173e69f85
...
be59376522
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
be59376522 |
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,166 @@ 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. The ref must resolve to a TAGGED
|
||||
# release; see the note on untagged pins below.
|
||||
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 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][0-9.]*\).*/\1/p' || true
|
||||
}
|
||||
|
||||
ensure_golangci_lint() {
|
||||
if [ "$(golangci_lint_version)" = "$GOLANGCI_LINT_VERSION" ]; then
|
||||
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
|
||||
return 0
|
||||
fi
|
||||
gcl_bin="$(go env GOBIN)"
|
||||
[ -n "$gcl_bin" ] || gcl_bin="$(go env GOPATH)/bin"
|
||||
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.
|
||||
- **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.** 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.
|
||||
|
||||
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`.
|
||||
|
||||
**The pin must be a tagged release commit.** `GOLANGCI_LINT_VERSION` is a
|
||||
separate string because the ref is a hash and carries no readable version,
|
||||
so the two must be updated together. It works out to a clean `2.12.2` only
|
||||
because that commit is the `v2.12.2` tag commit: the go command resolves a
|
||||
commit query to a semver tag on that commit when one exists, so the built
|
||||
binary reports `2.12.2`. Pin to an **untagged** commit and the go command
|
||||
falls back to a pseudo-version — the binary then reports something like
|
||||
`2.12.3-0.20260506110758-c0d3ddc9cf3f`, and this comparison cannot express
|
||||
it: the parser above stops at the first `-`, so the value to match would
|
||||
have to be the truncated numeric prefix `2.12.3`, which is also the number
|
||||
of a real future release. Setting `GOLANGCI_LINT_VERSION` to the full string
|
||||
`--version` prints can never match, and bootstrap would then reinstall on
|
||||
every run and exit 1 blaming a shadowing binary that does not exist. Keep
|
||||
pins on tagged releases.
|
||||
|
||||
**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