Merge next: run all linting in Docker (closes #41)
Pins golangci-lint by digest in Dockerfile.lint and removes the host lint path entirely, killing the false-green class that started the issue.
This commit was merged in pull request #43.
This commit is contained in:
9
.dockerignore
Normal file
9
.dockerignore
Normal file
@@ -0,0 +1,9 @@
|
|||||||
|
# The lint build reads the Go sources, go.mod/go.sum and .golangci.yml;
|
||||||
|
# none of that comes out of .git, so keep the build context small.
|
||||||
|
#
|
||||||
|
# This file is part of the lint gate, not housekeeping: only what reaches
|
||||||
|
# the container gets linted, so excluding a Go source here silently drops
|
||||||
|
# it from the lint (a self-contained file yields `0 issues.` at exit 0 with
|
||||||
|
# the violation still in the tree; it fails loudly only if other code still
|
||||||
|
# references it). Never exclude Go sources, go.mod/go.sum or .golangci.yml.
|
||||||
|
.git
|
||||||
39
Dockerfile.lint
Normal file
39
Dockerfile.lint
Normal file
@@ -0,0 +1,39 @@
|
|||||||
|
# Lint-only image: used by script/lint on machines where the docker
|
||||||
|
# daemon is remote (no bind mounts possible) — the repo is COPYed into
|
||||||
|
# the build context and golangci-lint runs as a build step, so a
|
||||||
|
# successful build means a clean lint.
|
||||||
|
#
|
||||||
|
# Two stages on purpose. script/lint builds with --no-cache-filter=lint so
|
||||||
|
# that the lint stage re-executes on every run, including on an unchanged
|
||||||
|
# tree (caching of the lint result is explicitly waived: a cached build
|
||||||
|
# lints nothing). Keeping `go mod download` in a separate `deps` stage
|
||||||
|
# means busting the lint stage does not also re-fetch the module cache
|
||||||
|
# over the network every time.
|
||||||
|
|
||||||
|
# golangci/golangci-lint:v2.12.2 (Debian-based), 2026-08-07
|
||||||
|
FROM golangci/golangci-lint:v2.12.2@sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240 AS deps
|
||||||
|
|
||||||
|
WORKDIR /src
|
||||||
|
|
||||||
|
# Copy go mod files first for better layer caching
|
||||||
|
COPY go.mod go.sum ./
|
||||||
|
RUN go mod download
|
||||||
|
|
||||||
|
# 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
|
||||||
|
COPY . .
|
||||||
|
|
||||||
|
# No `golangci-lint config verify` step here, deliberately (sneak/homoicon
|
||||||
|
# has one). It resolves its JSON schema over a live, unpinned HTTPS call:
|
||||||
|
# an unpinned network input inside the one step whose whole purpose is a
|
||||||
|
# pinned, reproducible gate, and a schema-host outage would show up as a
|
||||||
|
# red build. `golangci-lint run` already fails on a malformed config.
|
||||||
|
RUN golangci-lint run --config .golangci.yml ./...
|
||||||
16
Makefile
16
Makefile
@@ -1,8 +1,10 @@
|
|||||||
# Development convenience targets. This repo is exempt from the standard
|
# Development convenience targets. This repo is exempt from the standard
|
||||||
# policy scaffold (no Dockerfile, CI, or REPO_POLICIES.md); this Makefile
|
# policy scaffold (no CI config, no REPO_POLICIES.md, no application
|
||||||
# is only a thin wrapper around the Go toolchain, golangci-lint, and
|
# Dockerfile) except for the lint container: per sneak's 2026-08-09
|
||||||
# prettier so `make fmt` / `make check` behave the same as in sneak's
|
# ruling, linting runs in docker only, so Dockerfile.lint and script/lint
|
||||||
# other repos.
|
# are part of this repo. This Makefile is otherwise only a thin wrapper
|
||||||
|
# around the Go toolchain and prettier so `make fmt` / `make check` behave
|
||||||
|
# the same as in sneak's other repos.
|
||||||
|
|
||||||
GO_PKGS := ./...
|
GO_PKGS := ./...
|
||||||
MD_FILES := $(shell git ls-files '*.md')
|
MD_FILES := $(shell git ls-files '*.md')
|
||||||
@@ -26,9 +28,11 @@ fmt-check:
|
|||||||
fi
|
fi
|
||||||
$(PRETTIER) --check $(MD_FILES)
|
$(PRETTIER) --check $(MD_FILES)
|
||||||
|
|
||||||
# Run the house linter (config in .golangci.yml).
|
# Run the house linter. golangci-lint is never installed on the host: the
|
||||||
|
# work happens inside the pinned container built by Dockerfile.lint, and
|
||||||
|
# this target is a thin shim over the script that builds it.
|
||||||
lint:
|
lint:
|
||||||
golangci-lint run $(GO_PKGS)
|
./script/lint
|
||||||
|
|
||||||
# Run the test suite. Quiet on success; on failure, rerun verbosely for the
|
# Run the test suite. Quiet on success; on failure, rerun verbosely for the
|
||||||
# full output and still fail the target (the first run already proved the
|
# full output and still fail the target (the first run already proved the
|
||||||
|
|||||||
10
README.md
10
README.md
@@ -77,10 +77,12 @@ sequences, dungeon-generation golden checks, and an RNG compatibility test
|
|||||||
against the original C generator.
|
against the original C generator.
|
||||||
|
|
||||||
For development, the `Makefile` wraps the toolchain: `make fmt` (gofmt +
|
For development, the `Makefile` wraps the toolchain: `make fmt` (gofmt +
|
||||||
prettier), `make lint` (golangci-lint), `make test` (the suite, under the race
|
prettier), `make lint` (`script/lint`, which runs golangci-lint inside the
|
||||||
detector with coverage and a timeout), and `make check` (all three). Use the
|
pinned container built from `Dockerfile.lint` — it is never installed on the
|
||||||
targets rather than invoking `go test` directly — they carry the flags the
|
host, so docker is required), `make test` (the suite, under the race detector
|
||||||
project relies on.
|
with coverage and a timeout), and `make check` (all three). Use the targets
|
||||||
|
rather than invoking `go test` directly — they carry the flags the project
|
||||||
|
relies on.
|
||||||
|
|
||||||
## License
|
## License
|
||||||
|
|
||||||
|
|||||||
81
TODO.md
81
TODO.md
@@ -35,6 +35,67 @@ is finished.
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
|
- 2026-08-10 Linting moved into a container
|
||||||
|
(https://git.eeqj.de/sneak/rgoue/issues/41). `golangci-lint` is no longer
|
||||||
|
invoked on the host anywhere in the repo: `Dockerfile.lint` pins
|
||||||
|
`golangci/golangci-lint:v2.12.2` by digest and runs the linter as a build
|
||||||
|
step, so a successful build is a clean lint, and `make lint` is now a shim
|
||||||
|
over `script/lint`. This is what killed the false green seen earlier, where a
|
||||||
|
branch that was genuinely red with a `goconst` finding reported `0 issues` off
|
||||||
|
the shared host cache; a container per run has its own cache and lock.
|
||||||
|
|
||||||
|
Two deliberate divergences from the `sneak/homoicon` reference. The image
|
||||||
|
has two stages rather than one — a cached `deps` stage holding
|
||||||
|
`go mod download`, then `FROM deps AS lint` with the copy and the lint run —
|
||||||
|
and `script/lint` builds with `--no-cache-filter=lint`. Caching of the lint
|
||||||
|
result is explicitly waived (a cached build lints nothing), and splitting
|
||||||
|
the stages means busting the lint layer does not also re-fetch the module
|
||||||
|
cache over the network on every run. And `golangci-lint config verify` is
|
||||||
|
left out: it resolves its JSON schema over a live, unpinned HTTPS call,
|
||||||
|
which is an unpinned network input inside the one step whose purpose is a
|
||||||
|
pinned reproducible gate, and a schema-host outage would surface as a red
|
||||||
|
build. `golangci-lint run` already fails on a malformed config.
|
||||||
|
|
||||||
|
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 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. 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`
|
||||||
|
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. The same applies to `.dockerignore`, which is part of this gate
|
||||||
|
rather than housekeeping: only what reaches the container is linted, so
|
||||||
|
excluding a Go source there drops it from the lint silently — verified, a
|
||||||
|
planted violation plus that one path in `.dockerignore` gives `0 issues.` at
|
||||||
|
exit 0 with the violation still in the tree, and it fails loudly only when
|
||||||
|
other code still references the excluded file.
|
||||||
|
|
||||||
- 2026-08-09 `TestAutoSaveOnSignalRacesTurnLoop` de-flaked at the cause
|
- 2026-08-09 `TestAutoSaveOnSignalRacesTurnLoop` de-flaked at the cause
|
||||||
(`fix/autosave-turn-budget-36`, closes #36). The failure text was captured
|
(`fix/autosave-turn-budget-36`, closes #36). The failure text was captured
|
||||||
before anything was changed and it is **not** a data race: the assertion was
|
before anything was changed and it is **not** a data race: the assertion was
|
||||||
@@ -705,7 +766,11 @@ is finished.
|
|||||||
24 long lines wrapped or their comments tightened, control bytes in
|
24 long lines wrapped or their comments tightened, control bytes in
|
||||||
`term/tcell.go` as character literals, and two `wsl_v5` defer cuddles. The
|
`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;
|
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
|
- 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
|
reference on modern-rogue with a DUMP mode (testdata/c_seedcompat.patch) that
|
||||||
@@ -853,7 +918,13 @@ is finished.
|
|||||||
per-game dungeon dimensions instead of the 80x24 constants; open design
|
per-game dungeon dimensions instead of the 80x24 constants; open design
|
||||||
questions are resize policy, gameplay tuning at larger sizes, and a --classic
|
questions are resize policy, gameplay tuning at larger sizes, and a --classic
|
||||||
80x24 mode.
|
80x24 mode.
|
||||||
2. Note: this repo is exempt from the standard policy scaffold. A minimal dev
|
2. Note: this repo is exempt from the standard policy scaffold, but the
|
||||||
Makefile (fmt/fmt-check/lint/test/check targets) exists per sneak's
|
exemption is narrower than it was. A minimal dev Makefile
|
||||||
2026-07-07 request, but do not add a Dockerfile, CI config, or
|
(fmt/fmt-check/lint/test/check targets) exists per sneak's 2026-07-07
|
||||||
REPO_POLICIES.md.
|
request. `Dockerfile.lint` and `script/lint` are now also permitted, and
|
||||||
|
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.
|
||||||
|
|||||||
65
script/lint
Executable file
65
script/lint
Executable file
@@ -0,0 +1,65 @@
|
|||||||
|
#!/bin/sh
|
||||||
|
# script/lint: run the linter. golangci-lint is never installed locally:
|
||||||
|
# it runs via docker only, one way, everywhere — script/lint builds
|
||||||
|
# Dockerfile.lint, which COPYs the repo into the pinned golangci-lint
|
||||||
|
# image and lints as a build step. This works even when the docker daemon
|
||||||
|
# is remote and bind mounts are impossible.
|
||||||
|
#
|
||||||
|
# --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. It and --target must both stay,
|
||||||
|
# for the reason in the next paragraph: do not "simplify" either of those
|
||||||
|
# two away.
|
||||||
|
#
|
||||||
|
# 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:
|
||||||
|
#
|
||||||
|
# 1. $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.
|
||||||
|
#
|
||||||
|
# 2. .dockerignore decides what reaches the container, and only what
|
||||||
|
# reaches it gets linted. Excluding a Go file there removes it from the
|
||||||
|
# lint with no warning: verified by planting a real violation and adding
|
||||||
|
# just that file's path to .dockerignore, which produced `0 issues.` at
|
||||||
|
# exit 0 with the violation still sitting in the working tree. It shows
|
||||||
|
# up only if the rest of the package still references the excluded file,
|
||||||
|
# in which case the build fails loudly on `undefined:` typecheck errors;
|
||||||
|
# a self-contained file drops out silently. So .dockerignore is part of
|
||||||
|
# this gate, not housekeeping — keep it to build inputs the lint does
|
||||||
|
# not read, and never exclude Go sources, go.mod/go.sum or
|
||||||
|
# .golangci.yml.
|
||||||
|
#
|
||||||
|
# --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)"
|
||||||
|
|
||||||
|
# Must match the stage name in Dockerfile.lint.
|
||||||
|
stage=lint
|
||||||
|
|
||||||
|
main() {
|
||||||
|
cd "$ROOT"
|
||||||
|
docker build \
|
||||||
|
--target "$stage" \
|
||||||
|
--no-cache-filter="$stage" \
|
||||||
|
--output=type=cacheonly \
|
||||||
|
-f Dockerfile.lint .
|
||||||
|
}
|
||||||
|
|
||||||
|
main "$@"
|
||||||
Reference in New Issue
Block a user