Run all linting in Docker via Dockerfile.lint (closes #46)
All checks were successful
check / check (push) Successful in 1m15s
All checks were successful
check / check (push) Successful in 1m15s
Per the owner ruling, the linter runs inside a container invoked through the script/ entrypoint and is never installed on a host. A new root Dockerfile.lint COPYs the repo into the digest-pinned golangci/golangci-lint:v2.12.2 image and runs `golangci-lint config verify` and `golangci-lint run` as build steps, so a successful build IS a clean lint. script/lint is reduced to building it, which also works when the docker daemon is remote and bind mounts are impossible. script/bootstrap loses the `go install`, the pin constants, the version parser and verify_golangci_lint outright rather than being hardened: with nothing linting on the host, the $GOPATH/bin versus PATH shadowing problem those existed to diagnose has no subject. It keeps the git/make/go presence checks and `go mod download`, and warns rather than fails when docker is absent. Two traps. A lint build on an unchanged tree returns success in well under a second having run no linter, which is #32 and #39 over again. Caching is waived by ruling, so Dockerfile.lint carries ARG CHECK_EPOCH referenced inside every gate RUN -- BuildKit hashes the expanded command, not the declaration, so a declared but unreferenced ARG invalidates nothing -- and script/lint passes "$(date +%s)-$$". The PID is in that value because two lint runs land inside the same second easily and a bare epoch would cache the second one. Nothing inside an image build may shell out to docker. The main Dockerfile's lint stage therefore invokes golangci-lint directly instead of `make lint`, and its build stage runs `make test` and `make fmt-check` instead of the `make check` aggregate, which reaches script/lint. Those two remain `make` invocations rather than the bare scripts because the Makefile's `export CGO_ENABLED = 0` only applies to what it invokes, and today's `make check` gets it. COPY --from=lint /usr/bin/golangci-lint is replaced by COPY --from=lint /src/go.sum /dev/null. The copied binary was the only edge forcing BuildKit to finish linting before the build stage starts; dropping it without replacing the edge would have ended fail-fast linting silently under a still-green build. That no-op copy is the ordering edge canonical REPO_POLICIES.md prescribes. Nothing in the build stage runs the linter any more, so the binary itself is not wanted there, and ENV PATH=/home/builder/go/bin:$PATH goes with the `go install` that justified it. script/verify-linter-pin is retired -- deleted along with its README entry -- because both of its subjects ceased to exist in this same change: it compared a linter binary against GOLANGCI_LINT_VERSION in script/bootstrap, and there is now neither a binary crossing between stages nor a version pin in bootstrap. The drift it guarded has not gone away, it has moved. The linter is still pinned twice, now as the FROM line of Dockerfile.lint and the FROM line of the Dockerfile lint stage, with nothing syncing them, which is exactly what #42 made a build failure. Its replacement is one new script/verify-lint-image-pin that compares those two references to each other and deliberately restates neither pin: a hardcoded expected digest would be a third copy and the same drift one file further out. It runs as a gate in both files, so `make lint`, `make check` and `make docker` all catch drift, and an unreadable reference is a hard failure rather than a vacuous pass. `golangci-lint config verify` is included per the ruling. The concern about its unpinned live HTTPS schema fetch was measured rather than assumed: under --network none the pinned binary both passes a valid config and rejects an invalid one with the jsonschema error, so it validates against a schema it embeds and linting needs no network beyond pulling the image. The README states that rather than a requirement that does not exist. Verified. `make lint` green with every PATH directory containing a golangci-lint removed and `command -v golangci-lint` empty. Two consecutive script/lint runs on an untouched tree both executed the linter, 27.7s and 28.7s in the lint step under distinct epochs with the COPY layer CACHED above them. A planted unused variable failed script/lint with that exact finding and failed `make docker` at the lint stage with the build stage stopped before its COPY --from=lint, then reverted clean. The drift guard fails on tag-only, digest-only and unreadable-reference cases, naming both sides. `make check` green; `make docker` green in 5m35s with all six gates executing and the test gate reporting real coverage rather than a cached ok. In the builder image with the Go test cache off, --user 0:0 still fails TestScanHardlinkRunFailsTogether where the unprivileged user passes, so the non-root quirk is intact.
This commit is contained in:
82
TODO.md
82
TODO.md
@@ -29,6 +29,88 @@
|
||||
|
||||
# Completed Steps
|
||||
|
||||
- run all linting in Docker via `Dockerfile.lint` and `script/lint`
|
||||
(2026-08-10, branch `next`, closes
|
||||
https://git.eeqj.de/sneak/sfdupes/issues/46): per the owner ruling, the
|
||||
linter runs inside a container invoked through the `script/`
|
||||
entrypoint and is never installed on a host. New root
|
||||
`Dockerfile.lint` COPYs the repo into the digest-pinned
|
||||
`golangci/golangci-lint:v2.12.2` image and runs
|
||||
`golangci-lint config verify` and `golangci-lint run` as build
|
||||
steps, so a successful build IS a clean lint; `script/lint` is
|
||||
reduced to building it. `script/bootstrap` loses the `go install`,
|
||||
the pin constants, the version parser and `verify_golangci_lint`
|
||||
outright rather than hardening them — with nothing linting on the
|
||||
host, the `$GOPATH/bin` versus `PATH` problem that motivated them has
|
||||
no subject — and now warns rather than fails when `docker` is absent.
|
||||
Two traps handled. A lint build on an unchanged tree returns success
|
||||
in well under a second having run no linter, which is
|
||||
https://git.eeqj.de/sneak/sfdupes/issues/32 and
|
||||
https://git.eeqj.de/sneak/sfdupes/issues/39 again, so
|
||||
`Dockerfile.lint` carries `ARG CHECK_EPOCH` referenced
|
||||
inside every gate `RUN` (BuildKit hashes the expanded command, not
|
||||
the declaration) and `script/lint` passes `"$(date +%s)-$$"` — the
|
||||
PID matters because two lint runs land inside the same second easily.
|
||||
And nothing inside an image build may shell out to docker, so the
|
||||
main `Dockerfile`'s lint stage now invokes `golangci-lint` directly
|
||||
instead of `make lint`, and its build stage runs `make test` and
|
||||
`make fmt-check` instead of the `make check` aggregate (`make`, not
|
||||
the scripts bare, because the Makefile's `export CGO_ENABLED = 0`
|
||||
only reaches what it invokes). `COPY --from=lint`
|
||||
`/usr/bin/golangci-lint` is replaced by
|
||||
`COPY --from=lint /src/go.sum /dev/null`: the copied binary was the
|
||||
only edge forcing BuildKit to finish linting before the build stage
|
||||
starts, and dropping it without replacing the edge would have ended
|
||||
fail-fast linting silently under a still-green build. That is
|
||||
canonical `REPO_POLICIES.md:107`'s ordering edge, restored.
|
||||
`ENV PATH=/home/builder/go/bin:$PATH` is gone with the `go install`
|
||||
that justified it. `script/verify-linter-pin` is retired, deleted
|
||||
along with its README entry, because both of its subjects ceased to
|
||||
exist in the same change: it compared a linter binary against
|
||||
`GOLANGCI_LINT_VERSION` in `script/bootstrap`, and there is now
|
||||
neither a binary crossing between stages nor a version pin in
|
||||
bootstrap. The drift it guarded has not gone away, it has moved — the
|
||||
linter is still pinned twice, now as the `FROM` line of
|
||||
`Dockerfile.lint` and the `FROM` line of the `Dockerfile` lint stage,
|
||||
with nothing syncing them, which is exactly what
|
||||
https://git.eeqj.de/sneak/sfdupes/issues/42 made a build failure. Its
|
||||
replacement is one new `script/verify-lint-image-pin`,
|
||||
run as a gate in both files, which compares the two references to
|
||||
each other and deliberately restates neither: a hardcoded expected
|
||||
digest would be a third copy and the same drift one file further out.
|
||||
`golangci-lint config verify` is included per the ruling, and the
|
||||
concern about its unpinned live HTTPS schema fetch was measured
|
||||
rather than assumed — under `--network none` the pinned binary both
|
||||
passes a valid config and rejects an invalid one with the jsonschema
|
||||
error, so it validates from an embedded schema and makes no network
|
||||
call of its own. The README scopes that to the gate steps rather
|
||||
than to linting as a whole: `Dockerfile.lint` runs `go mod download`
|
||||
above them, so a cold cache still needs the network and only a warm
|
||||
one lints offline. Verified: `make lint` green with every `PATH`
|
||||
directory containing a `golangci-lint` removed
|
||||
(`/home/user/go/bin`, `/home/user/.local/bin`, `/usr/local/bin`;
|
||||
`command -v golangci-lint` empty); two consecutive `script/lint` runs
|
||||
on an untouched tree both executed the linter, 27.7s and 28.7s in the
|
||||
lint step under distinct epochs with the `COPY . .` layer `CACHED`
|
||||
above them, at 42.2s and 41.8s wall clock — the no-cache rule was not
|
||||
weakened to shorten that. Negative control: a planted
|
||||
`var unusedIssue46Sentinel = 1` failed `script/lint` with
|
||||
`report.go:173:5: var unusedIssue46Sentinel is unused (unused)`, and
|
||||
failed `make docker` at `[lint 9/9]` with the build stage stopped at
|
||||
`[builder 3/12]` — `COPY --from=lint`, `script/bootstrap`, the test
|
||||
gate and `make build` all zero occurrences — then reverted clean. The
|
||||
drift guard fails on a tag-only disagreement, on a digest-only
|
||||
disagreement, and on an unreadable reference, naming both sides.
|
||||
`make docker` green in 5m35s with all six gates executing under one
|
||||
epoch (lint 37.6s, test 25.2s reporting
|
||||
`ok sneak.berlin/go/sfdupes 1.938s coverage: 88.5%`, not `(cached)`).
|
||||
The non-root quirk still holds: in the builder image with the Go test
|
||||
cache off, `--user 0:0` fails `TestScanHardlinkRunFailsTogether`
|
||||
(exit 1) where the unprivileged user passes (exit 0). Noted for
|
||||
follow-up, not fixed here: `golangci-lint` warns that the
|
||||
`gomodguard` linter is deprecated since v2.12.0 in favour of
|
||||
`gomodguard_v2`.
|
||||
|
||||
- install the Docker build stage's prerequisites by running
|
||||
`script/bootstrap` instead of `apk add --no-cache make` inline
|
||||
(2026-08-09, branch `dockerfile-bootstrap`, closes #42): canonical
|
||||
|
||||
Reference in New Issue
Block a user