From 19f0585714695aa711b0a70e601ded7770118bd6 Mon Sep 17 00:00:00 2001 From: bigbes Date: Sun, 9 Aug 2026 01:32:26 +0300 Subject: [PATCH] ci: drop the 'all' from go mod download, which rewrote go.sum MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cache warm-up was 'go mod download all'. That resolves the entire module graph, test dependencies of dependencies included, and appends their hashes to go.sum — 145 lines in this repository, measured in a fresh clone of this branch. Which makes it the bug this wave exists to remove. A modified tracked file in the checkout when go build runs is what stamps vcs.modified into the binary, the same failure the version task avoids by not sed-ing the APKBUILD; the warm-up would have reintroduced it one task later. The bench sibling proved it first: build #359 stopped at check-version with a -dirty binary and 'M go.sum' named as the thing in the way, 228 lines there. Without 'all' go.sum is untouched, go mod verify still passes, and what is fetched is what the main module builds, which is all a warm-up needs. Also removes the sentence that let this through. Both the manifest and docs/ci.md claimed -mod=readonly is the default so neither command can rewrite go.mod or go.sum. It is false, and it sat directly above the line that disproved it. readonly governs updates to the module requirements, not writes to go.sum: measured here with the flag set explicitly on the command line, 'go mod download all' still appends the same 145 lines and still exits 0, and go mod verify passes after it, so verification guards nothing either. Setting the flag on the command line is the point. This machine carries go env -w GOFLAGS=-mod=mod, as bench's did, so a run that omits it proves nothing about the builder. What remains is the git status --porcelain at the end of the task, and it is now documented as the check rather than as belt-and-braces: it is the only thing that would notice a go.sum rewrite before the compile, and on bench it is what did. docs/ci.md warns against restoring the 'all' and against replacing the print with an appeal to a flag. compare's README makes no claim about module hygiene in CI, so nothing to correct there --- .build.yml | 22 +++++++++++++----- docs/ci.md | 65 +++++++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 76 insertions(+), 11 deletions(-) diff --git a/.build.yml b/.build.yml index 2d62dbd1b648cdf9b991761e731003cbc36b2bfd..218241f6679655510f933d59e7c7095c8ccfd7c8 100644 --- a/.build.yml +++ b/.build.yml @@ -115,12 +115,24 @@ tasks: echo "restored module cache did not verify — discarding it" rm -rf ~/go/pkg/mod fi - go mod download all + # `go mod download`, and NOT `go mod download all` — do not add the `all` + # back as an optimization. It resolves the whole module graph, including + # test dependencies of dependencies, and APPENDS their hashes to go.sum: + # 145 lines in this repository, 228 in the bench sibling, whose build #359 + # packaged a "-dirty" binary because of it. A modified tracked file in the + # checkout at `go build` time is the exact stamp this pipeline exists to + # prevent. Without `all` go.sum is untouched and `go mod verify` still + # passes. See docs/ci.md#not-go-mod-download-all. + go mod download go mod verify - # -mod=readonly is the default, so neither line above can rewrite go.mod - # or go.sum — but an untracked or modified file here is a "+dirty" stamp - # in the packaged binary, so say so out loud rather than trusting the - # flag. docs/ci.md#cache_restore. + # This print is NOT belt-and-braces. It is the ONLY thing here that + # catches a go.sum rewrite, and no flag stands behind it. Do not replace + # it with an appeal to -mod=readonly: readonly governs updates to the + # module REQUIREMENTS, it does not stop writes to go.sum. Measured with + # GOFLAGS=-mod=readonly explicitly on the command line — `go mod download + # all` still appended the same 145 lines and still exited 0. A modified or + # untracked file here is a "+dirty" stamp in the packaged binary. + # docs/ci.md#not-go-mod-download-all. git status --porcelain - test: | cd "$REPO" diff --git a/docs/ci.md b/docs/ci.md index 4c0753e69af0cb96e1fbbcbfa750654a046388fc..661fcf99717799c151266400a6d449c6547a98ea 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -166,16 +166,69 @@ in the code under test. So: make the tree writable (the module cache is mode 555 and `rm -rf` cannot remove it otherwise), ask `go mod verify` whether what came back is intact, and -throw the whole thing away if it is not. Then `go mod download all` and verify -for real, and let *that* failure be fatal. +throw the whole thing away if it is not. Then `go mod download` and verify for +real, and let *that* failure be fatal. Do not soften the final `go mod verify` to `|| true`. A build that proceeds with a module cache it could not verify is a build whose result means nothing. -The second `git status --porcelain` is there because `-mod=readonly` is the -default and neither command *can* rewrite `go.mod` or `go.sum` — but a dirty -tree at this point is a `+dirty` apk, so it is said out loud rather than trusted -to a flag. +### Not `go mod download all` + +The warm-up is `go mod download`. **The `all` must not come back**, and it is +worth knowing why, because `all` is exactly the shape a future reader adds as an +obvious improvement — warm *everything*, surely, so nothing is fetched later. + +`go mod download all` resolves the entire module graph, test dependencies of +dependencies included, and **appends the hashes of everything it resolved to +`go.sum`**. Measured on this tree: 145 lines added. In the bench sibling, 228 — +and that is not a hypothetical, it is what failed its build #359, which stopped +at `check-version` with a `-dirty` binary and `M go.sum` named as the thing in +the way. + +A modified tracked file in the checkout when `go build` runs is precisely the +stamp this whole pipeline is arranged to avoid — the same failure the `version` +task refuses to cause by not `sed`-ing the APKBUILD. Warming the cache with a +command that edits the tree would have reintroduced it one task later. + +Without `all`, `go.sum` is untouched, `go mod verify` still passes afterwards, +and what gets fetched is what the main module actually builds. That is all a +cache warm-up needs; the packages `all` would have added are ones no build in +this pipeline compiles. + +Verified in a fresh clone of this branch — `go mod download` followed by +`git status --porcelain` leaves the tree clean. compare has the smallest +dependency graph of the six services, so this is where `all` came closest to +being harmless, and it still rewrote 145 lines. + +### `-mod=readonly` does not save you + +There used to be a sentence here, and another above the `git status` line in the +manifest, saying `-mod=readonly` is the default so neither command *can* rewrite +`go.mod` or `go.sum`. **It was false**, and it is a large part of why nobody +looked at `go mod download all` for as long as they did: it sat directly above +the line that disproved it and invited the reader to skip both. + +`readonly` governs updates to the module **requirements**. It does not stop +writes to `go.sum`. Measured on this tree rather than inherited, with the flag +set explicitly on the command line: + +| command | `go.sum` | exit | +|---|---|---| +| `GOFLAGS=-mod=readonly go mod download all` | **+145 lines** | 0 | +| `GOFLAGS=-mod=readonly go mod download` | untouched | 0 | + +Both still pass `go mod verify` afterwards, so verification is no guard either. + +Set the flag **on the command line** when reproducing this. `go env -w` writes a +persistent `GOFLAGS`, and a machine carrying `go env -w GOFLAGS=-mod=mod` — this +one does, and bench's did too — makes a run that omits the flag prove nothing +about what the builder would do. Check with `go env GOFLAGS` first. + +So the `git status --porcelain` at the end of the task is not belt-and-braces, +and no flag stands behind it: **it is the check**. It is the only thing in this +pipeline that would notice a `go.sum` rewrite before the compile, and on the +bench sibling it is what did. Do not delete it as redundant and do not replace +it with an appeal to a flag. ## test