d709d88e79d71c3406e698e5543065fdccb1399c
4
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
d709d88e79 |
Merge main into next
check / check (push) Has been cancelled
Reconciles next with three merges it predated: the 60s/90s test budget, thin cmd/ entrypoints, and the .golangci.yml prohibition scoped to vendored copies. Conflicts: - prompts/REPO_POLICIES.md frontmatter: took main's later last_modified. - prompts/NEW_REPO_CHECKLIST.md: main's test-budget line, next's containerised-lint line. The two edits touch adjacent list items only. - prompts/REPO_POLICIES.md .golangci.yml bullet: both sides rewrote it. Took main's vendored-copy scoping and the change-by-re-vendoring sentence, and kept next's digest pin in Dockerfile.lint, which supersedes the host go install that main's text still describes. The test-budget text is main's verbatim on both sides of every conflict. |
||
|
|
0620416869 |
Give golangci-lint per-checkout cache and lock state (closes #30)
check / check (push) Successful in 7s
A golangci-lint result on a host running many concurrent workers does not reliably belong to the tree that asked for it. Two independent mechanisms, which have repeatedly been mistaken for one: The result cache is keyed 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 under the other tree's path. This produced a confirmed false green as well as the loud false reds. Moving workers from shared worktrees to their own clones does not address it — two clones collide exactly as two worktrees did — and removes only the foreign-path artefact that made the defect noticeable. The concurrency lock is $TMPDIR/golangci-lint.lock (pkg/commands/run.go, acquireFileLock), host-global and independent of GOLANGCI_LINT_CACHE, with a five-second acquire timeout, so it fails when the host is busiest. A private cache directory does not isolate it. Setting only the cache closes the contamination half and leaves runs failing red on a condition that is not a result at all. REPO_POLICIES.md now carries the canonical Go script/lint: both variables scoped into a .lint-cache/ directory inside the checkout, above any container-versus-host branch so every path reaching the linter gets them; --allow-serial-runners, which keeps the mutual-exclusion guard and queues rather than aborting, for the same-checkout overlap TMPDIR scoping cannot cover, with --allow-parallel-runners rejected because it deletes the guard; and a bounded retry that treats the lock error as VOID rather than as findings, exiting 75 on exhaustion so it is neither a pass nor a failure. Detection is on the stderr stream and never on exit status: findings go to stdout, so a finding quoting the lock message in source cannot be retried away, and the exit status is not a stable discriminator anyway. The interim void rule is recorded with the ../ clause that the original filter missed, and with its limit stated — it catches contamination that names foreign files, not contamination that suppresses findings. Both checklists gained the corresponding items, since a half-fix that sets only the cache reads as complete. GOCACHE was measured rather than assumed and does not need isolating: with the two variables scoped per checkout and GOCACHE shared at the host default, each checkout reported its own paths. Verified with the snippet extracted from the committed document and executed as a consuming repo would adopt it, each control paired against the pre-fix form: contamination reproduced on the pre-fix script and absent on the adopted one; a stub linter colliding twice then clearing, with the retry engaging and succeeding; exhaustion exiting 75 with a VOID message; a genuine finding whose text quotes the lock message reported as findings with no retry; and a real held lock failing the pre-fix script with exit 3 while the adopted script, inheriting the same environment, completed in one second. |
||
|
|
d173e69f85 |
Make the pinned golangci-lint actually reach the host (closes #28)
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. |
||
|
|
51c394552e |
Bust the Docker check-layer cache with a per-invocation CHECK_EPOCH (closes #26)
check / check (push) Successful in 7s
script/cibuild was a plain `docker build .`, and the Dockerfile does `COPY . .` followed by `RUN make check`. Docker invalidates a COPY layer only when the copied content changes, so on an unchanged tree the check layer was served from cache, the suite never ran, and the build still exited 0. Measured here: run 1 took 18.5s and ran the suite; run 2 on a byte-identical tree took 0.286s with `RUN make check` CACHED. script/cibuild and script/docker now assign a per-invocation nonce on its own line and pass it as --build-arg CHECK_EPOCH. The Dockerfile declares ARG CHECK_EPOCH, guards it with `[ -n "$CHECK_EPOCH" ] || exit 1`, and expands it into the check command. Post-fix, two consecutive runs both execute make check (17.4s / 8.1s) with `RUN script/bootstrap` still CACHED, so dependency layers are untouched and the build ceiling is not at risk. The guard is what makes a bare `docker build .` — the command REPO_POLICIES named verbatim — fail closed rather than reuse the empty and therefore stable cache key; verified failing in 0.455s. Holding the epoch constant restores the false green (run 2 fully CACHED), which pins the varying value as the operative mechanism rather than a coincidence. Because the guard references $CHECK_EPOCH it is itself value-keyed: BuildKit renders the epoch into that layer's description and re-runs the layer when the value changes. Each stage therefore has two independent invalidation points, the guard and the expansion, and the guard always precedes the check RUN. Both are kept and the prose now records this; the expansion remains defence in depth and is what puts the epoch in the build log. The false guarantee was org-canonical text in more than one document, so it is corrected everywhere it appeared rather than only where the issue first found it. REPO_POLICIES.md carried it in two places, and its Go multistage template had check steps in two stages; ARG is stage-scoped, so both stages get the treatment or the fleet inherits the half-fixed shape. CODE_STYLEGUIDE_GO.md restated the guarantee for the bare command this change makes fail closed. NEW_REPO_CHECKLIST.md specified the pre-fix script/cibuild verbatim, so every new repo would have been born with the false green, and EXISTING_REPO_CHECKLIST.md ended on a `docker build` acceptance item that the guard makes unsatisfiable by design — an agent working that checklist would have been led to delete the guard to tick the last box. Both checklists' Dockerfile criteria were also satisfiable by a Dockerfile whose check layers are still frozen, and now require the ARG and guard in every check-running stage. REPO_POLICIES.md's own Dockerfile criterion carried that same incomplete form; it is tightened by cross-reference to the CHECK_EPOCH rule rather than by duplicating the canonical block. The Go template's Key points gain a caveat that the cache-bust turns the `COPY --from=lint` no-op into a content-cache hit, so a repo using a file-dependency trick for stage ordering must re-prove that ordering on a warm cache after adopting it. That was re-proved in another repo in the org which uses the trick with a marker file, where the ordering held; the caveat states explicitly that it was not verified here, this repo being single-stage with no lint stage to order against. |