diff --git a/Dockerfile.lint b/Dockerfile.lint index 443f421..28febae 100644 --- a/Dockerfile.lint +++ b/Dockerfile.lint @@ -19,8 +19,13 @@ 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. +# This stage must stay the one that runs golangci-lint, and its name must +# match $stage in script/lint, which passes that single name to both +# --target and --no-cache-filter. Renaming here without updating script/lint +# fails the build loudly (--target rejects a name that is not in this file), +# so a mismatch cannot pass silently — but moving the lint step to another +# stage, or adding a stage after this one, would not be caught. Change the +# two files together. FROM deps AS lint # Copy source code diff --git a/TODO.md b/TODO.md index 7e104e0..b979a44 100644 --- a/TODO.md +++ b/TODO.md @@ -66,16 +66,30 @@ is finished. 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. + Hardened 2026-08-10 after review. `script/lint` now also passes `--target` + and `--output=type=cacheonly`. `--target` is what makes `--no-cache-filter` + trustworthy: BuildKit silently ignores the filter when no stage matches its + argument, 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 is not in the + file. `--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. + + Corrected 2026-08-10 after a second review, which was right to reject the + claim first made here that the two flags "validate each other". They did + not: `--target` validates only its own argument, so a typo confined to + `--no-cache-filter` still built `CACHED` at exit 0 — the original defect, + surviving in the narrow case. The duplicated stage name was the defect, so + it is now written once, as `stage=lint` in `script/lint`, and passed to both + flags. The true property is that there is only one name to get wrong, and + `--target` rejects it loudly if it is not a stage in `Dockerfile.lint`, so a + typo or a stale rename is a hard error rather than a silent skip. What + remains on the editor, and is not checked by anything: `$stage` must name + the stage that actually runs `golangci-lint`. `--target` verifies the name + exists, not that it is the right stage, and it stops the build there — so + moving the lint step to another stage, or adding a stage after it, would go + unnoticed. - 2026-08-09 `TestAutoSaveOnSignalRacesTurnLoop` de-flaked at the cause (`fix/autosave-turn-budget-36`, closes #36). The failure text was captured diff --git a/script/lint b/script/lint index b0ad2cf..94d31bf 100755 --- a/script/lint +++ b/script/lint @@ -5,18 +5,28 @@ # image and lints as a build step. This works even when the docker daemon # is remote and bind mounts are impossible. # -# --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. +# --no-cache-filter 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. Both flags below must stay: do +# not "simplify" either one away. # -# --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. +# The stage name is written ONCE, in $stage, and passed to both --target +# and --no-cache-filter, so the two flags cannot come to name different +# stages. That is the whole point of the variable. BuildKit silently +# ignores --no-cache-filter when no stage matches its argument: a filter +# naming a stage that does not exist is a no-op, the lint layer is served +# from cache, and script/lint reports green having linted nothing — the +# false green this setup exists to prevent. --target, by contrast, fails +# loudly on a name that is not in the file. With a single shared name, a +# typo or a stale rename therefore becomes a hard error instead of a +# silent skip, because the one name reaches both flags. +# +# What the tooling does NOT check, and is left to whoever edits this: +# $stage must name the stage in Dockerfile.lint that actually runs +# golangci-lint. --target verifies that the name exists, not that it is +# the right stage, and it stops the build at that stage — so moving the +# lint step into a different stage, or adding a stage after this one, +# would not be caught here. Keep this file and Dockerfile.lint in sync. # # --output=type=cacheonly skips the image export. Nothing consumes the # image — the deliverable of this build is an exit code — and exporting it @@ -26,11 +36,14 @@ set -eu ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" +# Must match the stage name in Dockerfile.lint. +stage=lint + main() { cd "$ROOT" docker build \ - --target lint \ - --no-cache-filter=lint \ + --target "$stage" \ + --no-cache-filter="$stage" \ --output=type=cacheonly \ -f Dockerfile.lint . }