Compare commits
2 Commits
35858dab66
...
lint-polic
| Author | SHA1 | Date | |
|---|---|---|---|
| dcf75f6c72 | |||
| 12e8db8b0e |
27
README.md
27
README.md
@@ -118,15 +118,10 @@ alpine. We provide:
|
|||||||
`script/docker` for the image tag
|
`script/docker` for the image tag
|
||||||
- `script/test` — run the test suite (no tests defined here)
|
- `script/test` — run the test suite (no tests defined here)
|
||||||
- `script/lint` — lint the markdown files, by building `Dockerfile.lint`. The
|
- `script/lint` — lint the markdown files, by building `Dockerfile.lint`. The
|
||||||
lint verdict comes only from the container; nothing on the host produces one.
|
linter runs in a container, always: it is never installed on the host and
|
||||||
(The prettier in `node_modules` that `script/fmt` and `script/fmt-check` use
|
never invoked there. Linting happens as a build step, so a successful build is
|
||||||
is the same binary, which is why this says "verdict" rather than "never on the
|
a clean lint, and the same per-invocation `CHECK_EPOCH` nonce used elsewhere
|
||||||
host" — see the scope note in `prompts/REPO_POLICIES.md`.) Linting happens as
|
is what stops Docker serving that lint from cache on an unchanged tree
|
||||||
a build step, so a successful build is a clean lint, and the same
|
|
||||||
per-invocation `CHECK_EPOCH` nonce used elsewhere is what stops Docker serving
|
|
||||||
that lint from cache on an unchanged tree. A failure that names no finding —
|
|
||||||
daemon down, image unpullable — is not a lint result: read which build step
|
|
||||||
failed, fix that, and re-run
|
|
||||||
- `script/fmt` — format all markdown files with prettier (writes)
|
- `script/fmt` — format all markdown files with prettier (writes)
|
||||||
- `script/fmt-check` — check formatting (read-only)
|
- `script/fmt-check` — check formatting (read-only)
|
||||||
- `script/check` — run all checks: `test`, `lint`, `fmt-check` (our own
|
- `script/check` — run all checks: `test`, `lint`, `fmt-check` (our own
|
||||||
@@ -135,14 +130,12 @@ alpine. We provide:
|
|||||||
(byte-identical across repos); passes the same `CHECK_EPOCH` nonce as
|
(byte-identical across repos); passes the same `CHECK_EPOCH` nonce as
|
||||||
`script/cibuild`
|
`script/cibuild`
|
||||||
- `script/cibuild` — cd to the repo root, run `script/lint` first, then assign
|
- `script/cibuild` — cd to the repo root, run `script/lint` first, then assign
|
||||||
`epoch="$(date +%s%N)$$"` and `version="$(git describe ...)"` on their own
|
`epoch="$(date +%s%N)$$"` and
|
||||||
lines and
|
`docker build --build-arg CHECK_EPOCH="$epoch" .` (what CI runs). Two
|
||||||
`docker build --build-arg CHECK_EPOCH="$epoch" --build-arg VERSION="$version" .`
|
container builds: the lint image, then the main image, which runs
|
||||||
(what CI runs; both build args are mandatory). Two container builds: the lint
|
`script/test` and `script/fmt-check` but deliberately not `make check` — that
|
||||||
image, then the main image, which runs `script/test` and `script/fmt-check`
|
would nest a docker build inside a build step. A bare `docker build .` fails
|
||||||
but deliberately not `make check` — that would nest a docker build inside a
|
closed on purpose
|
||||||
build step. So `script/cibuild` is what proves the branch green; a bare
|
|
||||||
`docker build .` never lints, and fails closed on purpose
|
|
||||||
- `script/precommit` — run by the git pre-commit hook (our own extension); calls
|
- `script/precommit` — run by the git pre-commit hook (our own extension); calls
|
||||||
`script/check`
|
`script/check`
|
||||||
- `script/install-precommit` — installs the git pre-commit hook (our own
|
- `script/install-precommit` — installs the git pre-commit hook (our own
|
||||||
|
|||||||
29
TODO.md
29
TODO.md
@@ -21,6 +21,21 @@ fmt-check, and commit.
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
|
- 2026-08-10: Closed three gaps the containerised-lint rule left between the
|
||||||
|
canonical text and the first repos to implement it. `.dockerignore` excluding
|
||||||
|
the agent scratch directory is now stated as a correctness precondition of
|
||||||
|
that rule rather than a context-size measure: the lint image lints whatever
|
||||||
|
`COPY . .` copies, and toolchains discover files by walking the tree instead
|
||||||
|
of reading `.gitignore`, so a nested worktree puts the foreign-tree false reds
|
||||||
|
back inside the container — `sneak/quak` measured the same discovery mechanism
|
||||||
|
taking a test count from 210 to 1050. The cache-bust arg is fixed at
|
||||||
|
`CHECK_EPOCH` in `Dockerfile.lint` as well, because a per-file name is
|
||||||
|
invisible to the grep that proves every build is busted, making a renamed
|
||||||
|
guard indistinguishable from a missing one. And the formatting check is now
|
||||||
|
required to run in exactly one of the two images, with either placement
|
||||||
|
allowed: splitting lint out of the `Dockerfile` is precisely when `fmt-check`
|
||||||
|
gets dropped from both, and running the formatter beside the linters is the
|
||||||
|
better shape where it is the same pinned dependency.
|
||||||
- 2026-08-10: Moved every lint run into a container, on the owner's ruling, and
|
- 2026-08-10: Moved every lint run into a container, on the owner's ruling, and
|
||||||
made this repo do it rather than merely document it. `script/lint` is now
|
made this repo do it rather than merely document it. `script/lint` is now
|
||||||
`docker build -f Dockerfile.lint .` and nothing else; the linter is never
|
`docker build -f Dockerfile.lint .` and nothing else; the linter is never
|
||||||
@@ -49,18 +64,8 @@ fmt-check, and commit.
|
|||||||
measurement, not preference — a bogus config key passes `golangci-lint run`
|
measurement, not preference — a bogus config key passes `golangci-lint run`
|
||||||
with `0 issues` and fails `config verify`, and every case reproduced
|
with `0 issues` and fails `config verify`, and every case reproduced
|
||||||
byte-identically under `docker run --network none`, so the schema is embedded
|
byte-identically under `docker run --network none`, so the schema is embedded
|
||||||
in the pinned binary and the line costs no network. Two positions are stated
|
in the pinned binary and the line costs no network. Verified with two
|
||||||
rather than left as gaps, because canon that omits them gets re-derived
|
consecutive runs on an unchanged tree both executing the linter, a planted
|
||||||
wrongly: `docker build` returns 1 both for findings and for a build that never
|
|
||||||
reached the lint step, and the exit-75 VOID machinery is deliberately not
|
|
||||||
restored — the dangerous direction is closed since an unrunnable lint fails
|
|
||||||
closed, BuildKit already names the failing step, and the failure is not
|
|
||||||
transient, so what survives is the reading rule that a run which did not lint
|
|
||||||
is not a verdict. And the rule is about linters: `script/fmt` and
|
|
||||||
`script/fmt-check` run on the host by necessity, which in a repo whose
|
|
||||||
formatter is its linter — this one — means that exact command does run there,
|
|
||||||
recorded as a known and bounded gap rather than papered over. Verified with
|
|
||||||
two consecutive runs on an unchanged tree both executing the linter, a planted
|
|
||||||
violation caught and reverted, the bare-build guard firing, and the main image
|
violation caught and reverted, the bare-build guard firing, and the main image
|
||||||
building without attempting a nested build.
|
building without attempting a nested build.
|
||||||
- 2026-08-09: Made a golangci-lint result belong to the tree that asked for it.
|
- 2026-08-09: Made a golangci-lint result belong to the tree that asked for it.
|
||||||
|
|||||||
@@ -112,13 +112,10 @@ last_modified: 2026-08-10
|
|||||||
1. For anything beyond a simple script or tool, or anything that is going to
|
1. For anything beyond a simple script or tool, or anything that is going to
|
||||||
run in any sort of "production" anywhere, make sure it passes
|
run in any sort of "production" anywhere, make sure it passes
|
||||||
`golangci-lint`. Run it with `make lint`, which builds `Dockerfile.lint`:
|
`golangci-lint`. Run it with `make lint`, which builds `Dockerfile.lint`:
|
||||||
the linter runs in a container, always, and `golangci-lint` is not installed
|
the linter runs in a container, always, and is never installed on the host.
|
||||||
on the host at all — no repo's `script/bootstrap` installs it any more. A
|
A `golangci-lint` invoked directly on a shared host reads a result cache
|
||||||
`golangci-lint` invoked directly on a shared host reads a result cache keyed
|
keyed on file content rather than location and a host-global lock, so its
|
||||||
on file content rather than location and a host-global lock, so its answer
|
answer may belong to another checkout entirely.
|
||||||
may belong to another checkout entirely, and a `make lint` is the only
|
|
||||||
verdict worth recording. A failure that names no finding is not a verdict
|
|
||||||
either: read which build step failed before concluding anything.
|
|
||||||
|
|
||||||
1. Write a `Dockerfile` for every repo, even if it only runs the tests. It runs
|
1. Write a `Dockerfile` for every repo, even if it only runs the tests. It runs
|
||||||
the non-lint checks; linting lives in `Dockerfile.lint` and is run by
|
the non-lint checks; linting lives in `Dockerfile.lint` and is run by
|
||||||
|
|||||||
@@ -52,7 +52,15 @@ with your task.
|
|||||||
containerised-lint rule in `REPO_POLICIES.md` for the canonical file. Its
|
containerised-lint rule in `REPO_POLICIES.md` for the canonical file. Its
|
||||||
base image is pinned by sha256 with a version/date comment, it carries
|
base image is pinned by sha256 with a version/date comment, it carries
|
||||||
`ARG CHECK_EPOCH` **after** the dependency layer with the guard below it,
|
`ARG CHECK_EPOCH` **after** the dependency layer with the guard below it,
|
||||||
and it invokes the linter directly rather than through `make lint`.
|
and it invokes the linter directly rather than through `make lint`. The
|
||||||
|
arg is named `CHECK_EPOCH` in this file too — a repo that calls it
|
||||||
|
`LINT_EPOCH` here is missed by the grep that checks every build is
|
||||||
|
cache-busted.
|
||||||
|
- [ ] The formatting check runs in exactly one of the two images — either
|
||||||
|
`script/fmt-check` in the `Dockerfile` or the formatter beside the linters
|
||||||
|
in `Dockerfile.lint`, whichever puts it on the pinned toolchain. Neither
|
||||||
|
image running it is the failure to look for here, since moving lint out of
|
||||||
|
the `Dockerfile` is exactly when it gets dropped.
|
||||||
- [ ] `.dockerignore` excludes the repo's own host-built artifacts (compiled
|
- [ ] `.dockerignore` excludes the repo's own host-built artifacts (compiled
|
||||||
binaries, test binaries, coverage output), written root-anchored —
|
binaries, test binaries, coverage output), written root-anchored —
|
||||||
`/myapp`, never `**/myapp`, which would also match `cmd/myapp/`. An
|
`/myapp`, never `**/myapp`, which would also match `cmd/myapp/`. An
|
||||||
@@ -61,8 +69,11 @@ with your task.
|
|||||||
- [ ] `.dockerignore` excludes `.claude`, root-anchored and with no `**/`
|
- [ ] `.dockerignore` excludes `.claude`, root-anchored and with no `**/`
|
||||||
prefix. Agent worktrees are entire checkouts of the repo, so they inflate
|
prefix. Agent worktrees are entire checkouts of the repo, so they inflate
|
||||||
the context by a multiple of it and can copy another session's unreviewed
|
the context by a multiple of it and can copy another session's unreviewed
|
||||||
work into an image layer. Confirm by enumerating the image, not by reading
|
work into an image layer — and `Dockerfile.lint` then lints that checkout
|
||||||
the file — `.gitignore` hides these from `git status` too.
|
as though it were this one, because toolchains discover files by walking
|
||||||
|
the tree and never read `.gitignore` (`sneak/quak`: 210 discovered tests
|
||||||
|
became 1050). Confirm by enumerating the image, not by reading the file —
|
||||||
|
`.gitignore` hides these from `git status` too.
|
||||||
- [ ] **Do agents in this repo run anywhere other than the repo root?** The
|
- [ ] **Do agents in this repo run anywhere other than the repo root?** The
|
||||||
scratch directory is created in the agent's working directory, so the
|
scratch directory is created in the agent's working directory, so the
|
||||||
canonical anchored entry misses `services/api/.claude/` in a monorepo with
|
canonical anchored entry misses `services/api/.claude/` in a monorepo with
|
||||||
@@ -111,25 +122,16 @@ with your task.
|
|||||||
`script/install-precommit`, shimmed by `make hooks`) runs it
|
`script/install-precommit`, shimmed by `make hooks`) runs it
|
||||||
- [ ] README has an **Entrypoints** section documenting the `script/`
|
- [ ] README has an **Entrypoints** section documenting the `script/`
|
||||||
entrypoints and linking the standard
|
entrypoints and linking the standard
|
||||||
- [ ] `script/lint` is the canonical container build and nothing else, and no
|
- [ ] `script/lint` is the canonical container build and nothing else. No host
|
||||||
host invocation anywhere in the repo can produce a lint **verdict** — grep
|
linter invocation survives anywhere in the repo — grep for the linter's
|
||||||
for the linter's own name across `script/`, the `Makefile` and CI config,
|
own name in `script/`, the `Makefile` and CI config, not just in
|
||||||
not just in `script/lint`. An existing repo is where a second path to the
|
`script/lint`. An existing repo is where a second path to the linter is
|
||||||
linter is likeliest to exist: a `make lint-fast`, a container-versus-host
|
likeliest to exist: a `make lint-fast`, a container-versus-host branch, or
|
||||||
branch, or a CI step that calls the binary directly. **Two hits are
|
a CI step that calls the binary directly.
|
||||||
expected and are not the defect**, so triage rather than delete: any
|
- [ ] `script/bootstrap` installs no linter. Delete the golangci-lint install
|
||||||
`script/fmt` (a formatter must run on the host — that is its job), and, in
|
block, its version and ref variables, and its call site: nothing invokes a
|
||||||
a repo whose formatter is also its linter, `script/fmt-check`, whose
|
host linter any more, so all it can still do is put a differently
|
||||||
command will be the same string as the one in `Dockerfile.lint`. See the
|
versioned binary where somebody runs it by hand and believes the result.
|
||||||
scope note in the containerised-lint rule. Everything else the grep finds
|
|
||||||
is a real second path and goes.
|
|
||||||
- [ ] `script/bootstrap` installs no linter **for the purpose of linting**.
|
|
||||||
Delete a dedicated install — the golangci-lint block, its version and ref
|
|
||||||
variables, and its call site: nothing invokes a host linter any more, so
|
|
||||||
all it can still do is put a differently versioned binary where somebody
|
|
||||||
runs it by hand and believes the result. A JS repo's `yarn install` stays;
|
|
||||||
it brings a linter along with every other dependency, which is unavoidable
|
|
||||||
and harmless as long as no verdict is taken from it.
|
|
||||||
- [ ] The per-checkout lint state is gone: no `GOLANGCI_LINT_CACHE` or `TMPDIR`
|
- [ ] The per-checkout lint state is gone: no `GOLANGCI_LINT_CACHE` or `TMPDIR`
|
||||||
exports, no `--allow-serial-runners`, and `.lint-cache/` removed from
|
exports, no `--allow-serial-runners`, and `.lint-cache/` removed from
|
||||||
`.gitignore` and `.dockerignore`. A container has its own cache and its
|
`.gitignore` and `.dockerignore`. A container has its own cache and its
|
||||||
|
|||||||
@@ -93,7 +93,16 @@ Template files can be fetched from:
|
|||||||
`golangci-lint run`; other repos use the same pattern around their own
|
`golangci-lint run`; other repos use the same pattern around their own
|
||||||
linter (eslint, ruff, prettier). Copy the canonical file from
|
linter (eslint, ruff, prettier). Copy the canonical file from
|
||||||
`REPO_POLICIES.md`. The linter is invoked directly there, never via
|
`REPO_POLICIES.md`. The linter is invoked directly there, never via
|
||||||
`make lint`, which would recurse.
|
`make lint`, which would recurse. The arg keeps the name `CHECK_EPOCH` in
|
||||||
|
this file as well, so one grep covers both builds.
|
||||||
|
- The formatting check runs in exactly one of the two images: either
|
||||||
|
`script/fmt-check` in the `Dockerfile`, or the formatter beside the
|
||||||
|
linters in `Dockerfile.lint` where that is the same pinned dependency.
|
||||||
|
Never neither, never both.
|
||||||
|
- `.dockerignore` must exclude the agent scratch directory before this image
|
||||||
|
is trusted: it lints whatever is in the build context, and toolchains walk
|
||||||
|
the tree rather than reading `.gitignore`, so an agent worktree that
|
||||||
|
reaches the context is linted as though it were the repo.
|
||||||
- [ ] Gitea Actions workflow at `.gitea/workflows/check.yml` that runs
|
- [ ] Gitea Actions workflow at `.gitea/workflows/check.yml` that runs
|
||||||
`script/cibuild` on push — reference
|
`script/cibuild` on push — reference
|
||||||
`https://git.eeqj.de/sneak/prompts/raw/branch/main/.gitea/workflows/check.yml`
|
`https://git.eeqj.de/sneak/prompts/raw/branch/main/.gitea/workflows/check.yml`
|
||||||
@@ -122,12 +131,11 @@ are thin shims calling them. Model scripts:
|
|||||||
timeout)
|
timeout)
|
||||||
- [ ] `script/lint` / `make lint` — builds `Dockerfile.lint` and nothing else:
|
- [ ] `script/lint` / `make lint` — builds `Dockerfile.lint` and nothing else:
|
||||||
`epoch="$(date +%s%N)$$"` on its own line, then
|
`epoch="$(date +%s%N)$$"` on its own line, then
|
||||||
`docker build --build-arg CHECK_EPOCH="$epoch" -f Dockerfile.lint .`. No
|
`docker build --build-arg CHECK_EPOCH="$epoch" -f Dockerfile.lint .`. The
|
||||||
lint verdict may come from a host invocation. Copy the canonical script
|
linter is never installed on the host and never invoked there. Copy the
|
||||||
from `REPO_POLICIES.md`; its executable lines are identical across repos.
|
canonical script from `REPO_POLICIES.md`; it is byte-identical across
|
||||||
Without the nonce this script exits 0 on an unchanged tree having linted
|
repos. Without the nonce this script exits 0 on an unchanged tree having
|
||||||
nothing, and without `-f Dockerfile.lint` it builds the main image and
|
linted nothing.
|
||||||
lints nothing at all.
|
|
||||||
- [ ] `script/fmt` / `make fmt` — formats code (writes)
|
- [ ] `script/fmt` / `make fmt` — formats code (writes)
|
||||||
- [ ] `script/fmt-check` / `make fmt-check` — checks formatting (read-only)
|
- [ ] `script/fmt-check` / `make fmt-check` — checks formatting (read-only)
|
||||||
- [ ] `script/check` / `make check` — runs `test`, `lint`, `fmt-check`; must not
|
- [ ] `script/check` / `make check` — runs `test`, `lint`, `fmt-check`; must not
|
||||||
|
|||||||
@@ -94,17 +94,12 @@ style conventions are in separate documents:
|
|||||||
contributor should be able to understand the entire development workflow by
|
contributor should be able to understand the entire development workflow by
|
||||||
reading the Makefile.
|
reading the Makefile.
|
||||||
|
|
||||||
- Every repo should have a `Dockerfile`. It must run the repo's **non-lint**
|
- Every repo should have a `Dockerfile`. It must run the repo's checks as build
|
||||||
checks as build steps, so the build fails on a branch those checks reject —
|
steps so the build fails if the branch is not green — which requires
|
||||||
which requires `ARG CHECK_EPOCH` and its guard in every stage containing a
|
`ARG CHECK_EPOCH` and its guard in every stage containing a check-running
|
||||||
check-running `RUN`, per the `CHECK_EPOCH` rule below. Without them a
|
`RUN`, per the `CHECK_EPOCH` rule below. Without them a Dockerfile satisfies
|
||||||
Dockerfile satisfies this criterion while its check layers are served from
|
this criterion while its check layers are served from cache, so the build
|
||||||
cache, so it cannot fail on a branch those checks would have rejected.
|
cannot fail on a branch that is not green.
|
||||||
|
|
||||||
**A green `docker build .` does not mean the branch is green**, because this
|
|
||||||
file does not lint. Only `script/cibuild` carries that meaning: it runs
|
|
||||||
`script/lint` and then this build. Do not restate the older, stronger claim
|
|
||||||
anywhere — it was true only while lint ran inside this image.
|
|
||||||
|
|
||||||
**It runs the individual non-lint checks — `script/test` and
|
**It runs the individual non-lint checks — `script/test` and
|
||||||
`script/fmt-check` — and never `make check`.** `script/lint` is itself a
|
`script/fmt-check` — and never `make check`.** `script/lint` is itself a
|
||||||
@@ -113,7 +108,10 @@ style conventions are in separate documents:
|
|||||||
build step, where there is no daemon. Lint is not skipped by this: it runs
|
build step, where there is no daemon. Lint is not skipped by this: it runs
|
||||||
in its own container, and `script/cibuild` runs it first. Put a comment to
|
in its own container, and `script/cibuild` runs it first. Put a comment to
|
||||||
that effect directly above those `RUN` lines, because `make check` is what
|
that effect directly above those `RUN` lines, because `make check` is what
|
||||||
the next person will reach for.
|
the next person will reach for. Of the two, only `script/test` is fixed
|
||||||
|
here: a repo may run its formatter in `Dockerfile.lint` beside the linters
|
||||||
|
instead, and some should — see the containerised-lint rule below. It must
|
||||||
|
then run in that file and not in this one, and never in neither.
|
||||||
|
|
||||||
For non-server repos, the Dockerfile should bring up a development
|
For non-server repos, the Dockerfile should bring up a development
|
||||||
environment and run those checks. For server repos, they should run as an
|
environment and run those checks. For server repos, they should run as an
|
||||||
@@ -140,7 +138,7 @@ style conventions are in separate documents:
|
|||||||
RUN echo "check epoch: ${CHECK_EPOCH}" && <the check command>
|
RUN echo "check epoch: ${CHECK_EPOCH}" && <the check command>
|
||||||
```
|
```
|
||||||
|
|
||||||
and in both `script/cibuild` and `script/docker`:
|
and in `script/lint`, `script/cibuild` and `script/docker`:
|
||||||
|
|
||||||
```sh
|
```sh
|
||||||
epoch="$(date +%s%N)$$"
|
epoch="$(date +%s%N)$$"
|
||||||
@@ -152,16 +150,12 @@ style conventions are in separate documents:
|
|||||||
.
|
.
|
||||||
```
|
```
|
||||||
|
|
||||||
`script/lint` needs the same nonce but is **not** this command: it builds a
|
|
||||||
different file and passes no version. Do not copy the block above into it —
|
|
||||||
a `docker build` with no `-f Dockerfile.lint` builds the main image instead,
|
|
||||||
which is a lint that silently lints nothing. Its canonical form is in the
|
|
||||||
containerised-lint rule below; copy that one.
|
|
||||||
|
|
||||||
The `VERSION` lines are there for a different reason, covered by the
|
The `VERSION` lines are there for a different reason, covered by the
|
||||||
git-describe rule below; they are shown here so the two rules do not each
|
git-describe rule below; they are shown here so the two rules do not each
|
||||||
document half a command. All four `CHECK_EPOCH` elements are load-bearing;
|
document half a command. `script/lint` passes only `CHECK_EPOCH`, since no
|
||||||
none is optional, and each guards a failure mode that otherwise fails green:
|
version is embedded in a lint image. All four `CHECK_EPOCH` elements are
|
||||||
|
load-bearing; none is optional, and each guards a failure mode that
|
||||||
|
otherwise fails green:
|
||||||
- `ARG` is stage-scoped, so a single declaration leaves the other check
|
- `ARG` is stage-scoped, so a single declaration leaves the other check
|
||||||
stages frozen while the fix reviews as complete. Declare it in every stage
|
stages frozen while the fix reviews as complete. Declare it in every stage
|
||||||
that runs checks, immediately above the first such `RUN`.
|
that runs checks, immediately above the first such `RUN`.
|
||||||
@@ -192,29 +186,19 @@ style conventions are in separate documents:
|
|||||||
cached, so it does not push against the five-minute Docker build ceiling.
|
cached, so it does not push against the five-minute Docker build ceiling.
|
||||||
Blanket `--no-cache` is **not** an acceptable substitute, on `Dockerfile` or
|
Blanket `--no-cache` is **not** an acceptable substitute, on `Dockerfile` or
|
||||||
on `Dockerfile.lint`: it re-runs `go mod download` / `yarn install` on every
|
on `Dockerfile.lint`: it re-runs `go mod download` / `yarn install` on every
|
||||||
invocation, so a lint that should take seconds pays a dependency install
|
invocation, which makes linting network-dependent and pushes a lint that
|
||||||
each time and reaches the network on **every** run rather than only on the
|
should take seconds toward the build ceiling. Never reach for
|
||||||
first and after a manifest change. It is a difference of degree, not an
|
`docker builder prune` to achieve the same end — the build cache is shared
|
||||||
absolute — a container build is never fully offline-independent — but it is
|
with every other build on the host, including other people's.
|
||||||
the difference between a lint that usually needs nothing and one that always
|
|
||||||
does. Never reach for `docker builder prune` to achieve the same end: the
|
|
||||||
build cache is shared with every other build on the host, including other
|
|
||||||
people's.
|
|
||||||
|
|
||||||
- **Every lint run happens in a container, and `script/lint` is that container
|
- **Every lint run happens in a container, and `script/lint` is that container
|
||||||
build.** The precise claim, because it is the one that has to survive contact
|
build.** The linter is never installed on the host and never invoked there.
|
||||||
with a repo whose formatter and linter are the same binary: **no lint verdict
|
Every repo carries a `Dockerfile.lint` next to its `Dockerfile`; the linter
|
||||||
may come from a host invocation.** No `script/`, no `Makefile` target and no
|
runs as a **build step**, so a successful build _is_ a clean lint. Building
|
||||||
CI step may produce a lint result by running a linter on the host. That is
|
rather than bind-mounting is deliberate: it is what makes the pattern work
|
||||||
stronger than it sounds and weaker than "the binary is never on the host" —
|
unchanged where the docker daemon is remote and bind mounts are impossible.
|
||||||
see the scope note at the end of this rule, which says exactly which host
|
Docker is assumed available in every environment. Discarding the linter's
|
||||||
invocations remain legitimate and why. Every repo carries a `Dockerfile.lint`
|
cache on every run is the point of this rule, not a cost it pays.
|
||||||
next to its `Dockerfile`; the linter runs as a **build step**, so a successful
|
|
||||||
build _is_ a clean lint. Building rather than bind-mounting is deliberate: it
|
|
||||||
is what makes the pattern work unchanged where the docker daemon is remote and
|
|
||||||
bind mounts are impossible. Docker is assumed available in every environment.
|
|
||||||
Discarding the linter's cache on every run is the point of this rule, not a
|
|
||||||
cost it pays.
|
|
||||||
|
|
||||||
This closes a family of defects, every one of them an artefact of running
|
This closes a family of defects, every one of them an artefact of running
|
||||||
the linter on a shared host, and every one of them observed rather than
|
the linter on a shared host, and every one of them observed rather than
|
||||||
@@ -281,20 +265,14 @@ style conventions are in separate documents:
|
|||||||
RUN golangci-lint run --config .golangci.yml ./...
|
RUN golangci-lint run --config .golangci.yml ./...
|
||||||
```
|
```
|
||||||
|
|
||||||
and the canonical `script/lint`, whose executable lines are identical in
|
and the canonical `script/lint`, identical in every repo:
|
||||||
every repo (only the comment wording is a repo's own):
|
|
||||||
|
|
||||||
```sh
|
```sh
|
||||||
#!/bin/sh
|
#!/bin/sh
|
||||||
# script/lint: run the linter. This is the ONLY source of a lint verdict
|
# script/lint: run the linter. The linter is never installed on the host
|
||||||
# in this repo — the linter runs in a container, one way, everywhere, so
|
# and never invoked there — it runs in a container, one way, everywhere,
|
||||||
# a run cannot inherit another checkout's cache, another process's lock,
|
# so a run cannot inherit another checkout's cache, another process's
|
||||||
# or a host toolchain that differs from the pinned one.
|
# lock, or a host toolchain that differs from the pinned one.
|
||||||
#
|
|
||||||
# A failure here that names no finding is NOT a lint result: docker
|
|
||||||
# build exits 1 both for findings and for a build that never reached
|
|
||||||
# the lint step. BuildKit names the failing step; read it, fix the
|
|
||||||
# environment, and re-run.
|
|
||||||
set -eu
|
set -eu
|
||||||
|
|
||||||
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
|
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
|
||||||
@@ -322,17 +300,46 @@ style conventions are in separate documents:
|
|||||||
close, arriving through a new file. The `ARG` goes **after** the
|
close, arriving through a new file. The `ARG` goes **after** the
|
||||||
dependency layer so `go mod download` / `yarn install` stay cached and
|
dependency layer so `go mod download` / `yarn install` stay cached and
|
||||||
only the lint steps re-run. Blanket `--no-cache` also busts the dependency
|
only the lint steps re-run. Blanket `--no-cache` also busts the dependency
|
||||||
layer, so every lint reaches the network instead of only the first one.
|
layer, which makes every lint network-dependent.
|
||||||
- **Non-Go repos get the same pattern around their own linter** — `eslint`,
|
- **Non-Go repos get the same pattern around their own linter** — `eslint`,
|
||||||
`ruff`, `prettier`, `shellcheck` — because the ruling is every lint run,
|
`ruff`, `prettier`, `shellcheck` — because the ruling is every lint run,
|
||||||
not every Go lint run. Only the base image and the lint commands change;
|
not every Go lint run. Only the base image and the lint commands change;
|
||||||
the `WORKDIR`, dependency layer, `ARG CHECK_EPOCH`, guard and
|
the `WORKDIR`, dependency layer, `ARG CHECK_EPOCH`, guard and
|
||||||
expanded-value `RUN` are identical. A JS or docs repo bases on its pinned
|
expanded-value `RUN` are identical. A JS or docs repo bases on its pinned
|
||||||
node image, runs `script/bootstrap` as the dependency layer **inside the
|
node image, runs `script/bootstrap` as the dependency layer, and lints
|
||||||
image**, and lints with the linter from that image's `node_modules`. Note
|
with the linter from `node_modules`, which is also how it gets the version
|
||||||
what this does not claim: a developer also runs `script/bootstrap` on the
|
pinned in `package.json` rather than whatever is on the host.
|
||||||
host, so a copy of that linter exists there too. What the rule forbids is
|
- **The lint container lints whatever is in the build context, so
|
||||||
taking a **verdict** from it.
|
`.dockerignore` is part of this rule and not merely hygiene.** `COPY . .`
|
||||||
|
copies an agent scratch worktree — an entire second checkout of the repo —
|
||||||
|
into the lint image unless `.dockerignore` excludes it, and language
|
||||||
|
toolchains discover files by walking the tree rather than by reading
|
||||||
|
`.gitignore`, so `./...`, `eslint .` and `prettier --check .` all descend
|
||||||
|
into it. `sneak/quak` measured this on the same discovery mechanism in its
|
||||||
|
test runner: a nested `.claude/` worktree took the discovered test count
|
||||||
|
from 210 to 1050 (https://git.eeqj.de/sneak/quak/issues/30). Left in the
|
||||||
|
context it re-creates _inside_ the container the foreign-tree false reds
|
||||||
|
that moving lint into a container was adopted to end, and it does so in
|
||||||
|
the convincing form — the findings are real, they simply belong to another
|
||||||
|
checkout. See the `.dockerignore` rules below, and verify by enumerating
|
||||||
|
the image rather than by reading the patterns.
|
||||||
|
- **The build arg is named `CHECK_EPOCH` in `Dockerfile.lint` too**, not
|
||||||
|
`LINT_EPOCH` or any other per-file name, and `script/lint` passes it under
|
||||||
|
that name. Both files guard the same failure under the same contract, and
|
||||||
|
the single name is what lets a reviewer grep a repo for `CHECK_EPOCH` and
|
||||||
|
see every cache-bust it has. Rename it in one file and that grep silently
|
||||||
|
misses it, so a renamed guard and an absent guard read identically without
|
||||||
|
opening both Dockerfiles.
|
||||||
|
- **The formatting check runs in exactly one of the two images, and either
|
||||||
|
one is allowed.** The canonical `Dockerfile` above runs `script/fmt-check`
|
||||||
|
because that is where the non-lint checks live. A repo may instead run its
|
||||||
|
formatter in `Dockerfile.lint` beside the linters, which is the better
|
||||||
|
shape wherever the formatter is the same pinned dependency as the linter
|
||||||
|
(`prettier` out of `node_modules`, say), because it takes the last host
|
||||||
|
toolchain off the checked path for the same reason the linter came off it.
|
||||||
|
What is not allowed is running it in neither image, or in both. Whichever
|
||||||
|
image runs it carries the epoch guard, and `script/check` still runs all
|
||||||
|
three targets on the developer's side either way.
|
||||||
- **Keep `golangci-lint config verify`, and it costs no network.** The two
|
- **Keep `golangci-lint config verify`, and it costs no network.** The two
|
||||||
commands catch **disjoint** classes of defect, measured under the pinned
|
commands catch **disjoint** classes of defect, measured under the pinned
|
||||||
v2.12.2 against a config carrying one planted defect at a time: a bogus
|
v2.12.2 against a config carrying one planted defect at a time: a bogus
|
||||||
@@ -349,48 +356,9 @@ style conventions are in separate documents:
|
|||||||
`getent hosts golangci-lint.run` exits 2. The schema is embedded in the
|
`getent hosts golangci-lint.run` exits 2. The schema is embedded in the
|
||||||
pinned binary. Re-run that control when bumping the pin rather than
|
pinned binary. Re-run that control when bumping the pin rather than
|
||||||
treating the result as permanent.
|
treating the result as permanent.
|
||||||
- **No repo installs a linter on the host _in order to lint_ — remove any
|
- **No repo installs a linter on the host, in `script/bootstrap` or anywhere
|
||||||
such install from `script/bootstrap`.** A dedicated linter install (the
|
else.** A host install is now dead weight whose only remaining effect is
|
||||||
`go install` of golangci-lint is the case that existed) is now dead weight
|
to reintroduce the version skew above.
|
||||||
whose only remaining effect is to reintroduce the version skew above. This
|
|
||||||
does not forbid a host dependency install that happens to bring a linter
|
|
||||||
along with everything else, which is unavoidable in a JS repo and is fine
|
|
||||||
as long as no verdict is taken from it.
|
|
||||||
- **A failed `script/lint` that names no finding is not a lint result.** The
|
|
||||||
exit status alone cannot tell you which happened: `docker build` returns 1
|
|
||||||
both when the lint step fails on findings and when the build never got
|
|
||||||
that far — daemon unreachable, base image unpullable, disk full,
|
|
||||||
dependency layer failing. The per-checkout wrapper this rule replaced drew
|
|
||||||
that line explicitly, exiting 75 for a run that analysed nothing, and that
|
|
||||||
machinery is deliberately **not** restored. Three reasons, and they are a
|
|
||||||
position rather than an omission:
|
|
||||||
- **The dangerous direction is already closed.** The wrapper's exit 75
|
|
||||||
existed because a lock collision could be read as a _result_ on a run
|
|
||||||
that analysed nothing. A build that cannot run fails **closed**: it
|
|
||||||
can never read as clean. What is lost is diagnosability, not safety.
|
|
||||||
- **The failure is self-describing, where the lock collision was not.**
|
|
||||||
BuildKit prints the failing step verbatim —
|
|
||||||
`ERROR: failed to solve: process "/bin/sh -c <the lint command>"` for
|
|
||||||
a genuine finding, against a named earlier step or a daemon error for
|
|
||||||
anything else. The discriminator is already in the output and needs no
|
|
||||||
code. The lock message, by contrast, went to stderr while findings
|
|
||||||
went to stdout and was easy to lose.
|
|
||||||
- **It is not transient, so retrying is wrong.** The lock collision
|
|
||||||
cleared on a retry, which is what made an automatic retry worth
|
|
||||||
building. A dead daemon or a full disk does not, and a retry loop over
|
|
||||||
a failing dependency fetch hides a real reproducibility problem.
|
|
||||||
Rebuilding the distinction in the script would also mean reintroducing
|
|
||||||
per-invocation capture files and traps to get the exit status out from
|
|
||||||
under a pipe, plus matching on BuildKit's message format, which is not
|
|
||||||
a stable interface — and a mis-match in the "treat it as
|
|
||||||
infrastructure" direction would be the false green this whole rule
|
|
||||||
exists to prevent.
|
|
||||||
|
|
||||||
The substance of the VOID rule survives as a reading rule, and it is
|
|
||||||
binding: **do not record a lint verdict from a run that did not reach
|
|
||||||
the lint step, and do not "fix" anything on the strength of one.** Read
|
|
||||||
which step failed, fix the environment, and re-run.
|
|
||||||
|
|
||||||
- **`script/check` still runs `test`, `lint` and `fmt-check`**, so a
|
- **`script/check` still runs `test`, `lint` and `fmt-check`**, so a
|
||||||
developer and the pre-commit hook get all three. It therefore requires a
|
developer and the pre-commit hook get all three. It therefore requires a
|
||||||
docker daemon, and it must never be invoked from inside a build stage —
|
docker daemon, and it must never be invoked from inside a build stage —
|
||||||
@@ -403,27 +371,6 @@ style conventions are in separate documents:
|
|||||||
- If linting requires CGO or system libraries (e.g. `vips-dev`), install
|
- If linting requires CGO or system libraries (e.g. `vips-dev`), install
|
||||||
them in `Dockerfile.lint`.
|
them in `Dockerfile.lint`.
|
||||||
|
|
||||||
**Scope: this rule is about linters, and a formatter is not one.**
|
|
||||||
`script/fmt` writes to your working tree, so it can only run on the host;
|
|
||||||
`script/fmt-check` is its read-only twin and runs on the host too, as well
|
|
||||||
as inside the main image. Neither is a lint run and neither is in scope.
|
|
||||||
|
|
||||||
**The awkward case, stated rather than left for an adopter to trip over: in
|
|
||||||
a repo whose formatter _is_ its linter** — prettier over a markdown or docs
|
|
||||||
repo is the standard shape, and this repo is one — the command in
|
|
||||||
`Dockerfile.lint` and the command in `script/fmt-check` are the same string,
|
|
||||||
so that exact command does still run on the host. That is a real gap in the
|
|
||||||
absolute reading and it is accepted for two reasons: the mechanisms this
|
|
||||||
rule exists to close do not reach it (prettier's version is pinned in
|
|
||||||
`package.json` and installed into the repo's own `node_modules`, so there is
|
|
||||||
no shared content-keyed cache, no host-global lock, and no version to skew
|
|
||||||
against), and CI's verdict is containerised regardless, because
|
|
||||||
`script/fmt-check` also runs inside the main image build. What is **not**
|
|
||||||
acceptable is taking a lint verdict from the host copy: `script/lint` stays
|
|
||||||
the only source of one. If a repo's linter and formatter ever diverge in
|
|
||||||
version or configuration, this gap becomes a real defect and the check-mode
|
|
||||||
invocation has to move into the container too.
|
|
||||||
|
|
||||||
**What a consuming repo does to adopt this**, in order: add
|
**What a consuming repo does to adopt this**, in order: add
|
||||||
`Dockerfile.lint`; replace `script/lint` with the build above; delete the
|
`Dockerfile.lint`; replace `script/lint` with the build above; delete the
|
||||||
`lint` stage from its `Dockerfile` along with the
|
`lint` stage from its `Dockerfile` along with the
|
||||||
@@ -461,8 +408,8 @@ style conventions are in separate documents:
|
|||||||
# docker build (Dockerfile.lint), so `make check` here would nest a
|
# docker build (Dockerfile.lint), so `make check` here would nest a
|
||||||
# build inside a build step, where there is no daemon. Lint is not
|
# build inside a build step, where there is no daemon. Lint is not
|
||||||
# skipped — script/cibuild runs it first, in its own container.
|
# skipped — script/cibuild runs it first, in its own container.
|
||||||
RUN echo "check epoch: ${CHECK_EPOCH}" && script/fmt-check
|
RUN echo "check epoch: ${CHECK_EPOCH}" && make fmt-check
|
||||||
RUN script/test
|
RUN make test
|
||||||
|
|
||||||
# VERSION comes from the host via --build-arg; see the git-describe rule
|
# VERSION comes from the host via --build-arg; see the git-describe rule
|
||||||
# below. Never run `git describe` here: .dockerignore excludes .git, so
|
# below. Never run `git describe` here: .dockerignore excludes .git, so
|
||||||
@@ -624,14 +571,22 @@ style conventions are in separate documents:
|
|||||||
inflates by a multiple of the repo, and another session's unreviewed,
|
inflates by a multiple of the repo, and another session's unreviewed,
|
||||||
sometimes uncommitted work can be copied into an image layer. The directory is
|
sometimes uncommitted work can be copied into an image layer. The directory is
|
||||||
also created and destroyed constantly, so it invalidates `COPY . .` for
|
also created and destroyed constantly, so it invalidates `COPY . .` for
|
||||||
reasons that have nothing to do with the repo's own content. In `.gitignore`
|
reasons that have nothing to do with the repo's own content. **And because
|
||||||
the entry is `.claude/`, unanchored, which already matches at every depth. In
|
`Dockerfile.lint` and `Dockerfile` run their tooling over the copied context,
|
||||||
`.dockerignore` it is `.claude`, anchored and with **no** `**/` prefix: the
|
a worktree that reaches it is linted and tested as though it were the repo.**
|
||||||
directory occurs exactly once **where agents run at the repo root**, and the
|
Nothing else stops that: language toolchains discover files by walking the
|
||||||
prefixed form would also match any nested directory of that name and delete it
|
tree and do not read `.gitignore`, which is how `sneak/quak` saw a nested
|
||||||
from the build. It is not case-folded the way the secret patterns are, because
|
`.claude/` worktree take its discovered test count from 210 to 1050
|
||||||
tooling creates it in exactly one spelling, so a folded pattern would add no
|
(https://git.eeqj.de/sneak/quak/issues/30). This entry is therefore a
|
||||||
coverage.
|
correctness precondition of the containerised-lint rule above and not a size
|
||||||
|
optimisation — without it the foreign-tree false reds that rule exists to end
|
||||||
|
simply move inside the container. In `.gitignore` the entry is `.claude/`,
|
||||||
|
unanchored, which already matches at every depth. In `.dockerignore` it is
|
||||||
|
`.claude`, anchored and with **no** `**/` prefix: the directory occurs exactly
|
||||||
|
once **where agents run at the repo root**, and the prefixed form would also
|
||||||
|
match any nested directory of that name and delete it from the build. It is
|
||||||
|
not case-folded the way the secret patterns are, because tooling creates it in
|
||||||
|
exactly one spelling, so a folded pattern would add no coverage.
|
||||||
|
|
||||||
**Known gap that comes with the anchored form.** The directory is created in
|
**Known gap that comes with the anchored form.** The directory is created in
|
||||||
the agent's working directory, so the "exactly once, at the root" premise is
|
the agent's working directory, so the "exactly once, at the root" premise is
|
||||||
|
|||||||
@@ -4,12 +4,6 @@
|
|||||||
# Dockerfile's checks only actually run because CHECK_EPOCH is a fresh
|
# Dockerfile's checks only actually run because CHECK_EPOCH is a fresh
|
||||||
# nonce on every invocation; without it a warm cache turns this into a
|
# nonce on every invocation; without it a warm cache turns this into a
|
||||||
# green that proves nothing.
|
# green that proves nothing.
|
||||||
#
|
|
||||||
# Those checks are the NON-LINT ones. Linting left this image: it runs
|
|
||||||
# in its own container, built by script/lint, because script/lint is a
|
|
||||||
# docker build and cannot run inside one. So a green here does not mean
|
|
||||||
# the branch is green — script/cibuild, which runs script/lint first, is
|
|
||||||
# what means that.
|
|
||||||
set -eu
|
set -eu
|
||||||
|
|
||||||
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"
|
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"
|
||||||
|
|||||||
20
script/lint
20
script/lint
@@ -1,17 +1,11 @@
|
|||||||
#!/bin/sh
|
#!/bin/sh
|
||||||
# script/lint: run the linter. This is the ONLY source of a lint verdict
|
# script/lint: run the linter. The linter is never installed on the host
|
||||||
# in this repo — the linter runs in a container, one way, everywhere, so
|
# and never invoked there — it runs in a container, one way, everywhere,
|
||||||
# a run cannot inherit another checkout's cache, another process's lock,
|
# so a run cannot inherit another checkout's cache, another process's
|
||||||
# or a host toolchain that differs from the pinned one. Linting happens
|
# lock, or a host toolchain that differs from the pinned one. Linting
|
||||||
# as a build step (see Dockerfile.lint), so a successful build is a
|
# happens as a build step (see Dockerfile.lint), so a successful build
|
||||||
# clean lint, and it works where the docker daemon is remote and bind
|
# is a clean lint, and it works where the docker daemon is remote and
|
||||||
# mounts are impossible.
|
# bind mounts are impossible.
|
||||||
#
|
|
||||||
# A failure here that names no finding is NOT a lint result: docker
|
|
||||||
# build exits 1 both for findings and for a build that never reached the
|
|
||||||
# lint step (daemon down, image unpullable, disk full). BuildKit names
|
|
||||||
# the failing step; read it, fix the environment, and re-run. Do not
|
|
||||||
# record a verdict from a run that did not lint.
|
|
||||||
set -eu
|
set -eu
|
||||||
|
|
||||||
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
|
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
|
||||||
|
|||||||
Reference in New Issue
Block a user