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.
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.
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.
`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.
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.
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.
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.
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.
The Next Step section still named the scripts-to-rule-them-all
conversion, which landed in 3abeacf on 2026-07-26 and closed#1
without recording itself in Completed Steps. TODO.md therefore
pointed the next contributor at finished work.
The backlog is no longer file-shaped: the Gitea tracker is
authoritative, with the open issues under the 1.0.0 milestone
defining what remains before the tag. Next Step now names the
milestone instead of restating a single issue that will drift, and
Status says so explicitly.
Workflow is reconciled with how work actually happens now: take an
issue, branch, implement with tests, record the result in the same
commit, open a PR titled "... (closes #N)", pass an independent
review, merge.
The scripts-to-rule-them-all conversion gains its missing Completed
Steps entry, and the golangci-lint entry now records its merge
commit 38a01bd and issue #3 rather than only the branch date.
Docs-only: TODO.md is the only file touched.
Bump the pinned golangci-lint from v2.12.1 to v2.12.2 in the
Dockerfile lint stage (tagged, digest-pinned Debian image) and in
script/bootstrap (go install ref). Replace .golangci.yml with the
canonical config: linter settings (lll, funlen, cyclop, dupl) move
under linters.settings per the v2 schema so they are actually
applied, and the redundant issues.exclude-use-default key is
dropped. No new lint findings surfaced; make check is green.
Plain make now builds sfdupes (previously the default was all =
check + build); make build remains as an alias, so the Dockerfile
and existing habits keep working. The sfdupes target is phony: go
build's own cache decides what to recompile.
All PATH operands belong to a single scan: every operand seeds the
shared walk worker pool, and each pass (walk, stat, hash, update)
runs exactly once over the whole scan, so pass totals, percentages,
and ETAs are scan-global. The per-operand walk/hash/update cycles and
their stderr operand announcements are gone; duplicate paths from
overlapping operands are deduplicated before stat.
The update pass now commits in batched transactions (10k changes per
batch) instead of one scan-wide transaction: the filesystem is
authoritative and the database is an eventually-consistent reflection
of it, so scan-level atomicity buys nothing, while batches keep the
WAL small and let concurrent reports observe progress.
Phases are strictly sequential again — walk, stat, hash, update per
operand — with parallelism only inside each phase. The walk
enumerates paths with per-directory workers (no lstat of file
entries); the stat pass lstats every collected path with per-file
workers, restoring its exact-total/ETA progress bar and per-file
parallelism inside wide flat directories.
With per-operand walk/hash/update cycles, a multi-operand invocation
(e.g. scan /srv/*) showed pass totals that looked like the whole
run's: an operator watching operand 3 of 14 hash 300k files concluded
the other 20M files were being skipped. Print the operand path and
its position before each cycle.
Replace the single-goroutine WalkDir traversal with a per-directory
worker pool: workers read directories concurrently and lstat entries
while each directory is fresh in cache, recording size and mtime
during the walk. This folds the separate stat pass away (halving
metadata I/O per run) and overlaps metadata latency, which dominated
on busy pools — a sequential walk of a ~22M-file tree was observed
taking over 4 hours.
Each PATH operand now loads its scope, walks, hashes, and commits in
its own transaction, so an interrupted scan keeps every operand
completed so far; a later overlapping operand sees the records
committed by earlier ones and reuses them unchanged.
The walk pass is a single goroutine; on a busy ZFS pool it manages
only a few thousand directory entries per second and takes hours at
~20M files. Respecify it as a worker-pool traversal that reads
directories concurrently and records size/mtime during the walk,
folding away the separate stat pass and halving metadata I/O. Each
PATH operand now commits in its own transaction so an interrupted
scan keeps the operands completed so far.
scan now synchronizes a database that survives between runs
(SFDUPES_DATABASE, default /var/lib/sfdupes/db.sqlite) instead of
emitting a stream: operands are resolved to absolute paths, unchanged
files (same size, mtime not newer than recorded) are never re-read, new
and changed files are hashed, and records under the scanned operands
that were not verified this run are deleted; records outside the
operands are untouched. All changes commit in a single transaction, and
WAL journaling with a busy timeout keeps a report run during a cron
scan safe.
report and trees read the database (no positional arguments); the
NUL-terminated stream format, its parser, and the malformed-record
handling are gone. The driver is modernc.org/sqlite (pure Go), so
builds keep cgo disabled.
scan will maintain a persistent database of file signatures
(default /var/lib/sfdupes/db.sqlite, overridable via
SFDUPES_DATABASE) that survives between runs; rescans hash only new
or changed files (mtime/size) and remove records for vanished files,
so scan can be cronned daily. report and trees will read the
database instead of a scan stream.
scan now takes one or more PATH operands (directories or regular
files) via cobra flags instead of the -root flag with its /srv
default; invoking scan with no operand is a usage error and a
nonexistent operand is fatal. Filesystem boundaries are crossed by
default; the new -x/--one-file-system flag (GNU du/rsync convention)
stops the walk at each operand's filesystem, implemented by comparing
lstat device IDs with build-tagged helpers for darwin's int32 Dev.
Verified against a real mounted disk image: default crosses, -x does
not, --one-file-system is identical to -x.
