337b319542cdcaf9c2f445bf26f6aecb102a5487
5
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
5ca68804ac |
Fail the Docker build when the lint stage's linter is not the pin
check / check (push) Successful in 1m23s
The reordered COPY --from=lint did not make the two stages provably one toolchain, as the Dockerfile comment, the previous commit message and TODO.md all claimed. script/bootstrap compares its pin against whatever PATH resolves, and $GOPATH/bin sits ahead of /usr/local/bin, so any drift was absorbed: bootstrap rebuilt the pinned version from source, verified that, and the build went green with the lint stage having linted at one version and make check having run at another. Bumping the lint stage image without touching the pin was enough to produce it. New script/verify-linter-pin fails, naming both versions, unless a given golangci-lint binary is exactly the version script/bootstrap pins. The build stage runs it on the binary copied out of the lint stage, immediately after the copy and before bootstrap, so no reinstall can satisfy it. The pin is read out of script/bootstrap, which stays its single source of truth; a pin that cannot be read is a hard failure rather than a skip. The check takes no CHECK_EPOCH because its only inputs are the copied binary and script/, so Docker invalidates the layer exactly when a cached result would stop being true. The linter version is pinned independently in the lint stage's image digest and in GOLANGCI_LINT_VERSION, with nothing keeping them in sync; a half-applied bump is now a build failure instead of a silent split. ENV PATH keeps $GOPATH/bin, but its comment no longer claims a reinstall is the reason: bootstrap must be able to run and verify what it installs, and nothing in this image is shadowed by the entry. Verified: with the lint stage's linter faked to 2.11.0 after the gates had really run, the build fails at verify-linter-pin naming 2.11.0 and 2.12.2, with bootstrap and the check gate never reached; an unmodified make docker is green with all three gates run on a fresh epoch and real test results. A planted unused finding still fails at the lint stage with gate check absent from the log; the image still fails TestScanHardlinkRunFailsTogether under --user 0:0 and passes as uid 1000, both with the Go test cache disabled; and a second build serves bootstrap, the verify layer and the dependency layers CACHED while the gates go cold. |
||
|
|
964fc29ed3 |
Bust the Docker layer cache for the gate steps (closes #32)
check / check (push) Successful in 1m50s
script/cibuild and script/docker were bare docker build invocations with no cache control, and the Dockerfile copies the tree before running its gates. On an unchanged tree Docker served those layers from cache, so the gates never executed and the build still exited 0. A merge commit here has a tree byte-identical to the branch head it merges, so every merge CI run was almost certainly a full cache hit, and PR #31's reviewer caught make docker returning success as a 17-layer cache hit that proved nothing. Declare ARG CHECK_EPOCH in both stages and have the scripts pass --build-arg CHECK_EPOCH="$(date +%s)". ARG is scoped per stage and this Dockerfile has three gates across two of them (make fmt-check and make lint in the lint stage, make check in the build stage), so one declaration would have left a stage silently cacheable. BuildKit hashes the expanded command rather than the declaration, so each gate RUN echoes the epoch: an unreferenced ARG invalidates nothing, and the echo doubles as evidence in the build log that the layer really ran. Both declarations sit below the dependency layers, so the pinned base images, go mod download, apk add and the source copies keep their cache and only the gates go cold. The build-stage declaration sits after USER, so the drop to the unprivileged builder user still happens before make check and the chmod(0) permission tests stay real. |
||
|
|
9d06c13777 |
Verify the golangci-lint install actually took effect
check / check (push) Successful in 1m31s
`go install` writes into GOBIN (or GOPATH/bin), but the linter `make lint` runs is whichever golangci-lint PATH resolves first. On a host where a wrong-version binary sits ahead of that directory — a nix profile, apt, brew, apk, a tarball in /usr/local/bin, or the /usr/local/bin copy the Dockerfile builder stage makes — the install landed behind the shadow, changed nothing the gate uses, and bootstrap still printed "bootstrap complete" and exited 0. That leaves the local gate linting against a different ruleset than CI while affirmatively claiming otherwise, and every subsequent run reinstalls forever, so the second run is never a no-op. After installing, re-read the effective version. On a mismatch print the resolved binary, the install directory and both versions to stderr and exit non-zero. Do not reorder PATH or remove anyone's binary: diagnose and stop. Also: - stop discarding `golangci-lint --version` stderr, so a present but broken binary (missing shared library, wrong architecture) says why instead of silently yielding the empty string and reinstalling on every run forever. Only stdout is parsed, so the parse matrix is unchanged. - bound the `--version` call with timeout(1) where it exists, since bootstrap now executes a binary it previously only located and a wedged one would otherwise hang the script. Hosts without timeout(1) run it unbounded, as before. - use X.Y.Z in the parsing comment so the pinned version stays a single literal in the script. |
||
|
|
9e924721e6 |
Check the golangci-lint version in bootstrap, not just presence (closes #24)
check / check (push) Successful in 1m48s
script/bootstrap installed the pinned linter only when the command was absent, so on any host that already had some golangci-lint the pin was never consulted and a version bump was inert forever. That is how a host running v2.10.1 against a v2.12.2 pin got a green `make check` while `make docker` rejected the same commit: the local gate was linting with a different ruleset than CI, and the disagreement only surfaced after a push. The version is now a single value, GOLANGCI_LINT_VERSION, with the `go install` module ref derived from it, so a future bump cannot half-apply. A golangci_lint_version helper parses the installed version out of `golangci-lint --version` (the field after the word "version", with an optional leading "v" stripped, since the module ref carries one and the binary's output does not) and yields the empty string when the tool is absent or unreadable. Any version that is not the pin -- older, newer, absent or unparseable -- is reinstalled, so a first run upgrades and a second is a no-op. git, make and go keep their presence-only checks: they come from the host package manager, the repo pins no system toolchain versions, and go.mod governs the language version. That is now stated in a comment next to them rather than left ambiguous beside a tool that is version-checked. |
||
|
|
1a38570301 |
Cover the post-walk cancellation guard with a test that reaches it
check / check (push) Successful in 1m32s
TestSyncScanCancelledWalkKeepsRecords handed syncScan a context that was already cancelled. loadIndex is the first thing syncScan does, and its QueryContext fails on that context, so the scan returned before startWalk was ever called: no walk ran, no pool started, no write path was reachable, and all three of the test's assertions held for the wrong reason. The post-walk ctx.Err() guard, which is the highest-stakes line in the change, had no coverage at all — a panic in its body, or deleting it outright, left the suite green. Replace it with TestSyncScanCancelledMidWalkKeepsRecords, which cancels during the walk and so reaches the guard holding a genuinely partial census and a still-populated record index. The cancellation is driven by the scan's own progress rather than by a timer: walkClock is a context that cancels itself once its Done method has been consulted a set number of times, and since every blocking channel operation in the walk selects on Done — one consultation per event, a couple per directory, against the index load's fixed three — a threshold set to a quarter of the fixture's file count lands the cancellation deep inside the walk on every run. The census settles at around 380 of 2000 files, leaving some 1600 records that a complete-looking census would have handed to the update phase as deletions. The already-cancelled case is kept, renamed to what it actually tests and with its goroutine assertion dropped, since nothing that could leak is ever started. Direct tests cover the remaining cancellation branches of both pools: sendEvent abandoning a blocked send, walk workers dropping queued directories, a walk worker abandoning its subdirectory hand-off, dispatchDirs closing jobs on its way out, feedHashJobs doing the same, hashWorker dropping queued runs, and hashPhase leaving its result loop. Each is deterministic — the channels involved are unbuffered, unread or pre-filled, so the cancellation case is the only one that can be ready. Also correct two overstated claims. The hashLeakFiles comment described a mechanism that does not occur: the surplus is absorbed exactly by the two pool channels plus the workers in flight, so the feeder drains and exits, and what an abandoned pool leaves parked is the workers and the goroutine waiting on them. And the guard is defence in depth, not the sole barrier against data loss: the update phase's BeginTx fails on the same cancelled context before deleting anything today. The guard is what keeps that true once an interrupted scan is allowed to commit what it has. |