Guarantee the database is closed on every fatal exit path (closes #4) #29

Merged
clawbot merged 1 commits from db-close-on-fatal into main 2026-08-09 04:39:28 +02:00
6 changed files with 566 additions and 57 deletions

14
TODO.md
View File

@@ -29,6 +29,20 @@
# Completed Steps
- 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

137
main.go
View File

@@ -16,20 +16,35 @@
package main
import (
"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 +52,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 +118,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(args []string) error {
return runScan(args, scanWorkers, scanOneFS)
}),
}
scanCmd.Flags().IntVar(&scanWorkers, "workers", runtime.NumCPU(),
"concurrent workers for the walk and hash phases")
@@ -74,35 +131,55 @@ 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(_ []string) error {
return runReport()
}),
}
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(_ []string) error {
return runTrees()
}),
}
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.
func runE(fn func(args []string) error) func(*cobra.Command, []string) error {
return func(cmd *cobra.Command, args []string) error {
cmd.SilenceUsage = true
cmd.SilenceErrors = true
err := fn(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(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

@@ -28,23 +28,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() ([]scanRec, error) {
dbPath := databasePath()
db, err := openReportDatabase(dbPath)
if err != nil {
fatalf("%v", err)
return nil, err
}
defer func() { _ = db.Close() }()
recs, err := loadFileRows(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 +62,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() error {
recs, err := loadRecords()
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 +86,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 +96,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

25
scan.go
View File

@@ -49,25 +49,30 @@ 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.
func runScan(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)
if err != nil {
fatalf("%v", err)
return err
}
defer func() { _ = db.Close() }()
st, err := syncScan(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 +80,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:

View File

@@ -34,8 +34,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() error {
recs, err := loadRecords()
if err != nil {
return err
}
super, allDirs := buildHierarchy(recs)
super.compute()
@@ -44,9 +47,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 +62,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 +72,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