Compare commits

...

19 Commits

Author SHA1 Message Date
d43c1d31ac Merge branch 'next': run all linting in Docker (closes #46)
All checks were successful
check / check (push) Successful in 1m19s
2026-08-10 15:55:42 +02:00
d4eaf5fed2 Run all linting in Docker via Dockerfile.lint (closes #46)
All checks were successful
check / check (push) Successful in 1m38s
Per the owner ruling the linter runs in a container invoked through the
script/ entrypoint, never installed on a host. 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, and works with a remote docker daemon, where bind mounts
are impossible.

script/bootstrap loses the `go install`, the pin constants, the version
parser and verify_golangci_lint: with nothing linting on the host, the
$GOPATH/bin versus PATH shadowing they diagnosed has no subject. It
keeps the git/make/go presence checks and `go mod download`, and warns
rather than fails when docker is absent.

Traps for anyone changing this.

A lint build on an unchanged tree exits 0 in under a second having run
no linter -- #32 and #39 again. Caching is waived by ruling:
Dockerfile.lint carries ARG CHECK_EPOCH referenced inside every gate
RUN, because BuildKit hashes the expanded command and a declared but
unreferenced ARG invalidates nothing. script/lint passes
"$(date +%s)-$$"; the PID is there because two runs land in the same
second easily and a bare epoch would cache the second.

Nothing inside an image build may shell out to docker. The main
Dockerfile's lint stage therefore invokes golangci-lint directly rather
than `make lint`, and its build stage runs `make test` and
`make fmt-check` rather than the `make check` aggregate, which reaches
script/lint. Both stay `make` invocations rather than bare scripts
because the Makefile's `export CGO_ENABLED = 0` only reaches what it
invokes.

COPY --from=lint /usr/bin/golangci-lint becomes
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 now, so ENV PATH=/home/builder/go/bin:$PATH
goes with the `go install` that justified it.

script/verify-linter-pin is retired with its README entry: it compared
a linter binary against GOLANGCI_LINT_VERSION in script/bootstrap and
neither subject still exists. The drift moved rather than went away --
the linter is pinned twice, as the FROM line of Dockerfile.lint and the
FROM line of the Dockerfile lint stage, which is what #42 made a build
failure. script/verify-lint-image-pin compares those two references to
each other and restates neither pin; a hardcoded digest would be a
third copy and the same drift one file further out. It runs as a gate
in both files, and an unreadable reference is a hard failure rather
than a vacuous pass.

`golangci-lint config verify` is included per the ruling, and its
unpinned live HTTPS schema fetch was measured rather than assumed:
under --network none the pinned binary passes a valid config and
rejects an invalid one with the jsonschema error, so it validates
against a schema it embeds. That holds for the gate steps, none of
which makes a network call, but not for the build around them --
Dockerfile.lint runs `go mod download` above the gates, so a cold cache
needs the network and only a warm one lints offline, until go.mod or
go.sum changes.

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 under distinct epochs with the COPY layer
CACHED above them. A planted unused variable failed script/lint with
that finding, and failed `make docker` at the lint stage with the build
stage stopped before its COPY --from=lint; 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.
2026-08-10 13:47:31 +00:00
e6a91711b0 Merge branch 'dockerfile-bootstrap': bootstrap the builder stage, guard the linter pin (closes #42)
All checks were successful
check / check (push) Successful in 1m19s
The build stage installed prerequisites inline instead of running
script/bootstrap, which canonical REPO_POLICIES forbids. It now copies
script/ and the manifests and runs bootstrap.

The first attempt claimed that reordering COPY --from=lint made the
build fail loudly on linter drift. It did the opposite: bootstrap
reinstalled to GOPATH/bin, which ENV PATH placed ahead of the copied
binary, so verification passed by construction and lint and build
stages could silently run different linters. Rather than soften the
claim, the guarantee is now implemented. script/verify-linter-pin runs
against the copied binary before bootstrap and fails, naming both
versions, unless it matches the pin read out of script/bootstrap.

Reviewed independently. The reviewer reproduced the negative control in
both directions, including one the PR did not claim - bumping the pin
alone also fails the build - and established that no version output or
mangled pin file yields a silent pass.
2026-08-09 17:43:57 +02:00
clawbot
5ca68804ac Fail the Docker build when the lint stage's linter is not the pin
All checks were successful
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.
2026-08-09 15:29:13 +00:00
3a183aa64b Run script/bootstrap in the Docker build stage (closes #42)
All checks were successful
check / check (push) Successful in 1m16s
Canonical REPO_POLICIES.md:97 requires Dockerfiles to install
development prerequisites by running script/bootstrap rather than
duplicating installs inline. The build stage did the opposite: an
inline `apk add --no-cache make` and its own `go mod download`, so it
maintained a second, independent notion of the toolchain — the
local-versus-CI divergence #24 exists to close, reintroduced one layer
down.

The stage now copies script/ plus go.mod/go.sum and runs
script/bootstrap, which ends in `go mod download`.

COPY --from=lint /usr/bin/golangci-lint is kept and moved above the
bootstrap layer. It is the only edge making this stage depend on the
lint stage, so removing it as redundant would silently stop the build
gating on lint. Copying it first also puts it on PATH before bootstrap
runs, so bootstrap's version check compares the lint stage's linter
against the pin on every build: the two stages are now provably one
toolchain rather than two that happen to agree, and bootstrap does not
pay for a from-source build of its own linter.

$GOPATH/bin joins PATH so that if the copied binary ever stops matching
the pin, bootstrap's reinstall lands somewhere PATH resolves instead of
failing its own verification.

All of it sits above ARG CHECK_EPOCH, and the chown and USER builder
still precede make check.
2026-08-09 14:51:27 +00:00
47fd4e8def Merge branch 'refresh-repo-policies': re-vendor REPO_POLICIES.md (closes #20)
All checks were successful
check / check (push) Successful in 1m7s
The vendored policy copy had drifted to 368 lines against the canonical
408 while still declaring last_modified: 2026-07-06, so nothing about
it signalled staleness. Agents read this file to learn the rules, and
the drift has already caused real defects: the missing scripts-to-rule-
them-all section is why script/fmt lost its Markdown pass (#19) and why
the README had no Entrypoints section (#21).

Replaced wholesale with the canonical copy, sha256
117dde7f148ed3cd693b333312f6345a0a0ee84fadbbe5cca559ca6fed4a1775.
Verified byte-identical by diff; REPO_POLICIES.md is the only file
touched.

The drift turned out to be bidirectional: the vendored copy also held a
repo-memory paragraph the canonical file does not have, so re-vendoring
removes it. That is the correct action for a vendored copy — if the
paragraph should exist it belongs upstream — but it is the one part of
this diff that is not a pure restoration, and it is flagged for a
decision rather than buried.

Docs-only, so the adversarial review was skipped per the standing
exception. make check green.
2026-08-09 12:00:54 +02:00
99d757c31d Re-vendor REPO_POLICIES.md from the canonical copy (closes #20)
All checks were successful
check / check (push) Successful in 1m14s
The vendored copy had drifted to 368 lines against the canonical 408
while still declaring `last_modified: 2026-07-06`, so nothing about the
file signalled that it was stale. Agents working in this repo read the
vendored copy to learn the rules, which makes a silent 40-line gap a
source of real defects rather than untidiness: two have already been
traced to exactly this drift, `script/fmt` having dropped the prettier
Markdown pass (#19, still open) and the README having no Entrypoints
section (#21, since fixed).

Restored by copying the canonical file wholesale — no hand-editing, no
partial merge, no local adaptation, because it is a vendored copy and
its value comes from matching upstream byte for byte. What comes back:

- the entire Scripts to Rule Them All section, including the POSIX sh
  requirement, the repo-root discovery idiom, and the division between
  the standard's canonical scripts and our four extensions
- `bootstrap`, `setup` and `hooks` in the required Makefile target list
- the `script/precommit` paragraph and how the pre-commit hook is wired
  through `script/install-precommit`
- the README **Entrypoints** section requirement
- the Dockerfile bootstrap-layer guidance and the `script/cibuild`
  wording in the Gitea Actions bullet

Verified: `diff` against the canonical file is empty, sha256 is
117dde7f148ed3cd693b333312f6345a0a0ee84fadbbe5cca559ca6fed4a1775, and
`REPO_POLICIES.md` is the only changed path. `make check` is green.

No `TODO.md` entry accompanies this commit. The repo convention is to
record the work in the same commit, but the issue's definition of done
restricts the change to `REPO_POLICIES.md` alone; that scope discipline
is what lets a docs-only change skip adversarial review.
2026-08-09 09:57:43 +00:00
a5fa600c98 Merge branch 'readme-entrypoints': document the entrypoints, drop the tooling line (closes #21)
All checks were successful
check / check (push) Successful in 1m10s
Removes the tooling attribution sentence from the Description, adds the
required Entrypoints section documenting all twelve script/ entrypoints
written from reading each script, corrects the stale Build section that
still claimed the Makefile was the single source of truth after every
target became a shim, and fixes the scan summary example, whose figures
did not sum to the stated total.

Docs-only (README.md alone), so the adversarial review was skipped per
the standing exception. Scope verified independently: git diff
--name-only reports README.md and nothing else. make check green.
2026-08-09 09:53:49 +02:00
9322e8ddee README: drop the tooling attribution, document the entrypoints (closes #21)
All checks were successful
check / check (push) Successful in 1m1s
Four independent README defects, one docs-only pass. README.md is the
only file touched: the issue's definition of done restricts the change
to the README, so TODO.md is deliberately not updated here.

Remove the trailing attribution sentence from the Description. The
paragraph above it already carries authorship and licence, so nothing
is lost, and the naming it carried is not wanted in this repo's output.

Add the required Entrypoints section. House policy wants a README
section that opens by stating adherence to Scripts to Rule Them All,
links the standard, and documents every script/ entrypoint; grep for
"script/" in the README previously returned nothing, though all twelve
entrypoints exist and the Gitea workflow runs script/cibuild. Each
entry is written from reading the script, not from its name. Two
behaviours are called out because they are unusual and were hard-won:
script/bootstrap version-checks golangci-lint against its pin and
verifies the install is the copy PATH resolves rather than a shadowed
one, and script/docker and script/cibuild pass a fresh CHECK_EPOCH so
the Dockerfile's gate layers cannot be served from cache, which is
what keeps a green build from being one that ran nothing.

Fix the stale Build section. Every Makefile target became a thin shim
into script/, so the "Makefile is the single source of truth" claim is
no longer true; point at script/ instead. Document make bootstrap and
make setup, which exist and were undocumented. Drop the make clean
clause about a legacy local files.dat, a file nothing produces any
more; the remaining files.dat references in Makefile, .gitignore and
.dockerignore are #22 and are untouched.

Fix the scan summary example, which showed 123456 files seen against
components summing to 123400. removed is deliberately excluded from
the total, as the following paragraph says and as scan.go implements,
so the total is what was wrong.
2026-08-09 07:51:31 +00:00
a102b8fb06 Merge branch 'cibuild-cache-bust': make the Docker gates actually execute (closes #32)
All checks were successful
check / check (push) Successful in 1m12s
script/cibuild and script/docker were bare docker build invocations, so
on an unchanged tree Docker served the gate layers from cache: no
tests, no lint, exit 0. Confirmed live three times by reviewers, once
as a 17-layer total cache hit.

ARG CHECK_EPOCH is now declared in both stages, since ARG is scoped per
stage and this Dockerfile gates in two of them, and is interpolated
into each gate RUN so BuildKit's command hash actually changes. Both
scripts pass a fresh epoch. Dependency layers stay cached.

Reviewed independently. The reviewer re-established its own cache
baseline after a host-wide BuildKit prune landed mid-review, then
tested the counterfactual: holding the epoch constant reproduces
exactly the 17-layer false green this fixes. It also confirmed the
lint stage still gates the build stage by planting a finding, and that
the non-root drop remains load-bearing by showing the hardlink
permission test fails when the same image runs as root.
2026-08-09 09:47:02 +02:00
clawbot
964fc29ed3 Bust the Docker layer cache for the gate steps (closes #32)
All checks were successful
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.
2026-08-09 07:04:35 +00:00
b8ebe5f578 Merge branch 'bootstrap-version-check': check the linter version, not just presence (closes #24)
All checks were successful
check / check (push) Successful in 4s
script/bootstrap installed the pinned golangci-lint only when the
command was absent, so a version bump was inert on any host that
already had the tool. This one ran v2.10.1 against a v2.12.2 pin: local
make check went green while make docker rejected the same commit with
six goconst findings.

Bootstrap now compares the installed version against the pin, which
lives in exactly one place, and reinstalls on mismatch. It then
verifies the install actually took effect: go install writes to
GOPATH/bin, but the binary make lint runs is whatever PATH resolves, so
a wrong-version linter shadowing it earlier on PATH would otherwise
leave bootstrap printing success having changed nothing. On mismatch it
now names both paths on stderr and exits non-zero.

The first review caught that shadowing case. Re-reviewed independently
by a fresh reviewer, who rebuilt the reproduction from scratch across
seven PATH layouts, ran a 20-row version-parse matrix confirming no
input yields a false match, and forced a cold Docker build after
finding the cached one executed nothing. Confirmed along the way that
v2.10.1 was hiding no findings on main.
2026-08-09 08:38:36 +02:00
clawbot
9d06c13777 Verify the golangci-lint install actually took effect
All checks were successful
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.
2026-08-09 06:19:37 +00:00
clawbot
9e924721e6 Check the golangci-lint version in bootstrap, not just presence (closes #24)
All checks were successful
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.
2026-08-09 05:52:55 +00:00
076d82231b Merge branch 'hash-pool-cleanup': unwind the hash worker pool on error (closes #6)
All checks were successful
check / check (push) Successful in 5s
hashPhase returned early on a recordRun error, abandoning the feeder
goroutine and the hash workers, which parked forever on channel sends.
The pool is now owned: every blocking send selects on ctx.Done(), and
hashPhase defers a stop() that cancels and then drains results. ctx is
threaded from cmd.Context() through runScan, syncScan, both pools and
the database layer, as contextcheck requires.

The walk pool gets the same treatment. It does not leak today, because
walkPhase has no early return, but #5 introduces one. That change also
required a ctx.Err() guard after the walk: a cancelled walk yields a
partial size census, and without the guard the update phase reads every
unreached file as vanished and deletes its record.

First review failed this: the test covering that guard was vacuous,
cancelling before the walk began. Replaced with a deterministic
mid-walk cancellation. Re-reviewed independently by a fresh reviewer,
who reproduced both falsification checks, measured the census across
four parallelism settings, mutation-verified all nine cancellation
branches, ran 120 randomised mid-flight cancellations over 18,000 files
with zero records lost, and forced a cold Docker build after finding
the cached one proved nothing. Coverage 88.5%.
2026-08-09 07:46:43 +02:00
clawbot
1a38570301 Cover the post-walk cancellation guard with a test that reaches it
All checks were successful
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.
2026-08-09 05:12:43 +00:00
1399249957 Unwind the hash worker pool instead of abandoning it (closes #6)
All checks were successful
check / check (push) Successful in 1m2s
hashPhase returned the moment recordRun failed and left the pool
running: the feeder parked forever on a full jobs channel and every
worker on a full results channel. Until #4 landed the process exited
before that mattered; now that runScan returns an error and unwinds,
the goroutines are a real leak.

The pool is now an owned hashPool. Its context is derived from the
scan's, every blocking send in the feeder and the workers selects on
ctx.Done(), the feeder closes jobs on every path out so the workers'
range always terminates, and hashPhase defers pool.stop(), which
cancels and then drains results until the last goroutine has exited.
Draining is the half that matters: a worker already parked on a send
cannot observe the cancellation until a receiver frees it.

ctx comes from cmd.Context() and is threaded through runScan,
syncScan, both worker pools and the database layer as the first
parameter throughout, so graceful interrupt handling has a path to
hook into rather than a pool to rewrite.

The walk pool never leaked, because walkPhase always drains its
events to close, but it has the same unbounded-send shape and gets
the same treatment, together with a ctx.Err() guard after the walk: a
cancelled walk leaves a partial size census, and the update phase
would read every file it never reached as vanished and delete its
record.

Tests drive the scan entry point against a database whose insert
trigger aborts, with a fixture large enough that the failure lands
partway through the hash phase with more runs queued than either pool
channel can hold, and assert that the scan fails instead of hanging
and that runtime.NumGoroutine polls back to its pre-scan baseline.
2026-08-09 03:00:01 +00:00
2a055c0104 Merge branch 'db-close-on-fatal': close the database on every exit path (closes #4)
All checks were successful
check / check (push) Successful in 5s
fatalf called os.Exit(1), which skips deferred functions, so every
defer db.Close() was dead on the fatal path and the SQLite WAL was
never checkpointed. fatalf is gone; runScan, runReport and runTrees
return errors, and a single run(args, stderr) int in main.go is the
only exit point, which also makes these paths testable in-process.

Exit-code behaviour is unchanged: 0 success, 1 fatal, 2 usage. A runE
adapter keeps cobra from reclassifying runtime failures as usage errors
or printing usage text alongside them.

Independently reviewed. The reviewer verified all 15 documented exit
paths against binaries built from both base and head, confirmed the
sidecar WAL files no longer survive a fatal exit, and mutation-tested
the new assertions by reintroducing the defect to confirm they fail.
Coverage 64% to 86.9%. make check and make docker both green.
2026-08-09 04:39:28 +02:00
73841c9989 Guarantee the database is closed on every fatal exit path (closes #4)
All checks were successful
check / check (push) Successful in 57s
fatalf called os.Exit(1), which does not run deferred functions, so
every defer db.Close() was dead on the fatal path: the SQLite WAL was
left uncheckpointed and the -wal/-shm sidecars were left for the next
process to recover. It also made those paths impossible to exercise
in-process.

fatalf is gone. runScan, runReport, runTrees, loadRecords and
resolveRoots return their errors, so the deferred close always runs,
and the only exit point is run() in main.go.

Mapping errors to exit codes needs care: cobra prints the error and
the command's usage text for anything RunE returns, and main mapped
every Execute() error to exit 2. A runtime failure is not a usage
problem, so the runE adapter silences both for the subcommands and
marks their errors fatalError; run() reports a fatalError as
"sfdupes: ..." on stderr and exits 1, and leaves everything else --
cobra's own argument, flag and unknown-command errors, which cobra has
already reported with its usage text -- on exit 2. The bare
"sfdupes" invocation still prints usage and exits 2.

Exit codes and message text are unchanged: 0 on success even with
per-file warnings, 1 fatal, 2 usage, per README section "Error
handling and exit codes". Everything on stdout is still data only.

main_test.go drives the CLI in-process and covers all three: a fatal
error raised after the database is open (a database with no files
table) closes it and leaves no -wal or -shm behind for scan, report
and trees; a nonexistent PATH operand is fatal, not usage, and prints
no usage text; the usage errors still exit 2; and a scan that skipped
an unreadable file still exits 0.
2026-08-09 02:27:07 +00:00
19 changed files with 2205 additions and 240 deletions

View File

@@ -5,18 +5,57 @@ WORKDIR /src
COPY go.mod go.sum ./
RUN go mod download
COPY . .
RUN make fmt-check
RUN make lint
# Cache-buster for the gate layers, and only for them. Docker
# invalidates COPY only when the copied content changes, so on an
# unchanged tree the gates below would be served from cache and the
# build would exit 0 having run nothing. script/cibuild and
# script/docker pass a fresh CHECK_EPOCH on every invocation.
#
# Two properties this depends on. ARG is per-stage, so the build stage
# below declares it again; one declaration here would leave that
# stage's gate cacheable. And each gate RUN must reference the value,
# because BuildKit hashes the expanded command: a declared but
# unreferenced ARG invalidates nothing.
#
# It sits below the dependency layers deliberately. Everything above it
# (the pinned base image, go mod download) keeps its cache; only the
# gates go cold.
ARG CHECK_EPOCH
# The linter is invoked directly here, not through `make lint`. That
# target now runs `docker build -f Dockerfile.lint`, and a docker build
# cannot run a docker build: routing the gate through make would mean
# nesting docker inside this image. Same reason `make check` is gone
# from the build stage below. `make fmt-check` stays as it is — it is a
# gate, not the aggregate, and it shells out to nothing.
RUN echo "gate fmt-check, epoch ${CHECK_EPOCH}" && make fmt-check
# The FROM above and the one in Dockerfile.lint pin the same linter
# twice, and nothing else keeps them in sync; this fails the build when
# they disagree. See the script for why it restates neither pin.
RUN echo "gate lint-image-pin, epoch ${CHECK_EPOCH}" && \
script/verify-lint-image-pin
# Same config-schema check Dockerfile.lint runs, kept here so this build
# gates on exactly what script/lint gates on. It validates against a
# schema the pinned binary embeds, so it needs no network.
RUN echo "gate config verify, epoch ${CHECK_EPOCH}" && \
golangci-lint config verify --config .golangci.yml
RUN echo "gate lint, epoch ${CHECK_EPOCH}" && \
golangci-lint run --config .golangci.yml ./...
# Build stage
# golang:1.25-alpine, 2026-07-23
FROM golang@sha256:56961d79ea8129efddcc0b8643fd8a5416b4e6228cfd477e3fd61deb2672c587 AS builder
RUN apk add --no-cache make
# We never build or run as root. Create an unprivileged user and point
# HOME and the Go caches at its home so go build/test and golangci-lint
# can write their caches when we drop to it below.
# HOME and the Go caches at its home so go build and go test can write
# their caches when we drop to it below. $GOPATH/bin is deliberately not
# on PATH: script/bootstrap no longer `go install`s anything (the linter
# runs from a pinned image, never from a host install), so nothing lands
# there and adding it would only widen what this image resolves.
RUN adduser -D -u 1000 builder
ENV HOME=/home/builder
ENV GOPATH=/home/builder/go
@@ -24,12 +63,26 @@ ENV GOCACHE=/home/builder/.cache/go-build
WORKDIR /src
# Reuse the linter binary from the lint stage; the copy also forces
# BuildKit to complete linting before this stage proceeds.
COPY --from=lint /usr/bin/golangci-lint /usr/local/bin/golangci-lint
# No-op file copy whose only purpose is the build-graph edge: it is what
# makes this stage depend on the lint stage, and so what forces BuildKit
# to finish fmt-check, the pin guard and lint before compilation and
# tests start. Remove it and the fail-fast design dies silently — the
# build stops gating on lint and still exits 0. It replaces a copy of
# the linter binary itself, which is no longer wanted here: nothing in
# this stage runs the linter, because `make lint` is now a docker build
# and a docker build cannot run inside one.
COPY --from=lint /src/go.sum /dev/null
# Install development prerequisites the same way a developer does,
# rather than duplicating the installs inline. Only script/ and the
# dependency manifests are copied first, nothing else, so this layer
# stays cached until the scripts or the dependencies change — bootstrap
# ends in `go mod download`, which is why there is no separate
# invocation of it here.
COPY script/ script/
COPY go.mod go.sum ./
RUN go mod download
RUN script/bootstrap
COPY . .
# Hand the sources and caches to the unprivileged user, then drop root
@@ -40,7 +93,20 @@ USER builder
# Fail the build unless the branch is green. Runs as non-root so the
# permission-denied test paths are exercised legitimately (root would
# bypass the chmod(0) the tests rely on).
RUN make check
#
# The gates are the individual targets, not `make check`: that aggregate
# runs `script/lint`, which is now a docker build, and nothing inside an
# image build may shell out to docker. Lint is not skipped by this — it
# ran in the lint stage above, which this stage's COPY --from makes a
# prerequisite. `make`, not the scripts directly, because the Makefile's
# `export CGO_ENABLED = 0` applies only to what it invokes.
#
# Second per-stage declaration of the gate cache-buster; see the lint
# stage above for why one is not enough. It is placed after USER so the
# drop to the unprivileged user still happens before the checks run.
ARG CHECK_EPOCH
RUN echo "gate test, epoch ${CHECK_EPOCH}" && make test
RUN echo "gate fmt-check, epoch ${CHECK_EPOCH}" && make fmt-check
RUN make build

59
Dockerfile.lint Normal file
View File

@@ -0,0 +1,59 @@
# Lint-only image: this is how the linter runs, everywhere. The repo is
# COPYed into the pinned golangci-lint image and the linter runs as a
# build step, so a successful build IS a clean lint. golangci-lint is
# never installed on a host — one toolchain, pinned by digest, identical
# on a laptop and in CI — and this works even when the docker daemon is
# remote and bind mounts are impossible.
#
# script/lint builds this file. It is a separate image from the lint
# stage of the main Dockerfile because script/lint must not depend on
# the rest of that build; the two FROM lines are kept identical by
# script/verify-lint-image-pin, run as a gate below.
# golangci/golangci-lint:v2.12.2 (Debian-based), 2026-08-07
FROM golangci/golangci-lint:v2.12.2@sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240
WORKDIR /src
# Dependency layers first, so they stay cached across lint runs.
COPY go.mod go.sum ./
RUN go mod download
COPY . .
# Cache-buster for the gate layers, and only for them. Caching of the
# lint run is waived by ruling: COPY is invalidated only by changed
# content, so on an unchanged tree the gates below would be served from
# cache and this build would exit 0 in under a second having run no
# linter at all. That exact false green has bitten this repo twice
# already (#32, #39). script/lint passes a fresh value on every
# invocation.
#
# Each gate RUN must reference the value, because BuildKit hashes the
# expanded command and not the ARG declaration: a declared but
# unreferenced ARG invalidates nothing. The ARG sits below the
# dependency layers deliberately — everything above it keeps its cache,
# only the gates go cold.
ARG CHECK_EPOCH
# The linter version is pinned in two places, here and in the main
# Dockerfile's lint stage. Nothing else keeps them in sync, so a
# half-applied bump is a build failure; see the script.
RUN echo "gate lint-image-pin, epoch ${CHECK_EPOCH}" && \
script/verify-lint-image-pin
# Validates .golangci.yml against golangci-lint's JSON schema. The
# concern about this step was that it fetches that schema over a live,
# unpinned HTTPS call; measured on the pinned image, it does not. The
# binary carries the schema for its own version, so under
# `--network none` this both passes on a valid config and still rejects
# an invalid one with the jsonschema error. That holds for the gate
# steps generally — none of them makes a network call — but not for
# this build as a whole: `go mod download` above needs the network on a
# cold cache, and under `--network none` a first build fails there
# before reaching any gate. That layer stays cached, so only a warm
# cache lints offline, until go.mod or go.sum changes.
RUN echo "gate config verify, epoch ${CHECK_EPOCH}" && \
golangci-lint config verify --config .golangci.yml
RUN echo "gate lint, epoch ${CHECK_EPOCH}" && \
golangci-lint run --config .golangci.yml ./...

124
README.md
View File

@@ -16,9 +16,6 @@ expensive. `scan` maintains a persistent SQLite database of file
signatures that survives between runs, so it can be run from cron and
the reports can be generated at any time from the most recent scan.
This tool was created by [@sneak](https://sneak.berlin) to scratch an itch,
using Claude Code/Fable.
This README is the complete and authoritative specification.
## Getting Started
@@ -283,7 +280,7 @@ the workers.
files seen this run broken down by disposition, plus skips:
```
scan: 123456 files seen (1200 added, 34 updated, 56 removed, 122166 unchanged), 3 skipped
scan: 123400 files seen (1200 added, 34 updated, 56 removed, 122166 unchanged), 3 skipped
```
(`removed` counts deleted database records, which are not part of the
@@ -443,22 +440,131 @@ Additional requirements:
- `2`: usage error (including `scan` with no `PATH` operand and
`report`/`trees` with any positional argument).
## Entrypoints
This repository adheres to the
[Scripts to Rule Them All](https://github.com/github/scripts-to-rule-them-all)
standard: the normalized executables in `script/` are the entrypoints
for the development workflow, and the `Makefile` targets are thin
shims that call them. Every script is POSIX `sh`, resolves the
repository root itself so it can be run from any working directory,
and may be invoked directly. The provided entrypoints are:
- `script/bootstrap` — install everything needed to build and
develop this repository, idempotently, assuming nothing is
present. `git`, `make`, and `go` come from the first of nix, apt,
brew, or apk found on the host, and are presence-checked only.
`golangci-lint` is deliberately **not** installed: it runs from a
digest-pinned image via `script/lint` and never from a host
install, so there is no host copy to drift from the pin. A missing
`docker` is warned about rather than installed or treated as
fatal — everything except linting works without it. Ends with
`go mod download`.
- `script/setup` — make a fresh clone ready for development: runs
`script/bootstrap`, then `script/install-precommit`.
- `script/projectname` — print this project's name (`sfdupes`).
Scripts that need the name call it, so they stay identical across
repositories.
- `script/test` — run the test suite with a 30-second timeout and
coverage enabled, rerunning verbosely on failure so the logs show
which test failed.
- `script/lint` — run the linter. It builds `Dockerfile.lint`, which
copies the repository into the digest-pinned
`golangci/golangci-lint` image and runs
`golangci-lint config verify` and `golangci-lint run` as build
steps, so a successful build is a clean lint. The linter is never
run on the host, which makes a working `docker` the one
prerequisite for linting — and therefore for `make check` and the
pre-commit hook. Offline machines: the gate steps themselves make
no network calls. `golangci-lint run` does not, and neither does
`golangci-lint config verify` — it validates against a schema the
pinned binary embeds, measured under `--network none` to both
pass a valid config and reject an invalid one. The build around
them does. `Dockerfile.lint` runs `go mod download` before the
gates and this module has external dependencies, so a first lint
on a machine with a cold BuildKit cache reaches the network there
(as well as pulling the pinned image); under `--network none` it
fails at that step, before any gate. That layer sits above the
gates and stays cached, so once it is warm `script/lint` — and
with it `make check` — runs entirely offline, until `go.mod` or
`go.sum` changes and the download layer goes cold again. Because
the daemon only ever sees a build context, this works when the
docker daemon is remote and bind mounts are impossible.
- `script/fmt` — format the Go sources in place (`gofmt -s -w`).
Markdown is not formatted.
- `script/fmt-check` — the read-only counterpart of `script/fmt`:
prints any unformatted file and exits non-zero instead of writing.
- `script/check` — run `script/test`, `script/lint`, and
`script/fmt-check`, in that order. Modifies nothing. Needs
`docker`, because `script/lint` does.
- `script/docker` — build the Docker image, tagged with the name
from `script/projectname`. The `Dockerfile` runs the gates as
build steps, so this is also the check a developer or reviewer
runs by hand.
- `script/cibuild` — build the Docker image untagged. This is what
the Gitea workflow runs on push; because the gates run as build
steps, a successful build implies the repository is green.
- `script/precommit` — run by the git pre-commit hook: `go mod tidy`
must be a no-op (a resulting change to `go.mod` or `go.sum` fails
the commit), then `script/check`.
- `script/install-precommit` — install the git pre-commit hook that
runs `script/precommit`. The hook is written to the common git
directory, so the main checkout and every worktree share it.
- `script/verify-lint-image-pin` — fail unless the
`golangci/golangci-lint` reference in `Dockerfile.lint` and the
one in the `Dockerfile` lint stage are the same image at the same
digest, naming both if not. The linter is pinned in those two
files and nothing else keeps them in sync, so a bump applied to
one alone would leave `make lint` and the `Dockerfile`'s
fail-fast lint stage checking the same tree against different
rulesets, both green. The guard restates neither pin — a third
copy would be the same drift one file further out — and runs as a
gate in both files, so `make lint`, `make check` and `make docker`
all catch it.
`script/verify-linter-pin` used to live here. It compared a linter
binary against a version pin in `script/bootstrap`, and both of its
subjects are gone: no linter binary is copied between build stages any
more, and bootstrap pins no version because it installs no linter. The
drift it existed to catch has moved from binary-versus-pin to
pin-versus-pin, which is what `script/verify-lint-image-pin` above
checks.
`script/lint`, `script/docker` and `script/cibuild` all pass a freshly
computed `CHECK_EPOCH` build argument, and the gate steps in
`Dockerfile.lint` and `Dockerfile` reference it. Without that, an
unchanged tree lets Docker serve the gate layers from cache and the
build exits 0 having executed no tests and no lint — a green it never
earned, and one this repository has produced twice. `CHECK_EPOCH`
invalidates the gate layers on every run while leaving the pinned base
images and the dependency layers cached. `script/lint`'s value carries
the process id as well as the epoch, because two lint runs land inside
the same second easily and a bare epoch would cache the second one.
## Build
The `Makefile` is the single source of truth for all operations:
The `script/` entrypoints above are where the implementations live;
the `Makefile` targets are shims onto them, except `build`, which
carries the compile recipe:
- `make` / `make build` — build the `sfdupes` binary (cgo
disabled); building is the default target.
- `make bootstrap` — install the build and development
dependencies.
- `make setup` — prepare a fresh clone: `bootstrap` plus the
pre-commit hook.
- `make test` — run the test suite (30-second timeout; reruns with
`-v` on failure).
- `make lint` — run `golangci-lint` with the repo config.
- `make lint` — run `golangci-lint` with the repo config, in Docker
(see `script/lint`); requires `docker`.
- `make fmt` / `make fmt-check` — format Go sources / verify
formatting without writing.
- `make check` — `test`, `lint`, and `fmt-check`; modifies nothing.
- `make docker` — build the Docker image, which runs `make check` as
a build stage.
Requires `docker`, via `lint`.
- `make docker` — build the Docker image, which runs the gates as
build stages.
- `make hooks` — install the pre-commit hook.
- `make clean` — remove the binary and any legacy local `files.dat`.
- `make clean` — remove the binary.
### Definition of done

View File

@@ -34,10 +34,46 @@ style conventions are in separate documents:
every file before committing. There are zero exceptions to this rule.
- Every repo with software must have a root `Makefile` with these targets:
`make test`, `make lint`, `make fmt` (writes), `make fmt-check` (read-only),
`make check` (prereqs: `test`, `lint`, `fmt-check`), `make docker`, and
`make hooks` (installs pre-commit hook). A model Makefile is at
`https://git.eeqj.de/sneak/prompts/raw/branch/main/Makefile`.
`make bootstrap`, `make setup`, `make test`, `make lint`, `make fmt` (writes),
`make fmt-check` (read-only), `make check` (runs `test`, `lint`, `fmt-check`),
`make docker`, and `make hooks` (installs pre-commit hook). A model Makefile
is at `https://git.eeqj.de/sneak/prompts/raw/branch/main/Makefile`.
- Repos follow the
[Scripts to Rule Them All](https://github.com/github/scripts-to-rule-them-all)
pattern: the implementation of each Makefile target lives in an executable
script in `script/` (`script/bootstrap`, `script/setup`, `script/test`,
`script/lint`, `script/fmt`, `script/fmt-check`, `script/check`,
`script/docker`), and the Makefile targets are thin shims that call them. The
scripts must be POSIX sh (`#!/bin/sh`, `set -eu`, no bashisms) so they run in
minimal containers (e.g. alpine images have no bash); locate the repo root
with `$(cd "$(dirname "$0")/.." && pwd -P)` and `cd` there before acting. From
the standard's canonical set we use `bootstrap`, `setup` (make the repo ready
for development after a fresh clone: runs `bootstrap`, then
`install-precommit`, plus any repo-specific initialization), `test`, and
`cibuild`. `script/bootstrap` installs all dependencies idempotently and
assumes nothing is present: base tools come from nix, apt, brew, or apk
(detected in that order; apt runs noninteractive). For node it uses the
installed node if present; otherwise it installs a PINNED node version via
nvm, first installing nvm itself if missing — from a hash-verified GitHub
release archive (never `curl | sh`), with bash installed as an explicit
prerequisite since nvm requires bash. yarn is then pinned via
`corepack prepare yarn@<version> --activate`. Never install "latest" or "lts";
always exact versions. `script/cibuild` runs the CI build: it changes to the
repo root and runs `docker build .`; the Gitea workflow calls it. Four further
scripts are our own extensions to the standard: `script/check` runs
`script/test`, `script/lint`, and `script/fmt-check`; `script/precommit` is
what the git pre-commit hook runs, and it calls `script/check`;
`script/install-precommit` installs the git pre-commit hook (the `make hooks`
target shims to it); and `script/projectname` (literally that filename) simply
outputs the project's name. Scripts that need the name call
`script/projectname` — e.g. `script/docker` assembles its image tag from it —
so those scripts stay byte-identical across all repos. Repo-type-specific
pre-commit extras (e.g. `go mod tidy` verification in Go repos) belong in
`script/precommit`, not in the hook itself. Model scripts are at
`https://git.eeqj.de/sneak/prompts/raw/branch/main/script/<name>`. The README
must document the provided scripts in an **Entrypoints** section (see the
README requirements below).
- Always use Makefile targets (`make fmt`, `make test`, `make lint`, etc.)
instead of invoking the underlying tools directly. The Makefile is the single
@@ -57,7 +93,11 @@ style conventions are in separate documents:
as a build step so the build fails if the branch is not green. For non-server
repos, the Dockerfile should bring up a development environment and run
`make check`. For server repos, `make check` should run as an early build
stage before the final image is assembled.
stage before the final image is assembled. Dockerfiles install development
prerequisites by running `script/bootstrap` rather than duplicating installs
inline; COPY `script/` and the dependency manifests (`package.json` +
`yarn.lock`, `go.mod` + `go.sum`, etc.) before running it so the bootstrap
layer stays cached until dependencies change.
- **Dockerfiles must use a separate lint stage for fail-fast feedback.** Go
repos use a multistage build where linting runs in an independent stage based
@@ -127,8 +167,9 @@ style conventions are in separate documents:
artifacts or heavier dependencies.
- Every repo should have a Gitea Actions workflow (`.gitea/workflows/`) that
runs `docker build .` on push. Since the Dockerfile already runs `make check`,
a successful build implies all checks pass.
runs `script/cibuild` (which runs `docker build .`) on push. Since the
Dockerfile already runs `make check`, a successful build implies all checks
pass.
- Use platform-standard formatters: `black` for Python, `prettier` for
JS/CSS/Markdown/HTML, `go fmt` for Go. Always use default configuration with
@@ -136,9 +177,11 @@ style conventions are in separate documents:
Markdown (hard-wrap at 80 columns). Documentation and writing repos (Markdown,
HTML, CSS) should also have `.prettierrc` and `.prettierignore`.
- Pre-commit hook: `make check` if local testing is possible, otherwise
`make lint && make fmt-check`. The Makefile should provide a `make hooks`
target to install the pre-commit hook.
- Pre-commit hook: runs `script/precommit`, which calls `script/check`. If local
testing is not possible in the repo, `script/precommit` may skip `script/test`
and run only `script/lint` and `script/fmt-check`. The hook is installed by
`script/install-precommit`; the Makefile must provide a `make hooks` target
that shims to it.
- All repos with software must have tests that run via the platform-standard
test framework (`go test`, `pytest`, `jest`/`vitest`, etc.). If no meaningful
@@ -297,6 +340,10 @@ style conventions are in separate documents:
"µPaaS is an MIT-licensed Go web application by @sneak that receives
git-frontend webhooks and deploys applications via Docker in realtime."
- **Getting Started**: Copy-pasteable install/usage code block.
- **Entrypoints**: Opens by stating that the repo adheres to the
[Scripts to Rule Them All](https://github.com/github/scripts-to-rule-them-all)
standard (with that link), then documents each provided `script/`
entrypoint and its purpose.
- **Rationale**: Why does this exist?
- **Design**: How is the program structured?
- **TODO**: Update meticulously, even between commits. When planning, put
@@ -326,16 +373,6 @@ style conventions are in separate documents:
- All repos should have an `.editorconfig` enforcing the project's indentation
settings.
- **Claude Code repo memory is versioned in the repo**, not left only in
`~/.claude` on one machine. Each memory is one file at
`.claude/memory/<memory>.md`, and every memory file must be `@`-imported
from `.claude/CLAUDE.md` (one `- @memory/<memory>.md` list line per file;
relative import paths resolve against `.claude/`, and Claude Code expands
the imports into context at session launch). When adding a memory, add both
the file and its import line. A root `MEMORY.md` is a violation — Claude
Code never auto-loads it; split it into `.claude/memory/` files. Repos with
no memories yet need no `.claude/` scaffolding.
- Avoid putting files in the repo root unless necessary. Root should contain
only project-level config files (`README.md`, `Makefile`, `Dockerfile`,
`LICENSE`, `.gitignore`, `.editorconfig`, `REPO_POLICIES.md`, and
@@ -361,6 +398,9 @@ style conventions are in separate documents:
- `README.md`, `.git`, `.gitignore`, `.editorconfig`
- `LICENSE`, `REPO_POLICIES.md` (copy from the `prompts` repo)
- `Makefile`
- `script/` entrypoints (`bootstrap`, `setup`, `projectname`, `test`,
`lint`, `fmt`, `fmt-check`, `check`, `docker`, `cibuild`, `precommit`,
`install-precommit`)
- `Dockerfile`, `.dockerignore`
- `.gitea/workflows/check.yml`
- Go: `go.mod`, `go.sum`, `.golangci.yml`

271
TODO.md
View File

@@ -29,6 +29,277 @@
# 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
`REPO_POLICIES.md:97` requires it, and the inline install left the
build stage maintaining its own notion of the toolchain — exactly
the divergence #24 exists to close, one layer down. The stage now
copies `script/` plus `go.mod`/`go.sum` and runs `script/bootstrap`,
which ends in `go mod download`, so the separate invocation of that
is gone. `COPY --from=lint /usr/bin/golangci-lint` stays, and moves
above the bootstrap layer. It is the only edge making this stage
depend on the lint stage, so deleting it as redundant would end
fail-fast linting silently. Letting bootstrap install its own linter
here would have reintroduced the second toolchain and paid for a
from-source build of it. What makes the two stages provably one
toolchain rather than two that happen to agree is a new
`script/verify-linter-pin`, run in the build stage on the binary
that arrives from the lint stage, before bootstrap: it fails the
build naming both versions unless that binary is the version
`script/bootstrap` pins. Bootstrap's own check could not serve that
purpose — it reinstalls its pin from source and then verifies
whatever `PATH` resolves, so drift self-heals silently and a lint
stage image bumped on its own would lint at the new version while
`make check` ran at the old one, green. The linter version is pinned
in two independent places (the lint stage image digest and
`GOLANGCI_LINT_VERSION`) and nothing else keeps them in sync, so a
half-applied bump is now a build failure. The pin is read out of
`script/bootstrap`, which stays the single source of truth; a pin
that cannot be read is a hard failure, not a skip. The check needs
no `CHECK_EPOCH`: its only inputs are the copied binary and
`script/`, so Docker invalidates the layer exactly when a cached
result would stop being true, and it is documented with the other
entrypoints in the README. `$GOPATH/bin` joins `PATH` because
that is where bootstrap's `go install` lands and bootstrap verifies
its installs against what `PATH` resolves — nothing in the image is
shadowed by it, the directory does not exist until bootstrap runs.
Everything added sits above `ARG CHECK_EPOCH`, and the `chown` and
`USER builder` still precede `make check`. Verified: the guard fails
the build with both versions named when the lint stage's linter is
faked to a different version, and an unmodified build still passes
it; bootstrap runs clean under Alpine's `sh` and its `apk` branch,
installing `git` and `make` and finding the copied
linter already at the pin; a second build served the bootstrap and
dependency layers `CACHED` while both gates ran with a fresh epoch;
a planted `unused` finding failed the build at the lint gate in
48.9s with the build stage's `make check` never starting; and the
suite run in the image as `--user 0:0` fails
`TestScanHardlinkRunFailsTogether`, so the drop to the unprivileged
user is still load-bearing. That last check needs the Go test cache
disabled — the first attempt reported `ok ... (cached)` as root,
reusing the result the build-time run had left in the shared cache,
which would have read as a pass. Build wall time, on a shared host
running many concurrent builds and so noisy: 2m13s on an unchanged
tree, 2m17s and 4m29s for two builds after a source change, 5m14s
cold. Only the cold one breaches the policy ceiling, and not because
of this change — `chown -R builder:builder /src /home/builder` walks
the module cache and re-runs on every source change, and it alone
varied between 77s and 210s across those four builds, which is also
the whole spread in the totals. The same cold measurement against
`main` is 5m03s with a 209s `chown`. Filed as #43
- bust the Docker layer cache for the gate steps, so `script/cibuild`
and `script/docker` cannot report a green they did not earn
(2026-08-09, branch `cibuild-cache-bust`, closes #32): both scripts
were bare `docker build` invocations with no cache control, and the
`Dockerfile` copies the tree before running its gates, so on an
unchanged tree Docker served those layers from cache and the build
exited 0 having executed nothing. That is not hypothetical here —
every merge this repo has done is a non-fast-forward merge of an
undiverged branch, so each merge commit's tree is byte-identical to
the branch head's and each merge CI run was almost certainly a full
cache hit; and PR #31's reviewer found `make docker` returning
success as a 17-layer cache hit, catching it only by being
suspicious. The fix is `ARG CHECK_EPOCH` with the scripts passing
`--build-arg CHECK_EPOCH="$(date +%s)"`. Two details make or break
it. `ARG` is scoped per stage and this `Dockerfile` has three gates
across two — `make fmt-check` and `make lint` in the lint stage,
`make check` in the build stage — so a single declaration would have
left one stage silently cacheable; it is declared in both. And
BuildKit hashes the expanded command, not the declaration, so a
declared-but-unreferenced `ARG` invalidates nothing: each gate `RUN`
echoes the epoch, which also puts the value in the build log as
evidence the layer really ran. Placement is below the dependency
layers on purpose — a build that goes cold every time would be a
different bug, not a fix. Verified by running each script twice back
to back on an unchanged tree under `BUILDKIT_PROGRESS=plain`: all
three gates executed on all four runs, each with a fresh epoch in
the log (`script/cibuild` 78.8s then 61.1s; `script/docker` 61.1s
then 53.4s), and twelve steps were still served `CACHED` in the
steady state — both `go mod download`s, `apk add`, `adduser`, the
`chown`, every `go.mod`/`go.sum` and source copy, the linter copy
out of the lint stage, and the binary copy into the runtime stage.
The lint stage still gates the build stage: with a deliberate
`unused` finding planted in the tree, the build failed at
`make lint` in 36.1s and the build-stage `make check` never started.
The build stage also still drops to the unprivileged `builder` user
before `make check`, which the suite depends on rather than merely
prefers: forcing the same image to run the tests as root fails
`TestScanHardlinkRunFailsTogether`, because root reads straight
through the `chmod(0)` the test uses to prove hard links are read
once. This is the local fix only; propagating it to the canonical
templates is `prompts` #26
- check the installed golangci-lint version in `script/bootstrap`
instead of only its presence (2026-08-09, branch
`bootstrap-version-check`, closes #24): `missing golangci-lint` meant
any linter already on `PATH` satisfied the check, so the pin was never
consulted and the v2.12.2 bump from #3 was inert on every host that
already had one — this host ran v2.10.1 against a v2.12.2 pin,
`make check` went green, and `make docker` then rejected the same
commit with findings the local gate never saw. The version now lives
in one place, `GOLANGCI_LINT_VERSION`, with the `go install` module
ref derived from it so a bump cannot half-apply; a
`golangci_lint_version` helper parses `golangci-lint --version`
(taking the field after the word `version` and tolerating an optional
leading `v`, which the module ref carries and the binary's output does
not), and any version that is not the pin — older, newer, absent or
unparseable — is reinstalled. The install is then verified against the
binary `PATH` actually resolves: `go install` writes into `GOBIN` (or
`GOPATH/bin`) while `make lint` runs whichever `golangci-lint` comes
first on `PATH`, so a wrong-version one sitting ahead of it — nix,
apt, brew, apk, or the `/usr/local/bin` copy the `Dockerfile` builder
stage makes — would swallow the install and leave the local gate
disagreeing with CI under an affirmative `bootstrap complete`.
Bootstrap now re-reads the effective version after installing and, on
a mismatch, prints both paths and both versions to stderr and exits
non-zero instead of claiming success; it does not reorder anyone's
`PATH` or delete their binary. The `--version` call keeps its stderr
connected, so a present-but-broken binary says why rather than
reinstalling forever in silence, and is bounded by `timeout(1)` where
that exists, so a wedged binary cannot hang bootstrap. `git`, `make`
and `go` keep their presence-only checks and now say why in a
comment: they are host package-manager tools the repo deliberately
does not pin, with `go.mod` governing the language version and the
digest-pinned images covering reproducible builds. Verified on this
host by bootstrapping from v2.10.1 to v2.12.2 and running it again to
a no-op, plus stub runs of the real script under `dash` covering a
thirteen-input parse matrix (absent, older, newer, host-style,
image-style, leading-`v`, stderr-only, empty, non-zero exit, impostor
binary, `(devel)`, trailing `version`), a shadowed install that must
exit non-zero, an install destination not on `PATH` at all, `GOBIN`
set, and a wedged binary that must hit the timeout; `make check` and
`make lint` are clean at v2.12.2, so v2.10.1 was not hiding any
findings on `main`
- unwind the hash worker pool on the error path (2026-08-09, branch
`hash-pool-cleanup`, closes #6): `hashPhase` used to return the
moment `recordRun` failed and abandon the pool — the feeder parked
forever on a full `jobs` channel and every worker on a full
`results` channel. That only stopped being invisible when #4 landed
and `runScan` began unwinding instead of calling `os.Exit`. The
pool is now an owned, context-aware `hashPool`: every blocking send
in the feeder and the workers selects on `ctx.Done()`, `jobs` is
closed on every path out, and `hashPhase` defers `pool.stop()`,
which cancels and then drains `results` until the last goroutine
has exited — draining is what frees a worker already parked on a
send. `ctx` is threaded from `cmd.Context()` through `runScan`,
`syncScan`, both worker pools and the whole database layer (it is
the first parameter everywhere), so #5 can hand this path a signal
and needs to add nothing else. The walk pool never leaked, because
`walkPhase` always drains its events to close, but it has the same
unbounded-send shape and #5 will give it an early return, so it
gets the same treatment plus a `ctx.Err()` guard after the walk: a
cancelled walk yields a partial size census, and every file it never
reached looks vanished to the update phase. That phase's own
`BeginTx` fails on the same cancelled context before deleting
anything, so the guard is defence in depth rather than the only
barrier — but it is the one that survives #5 deciding an interrupted
scan may commit what it has. Tests drive `run(scan)` against a
database whose insert trigger aborts, and assert both that the scan
fails instead of hanging and that `runtime.NumGoroutine()` polls
back to its pre-scan baseline; a second set cancels a scan part-way
through the walk — deterministically, by counting the scan's own
consultations of `ctx.Done()` rather than racing a timer — and
asserts that it stops at the guard holding a partial census and a
still-populated record index, with every record intact. The
remaining cancellation branches of both pools are covered by direct
tests of `sendEvent`, the walk workers, `dispatchDirs`,
`feedHashJobs`, `hashWorker` and `hashPhase`
- guarantee the database is closed on every fatal exit path
(2026-08-09, branch `db-close-on-fatal`, closes #4): `fatalf` and
its `os.Exit(1)` are gone, so the deferred `db.Close()` — and with
it the SQLite WAL checkpoint — now actually runs when a subcommand
fails; `runScan`, `runReport`, `runTrees`, `loadRecords` and
`resolveRoots` return errors instead. The single exit point is `run`
in `main.go`: it maps a `fatalError` (anything a subcommand
returned) to exit 1 and cobra's own argument and flag errors to exit
2, which keeps a runtime failure from being reported as a usage
error or printing the usage text. New `main_test.go` drives the CLI
in-process and asserts the exit codes from README §Error handling
plus the stdout/stderr split, including that a fatal error raised
after the database is open leaves no `-wal`/`-shm` sidecar behind
for `scan`, `report` or `trees`
- update golangci-lint to v2.12.2 with the canonical config
(2026-08-09, branch `golangci-v2.12.2`, merged as `38a01bd`,
closes #3): bumped the pinned linter in the `Dockerfile` lint

489
cancel_test.go Normal file
View File

@@ -0,0 +1,489 @@
package main
import (
"context"
"database/sql"
"errors"
"os"
"path/filepath"
"strconv"
"sync"
"sync/atomic"
"testing"
"time"
)
// poolUnwind bounds how long a goroutine is given to leave a pool
// after its context is cancelled. Only a failing run ever waits this
// long: a pool that ignored its cancellation parks forever, and this
// is what turns that into a failed assertion instead of a suite that
// hangs until the test binary's own timeout.
const poolUnwind = 2 * time.Second
// walkClock is a context whose cancellation is driven by the scan's
// own progress rather than by the wall clock: it cancels itself the
// moment its Done method has been consulted n times. That is what
// makes "cancel in the middle of the walk" reproducible instead of a
// race against a timer.
//
// The accounting behind the n chosen by each test: every blocking
// channel operation in the walk selects on Done, so the walk spends
// one consultation per file event plus a couple per directory, while
// the index load that runs ahead of it spends a small fixed number
// (three) whatever the record count.
type walkClock struct {
n int64
seen atomic.Int64
once sync.Once
done chan struct{}
}
// newWalkClock returns a context that cancels itself on the nth
// consultation of its Done method.
func newWalkClock(n int64) *walkClock {
return &walkClock{n: n, done: make(chan struct{})}
}
// Done returns the cancellation channel, cancelling the context on the
// nth call and on every call after it. The same channel is returned
// throughout, so a caller that took it before the cancellation still
// observes the close.
func (c *walkClock) Done() <-chan struct{} {
if c.seen.Add(1) >= c.n {
c.once.Do(func() { close(c.done) })
}
return c.done
}
// Err reports the cancellation without consuming a consultation, which
// is what lets the post-walk guard read it without disturbing the
// count.
func (c *walkClock) Err() error {
select {
case <-c.done:
return context.Canceled
default:
return nil
}
}
// Deadline reports no deadline: this context is cancelled by progress,
// never by time.
func (c *walkClock) Deadline() (time.Time, bool) {
return time.Time{}, false
}
// Value carries nothing.
func (c *walkClock) Value(_ any) any {
return nil
}
// walkCancelDirs and walkCancelFilesPerDir shape the fixture for the
// mid-walk cancellation test. Spreading the files over directories is
// load-bearing: it is what bounds how much of the tree can still be
// walked after the cancellation, since the workers drop every
// directory still queued and only the handful already in flight can
// emit anything more.
const (
walkCancelDirs = 100
walkCancelFilesPerDir = 20
walkCancelFiles = walkCancelDirs * walkCancelFilesPerDir
walkCancelWorkers = 4
walkCancelInFlightDirs = walkCancelWorkers * walkCancelFilesPerDir
)
// walkCancelAtDone is the consultation on which the fixture's context
// cancels itself. A quarter of the file count is far past the index
// load's fixed handful and far short of the walk's total, so the
// cancellation lands deep inside the walk and nowhere near either end
// of it.
const walkCancelAtDone = walkCancelFiles / 4
// buildWalkCancelTree writes walkCancelFiles empty files spread over
// walkCancelDirs subdirectories. Zero-length files are never opened by
// the hasher, so the fixture costs directory entries and no read I/O
// while still giving the walk thousands of events to emit.
func buildWalkCancelTree(t *testing.T) string {
t.Helper()
dir := t.TempDir()
for i := range walkCancelDirs {
sub := filepath.Join(dir, "d"+strconv.Itoa(i))
err := os.Mkdir(sub, 0o750)
if err != nil {
t.Fatal(err)
}
writeEmptyFiles(t, sub, walkCancelFilesPerDir)
}
return dir
}
// assertRecordsIntact fails when the database no longer holds exactly
// the records it held before, reporting the first difference rather
// than dumping thousands of paths.
func assertRecordsIntact(t *testing.T, db *sql.DB, before []string) {
t.Helper()
got := recordPaths(dbRecords(t, db))
if len(got) != len(before) {
t.Fatalf("%d records after the cancelled scan, want %d",
len(got), len(before))
}
for i := range got {
if got[i] != before[i] {
t.Fatalf("record %d = %q after the cancelled scan, want %q",
i, got[i], before[i])
}
}
}
// TestSyncScanCancelledMidWalkKeepsRecords is the regression net under
// the post-walk guard. The scan is cancelled part-way through the
// walk, so it reaches the guard holding a genuinely partial size
// census and a still-populated index of records the walk never got to.
// Every one of those records would look vanished to the update phase.
// The guard is what stops the scan there, and this test is what
// notices if it stops doing so: deleting the guard, or making it
// unreachable, makes the scan carry its truncated view into a later
// phase and fail there instead, with a wrapped error rather than the
// bare cancellation.
//
//nolint:paralleltest // counts goroutines: must not run beside others
func TestSyncScanCancelledMidWalkKeepsRecords(t *testing.T) {
dir := buildWalkCancelTree(t)
db := openTestDB(t)
st := syncTree(t, db, dir)
if st.added != walkCancelFiles {
t.Fatalf("setup scan added %d records, want %d",
st.added, walkCancelFiles)
}
before := recordPaths(dbRecords(t, db))
base := baselineGoroutines(t)
st, err := syncScan(newWalkClock(walkCancelAtDone), db,
[]string{dir}, walkCancelWorkers, false)
assertWalkGuardAborted(t, st, err)
assertRecordsIntact(t, db, before)
if got := settledGoroutines(t, base); got > base {
t.Errorf("goroutines = %d after the cancelled scan, want %d back",
got, base)
}
}
// assertWalkGuardAborted checks that the scan stopped at the post-walk
// guard: with a census that is neither empty (the walk really ran)
// nor complete (it really was cut short), and with the guard's own
// bare cancellation as the error. A wrapped error means the partial
// census was carried past the guard into the hash or update phase,
// which is the failure this test exists to catch.
func assertWalkGuardAborted(t *testing.T, st scanStats, err error) {
t.Helper()
if !errors.Is(err, context.Canceled) {
t.Fatalf("syncScan cancelled mid-walk = %v, want %v",
err, context.Canceled)
}
if errors.Unwrap(err) != nil {
t.Errorf("syncScan reported %q, want the guard's bare "+
"cancellation: a wrapped error means the truncated census "+
"reached a later phase", err)
}
if st.unchanged == 0 {
t.Fatalf("stats = %+v: the census is empty, so the walk never "+
"ran and the guard was reached for the wrong reason", st)
}
if st.unchanged >= walkCancelFiles {
t.Fatalf("stats = %+v: the census covers the whole tree, so the "+
"walk was not cut short", st)
}
// The workers drop every directory still queued once the scan is
// cancelled, so only the directories already in flight can add to
// the census after the fact. A census beyond that bound would mean
// the cancellation was not observed where it should have been.
limit := walkCancelAtDone + walkCancelInFlightDirs
if st.unchanged > limit {
t.Errorf("census covers %d files, want at most %d: the walk kept "+
"taking directories off the queue after cancellation",
st.unchanged, limit)
}
if st.removed != 0 {
t.Errorf("stats = %+v: the scan counted records for removal from "+
"a partial census", st)
}
}
// TestSyncScanCancelledBeforeLoadIndex covers the trivial end of the
// cancellation path: a scan handed a context that is already cancelled
// fails in the index load, before the walk pool is ever started. It
// says nothing about the post-walk guard — nothing downstream of
// loadIndex runs at all — only that the failure surfaces as a
// cancellation and that no record is touched on the way out.
func TestSyncScanCancelledBeforeLoadIndex(t *testing.T) {
t.Parallel()
dir := buildSmokeTree(t)
db := openTestDB(t)
syncTree(t, db, dir)
before := recordPaths(dbRecords(t, db))
ctx, cancel := context.WithCancel(t.Context())
cancel()
st, err := syncScan(ctx, db, []string{dir}, walkCancelWorkers, false)
if !errors.Is(err, context.Canceled) {
t.Fatalf("syncScan on a cancelled context = %v, want %v",
err, context.Canceled)
}
if st != (scanStats{}) {
t.Errorf("stats = %+v, want none: the scan gave up in the index "+
"load, before any phase ran", st)
}
assertRecordsIntact(t, db, before)
}
// drainClosed counts the values received from ch until it closes,
// failing the test if it does not close within poolUnwind. A pool that
// ignored its cancellation leaves its channel open with its goroutines
// parked, and this is what reports that as an assertion.
func drainClosed[T any](t *testing.T, ch <-chan T, what string) int {
t.Helper()
counted := make(chan int, 1)
go func() {
n := 0
for range ch {
n++
}
counted <- n
}()
select {
case n := <-counted:
return n
case <-time.After(poolUnwind):
t.Fatalf("%s stayed open after cancellation", what)
return 0
}
}
// awaitReturn fails the test if done is not closed within poolUnwind.
func awaitReturn(t *testing.T, done <-chan struct{}, what string) {
t.Helper()
select {
case <-done:
case <-time.After(poolUnwind):
t.Fatalf("%s did not return after cancellation", what)
}
}
// cancelledContext returns a context that is already cancelled.
func cancelledContext(t *testing.T) context.Context {
t.Helper()
ctx, cancel := context.WithCancel(t.Context())
cancel()
return ctx
}
// TestSendEventAbandonsBlockedSend checks that a walk goroutine with an
// event to deliver and nobody to deliver it to leaves on cancellation
// instead of holding the pool open. The channel here is unbuffered and
// unread, so the send can never complete.
func TestSendEventAbandonsBlockedSend(t *testing.T) {
t.Parallel()
done := make(chan struct{})
events := make(chan walkEvent)
go func() {
defer close(done)
sendEvent(cancelledContext(t), events, walkEvent{})
}()
awaitReturn(t, done, "sendEvent")
}
// TestWalkWorkersDropQueuedDirs checks that cancelled walk workers keep
// reading jobs and drop the directories rather than stopping their
// read: the range over jobs has to run out for the pool to tear down
// and close its event stream.
func TestWalkWorkersDropQueuedDirs(t *testing.T) {
t.Parallel()
dir := t.TempDir()
writeEmptyFiles(t, dir, walkCancelFilesPerDir)
jobs, _, events := startWalkWorkers(cancelledContext(t), 2, false)
for range 4 {
jobs <- dirJob{path: dir}
}
close(jobs)
if n := drainClosed(t, events, "the walk event stream"); n != 0 {
t.Errorf("cancelled walk workers emitted %d events, want none", n)
}
}
// TestWalkWorkerAbandonsSubdirHandoff checks the other blocking send a
// walk worker makes: handing discovered subdirectories back to the
// dispatcher. Once the dispatcher has left, nothing drains that
// channel, and a worker parked on it would hold the pool open forever.
func TestWalkWorkerAbandonsSubdirHandoff(t *testing.T) {
t.Parallel()
dir := t.TempDir()
writeEmptyFiles(t, dir, 1)
err := os.Mkdir(filepath.Join(dir, "sub"), 0o750)
if err != nil {
t.Fatal(err)
}
ctx, cancel := context.WithCancel(t.Context())
defer cancel()
jobs, subdirs, events := startWalkWorkers(ctx, 1, false)
// Fill the hand-back channel to its capacity — one slot per worker
// — so the worker's own hand-back is certain to block.
subdirs <- nil
jobs <- dirJob{path: dir}
// The file event proves the worker has read the directory and has
// nothing left to do but the blocked hand-back.
ev := <-events
if ev.fail {
t.Fatalf("walk event = %+v, want the fixture file", ev)
}
cancel()
close(jobs)
drainClosed(t, events, "the walk event stream")
}
// TestDispatchDirsClosesJobsWhenCancelled checks that a dispatcher
// leaving on cancellation closes the job channel on its way out. The
// workers range over that channel; a dispatcher that returned without
// closing it would strand every one of them.
func TestDispatchDirsClosesJobsWhenCancelled(t *testing.T) {
t.Parallel()
// Unbuffered and unread: with no worker pool behind it, the
// dispatcher can only leave through its cancellation case.
jobs := make(chan dirJob)
subdirs := make(chan []dirJob)
initial := []dirJob{{path: "/a"}, {path: "/b"}}
dispatchDirs(cancelledContext(t), initial, jobs, subdirs)
if n := drainClosed(t, jobs, "the walk job queue"); n > len(initial) {
t.Errorf("dispatcher queued %d jobs, want at most %d",
n, len(initial))
}
}
// TestFeedHashJobsClosesJobsWhenCancelled checks that the hash feeder
// abandons the runs it has not queued yet and still closes the job
// channel, which is what lets the workers' range terminate.
func TestFeedHashJobsClosesJobsWhenCancelled(t *testing.T) {
t.Parallel()
done := make(chan struct{})
// Unbuffered and unread until the feeder has returned, so the only
// way out of the feeder is its cancellation case.
jobs := make(chan []fileRec)
runs := [][]fileRec{{{path: "a"}}, {{path: "b"}}}
go func() {
defer close(done)
feedHashJobs(cancelledContext(t), runs, jobs)
}()
awaitReturn(t, done, "feedHashJobs")
if _, ok := <-jobs; ok {
t.Error("the hash job channel was left open after cancellation")
}
}
// TestHashWorkerDropsQueuedRuns checks that a cancelled hash worker
// keeps reading jobs and drops the runs rather than reading files
// nobody wants the hashes of — while still letting the range run out
// so the pool tears down. The queued run names a file that does not
// exist, so a worker that hashed it anyway would produce a result.
func TestHashWorkerDropsQueuedRuns(t *testing.T) {
t.Parallel()
done := make(chan struct{})
jobs := make(chan []fileRec, 1)
results := make(chan hashResult, 1)
run := []fileRec{{path: filepath.Join(t.TempDir(), "missing"), size: 1}}
jobs <- run
close(jobs)
go func() {
defer close(done)
hashWorker(cancelledContext(t), jobs, results)
}()
awaitReturn(t, done, "hashWorker")
select {
case r := <-results:
t.Errorf("cancelled hash worker produced %+v, want the run dropped",
r)
default:
}
}
// TestHashPhaseCancelledReturnsContextError checks the result loop's
// own exit: with the pool cancelled, no result will ever arrive, and
// the loop must leave through the cancellation rather than wait for a
// receive that cannot happen.
func TestHashPhaseCancelledReturnsContextError(t *testing.T) {
t.Parallel()
s := &scanState{
db: openTestDB(t),
toHash: []fileRec{{path: "a", size: 1, dev: 1, ino: 1}},
}
err := s.hashPhase(cancelledContext(t), 2)
if !errors.Is(err, context.Canceled) {
t.Fatalf("hashPhase on a cancelled context = %v, want %v",
err, context.Canceled)
}
}

47
db.go
View File

@@ -96,7 +96,7 @@ func openDB(path string) (*sql.DB, error) {
// openScanDatabase opens the database for the scan subcommand, creating
// the file, its parent directory, and the schema as needed.
func openScanDatabase(path string) (*sql.DB, error) {
func openScanDatabase(ctx context.Context, path string) (*sql.DB, error) {
err := os.MkdirAll(filepath.Dir(path), dbDirPerm)
if err != nil {
return nil, fmt.Errorf("create database directory: %w", err)
@@ -107,7 +107,7 @@ func openScanDatabase(path string) (*sql.DB, error) {
return nil, err
}
err = initSchema(db)
err = initSchema(ctx, db)
if err != nil {
_ = db.Close()
@@ -120,7 +120,9 @@ func openScanDatabase(path string) (*sql.DB, error) {
// openReportDatabase opens an existing database for the report and
// trees subcommands. A missing database file is an error directing the
// user to run scan first; the schema version must match exactly.
func openReportDatabase(path string) (*sql.DB, error) {
func openReportDatabase(ctx context.Context,
path string,
) (*sql.DB, error) {
_, err := os.Stat(path)
if errors.Is(err, fs.ErrNotExist) {
return nil, fmt.Errorf("%s: %w", path, errNoDatabase)
@@ -135,7 +137,7 @@ func openReportDatabase(path string) (*sql.DB, error) {
return nil, err
}
v, err := userVersion(db)
v, err := userVersion(ctx, db)
if err != nil {
_ = db.Close()
@@ -154,15 +156,15 @@ func openReportDatabase(path string) (*sql.DB, error) {
// initSchema creates the schema on a fresh database and verifies the
// schema version on an existing one.
func initSchema(db *sql.DB) error {
v, err := userVersion(db)
func initSchema(ctx context.Context, db *sql.DB) error {
v, err := userVersion(ctx, db)
if err != nil {
return err
}
switch v {
case 0:
return createSchema(db)
return createSchema(ctx, db)
case schemaVersion:
return nil
default:
@@ -173,9 +175,7 @@ func initSchema(db *sql.DB) error {
// createSchema applies the schema to a fresh database and stamps the
// schema version.
func createSchema(db *sql.DB) error {
ctx := context.Background()
func createSchema(ctx context.Context, db *sql.DB) error {
_, err := db.ExecContext(ctx, createTableSQL)
if err != nil {
return fmt.Errorf("create schema: %w", err)
@@ -191,11 +191,10 @@ func createSchema(db *sql.DB) error {
}
// userVersion reads the database's PRAGMA user_version.
func userVersion(db *sql.DB) (int, error) {
func userVersion(ctx context.Context, db *sql.DB) (int, error) {
var v int
err := db.QueryRowContext(context.Background(),
"PRAGMA user_version").Scan(&v)
err := db.QueryRowContext(ctx, "PRAGMA user_version").Scan(&v)
if err != nil {
return 0, fmt.Errorf("read schema version: %w", err)
}
@@ -204,8 +203,8 @@ func userVersion(db *sql.DB) (int, error) {
}
// loadFileRows reads every record from the files table.
func loadFileRows(db *sql.DB) ([]scanRec, error) {
rows, err := db.QueryContext(context.Background(),
func loadFileRows(ctx context.Context, db *sql.DB) ([]scanRec, error) {
rows, err := db.QueryContext(ctx,
"SELECT path, size, mtime, head, tail FROM files")
if err != nil {
return nil, fmt.Errorf("read records: %w", err)
@@ -242,10 +241,10 @@ func loadFileRows(db *sql.DB) ([]scanRec, error) {
// it carries hashes to fn. Scan change detection needs no hash
// values, and skipping the hash columns keeps the scan's in-memory
// index small on multi-million-file databases.
func loadFileMeta(db *sql.DB,
func loadFileMeta(ctx context.Context, db *sql.DB,
fn func(path string, size, mtime int64, hashed bool),
) error {
rows, err := db.QueryContext(context.Background(),
rows, err := db.QueryContext(ctx,
"SELECT path, size, mtime, head <> '' FROM files")
if err != nil {
return fmt.Errorf("read records: %w", err)
@@ -286,18 +285,18 @@ const updateBatchSize = 10000
// applyChanges writes one scan's database changes — upserts for new and
// changed files, deletes for vanished ones — in batched transactions.
// Progress is rendered on prog (one increment per change).
func applyChanges(db *sql.DB, upserts []scanRec, deletes []string,
prog *progress,
func applyChanges(ctx context.Context, db *sql.DB, upserts []scanRec,
deletes []string, prog *progress,
) error {
for batch := range slices.Chunk(upserts, updateBatchSize) {
err := applyBatch(db, batch, nil, prog)
err := applyBatch(ctx, db, batch, nil, prog)
if err != nil {
return err
}
}
for batch := range slices.Chunk(deletes, updateBatchSize) {
err := applyBatch(db, nil, batch, prog)
err := applyBatch(ctx, db, nil, batch, prog)
if err != nil {
return err
}
@@ -308,11 +307,9 @@ func applyChanges(db *sql.DB, upserts []scanRec, deletes []string,
// applyBatch commits one batch of upserts and deletes in a single
// transaction.
func applyBatch(db *sql.DB, upserts []scanRec, deletes []string,
prog *progress,
func applyBatch(ctx context.Context, db *sql.DB, upserts []scanRec,
deletes []string, prog *progress,
) error {
ctx := context.Background()
tx, err := db.BeginTx(ctx, nil)
if err != nil {
return fmt.Errorf("begin transaction: %w", err)

View File

@@ -22,7 +22,7 @@ func testDBPath(t *testing.T) string {
func openTestDB(t *testing.T) *sql.DB {
t.Helper()
db, err := openScanDatabase(testDBPath(t))
db, err := openScanDatabase(t.Context(), testDBPath(t))
if err != nil {
t.Fatal(err)
}
@@ -52,12 +52,12 @@ func TestOpenScanDatabaseCreates(t *testing.T) {
// The parent directory does not exist yet; scan must create it.
path := filepath.Join(t.TempDir(), "nested", "dir", "db.sqlite")
db, err := openScanDatabase(path)
db, err := openScanDatabase(t.Context(), path)
if err != nil {
t.Fatalf("openScanDatabase: %v", err)
}
v, err := userVersion(db)
v, err := userVersion(t.Context(), db)
if err != nil || v != schemaVersion {
t.Fatalf("userVersion = %d, %v; want %d, nil", v, err, schemaVersion)
}
@@ -65,14 +65,14 @@ func TestOpenScanDatabaseCreates(t *testing.T) {
_ = db.Close()
// Reopening an existing database must succeed and find the schema.
db, err = openScanDatabase(path)
db, err = openScanDatabase(t.Context(), path)
if err != nil {
t.Fatalf("reopen: %v", err)
}
defer func() { _ = db.Close() }()
recs, err := loadFileRows(db)
recs, err := loadFileRows(t.Context(), db)
if err != nil || len(recs) != 0 {
t.Fatalf("loadFileRows = %v, %v; want empty, nil", recs, err)
}
@@ -81,7 +81,7 @@ func TestOpenScanDatabaseCreates(t *testing.T) {
func TestOpenReportDatabaseMissing(t *testing.T) {
t.Parallel()
_, err := openReportDatabase(testDBPath(t))
_, err := openReportDatabase(t.Context(), testDBPath(t))
if !errors.Is(err, errNoDatabase) {
t.Fatalf("err = %v, want errNoDatabase", err)
}
@@ -92,7 +92,7 @@ func TestOpenReportDatabaseVersionMismatch(t *testing.T) {
path := testDBPath(t)
db, err := openScanDatabase(path)
db, err := openScanDatabase(t.Context(), path)
if err != nil {
t.Fatal(err)
}
@@ -104,7 +104,7 @@ func TestOpenReportDatabaseVersionMismatch(t *testing.T) {
_ = db.Close()
_, err = openReportDatabase(path)
_, err = openReportDatabase(t.Context(), path)
if !errors.Is(err, errSchemaVersion) {
t.Fatalf("err = %v, want errSchemaVersion", err)
}
@@ -115,14 +115,14 @@ func TestOpenReportDatabaseOK(t *testing.T) {
path := testDBPath(t)
db, err := openScanDatabase(path)
db, err := openScanDatabase(t.Context(), path)
if err != nil {
t.Fatal(err)
}
_ = db.Close()
db, err = openReportDatabase(path)
db, err = openReportDatabase(t.Context(), path)
if err != nil {
t.Fatalf("openReportDatabase: %v", err)
}
@@ -142,12 +142,13 @@ func TestApplyChangesRoundTrip(t *testing.T) {
{size: 1, mtime: 10, head: "h1", tail: "t1", path: "/a/x"},
}
err := applyChanges(db, recs, nil, newProgress("update", 2))
err := applyChanges(t.Context(), db, recs, nil,
newProgress("update", 2))
if err != nil {
t.Fatalf("applyChanges: %v", err)
}
got, err := loadFileRows(db)
got, err := loadFileRows(t.Context(), db)
if err != nil {
t.Fatal(err)
}
@@ -164,13 +165,13 @@ func TestApplyChangesRoundTrip(t *testing.T) {
// removes exactly its path.
upd := scanRec{size: 3, mtime: 30, head: "h3", tail: "t3", path: "/a/x"}
err = applyChanges(db, []scanRec{upd},
err = applyChanges(t.Context(), db, []scanRec{upd},
[]string{"/a/tab\tnew\nline"}, newProgress("update", 2))
if err != nil {
t.Fatalf("applyChanges: %v", err)
}
got, err = loadFileRows(db)
got, err = loadFileRows(t.Context(), db)
if err != nil {
t.Fatal(err)
}
@@ -197,12 +198,13 @@ func TestApplyChangesBatching(t *testing.T) {
})
}
err := applyChanges(db, recs, nil, newProgress("update", int64(n)))
err := applyChanges(t.Context(), db, recs, nil,
newProgress("update", int64(n)))
if err != nil {
t.Fatalf("applyChanges: %v", err)
}
got, err := loadFileRows(db)
got, err := loadFileRows(t.Context(), db)
if err != nil || len(got) != n {
t.Fatalf("loadFileRows = %d rows, %v; want %d", len(got), err, n)
}
@@ -212,12 +214,13 @@ func TestApplyChangesBatching(t *testing.T) {
deletes = append(deletes, r.path)
}
err = applyChanges(db, nil, deletes, newProgress("update", int64(n)))
err = applyChanges(t.Context(), db, nil, deletes,
newProgress("update", int64(n)))
if err != nil {
t.Fatalf("applyChanges deletes: %v", err)
}
got, err = loadFileRows(db)
got, err = loadFileRows(t.Context(), db)
if err != nil || len(got) != 0 {
t.Fatalf("loadFileRows = %d rows, %v; want 0", len(got), err)
}

142
main.go
View File

@@ -16,20 +16,36 @@
package main
import (
"context"
"errors"
"fmt"
"io"
"os"
"runtime"
"github.com/spf13/cobra"
)
// Exit codes: 0 is success (even with per-file warnings), exitFatal is
// a fatal error, exitUsage is a usage error.
// Exit codes: exitOK is success (even with per-file warnings),
// exitFatal is a fatal error, exitUsage is a usage error.
const (
exitOK = 0
exitFatal = 1
exitUsage = 2
)
// The subcommand names, as typed on the command line.
const (
cmdScan = "scan"
cmdReport = "report"
cmdTrees = "trees"
)
// errNoSubcommand is returned by the root command when it is invoked
// without a subcommand. That is a usage error, and the usage text
// cobra prints for it is the whole message.
var errNoSubcommand = errors.New("no subcommand")
// Version is the build version, injected at link time via -ldflags
// (see the Makefile); "dev" for a plain go build.
//
@@ -37,22 +53,64 @@ const (
var Version = "dev"
func main() {
os.Exit(run(os.Args[1:], os.Stderr))
}
// run executes args against the command tree and returns the process
// exit code. It is the program's single exit point: the subcommands
// return their errors instead of exiting, so every deferred cleanup —
// above all closing the database, which checkpoints the SQLite WAL —
// runs before the process ends.
func run(args []string, stderr io.Writer) int {
// A nil slice makes cobra fall back to os.Args, which would let a
// test binary's own flags reach the command tree.
if args == nil {
args = []string{}
}
root := newRootCommand(stderr)
root.SetArgs(args)
err := root.Execute()
var fatal fatalError
switch {
case err == nil:
return exitOK
case errors.As(err, &fatal):
// The command ran and failed: a runtime error, reported
// without the usage text that a usage error gets.
_, _ = fmt.Fprintf(stderr, "sfdupes: %v\n", err)
return exitFatal
default:
// A usage error: cobra has already printed the message and
// the usage text.
return exitUsage
}
}
// newRootCommand builds the command tree. Everything on stdout is
// machine-readable data; all human-facing output (help, usage, errors)
// goes to stderr.
func newRootCommand(stderr io.Writer) *cobra.Command {
root := &cobra.Command{
Use: "sfdupes",
Short: "Find candidate duplicate files by size and head/tail SHA-256",
Version: Version,
Args: cobra.NoArgs,
Run: func(cmd *cobra.Command, _ []string) {
// A missing subcommand prints usage and exits 2.
_ = cmd.Usage()
RunE: func(cmd *cobra.Command, _ []string) error {
// A missing subcommand prints usage and exits 2: cobra
// prints the usage text for the returned error, and run
// maps everything that is not a fatal error to exit 2.
cmd.SilenceErrors = true
os.Exit(exitUsage)
return errNoSubcommand
},
}
// Everything on stdout is machine-readable data; all human-facing
// output (help, usage, errors) goes to stderr.
root.SetOut(os.Stderr)
root.SetErr(os.Stderr)
root.SetOut(stderr)
root.SetErr(stderr)
root.CompletionOptions.DisableDefaultCmd = true
var (
@@ -61,12 +119,12 @@ func main() {
)
scanCmd := &cobra.Command{
Use: "scan [--workers N] [-x] PATH...",
Use: cmdScan + " [--workers N] [-x] PATH...",
Short: "Walk trees and synchronize the scan database",
Args: cobra.MinimumNArgs(1),
Run: func(_ *cobra.Command, args []string) {
runScan(args, scanWorkers, scanOneFS)
},
RunE: runE(func(ctx context.Context, args []string) error {
return runScan(ctx, args, scanWorkers, scanOneFS)
}),
}
scanCmd.Flags().IntVar(&scanWorkers, "workers", runtime.NumCPU(),
"concurrent workers for the walk and hash phases")
@@ -74,35 +132,59 @@ func main() {
"do not cross filesystem boundaries")
reportCmd := &cobra.Command{
Use: "report",
Use: cmdReport,
Short: "Read the scan database and print the file-level duplicates report",
Args: cobra.NoArgs,
Run: func(_ *cobra.Command, _ []string) {
runReport()
},
RunE: runE(func(ctx context.Context, _ []string) error {
return runReport(ctx)
}),
}
treesCmd := &cobra.Command{
Use: "trees",
Use: cmdTrees,
Short: "Read the scan database and print the duplicate-tree report",
Args: cobra.NoArgs,
Run: func(_ *cobra.Command, _ []string) {
runTrees()
},
RunE: runE(func(ctx context.Context, _ []string) error {
return runTrees(ctx)
}),
}
root.AddCommand(scanCmd, reportCmd, treesCmd)
err := root.Execute()
return root
}
// runE adapts a subcommand implementation to cobra's RunE. Cobra
// prints the error and the command's usage text for every error RunE
// returns, but a subcommand that ran and failed has no usage problem
// to report: both are silenced here, and the error is marked fatal so
// that run reports it on stderr and exits 1 rather than 2. The command's
// context is handed to the implementation: cancelling it unwinds the
// scan's worker pools.
func runE(
fn func(ctx context.Context, args []string) error,
) func(*cobra.Command, []string) error {
return func(cmd *cobra.Command, args []string) error {
cmd.SilenceUsage = true
cmd.SilenceErrors = true
err := fn(cmd.Context(), args)
if err != nil {
// Cobra has already printed the error and usage to stderr;
// an invalid subcommand or bad arguments is a usage error.
os.Exit(exitUsage)
return fatalError{err: err}
}
return nil
}
}
// fatalf reports a fatal error and exits 1.
func fatalf(format string, args ...any) {
fmt.Fprintf(os.Stderr, "sfdupes: "+format+"\n", args...)
os.Exit(exitFatal)
// fatalError marks a runtime failure, as opposed to the usage errors
// cobra itself produces while parsing arguments and flags. Both come
// out of Execute as plain errors, so the wrapper is what tells run to
// report this one as "sfdupes: ..." and exit 1.
type fatalError struct {
err error
}
func (e fatalError) Error() string { return e.err.Error() }
func (e fatalError) Unwrap() error { return e.err }

397
main_test.go Normal file
View File

@@ -0,0 +1,397 @@
package main
import (
"bytes"
"context"
"errors"
"io"
"io/fs"
"os"
"path/filepath"
"strconv"
"strings"
"testing"
)
// usageMarker is the first line of cobra's usage text, which a usage
// error prints and a runtime failure must not.
const usageMarker = "Usage:"
// walSuffixes are the SQLite sidecar files a WAL-mode database keeps
// while it is open. A clean close checkpoints the WAL and removes
// both; finding either afterwards means the database was never closed.
//
//nolint:gochecknoglobals // a constant list, immutable by convention
var walSuffixes = []string{"-wal", "-shm"}
// assertNoSidecars fails when a WAL sidecar is still present beside the
// database at path.
func assertNoSidecars(t *testing.T, path string) {
t.Helper()
for _, suffix := range walSuffixes {
_, err := os.Stat(path + suffix)
if err == nil {
t.Errorf("%s%s still present: the database was not closed",
path, suffix)
continue
}
if !errors.Is(err, fs.ErrNotExist) {
t.Fatal(err)
}
}
}
// captureStdout redirects os.Stdout to a file for the rest of the test
// and returns a function reading back everything written to it. Only
// machine-readable data belongs on stdout (README design goal 4), so
// the tests assert on it directly.
func captureStdout(t *testing.T) func() string {
t.Helper()
f, err := os.Create(filepath.Join(t.TempDir(), "stdout"))
if err != nil {
t.Fatal(err)
}
saved := os.Stdout
os.Stdout = f
t.Cleanup(func() {
os.Stdout = saved
_ = f.Close()
})
return func() string {
// Read what has been written without disturbing the write
// offset, so the capture can be inspected more than once.
size, err := f.Seek(0, io.SeekCurrent)
if err != nil {
t.Fatal(err)
}
if size == 0 {
return ""
}
b := make([]byte, size)
_, err = f.ReadAt(b, 0)
if err != nil {
t.Fatal(err)
}
return string(b)
}
}
// brokenDatabase writes a database that opens cleanly and passes the
// schema-version check but has no files table, so the first query
// fails with the database already open: a fatal error on a path that
// owns an open database.
func brokenDatabase(t *testing.T) string {
t.Helper()
path := testDBPath(t)
db, err := openDB(path)
if err != nil {
t.Fatal(err)
}
_, err = db.ExecContext(context.Background(),
"PRAGMA user_version = "+strconv.Itoa(schemaVersion))
if err != nil {
t.Fatal(err)
}
err = db.Close()
if err != nil {
t.Fatal(err)
}
return path
}
func TestOpenDatabaseKeepsWALWhileOpen(t *testing.T) {
t.Parallel()
// The premise of the fatal-path tests below: an open database has
// a -wal sidecar, so its absence afterwards is evidence that the
// database was closed and its WAL checkpointed.
path := testDBPath(t)
db, err := openScanDatabase(t.Context(), path)
if err != nil {
t.Fatal(err)
}
_, err = os.Stat(path + "-wal")
if err != nil {
t.Fatalf("no -wal beside an open database: %v", err)
}
err = db.Close()
if err != nil {
t.Fatal(err)
}
assertNoSidecars(t, path)
}
func TestRunFatalAfterOpenClosesDatabase(t *testing.T) {
// Every subcommand that owns an open database must close it when
// it fails: no os.Exit between the open and the return.
cases := map[string][]string{
cmdScan: {cmdScan},
cmdReport: {cmdReport},
cmdTrees: {cmdTrees},
}
for name, args := range cases {
t.Run(name, func(t *testing.T) {
path := brokenDatabase(t)
t.Setenv(databaseEnv, path)
if name == cmdScan {
args = append(args, t.TempDir())
}
var stderr bytes.Buffer
stdout := captureStdout(t)
code := run(args, &stderr)
if code != exitFatal {
t.Errorf("run(%v) = %d, want %d", args, code, exitFatal)
}
assertNoSidecars(t, path)
assertFatalOutput(t, stderr.String(), stdout())
// Proof that the failure happened after the open: only a
// query against the opened database can report this.
if !strings.Contains(stderr.String(), "no such table: files") {
t.Errorf("stderr = %q, want the failure to come from a "+
"query on the open database", stderr.String())
}
})
}
}
func TestRunMissingOperandIsFatalNotUsage(t *testing.T) {
// README §Error handling: a PATH operand that does not exist is a
// fatal error (1), not a usage error (2) — and a runtime failure
// must not dump the usage text.
t.Setenv(databaseEnv, testDBPath(t))
var stderr bytes.Buffer
stdout := captureStdout(t)
missing := filepath.Join(t.TempDir(), "nope")
code := run([]string{cmdScan, missing}, &stderr)
if code != exitFatal {
t.Errorf("run(scan %s) = %d, want %d", missing, code, exitFatal)
}
assertFatalOutput(t, stderr.String(), stdout())
}
// assertFatalOutput checks that a fatal error was reported the way
// README §Error handling and design goal 4 require: the message on
// stderr, prefixed with the program name, no usage text, and nothing
// at all on stdout.
func assertFatalOutput(t *testing.T, stderr, stdout string) {
t.Helper()
if !strings.Contains(stderr, "sfdupes: ") {
t.Errorf("stderr = %q, want a \"sfdupes: \" error report", stderr)
}
if strings.Contains(stderr, usageMarker) {
t.Errorf("stderr = %q, want no usage text for a runtime failure",
stderr)
}
if stdout != "" {
t.Errorf("stdout = %q, want nothing (data only)", stdout)
}
}
func TestRunUsageErrors(t *testing.T) {
// Usage errors keep exiting 2 with cobra's own report on stderr.
cases := map[string]struct {
args []string
want string
}{
"no subcommand": {[]string{}, usageMarker},
"scan without paths": {[]string{cmdScan}, usageMarker},
"report with args": {[]string{cmdReport, "x"}, usageMarker},
"trees with args": {[]string{cmdTrees, "x"}, usageMarker},
"unknown flag": {[]string{cmdScan, "--nope", "/"}, usageMarker},
"unknown subcommand": {[]string{"nope"}, "unknown command"},
}
for name, tc := range cases {
t.Run(name, func(t *testing.T) {
// No usage error may reach the database, so point it at a
// path that does not exist.
t.Setenv(databaseEnv, testDBPath(t))
var stderr bytes.Buffer
stdout := captureStdout(t)
code := run(tc.args, &stderr)
if code != exitUsage {
t.Errorf("run(%v) = %d, want %d", tc.args, code, exitUsage)
}
if !strings.Contains(stderr.String(), tc.want) {
t.Errorf("stderr = %q, want %q", stderr.String(), tc.want)
}
if got := stdout(); got != "" {
t.Errorf("stdout = %q, want nothing (data only)", got)
}
})
}
}
// TestRunHelpAndVersionSucceed checks that the two informational flags
// exit 0 and keep their human-facing output on stderr.
//
//nolint:paralleltest // captureStdout replaces the process-wide os.Stdout
func TestRunHelpAndVersionSucceed(t *testing.T) {
assertHumanOutput(t, "--help")
assertHumanOutput(t, "--version")
}
// assertHumanOutput runs sfdupes with one informational flag and checks
// that it succeeds with its output on stderr and stdout untouched
// (README design goal 4).
func assertHumanOutput(t *testing.T, arg string) {
t.Helper()
var stderr bytes.Buffer
stdout := captureStdout(t)
code := run([]string{arg}, &stderr)
if code != exitOK {
t.Errorf("run(%s) = %d, want %d", arg, code, exitOK)
}
if stderr.Len() == 0 {
t.Errorf("run(%s) wrote nothing to stderr", arg)
}
if got := stdout(); got != "" {
t.Errorf("stdout = %q, want nothing (data only)", got)
}
}
// scanFixture builds a small tree holding one duplicate pair and one
// unreadable file and scans it into the database the caller has
// pointed SFDUPES_DATABASE at. The unreadable file makes the scan warn
// and skip, which README §Error handling still calls a successful run.
// It returns the duplicate pair's paths.
func scanFixture(t *testing.T) []string {
t.Helper()
dir := t.TempDir()
dupes := []string{
writeFile(t, dir, "one/a.bin", pattern(1, 300)),
writeFile(t, dir, "two/a.bin", pattern(1, 300)),
}
// Same size as the pair, so the scan queues it for hashing and the
// read fails.
unreadable := writeFile(t, dir, "unreadable.bin", pattern(2, 300))
err := os.Chmod(unreadable, 0)
if err != nil {
t.Fatal(err)
}
var stderr bytes.Buffer
stdout := captureStdout(t)
code := run([]string{cmdScan, dir}, &stderr)
if code != exitOK {
t.Fatalf("run(scan) = %d, want %d; stderr: %s",
code, exitOK, stderr.String())
}
if got := stdout(); got != "" {
t.Errorf("scan stdout = %q, want nothing (data only)", got)
}
return dupes
}
func TestRunScanSucceedsDespiteWarnings(t *testing.T) {
path := testDBPath(t)
t.Setenv(databaseEnv, path)
scanFixture(t)
assertNoSidecars(t, path)
}
func TestRunReportSucceeds(t *testing.T) {
path := testDBPath(t)
t.Setenv(databaseEnv, path)
dupes := scanFixture(t)
var stderr bytes.Buffer
stdout := captureStdout(t)
code := run([]string{cmdReport}, &stderr)
if code != exitOK {
t.Fatalf("run(report) = %d, want %d; stderr: %s",
code, exitOK, stderr.String())
}
want := "first\tdupe\tsize\n" + dupes[0] + "\t" + dupes[1] + "\t300\n"
if got := stdout(); got != want {
t.Errorf("stdout = %q, want %q", got, want)
}
assertNoSidecars(t, path)
}
func TestRunTreesSucceeds(t *testing.T) {
path := testDBPath(t)
t.Setenv(databaseEnv, path)
dupes := scanFixture(t)
var stderr bytes.Buffer
stdout := captureStdout(t)
code := run([]string{cmdTrees}, &stderr)
if code != exitOK {
t.Fatalf("run(trees) = %d, want %d; stderr: %s",
code, exitOK, stderr.String())
}
// The two directories holding the duplicate pair are duplicate
// trees of each other.
want := "first\tdupe\tfiles\tsize\n" +
filepath.Dir(dupes[0]) + "\t" + filepath.Dir(dupes[1]) + "\t1\t300\n"
if got := stdout(); got != want {
t.Errorf("stdout = %q, want %q", got, want)
}
assertNoSidecars(t, path)
}

View File

@@ -2,6 +2,7 @@ package main
import (
"bufio"
"context"
"fmt"
"os"
"slices"
@@ -28,23 +29,26 @@ type scanRec struct {
// loadRecords opens the database and reads every file record for the
// report and trees subcommands. Any database problem — including a
// missing database — is fatal.
func loadRecords() []scanRec {
// missing database — is fatal. The error is returned rather than
// exiting, so that the deferred close — which checkpoints the SQLite
// WAL — always runs; the database is closed before the caller formats
// its output, so it stays closed even if that output fails.
func loadRecords(ctx context.Context) ([]scanRec, error) {
dbPath := databasePath()
db, err := openReportDatabase(dbPath)
db, err := openReportDatabase(ctx, dbPath)
if err != nil {
fatalf("%v", err)
return nil, err
}
defer func() { _ = db.Close() }()
recs, err := loadFileRows(db)
recs, err := loadFileRows(ctx, db)
if err != nil {
fatalf("database %s: %v", dbPath, err)
return nil, fmt.Errorf("database %s: %w", dbPath, err)
}
return recs
return recs, nil
}
// dupeGroup is one set of candidate-duplicate files: identical size,
@@ -59,15 +63,19 @@ type dupeGroup struct {
// from the database and prints the file-level duplicates report as TSV
// on stdout. It never touches the scanned filesystem; its only I/O is
// the database, stdout, and stderr.
func runReport() {
recs := loadRecords()
func runReport(ctx context.Context) error {
recs, err := loadRecords(ctx)
if err != nil {
return err
}
dupes := collectDupeGroups(recs)
out := bufio.NewWriterSize(os.Stdout, ioBufSize)
_, err := fmt.Fprintln(out, "first\tdupe\tsize")
_, err = fmt.Fprintln(out, "first\tdupe\tsize")
if err != nil {
fatalf("write stdout: %v", err)
return fmt.Errorf("write stdout: %w", err)
}
dupeFiles := 0
@@ -79,7 +87,7 @@ func runReport() {
_, err = fmt.Fprintf(out, "%s\t%s\t%d\n",
g.paths[0], p, g.size)
if err != nil {
fatalf("write stdout: %v", err)
return fmt.Errorf("write stdout: %w", err)
}
dupeFiles++
@@ -89,13 +97,15 @@ func runReport() {
err = out.Flush()
if err != nil {
fatalf("write stdout: %v", err)
return fmt.Errorf("write stdout: %w", err)
}
fmt.Fprintf(os.Stderr,
"report: %d records read, %d duplicate groups, %d dupe files, "+
"%s reclaimable\n",
len(recs), len(dupes), dupeFiles, humanBytes(reclaimable))
return nil
}
// collectDupeGroups groups records by signature and returns every group

310
scan.go
View File

@@ -2,6 +2,7 @@ package main
import (
"cmp"
"context"
"crypto/sha256"
"database/sql"
"encoding/hex"
@@ -49,25 +50,34 @@ type fileMeta struct {
// under the PATH operands. Only files whose size at least one other
// file shares are ever hashed: a size-unique file cannot be a
// duplicate. Flag parsing and the at-least-one-operand check are done
// by cobra.
func runScan(roots []string, workers int, oneFS bool) {
// by cobra. Errors are returned rather than exiting, so that the
// deferred close — which checkpoints the SQLite WAL — always runs.
// Cancelling ctx unwinds the worker pools and aborts the scan with the
// context's error.
func runScan(ctx context.Context, roots []string, workers int,
oneFS bool,
) error {
if workers < 1 {
workers = 1
}
roots = resolveRoots(roots)
roots, err := resolveRoots(roots)
if err != nil {
return err
}
dbPath := databasePath()
db, err := openScanDatabase(dbPath)
db, err := openScanDatabase(ctx, dbPath)
if err != nil {
fatalf("%v", err)
return err
}
defer func() { _ = db.Close() }()
st, err := syncScan(db, roots, workers, oneFS)
st, err := syncScan(ctx, db, roots, workers, oneFS)
if err != nil {
fatalf("update database %s: %v", dbPath, err)
return fmt.Errorf("update database %s: %w", dbPath, err)
}
fmt.Fprintf(os.Stderr,
@@ -75,31 +85,33 @@ func runScan(roots []string, workers int, oneFS bool) {
"%d unchanged), %d skipped\n",
st.added+st.updated+st.unchanged, st.added, st.updated,
st.removed, st.unchanged, st.skipped)
return nil
}
// resolveRoots converts each PATH operand to an absolute, lexically
// cleaned path (symlinks are not resolved) and verifies that it
// exists. Database records are keyed by absolute path, so scan results
// must not depend on the working directory.
func resolveRoots(roots []string) []string {
func resolveRoots(roots []string) ([]string, error) {
abs := make([]string, 0, len(roots))
for _, root := range roots {
a, err := filepath.Abs(root)
if err != nil {
fatalf("resolve %s: %v", root, err)
return nil, fmt.Errorf("resolve %s: %w", root, err)
}
// A nonexistent operand is a fatal error before any scanning.
_, err = os.Lstat(a)
if err != nil {
fatalf("%v", err)
return nil, err
}
abs = append(abs, a)
}
return abs
return abs, nil
}
// pruneRoots drops operands already covered by another operand:
@@ -161,28 +173,41 @@ type scanState struct {
// and update (record the size-unique files without reading them, and
// delete the records the scan no longer verifies). Records outside
// the roots are never touched.
func syncScan(db *sql.DB, roots []string, workers int,
oneFS bool,
func syncScan(ctx context.Context, db *sql.DB, roots []string,
workers int, oneFS bool,
) (scanStats, error) {
roots = pruneRoots(roots)
s := &scanState{db: db}
err := s.loadIndex(roots)
err := s.loadIndex(ctx, roots)
if err != nil {
return s.st, err
}
changed, unhashed := s.walkPhase(startWalk(roots, oneFS, workers))
changed, unhashed := s.walkPhase(startWalk(ctx, roots, oneFS, workers))
// A cancelled walk stops early, so its size census covers only part
// of the roots, and every file it never reached looks vanished to
// the update phase. Defence in depth rather than the only barrier:
// that phase would today fail on its first BeginTx with the same
// cancelled context before deleting anything. But it is the barrier
// that survives a later decision to let an interrupted scan commit
// what it has, and it turns a confusing failure deep in the update
// phase into a clean abort at the phase boundary.
err = ctx.Err()
if err != nil {
return s.st, err
}
s.partition(changed, unhashed)
err = s.hashPhase(workers)
err = s.hashPhase(ctx, workers)
if err != nil {
return s.st, err
}
return s.st, s.updatePhase()
return s.st, s.updatePhase(ctx)
}
// loadIndex indexes the database records under the scan roots for
@@ -190,7 +215,7 @@ func syncScan(db *sql.DB, roots []string, workers int,
// them: out-of-scope records join the size census so a scanned file
// can be recognized as a possible duplicate of a tree scanned
// separately into the same database.
func (s *scanState) loadIndex(roots []string) error {
func (s *scanState) loadIndex(ctx context.Context, roots []string) error {
// Indexing tens of millions of records takes real time; without a
// display the scan looks hung before the walk begins.
prog := newProgress("load", -1)
@@ -198,7 +223,7 @@ func (s *scanState) loadIndex(roots []string) error {
s.existing = make(map[string]fileMeta)
return loadFileMeta(s.db,
return loadFileMeta(ctx, s.db,
func(path string, size, mtime int64, hashed bool) {
prog.increment()
@@ -363,28 +388,29 @@ func sameInode(a, b fileRec) bool {
// reads, so the bar shows a real ETA. A run that fails to hash is
// warned about and skipped; stale records for its paths, if any, are
// deleted by the update phase.
func (s *scanState) hashPhase(workers int) error {
//
// Returning early — a failed database write, or a cancelled scan — must
// not strand the pool: the feeder would park forever on a full jobs
// channel and every worker on a full results channel. The deferred stop
// is what prevents that.
func (s *scanState) hashPhase(ctx context.Context, workers int) error {
runs := hashRuns(s.toHash)
s.toHash = nil
jobs := make(chan []fileRec, workQueueDepth)
results := make(chan hashResult, workQueueDepth)
startHashWorkers(jobs, results, workers)
go func() {
for _, run := range runs {
jobs <- run
}
close(jobs)
}()
pool := startHashPool(ctx, runs, workers)
defer pool.stop()
prog := newProgress("hash", int64(len(runs)))
defer prog.finish()
for range runs {
r := <-results
var r hashResult
select {
case r = <-pool.results:
case <-ctx.Done():
return ctx.Err()
}
prog.increment()
@@ -396,7 +422,7 @@ func (s *scanState) hashPhase(workers int) error {
continue
}
err := s.recordRun(r)
err := s.recordRun(ctx, r)
if err != nil {
return err
}
@@ -408,7 +434,7 @@ func (s *scanState) hashPhase(workers int) error {
// recordRun folds one hash result into the running batch: every path
// in the run (one file, or several hard links to it) gets a record
// with the shared hashes.
func (s *scanState) recordRun(r hashResult) error {
func (s *scanState) recordRun(ctx context.Context, r hashResult) error {
for _, rec := range r.run {
s.resolve(rec.path)
@@ -425,7 +451,7 @@ func (s *scanState) recordRun(r hashResult) error {
return nil
}
err := applyBatch(s.db, s.batch, nil, nil)
err := applyBatch(ctx, s.db, s.batch, nil, nil)
s.batch = s.batch[:0]
return err
@@ -436,7 +462,7 @@ func (s *scanState) recordRun(r hashResult) error {
// size-unique new or changed file, and deletions for every record the
// scan did not verify (vanished files, plus paths that failed to stat
// or hash).
func (s *scanState) updatePhase() error {
func (s *scanState) updatePhase(ctx context.Context) error {
deletes := make([]string, 0, len(s.existing))
for path := range s.existing {
deletes = append(deletes, path)
@@ -452,7 +478,7 @@ func (s *scanState) updatePhase() error {
defer prog.finish()
err := applyChanges(s.db, s.batch, nil, prog)
err := applyChanges(ctx, s.db, s.batch, nil, prog)
if err != nil {
return err
}
@@ -469,13 +495,13 @@ func (s *scanState) updatePhase() error {
})
}
err = applyBatch(s.db, recs, nil, prog)
err = applyBatch(ctx, s.db, recs, nil, prog)
if err != nil {
return err
}
}
return applyChanges(s.db, nil, deletes, prog)
return applyChanges(ctx, s.db, nil, deletes, prog)
}
// underAnyRoot reports whether path is any of the roots or lies under
@@ -525,34 +551,53 @@ type walkEvent struct {
// startWalk seeds every root into the shared walk worker pool and
// returns the event stream: one record per regular file, one warning
// event per per-path error. The channel is closed when the walk
// completes.
func startWalk(roots []string, oneFS bool, workers int) <-chan walkEvent {
jobs, subdirs, events := startWalkWorkers(workers, oneFS)
// completes, and also when ctx is cancelled — every goroutine in the
// pool abandons its blocking send in that case, so the consumer sees a
// truncated but properly terminated stream instead of a stalled one.
func startWalk(ctx context.Context, roots []string, oneFS bool,
workers int,
) <-chan walkEvent {
jobs, subdirs, events := startWalkWorkers(ctx, workers, oneFS)
go func() {
initial := make([]dirJob, 0, len(roots))
for _, root := range roots {
initial = append(initial, seedRoot(root, events)...)
initial = append(initial, seedRoot(ctx, root, events)...)
}
dispatchDirs(initial, jobs, subdirs)
dispatchDirs(ctx, initial, jobs, subdirs)
}()
return events
}
// sendEvent delivers one walk event, abandoning the send when the scan
// is cancelled. Every walk goroutine reaches the consumer through this
// one channel, so this is where a cancelled walk unwinds rather than
// parking on a buffer nobody is draining.
func sendEvent(ctx context.Context, events chan<- walkEvent,
ev walkEvent,
) {
select {
case events <- ev:
case <-ctx.Done():
}
}
// seedRoot turns one PATH operand into the walk's starting state: a
// regular-file operand is statted and emitted directly, a directory
// operand becomes an initial job, and a symlink or other non-regular
// operand yields nothing (symlinks are never followed, including as
// operands).
func seedRoot(root string, events chan<- walkEvent) []dirJob {
func seedRoot(ctx context.Context, root string,
events chan<- walkEvent,
) []dirJob {
fi, err := os.Lstat(root)
if err != nil {
events <- walkEvent{
sendEvent(ctx, events, walkEvent{
warn: fmt.Sprintf("walk %s: %v", root, err),
fail: true,
}
})
return nil
}
@@ -569,13 +614,13 @@ func seedRoot(root string, events chan<- walkEvent) []dirJob {
case fi.Mode().IsRegular():
dev, ino := inodeOfInfo(fi)
events <- walkEvent{rec: fileRec{
sendEvent(ctx, events, walkEvent{rec: fileRec{
path: root,
size: fi.Size(),
mtime: fi.ModTime().Unix(),
dev: dev,
ino: ino,
}}
}})
return nil
default:
@@ -586,8 +631,10 @@ func seedRoot(root string, events chan<- walkEvent) []dirJob {
// startWalkWorkers starts the walk worker pool. Each worker processes
// one directory at a time, emitting an event per regular file and
// handing discovered subdirectories back to the dispatcher; events is
// closed once every worker has finished.
func startWalkWorkers(workers int,
// closed once every worker has finished. A cancelled scan makes the
// workers drop the directories still queued rather than stop reading
// jobs, so the range always runs out and the pool always tears down.
func startWalkWorkers(ctx context.Context, workers int,
oneFS bool,
) (chan dirJob, chan []dirJob, chan walkEvent) {
jobs := make(chan dirJob, workQueueDepth)
@@ -599,7 +646,14 @@ func startWalkWorkers(workers int,
for range workers {
wg.Go(func() {
for job := range jobs {
subdirs <- walkOneDir(job, oneFS, events)
if ctx.Err() != nil {
continue
}
select {
case subdirs <- walkOneDir(ctx, job, oneFS, events):
case <-ctx.Done():
}
}
})
}
@@ -615,11 +669,15 @@ func startWalkWorkers(workers int,
// dispatchDirs feeds directory jobs to the walk workers, queueing
// newly discovered subdirectories (newest first, which keeps the
// frontier small) until every directory has been processed, then
// closes jobs.
func dispatchDirs(initial []dirJob, jobs chan<- dirJob,
subdirs <-chan []dirJob,
// closes jobs. jobs is closed on every path out, cancellation
// included: the workers range over it, and a dispatcher that returned
// without closing would strand all of them.
func dispatchDirs(ctx context.Context, initial []dirJob,
jobs chan<- dirJob, subdirs <-chan []dirJob,
) {
go func() {
defer close(jobs)
queue := slices.Clone(initial)
pending := len(queue)
@@ -640,23 +698,25 @@ func dispatchDirs(initial []dirJob, jobs chan<- dirJob,
case subs := <-subdirs:
pending += len(subs) - 1
queue = append(queue, subs...)
case <-ctx.Done():
return
}
}
close(jobs)
}()
}
// walkOneDir reads one directory, emitting an event per regular-file
// entry and a warning event per unreadable one, and returns the
// subdirectories to descend into.
func walkOneDir(job dirJob, oneFS bool, events chan<- walkEvent) []dirJob {
func walkOneDir(ctx context.Context, job dirJob, oneFS bool,
events chan<- walkEvent,
) []dirJob {
entries, err := os.ReadDir(job.path)
if err != nil {
events <- walkEvent{
sendEvent(ctx, events, walkEvent{
warn: fmt.Sprintf("walk %s: %v", job.path, err),
fail: true,
}
})
return nil
}
@@ -667,7 +727,7 @@ func walkOneDir(job dirJob, oneFS bool, events chan<- walkEvent) []dirJob {
p := filepath.Join(job.path, e.Name())
if e.IsDir() {
if sub, ok := subdirJob(p, e, job, oneFS, events); ok {
if sub, ok := subdirJob(ctx, p, e, job, oneFS, events); ok {
subs = append(subs, sub)
}
@@ -679,7 +739,7 @@ func walkOneDir(job dirJob, oneFS bool, events chan<- walkEvent) []dirJob {
continue
}
emitFile(p, e, events)
emitFile(ctx, p, e, events)
}
return subs
@@ -690,13 +750,15 @@ func walkOneDir(job dirJob, oneFS bool, events chan<- walkEvent) []dirJob {
// metadata is still hot; a path that fails to stat (or stops being a
// regular file) between the directory read and the lstat is warned
// about and skipped.
func emitFile(p string, e fs.DirEntry, events chan<- walkEvent) {
func emitFile(ctx context.Context, p string, e fs.DirEntry,
events chan<- walkEvent,
) {
info, err := e.Info()
if err != nil {
events <- walkEvent{
sendEvent(ctx, events, walkEvent{
warn: fmt.Sprintf("stat %s: %v", p, err),
fail: true,
}
})
return
}
@@ -707,21 +769,21 @@ func emitFile(p string, e fs.DirEntry, events chan<- walkEvent) {
dev, ino := inodeOfInfo(info)
events <- walkEvent{rec: fileRec{
sendEvent(ctx, events, walkEvent{rec: fileRec{
path: p,
size: info.Size(),
mtime: info.ModTime().Unix(),
dev: dev,
ino: ino,
}}
}})
}
// subdirJob applies the descent rules to directory p: never enter
// .zfs (ZFS snapshot pseudo-dirs would list every file once per
// snapshot), and with -x never enter a directory on a different
// filesystem than its operand.
func subdirJob(p string, e fs.DirEntry, parent dirJob, oneFS bool,
events chan<- walkEvent,
func subdirJob(ctx context.Context, p string, e fs.DirEntry,
parent dirJob, oneFS bool, events chan<- walkEvent,
) (dirJob, bool) {
if e.Name() == ".zfs" {
return dirJob{}, false
@@ -734,10 +796,10 @@ func subdirJob(p string, e fs.DirEntry, parent dirJob, oneFS bool,
info, err := e.Info()
if err != nil {
events <- walkEvent{
sendEvent(ctx, events, walkEvent{
warn: fmt.Sprintf("walk %s: %v", p, err),
fail: true,
}
})
return dirJob{}, false
}
@@ -781,22 +843,102 @@ type hashResult struct {
err error
}
// startHashWorkers starts the hash worker pool: workers read inode
// runs, hash each run's first path (all paths in a run are hard links
// to the same inode), write one result per run, and exit when jobs is
// closed.
func startHashWorkers(jobs <-chan []fileRec, results chan<- hashResult,
// hashPool owns every goroutine of the hash worker pool: the feeder
// that queues the inode runs and the workers that read them. Both block
// on channel sends, so both are cancellable — the pool's context is
// derived from the scan's, and stop cancels it and waits the goroutines
// out. The consumer must call stop on every path out of the phase, not
// just the happy one.
type hashPool struct {
results <-chan hashResult
cancel context.CancelFunc
done <-chan struct{}
}
// startHashPool starts the feeder and the workers over runs. Workers
// hash each run's first path (all paths in a run are hard links to the
// same inode) and write one result per run.
func startHashPool(ctx context.Context, runs [][]fileRec,
workers int,
) {
) *hashPool {
ctx, cancel := context.WithCancel(ctx)
jobs := make(chan []fileRec, workQueueDepth)
results := make(chan hashResult, workQueueDepth)
var wg sync.WaitGroup
wg.Go(func() { feedHashJobs(ctx, runs, jobs) })
for range workers {
wg.Go(func() { hashWorker(ctx, jobs, results) })
}
done := make(chan struct{})
go func() {
for run := range jobs {
head, tail, err := hashHeadTail(run[0].path, run[0].size)
results <- hashResult{
run: run, head: head, tail: tail, err: err,
}
}
wg.Wait()
close(done)
}()
return &hashPool{results: results, cancel: cancel, done: done}
}
// stop cancels the pool and blocks until every one of its goroutines
// has exited, draining results while it waits: a worker already parked
// on a send observes the cancellation only once a receiver frees it.
// Calling stop more than once is safe.
func (p *hashPool) stop() {
p.cancel()
for {
select {
case <-p.results:
case <-p.done:
return
}
}
}
// feedHashJobs queues every run for the workers, closing jobs on the
// way out — including when the scan is cancelled mid-queue, so that the
// workers' range over jobs always terminates.
func feedHashJobs(ctx context.Context, runs [][]fileRec,
jobs chan<- []fileRec,
) {
defer close(jobs)
for _, run := range runs {
select {
case jobs <- run:
case <-ctx.Done():
return
}
}
}
// hashWorker hashes one inode run at a time until jobs is closed or the
// scan is cancelled. A cancelled worker drops the runs still queued
// instead of stopping its reads of jobs: the range must run out for the
// pool to tear down, and reading a file nobody wants the hash of only
// delays that.
func hashWorker(ctx context.Context, jobs <-chan []fileRec,
results chan<- hashResult,
) {
for run := range jobs {
if ctx.Err() != nil {
continue
}
head, tail, err := hashHeadTail(run[0].path, run[0].size)
select {
case results <- hashResult{
run: run, head: head, tail: tail, err: err,
}:
case <-ctx.Done():
return
}
}
}

View File

@@ -1,6 +1,7 @@
package main
import (
"bytes"
"context"
"crypto/sha256"
"database/sql"
@@ -8,7 +9,9 @@ import (
"fmt"
"os"
"path/filepath"
"runtime"
"slices"
"strconv"
"strings"
"testing"
"time"
@@ -130,7 +133,7 @@ func collectWalk(t *testing.T, roots []string, oneFS bool,
errs int
)
for ev := range startWalk(roots, oneFS, workers) {
for ev := range startWalk(t.Context(), roots, oneFS, workers) {
if ev.fail {
errs++
@@ -359,7 +362,7 @@ const smokeTreeFiles = 15
func syncTree(t *testing.T, db *sql.DB, roots ...string) scanStats {
t.Helper()
st, err := syncScan(db, roots, 4, false)
st, err := syncScan(t.Context(), db, roots, 4, false)
if err != nil {
t.Fatalf("syncScan: %v", err)
}
@@ -371,7 +374,7 @@ func syncTree(t *testing.T, db *sql.DB, roots ...string) scanStats {
func dbRecords(t *testing.T, db *sql.DB) []scanRec {
t.Helper()
recs, err := loadFileRows(db)
recs, err := loadFileRows(t.Context(), db)
if err != nil {
t.Fatal(err)
}
@@ -815,6 +818,157 @@ func TestScanHardlinkRunFailsTogether(t *testing.T) {
}
}
// injectedWriteFailure is the message the injected database trigger
// aborts with, so the test can recognize its own failure in the error
// the scan reports.
const injectedWriteFailure = "injected write failure"
// hashLeakFiles is the size of the fixture for the hash-phase failure
// test. The batch commit inside the hash phase is what fails, so the
// tree must hold more than updateBatchSize files for the failure to
// happen at all, and the surplus over that is what is still queued
// when it does. That surplus is 2*workQueueDepth, which jobs, results
// and the workers in flight between them absorb exactly, so the feeder
// itself drains and exits; what an abandoned pool leaves parked is
// every worker, each holding a result nobody will ever receive, plus
// the goroutine waiting on them. That is what this test detects, and
// its margin over detecting nothing at all is the worker count —
// worth knowing before changing hashLeakWorkers or workQueueDepth.
const hashLeakFiles = updateBatchSize + 2*workQueueDepth
// hashLeakWorkers is the worker count for that scan: a fixed, modest
// number keeps the leak deterministic on any machine.
const hashLeakWorkers = 4
// goroutineSettle bounds how long a goroutine count is given to come
// back down to its target. Only a failing run ever waits this long.
const goroutineSettle = 5 * time.Second
// goroutinePoll is the interval between goroutine-count samples.
const goroutinePoll = 10 * time.Millisecond
// writeEmptyFiles creates n empty files directly in dir. Zero-length
// files are never opened by the hasher — their hashes are constant —
// so a fixture this size costs directory entries and no read I/O,
// while still queueing n runs through the hash pool.
func writeEmptyFiles(t *testing.T, dir string, n int) {
t.Helper()
for i := range n {
err := os.WriteFile(
filepath.Join(dir, strconv.Itoa(i)), nil, 0o600)
if err != nil {
t.Fatal(err)
}
}
}
// injectWriteFailure creates a scan database at path carrying the real
// schema plus a trigger that aborts every insert. Reads are untouched,
// so a scan loads its index and walks normally and then fails on the
// first record it tries to commit — a genuine database write failure
// partway through the hash phase.
func injectWriteFailure(t *testing.T, path string) {
t.Helper()
db, err := openScanDatabase(t.Context(), path)
if err != nil {
t.Fatal(err)
}
_, err = db.ExecContext(t.Context(),
"CREATE TRIGGER refuse_insert BEFORE INSERT ON files "+
"BEGIN SELECT RAISE(ABORT, '"+injectedWriteFailure+"'); END")
if err != nil {
t.Fatal(err)
}
err = db.Close()
if err != nil {
t.Fatal(err)
}
}
// baselineGoroutines waits for the goroutine count to stop moving and
// returns it. Handles closed by earlier tests take a moment to reap
// their driver goroutines, so a single sample would make the baseline
// itself flaky.
func baselineGoroutines(t *testing.T) int {
t.Helper()
deadline := time.Now().Add(goroutineSettle)
last := runtime.NumGoroutine()
for time.Now().Before(deadline) {
time.Sleep(goroutinePoll)
n := runtime.NumGoroutine()
if n == last {
return n
}
last = n
}
return last
}
// settledGoroutines polls runtime.NumGoroutine until it is back at or
// below want and returns the last count seen. Polling, rather than one
// sample after a fixed sleep, is what keeps this from being a race
// between the assertion and goroutines that are already exiting.
func settledGoroutines(t *testing.T, want int) int {
t.Helper()
deadline := time.Now().Add(goroutineSettle)
for {
n := runtime.NumGoroutine()
if n <= want || time.Now().After(deadline) {
return n
}
time.Sleep(goroutinePoll)
}
}
// TestScanHashWriteFailureUnwindsPool drives the scan entry point
// against a database that refuses every write. The hash phase gives up
// partway through with thousands of runs still queued, which used to
// leave the feeder parked on a full job channel and every worker parked
// on a full result channel for the life of the process.
func TestScanHashWriteFailureUnwindsPool(t *testing.T) {
path := testDBPath(t)
t.Setenv(databaseEnv, path)
dir := t.TempDir()
writeEmptyFiles(t, dir, hashLeakFiles)
injectWriteFailure(t, path)
base := baselineGoroutines(t)
var stderr bytes.Buffer
code := run([]string{
cmdScan, "--workers", strconv.Itoa(hashLeakWorkers), dir,
}, &stderr)
if code != exitFatal {
t.Fatalf("run(scan) = %d, want %d; stderr: %s",
code, exitFatal, stderr.String())
}
if !strings.Contains(stderr.String(), injectedWriteFailure) {
t.Errorf("stderr = %q, want the injected write failure",
stderr.String())
}
if got := settledGoroutines(t, base); got > base {
t.Errorf("goroutines = %d after the failed scan, want %d back",
got, base)
}
}
func TestHashRuns(t *testing.T) {
t.Parallel()

View File

@@ -2,17 +2,15 @@
# script/bootstrap: install all dependencies needed to build and develop
# this repo. Idempotent: every install is guarded by a check so already
# installed tools are skipped. Base tooling comes from nix, apt, brew,
# or apk (detected in that order); assumes nothing is present.
# golangci-lint is installed via `go install` pinned to the same version
# the Dockerfile lint stage uses (never "latest").
# or apk (detected in that order); assumes nothing is present (not git,
# make, or go). The linter is NOT installed: golangci-lint runs via
# docker only (script/lint), pinned by image digest, so the only lint
# prerequisite is a working docker — which is warned about, not
# installed, because everything except linting works without it.
set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
# Pinned versions, 2026-08-07 (same version as the Dockerfile lint stage).
# golangci-lint v2.12.2
GOLANGCI_LINT_REF="github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.12.2"
PKGMGR=""
SUDO=""
APT_UPDATED=""
@@ -63,13 +61,26 @@ missing() {
main() {
cd "$ROOT"
# System tooling, deliberately unpinned: these come from the host
# package manager and whatever version it ships is what the host
# gets, so a presence check is the right check. The repo pins no
# system toolchain versions — the Go language version is governed by
# go.mod, and builds that must be reproducible run in the Docker
# image, whose base images are pinned by digest.
if missing git; then pkg_install git git git git; fi
if missing make; then pkg_install gnumake make make make; fi
if missing go; then pkg_install go golang go go; fi
# Lint tooling, pinned via go install (installs into
# "$(go env GOPATH)/bin"; ensure that is on your PATH).
if missing golangci-lint; then go install "$GOLANGCI_LINT_REF"; fi
# Linting runs via docker only (script/lint), so docker is a lint
# prerequisite rather than something bootstrap installs. Warn, do
# not fail: everything except `make lint` — and, through it,
# `make check`, `make docker` and the pre-commit hook — works
# without it.
if missing docker; then
echo "bootstrap: WARNING: docker not found; make lint, make check" >&2
echo "bootstrap: and make docker require it. Install docker to" >&2
echo "bootstrap: run the linter." >&2
fi
go mod download

View File

@@ -1,14 +1,34 @@
#!/bin/sh
# script/cibuild: run the CI build. The Dockerfile runs make check (via
# script/check), so a successful build implies all checks pass. The
# Gitea workflow runs this on push.
# script/cibuild: run the CI build. The Gitea workflow runs this on
# push.
#
# The Dockerfile runs the gates individually as build steps, not the
# make check aggregate: the lint stage runs make fmt-check,
# script/verify-lint-image-pin, golangci-lint config verify and
# golangci-lint run; the build stage, dropped to an unprivileged user,
# runs make test and make fmt-check. Neither make lint nor make check
# appears, because both reach script/lint, which is itself a docker
# build, and a docker build cannot run inside one. Lint is not skipped
# by that — the linter is invoked directly in the lint stage, and the
# build stage's COPY --from=lint makes that stage a prerequisite, so
# BuildKit must finish it first. Between the two stages everything
# make check would run has run, which is why a successful build here
# implies the repo is green.
#
# That implication holds only because of CHECK_EPOCH. A COPY layer is
# invalidated by changed content, and a merge commit's tree is
# byte-identical to the branch head it merges, so without a fresh value
# here Docker serves the gate layers from cache and the build reports a
# green it never earned. Passing the current epoch invalidates the gate
# layers on every run while leaving the pinned base images and
# go mod download cached; see the Dockerfile for the placement.
set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
main() {
cd "$ROOT"
docker build .
docker build --build-arg CHECK_EPOCH="$(date +%s)" .
}
main "$@"

View File

@@ -1,6 +1,14 @@
#!/bin/sh
# script/docker: build the Docker image tagged with the project name.
# Identical in all repos; the tag comes from script/projectname.
# The tag comes from script/projectname.
#
# CHECK_EPOCH is passed for the same reason script/cibuild passes it:
# without it Docker serves the Dockerfile's gate layers from cache on an
# unchanged tree and this exits 0 having run neither the lint stage's
# gates nor the builder stage's test and fmt-check gates. This is the
# set of gates a developer or reviewer runs by hand, so a cached pass
# here is the most misleading result the repo can produce. Dependency
# layers sit above the ARG and stay cached.
set -eu
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"
@@ -8,7 +16,10 @@ ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)"
main() {
cd "$ROOT"
docker build -t "$("$SCRIPT_DIR/projectname")" .
docker build \
--build-arg CHECK_EPOCH="$(date +%s)" \
-t "$("$SCRIPT_DIR/projectname")" \
.
}
main "$@"

View File

@@ -1,12 +1,29 @@
#!/bin/sh
# script/lint: run the linter.
# script/lint: run the linter. golangci-lint is never installed on a
# host: it runs via docker only, one way, everywhere — this builds
# Dockerfile.lint, which COPYs the repo into the digest-pinned
# golangci-lint image and lints as a build step, so a successful build
# is a clean lint. The only prerequisite is a working docker. The gate
# steps make no network calls of their own, but Dockerfile.lint runs
# `go mod download` above them, so a cold cache does reach the network
# (as does pulling the pinned image); that layer stays cached, and once
# it is warm this runs offline until go.mod or go.sum changes.
#
# CHECK_EPOCH is what makes the result mean anything. Without it docker
# serves the gate layers from cache on an unchanged tree and this exits
# 0 in well under a second having run no linter. The PID is in the value
# as well as the epoch because two lint runs land inside the same second
# easily, and `date +%s` alone would cache the second one.
set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
main() {
cd "$ROOT"
golangci-lint run --config .golangci.yml ./...
docker build \
--build-arg CHECK_EPOCH="$(date +%s)-$$" \
-f Dockerfile.lint \
.
}
main "$@"

84
script/verify-lint-image-pin Executable file
View File

@@ -0,0 +1,84 @@
#!/bin/sh
# script/verify-lint-image-pin: fail unless the golangci-lint image
# referenced by Dockerfile.lint and the one referenced by the main
# Dockerfile's lint stage are the same image at the same digest. Our own
# extension to scripts-to-rule-them-all, not one of its entrypoints.
#
# The linter version is pinned in two independent files. That is the
# shape #42 turned into a build failure rather than tolerate: nothing
# else keeps the two in sync, and a bump applied to one file alone would
# leave `make lint` and the fail-fast lint stage of `make docker`
# linting the same tree against different rulesets, both green. This is
# the single guard that stops it, run as a gate in both files.
#
# It deliberately restates neither pin. A hardcoded expected digest here
# would be a third copy — one more thing to bump, and the same drift one
# file further out. It compares the two files to each other and knows
# nothing about which version is correct.
#
# A reference that cannot be read is a hard failure, not a skip: a
# comparison of two empty strings succeeds, which would turn this guard
# into exactly the unearned green it exists to prevent.
set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
LINT_DOCKERFILE="Dockerfile.lint"
MAIN_DOCKERFILE="Dockerfile"
# Echo the single golangci-lint image reference in the named Dockerfile.
# Scans every argument of every FROM instruction rather than assuming a
# field position, so `FROM --platform=... img AS stage` reads correctly.
# Exits non-zero, with a diagnosis, unless there is exactly one.
lint_image_ref() {
file="$1"
if [ ! -f "$file" ]; then
echo "verify-lint-image-pin: $file: not found" >&2
return 1
fi
refs="$(
awk '
toupper($1) == "FROM" {
for (i = 2; i <= NF; i++) {
if ($i ~ /^golangci\/golangci-lint[:@]/) {
print $i
}
}
}
' "$file"
)"
count="$(printf '%s' "$refs" | grep -c . || true)"
if [ "$count" -ne 1 ]; then
echo "verify-lint-image-pin: $file: expected exactly one" \
"golangci/golangci-lint FROM reference, found $count" >&2
return 1
fi
printf '%s\n' "$refs"
}
main() {
cd "$ROOT"
lint_ref="$(lint_image_ref "$LINT_DOCKERFILE")"
main_ref="$(lint_image_ref "$MAIN_DOCKERFILE")"
if [ "$lint_ref" != "$main_ref" ]; then
echo "verify-lint-image-pin: the linter image is pinned twice and" \
"the two pins disagree:" >&2
echo "verify-lint-image-pin: $LINT_DOCKERFILE: $lint_ref" >&2
echo "verify-lint-image-pin: $MAIN_DOCKERFILE: $main_ref" >&2
echo "verify-lint-image-pin: bump both FROM lines together, tag and" \
"digest, so script/lint and the Dockerfile lint stage keep" \
"running the same linter" >&2
exit 1
fi
echo "verify-lint-image-pin: $LINT_DOCKERFILE and $MAIN_DOCKERFILE" \
"agree on $lint_ref"
}
main "$@"

View File

@@ -2,6 +2,7 @@ package main
import (
"bufio"
"context"
"crypto/sha256"
"fmt"
"os"
@@ -34,8 +35,11 @@ type treeNode struct {
// maximal duplicate-tree groups as TSV on stdout. It never touches the
// scanned filesystem; its only I/O is the database, stdout, and
// stderr.
func runTrees() {
recs := loadRecords()
func runTrees(ctx context.Context) error {
recs, err := loadRecords(ctx)
if err != nil {
return err
}
super, allDirs := buildHierarchy(recs)
super.compute()
@@ -44,9 +48,9 @@ func runTrees() {
out := bufio.NewWriterSize(os.Stdout, ioBufSize)
_, err := fmt.Fprintln(out, "first\tdupe\tfiles\tsize")
_, err = fmt.Fprintln(out, "first\tdupe\tfiles\tsize")
if err != nil {
fatalf("write stdout: %v", err)
return fmt.Errorf("write stdout: %w", err)
}
dupeTrees := 0
@@ -59,7 +63,7 @@ func runTrees() {
_, err = fmt.Fprintf(out, "%s\t%s\t%d\t%d\n",
first.path, n.path, first.fileCount, first.totalSize)
if err != nil {
fatalf("write stdout: %v", err)
return fmt.Errorf("write stdout: %w", err)
}
dupeTrees++
@@ -69,13 +73,15 @@ func runTrees() {
err = out.Flush()
if err != nil {
fatalf("write stdout: %v", err)
return fmt.Errorf("write stdout: %w", err)
}
fmt.Fprintf(os.Stderr,
"trees: %d records read, %d duplicate tree groups, %d dupe trees, "+
"%s reclaimable\n",
len(recs), len(dupes), dupeTrees, humanBytes(reclaimable))
return nil
}
// buildHierarchy reconstructs the directory hierarchy from the record