diff --git a/Dockerfile b/Dockerfile index 267f8ae..7d7bd16 100644 --- a/Dockerfile +++ b/Dockerfile @@ -19,7 +19,34 @@ RUN go mod download # Copy source code COPY . . -# Run formatting check and linter +# Run formatting check and linter. +# +# CHECK_EPOCH must stay immediately above these RUNs. These layers are +# keyed on its value, so they are cache-eligible only for a value +# already built against this same tree. script/cibuild passes a fresh +# value on every invocation, which is what makes its green mean the +# checks really executed. +# +# The guarantee is conditional on that fresh value, not absolute. A +# build that omits --build-arg -- a bare `docker build .` -- gets an +# empty CHECK_EPOCH, and an empty string is a constant: the first such +# build runs the checks, and every one after it on an unchanged tree +# replays these layers from cache, never executing a check and still +# exiting 0, a green nothing earned. Gate through script/cibuild. +# Making the missing-arg case fail loudly instead is tracked in #91. +# +# CHECK_EPOCH is deliberately not referenced by the commands below: a +# declared-but-unreferenced ARG does enter BuildKit's cache key, which +# is measured on this host rather than assumed (PR #89). Upstream +# sneak/prompts #26 prefers expanding the value into the command so +# that the miss is contractual rather than dependent on that behavior +# staying as it is; adopting that here is tracked in #91. Do not delete +# this ARG as dead code -- the gate depends on it. +# +# ARG scope is per-stage, so the builder stage declares its own. +# Everything above this line (apk, go.mod, `go mod download`) is +# deliberately outside the busted range and keeps caching. +ARG CHECK_EPOCH RUN make fmt-check RUN make lint @@ -44,7 +71,11 @@ RUN go mod download # Copy source code COPY . . -# Run tests +# Run tests. See the CHECK_EPOCH comment in the lint stage, including +# the conditions the guarantee depends on; ARG scope is per-stage, so +# this stage needs its own declaration, and it must stay immediately +# above the check RUN. +ARG CHECK_EPOCH RUN make test # Build (pure Go, no CGO required since we use modernc.org/sqlite) diff --git a/README.md b/README.md index 552d00c..71328c4 100644 --- a/README.md +++ b/README.md @@ -617,10 +617,24 @@ them. We provide: lint findings. * `script/docker` — build the Docker image tagged via `script/projectname` -* `script/cibuild` — CI entrypoint: `docker build .` (the Dockerfile - runs the checks). This is the full CI-equivalent gate — it runs the - checks in the same containers CI does, from a clean copy of the tree, - so it also catches anything that depends on host state. +* `script/cibuild` — CI entrypoint: `docker build` (the `Dockerfile` + runs `make fmt-check` and `make lint` in its lint stage and `make + test` in its builder stage). This is the full CI-equivalent gate — it + runs the checks in the same containers CI does, from a clean copy of + the tree, so it also catches anything that depends on host state. It + passes a fresh `--build-arg CHECK_EPOCH`, which the `Dockerfile` + declares immediately above the check `RUN`s in both stages. Those + layers are keyed on that value, so a new value re-runs them even on a + byte-identical tree, and a green from this script means the checks + executed. Dependency and module layers sit above the `ARG` and still + cache, so a build is not cold. + + That guarantee is conditional on the fresh value, so **run the gate + through `script/cibuild`, not by invoking `docker build` yourself**. A + bare `docker build .` supplies no `CHECK_EPOCH`; the empty default is + a constant, so the second and every later build on an unchanged tree + serves all three check layers from cache, executes nothing, and still + exits 0. Issue #91 tracks making that case fail loudly instead. * `script/precommit` — pre-commit gate: `go mod tidy` + `go fmt` (must not change files), then `script/check` * `script/install-precommit` — install the git pre-commit hook that diff --git a/TODO.md b/TODO.md index 16f67b1..60235dc 100644 --- a/TODO.md +++ b/TODO.md @@ -19,6 +19,31 @@ or delete the branch. # Completed Steps +- 2026-08-09: Stopped `script/cibuild` from reporting a green it did + not earn (issue #85). A bare `docker build .` let Docker serve the + check layers from the layer cache whenever the tree had not changed: + the checks never executed and the build still exited 0. The fix is an + `ARG CHECK_EPOCH` declared immediately above the check `RUN`s in both + the lint stage and the builder stage (`ARG` scope is per-stage, so + each declares its own), with `script/cibuild` assigning + `epoch="$(date +%s)"` and passing `--build-arg CHECK_EPOCH="$epoch"`. + The assignment is separate on purpose: under `set -eu` a command + substitution that fails inside an argument does not abort the script, + which would leave an empty constant `CHECK_EPOCH` and restore the + very false green being fixed. Placement is the rest of the point — + the `ARG` sits below the `apk add`, `COPY go.mod go.sum`, and `go mod + download` layers, so only the checks are invalidated and the + dependency layers still cache. The guarantee is conditional on a + fresh value rather than absolute: a bare `docker build .` gets an + empty `CHECK_EPOCH` and can still serve the check layers from cache, + which `README.md` and the `Dockerfile` now say plainly, with issue + #91 tracking the upstream hardening (expanded `ARG` form, unset + guard, per-invocation epoch, `script/docker`) that would close it. + Verified by re-running the reproduction plus the withheld-`--build-arg` + counterfactual; the measurements are recorded once, in the PR #89 + verification comment, rather than restated here. `.golangci.yml`, the + lint-stage `FROM` line and its digest, `script/lint`, and + `.gitea/workflows/check.yml` are all untouched. - 2026-08-09: Corrected the `Vaultik.UI` doc comment (issue #84). It claimed the cli layer replaces the writer with a discarding one in `--cron` mode; the actual mechanism is `UI.SetQuiet(true)` in diff --git a/script/cibuild b/script/cibuild index 3da5857..44c62d0 100755 --- a/script/cibuild +++ b/script/cibuild @@ -1,6 +1,9 @@ #!/bin/sh -# script/cibuild: run the CI build. The Dockerfile runs script/check -# (via make check), so a successful build implies all checks pass. +# script/cibuild: run the CI build. The Dockerfile does not run +# script/check; it runs `make fmt-check` and `make lint` in its lint +# stage and `make test` in its builder stage. A successful build +# implies those three passed, provided they actually ran -- which is +# what the CHECK_EPOCH below is for. # Generic: needs no adaptation. The Gitea workflow runs this on push. set -eu @@ -8,7 +11,22 @@ ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" main() { cd "$ROOT" - docker build . + # The Dockerfile's check layers are keyed on CHECK_EPOCH, so a + # fresh value here is what forces them to re-run: without it an + # unchanged tree replays them from cache, the checks never execute, + # and the build still exits 0. The ARG sits immediately above the + # check RUNs, so dependency and module layers still cache. + # + # Assign the epoch on its own line rather than inline in the + # argument. Under `set -eu` a command substitution that fails + # inside an argument does NOT abort the script: CHECK_EPOCH would + # become an empty string, an empty string is a constant, and a + # constant CHECK_EPOCH is exactly the cached-check false green this + # script exists to prevent -- so the guard would disarm itself and + # still exit 0. As a bare assignment, `set -e` catches a failing + # `date` and no build starts. + epoch="$(date +%s)" + docker build --build-arg CHECK_EPOCH="$epoch" . } main "$@"