From 329c03f06e019a65eaaa85c29d4d0a4389b6305d Mon Sep 17 00:00:00 2001 From: sneak Date: Mon, 10 Aug 2026 12:55:40 +0000 Subject: [PATCH] build: make the lint cache-busting self-validating in script/lint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `--no-cache-filter=lint` is silently ignored by BuildKit when no stage matches the name, so the entire anti-false-green mechanism hung on one unvalidated magic string: renaming or mistyping the `lint` stage would have left the lint layer served from cache and `script/lint` reporting green having linted nothing. Reproduced here — with the filter pointed at a nonexistent stage and no `--target`, an unchanged tree built with `RUN golangci-lint run ... CACHED` and exited 0. `--target lint` closes it: a stage name that does not exist now fails loudly (`target stage "nosuchstage" could not be found`, exit 1) instead of passing. The two flags name the same stage from the same string and validate each other; both the script and the stage definition in Dockerfile.lint carry a comment saying they must be kept in sync. `--output=type=cacheonly` drops the image export. Nothing consumes the image — the deliverable of this build is an exit code — and the export cost seconds per run and left one dangling image behind every time, on a host where pruning is prohibited. The lint stage still executes and a lint failure still exits non-zero, both verified rather than assumed. TODO.md: the 2026-08-07 entry's claim that the repo has no linter pin and lints on the host is marked superseded in place rather than rewritten; the narrowed scaffold exemption now names `.dockerignore` alongside `Dockerfile.lint` and `script/lint`; and the specific wall-clock timings are replaced by the durable property they were evidence for, since they vary per host and per run. --- Dockerfile.lint | 2 ++ TODO.md | 38 ++++++++++++++++++++++++++++---------- script/lint | 20 +++++++++++++++++++- 3 files changed, 49 insertions(+), 11 deletions(-) diff --git a/Dockerfile.lint b/Dockerfile.lint index dfcafab..443f421 100644 --- a/Dockerfile.lint +++ b/Dockerfile.lint @@ -19,6 +19,8 @@ WORKDIR /src COPY go.mod go.sum ./ RUN go mod download +# Do not rename this stage without changing script/lint in the same commit: +# it passes both --target lint and --no-cache-filter=lint by this name. FROM deps AS lint # Copy source code diff --git a/TODO.md b/TODO.md index ab7646b..7e104e0 100644 --- a/TODO.md +++ b/TODO.md @@ -58,10 +58,24 @@ is finished. Verified rather than assumed, since a green docker build is the classic false green: two consecutive runs on an unchanged tree each showed the - `golangci-lint run` layer executing (11.6s and 9.7s, both `0 issues.`) while - the `deps` layers reported `CACHED`, and a deliberate `indent-error-flow` + `golangci-lint run` layer executing and reporting `0 issues.` while the + `deps` layers reported `CACHED`, and a deliberate `indent-error-flow` violation failed the build naming that finding plus the `unused` one before - a revert went clean again. + a revert went clean again. The durable property to check when touching any + of this is that the lint stage executes on every run and is never served + from cache; wall-clock durations vary per host and per run, so they are not + recorded here. + + Hardened 2026-08-10 after review. `script/lint` now also passes + `--target lint` and `--output=type=cacheonly`. `--target` is what makes + `--no-cache-filter=lint` trustworthy: BuildKit silently ignores the filter + when no stage matches the name, so a rename or typo of the `lint` stage + would have left the lint layer cached and `script/lint` green having linted + nothing — the same false green in a new place. `--target` fails loudly on a + name that does not exist, so the two flags validate each other and must be + kept in sync. `--output=type=cacheonly` skips the image export: nothing + consumes the image (the deliverable is an exit code), and exporting it cost + seconds per run and left a dangling image behind each time. - 2026-08-09 `TestAutoSaveOnSignalRacesTurnLoop` de-flaked at the cause (`fix/autosave-turn-budget-36`, closes #36). The failure text was captured @@ -733,7 +747,11 @@ is finished. 24 long lines wrapped or their comments tightened, control bytes in `term/tcell.go` as character literals, and two `wsl_v5` defer cuddles. The repo has no golangci-lint version pin to bump (no Dockerfile or CI; - `make lint` runs whatever `golangci-lint` is on the host). + `make lint` runs whatever `golangci-lint` is on the host). Superseded + 2026-08-10: there is a pin now, and no host lint path — `Dockerfile.lint` pins + the linter image by digest and `script/lint` runs it in a container. See the + 2026-08-10 entry at the top of this section + (https://git.eeqj.de/sneak/rgoue/issues/41). - 2026-07-24 Seed compatibility — item tables (seed-compat): instrumented the C reference on modern-rogue with a DUMP mode (testdata/c_seedcompat.patch) that @@ -885,9 +903,9 @@ is finished. exemption is narrower than it was. A minimal dev Makefile (fmt/fmt-check/lint/test/check targets) exists per sneak's 2026-07-07 request. `Dockerfile.lint` and `script/lint` are now also permitted, and - required: sneak's 2026-08-09 ruling - (https://git.eeqj.de/sneak/rgoue/issues/41) is that every repo lints in a - container invoked through `script/lint`, and being later and explicit it - overrides the 2026-07-07 exemption for those two files only. Still do not - add: CI config, `REPO_POLICIES.md`, an application `Dockerfile`, or any other - `script/` entrypoint. + required, along with the `.dockerignore` that scopes their build context: + sneak's 2026-08-09 ruling (https://git.eeqj.de/sneak/rgoue/issues/41) is that + every repo lints in a container invoked through `script/lint`, and being + later and explicit it overrides the 2026-07-07 exemption for those three + files only. Still do not add: CI config, `REPO_POLICIES.md`, an application + `Dockerfile`, or any other `script/` entrypoint. diff --git a/script/lint b/script/lint index 5e3b1b4..b0ad2cf 100755 --- a/script/lint +++ b/script/lint @@ -8,13 +8,31 @@ # --no-cache-filter=lint forces the lint stage to re-execute every run, so # an unchanged tree is still actually linted; the deps stage keeps its # cache, so the module download is not repeated. +# +# --target lint and --no-cache-filter=lint must BOTH be present, and both +# must keep naming the stage that Dockerfile.lint calls `lint`. Do not +# "simplify" either one away. BuildKit silently ignores --no-cache-filter +# when no stage matches the name: rename or typo the stage and the filter +# becomes a no-op, the lint layer is served from cache, and script/lint +# reports green having linted nothing — the exact false green this whole +# setup exists to prevent. --target fails loudly on a name that does not +# exist, so the two flags validate each other's magic string. +# +# --output=type=cacheonly skips the image export. Nothing consumes the +# image — the deliverable of this build is an exit code — and exporting it +# costs seconds per run and leaves a dangling image behind every time. The +# lint stage still executes and a lint failure still exits non-zero. set -eu ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" main() { cd "$ROOT" - docker build --no-cache-filter=lint -f Dockerfile.lint . + docker build \ + --target lint \ + --no-cache-filter=lint \ + --output=type=cacheonly \ + -f Dockerfile.lint . } main "$@"