Files
sfdupes/TODO.md
clawbot 9d06c13777
All checks were successful
check / check (push) Successful in 1m31s
Verify the golangci-lint install actually took effect
`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

238 lines
13 KiB
Markdown

# Workflow
- take an issue from the `1.0.0` milestone on the tracker; work not
yet on the tracker gets filed as an issue first
- branch (from `main`)
- do the work, with tests, in small focused commits
- record it at the top of Completed Steps (`TODO.md` changes in the
same commit as the work)
- push the branch and open a PR whose title ends with
` (closes #N)`
- an independent review gates the merge; every finding is addressed
or explicitly rebutted on the PR
- merge to `main` once the review passes
# Status
- pre-1.0
- the Gitea tracker is authoritative for the pre-1.0 backlog: the
open issues under the `1.0.0` milestone are what remains before
the tag, and this file records history and process, not the queue
# Next Step
- take the next issue from the `1.0.0` milestone on the tracker:
https://git.eeqj.de/sneak/sfdupes/milestone/17 — the milestone is
the source of truth for what is left before 1.0.0. Individual
issues are deliberately not restated here; a copy in this file
drifts out of date the moment the tracker moves
# Completed Steps
- 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
stage and `script/bootstrap` from v2.12.1 to v2.12.2, and replaced
`.golangci.yml` with the canonical file — the linter settings
(`lll`, `funlen`, `cyclop`, `dupl` thresholds) now live under
`linters.settings` per the v2 schema, so they are actually
applied; no new lint findings surfaced
- convert Makefile targets to scripts-to-rule-them-all `script/`
entrypoints like the other managed repos (2026-07-26, commit
`3abeacf`, closes #1): all 12 `script/` entrypoints exist
(`bootstrap`, `setup`, `projectname`, `test`, `lint`, `fmt`,
`fmt-check`, `check`, `docker`, `cibuild`, `precommit`,
`install-precommit`) and every Makefile target is now a thin shim
over them, matching the other managed repos
- make the binary the default Make target (2026-07-24, branch
`make-default-target`): plain `make` now builds `sfdupes`
(previously it ran `check` plus `build`); `make build` remains as
an alias
- scan-wide phases, concurrent operands, batched updates (2026-07-24,
branch `scan-wide-phases`): all operands seed the shared walk pool
and every pass runs once over the whole scan, so totals and ETAs
are scan-global; the per-operand walk/hash/update cycles and their
stderr announcements are gone; the update pass commits in batched
transactions — the filesystem is authoritative and the database an
eventually-consistent reflection, so scan-level atomicity is not
required
- split the stat pass back out of the walk (2026-07-24, branch
`parallel-phases`): 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 and the
stat pass lstats them with per-file workers, restoring the exact
total/ETA stat bar
- announce each operand on stderr before its passes (2026-07-24,
branch `scan-operand-progress`): with per-operand walk/hash/update
cycles, a multi-operand run (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 20M files were being skipped
- parallel walk (2026-07-24, branch `parallel-walk`): the walk pass
was a single goroutine and took hours at ~20M files on a busy pool
(observed: 22M files in 4h on a ZFS server); it is now a
per-directory worker-pool traversal that records size/mtime during
the walk (folding away the separate stat pass, halving metadata
I/O), and each `PATH` operand commits in its own transaction so an
interrupted scan keeps completed operands
- persistent scan database (2026-07-24, branch `persistent-database`):
`scan` now maintains a SQLite database (`modernc.org/sqlite`, pure
Go, cgo stays disabled) keyed by absolute path that survives between
runs — a rescan hashes only new or changed files (by mtime/size),
deletes records for files vanished from under the scanned operands,
and leaves records outside them untouched, so `scan` can be cronned
daily; `report` and `trees` read the database (no positional
arguments) instead of a scan stream. Database at
`/var/lib/sfdupes/db.sqlite`, overridable via `SFDUPES_DATABASE`;
WAL journaling plus a single-transaction update keep a report run
during a scan safe
- add the `origin` remote (`git@git.eeqj.de:sneak/sfdupes.git`), tag
`v0.0.1`, and push `main` plus tags (2026-07-23)
- `scan` CLI rework (2026-07-23, branch `scan-required-paths`): required
`PATH...` operands via cobra flags replacing the `/srv` `-root`
default; new `-x`/`--one-file-system` flag (GNU convention) to stop
at filesystem boundaries, which are crossed by default
- bring the repo into full policy compliance (2026-07-23, branch
`repo-policy-compliance`; checklist below)
- `git init` with README-only first commit; code baseline committed on
`main` (2026-07-22)
- implement `scan`, `report`, and `trees` subcommands (pre-git history)
# Future Steps
- possible later features (explicitly out of scope per README):
full-content verification of candidates, removal-script helpers
# Repo Policy Compliance
Audited 2026-07-22 against `REPO_POLICIES.md` (2026-07-06), the existing
repo checklist, and the Go styleguide. Code is already gofmt-clean, so no
standalone formatting commit is needed.
- [x] `.gitignore` missing — the compiled `sfdupes` binary and
`files.dat` sit untracked in the tree; needs OS/editor/Go
artifacts plus secrets patterns
- [x] `.editorconfig` missing
- [x] `LICENSE` missing and README has no License section (MIT assumed
from house convention — user to confirm)
- [x] `REPO_POLICIES.md` missing from repo root
- [x] `.golangci.yml` missing (install canonical copy); code must then
pass `make lint` (150 findings fixed; `make lint` is clean)
- [x] `Makefile` lacks required targets `test`, `lint`, `fmt`,
`fmt-check`, `docker`, `hooks`; `check` currently depends on
`build`, which writes the binary (`make check` must not modify
files)
- [x] no tests — `go test ./...` has nothing to run; policy requires
real tests with a 30-second timeout and the conditional `-v`
rerun pattern (suite covers parsing, grouping, digests,
suppression, hashing, and the scan pipeline; 64% coverage)
- [x] `Dockerfile` missing — Go multistage with hash-pinned images:
fail-fast lint stage, build stage running `make check`
- [x] `.dockerignore` missing
- [x] `.gitea/workflows/check.yml` missing (`docker build .` on push,
checkout action pinned by commit SHA)
- [x] README lacks required sections: Description first line
(name/purpose/category/license/author), Getting Started,
Rationale, TODO, License, Author
- [x] README non-goal "no git repository setup and no CI" is stale now
that the repo is under git with CI
- [x] pre-commit hook not installed (`make hooks` once the target
exists)
Accepted divergences (no action):
- flat single-package layout with `.go` files in the repo root — fine
for a small single-binary tool per the Go styleguide; the tracker
audit agrees
- `go test` runs without `-race` — the repo mandates `CGO_ENABLED=0`
(pure-Go builds) and the race detector requires cgo