From c6470906d0948e6faffe44fcea991e9bb78b9193 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Fri, 16 Oct 2026 12:02:46 +0000 Subject: [PATCH] Name the database each command uses on stderr (closes #111) scan, report and trees now print one line on stderr before anything else, naming the database and whether its path is the default or comes from SFDUPES_DATABASE; scan also says when the file is new. report and trees print it before opening the database, so a missing-database error follows it. A relative path gets the working directory put in front of it without cleaning: cleaning drops a ".." with the directory before it, which names another file when that directory is a symlink. README "Database" lists when scan writes each file. Unverified: the line on a terminal has no test of its own. Model: opus-5-5 --- README.md | 39 +++++++++-- TODO.md | 6 ++ cancel_test.go | 10 +-- db.go | 27 ++++++-- db_test.go | 38 +++++++++-- main_test.go | 182 +++++++++++++++++++++++++++++++++++++++++++++++-- report.go | 11 ++- scan.go | 26 +++++-- trees.go | 12 +++- 9 files changed, 316 insertions(+), 35 deletions(-) diff --git a/README.md b/README.md index 010587f..e5fbece 100644 --- a/README.md +++ b/README.md @@ -96,10 +96,11 @@ As a file `/etc/cron.d/sfdupes`: the first scan. To keep it elsewhere, set `SFDUPES_DATABASE=/path/to/db.sqlite` before the command on the same line. - `scan` writes nothing to stdout. Its stderr, appended here to - `/var/log/sfdupes.log`, holds a plain progress line as each phase starts and - then at most every 5 seconds, a warning for each path it skips, and the - summary line (see "Progress" and "`scan` mode"). The log grows with every - scan; rotate it like any other. + `/var/log/sfdupes.log`, holds a first line naming the database (see + "Database"), a plain progress line as each phase starts and then at most every + 5 seconds, a warning for each path it skips, and the summary line (see + "Progress" and "`scan` mode"). The log grows with every scan; rotate it like + any other. - Skipped paths do not fail a scan: it still exits 0, and cron sends nothing. A scan that fails, or is stopped by `SIGINT` or `SIGTERM`, exits 1 with the reason among the last lines of the log; `tail` prints them, and cron mails @@ -266,6 +267,22 @@ All three subcommands operate on a single SQLite database file: command-line flag. The path names the file exactly, whatever characters it holds (`?`, `#` and `%` included); a relative path is relative to the working directory. +- Each subcommand first prints one line on stderr naming the database it uses, + before anything else it prints (progress displays included), whether or not + stderr is a terminal. The line gives the path with the working directory put + in front of it when it is relative, and otherwise as set (a `..` in it stays, + so the line names the same file), escaped like the paths in a report (see + "Report output format"), and says where the path came from: + + ``` + scan: new database /var/lib/sfdupes/db.sqlite (default) + trees: database /srv/tmp/sfdupes/db.db (from SFDUPES_DATABASE) + ``` + + `scan` says `new database` when no file is at that path as it starts. + `report` and `trees` print the line before they open the database, so the + error for a missing database comes after it. + - `scan` creates the database (and its parent directory) on first use. `report` and `trees` require an existing database; a missing database file is a fatal error (exit 1) telling the user to run `scan` first. @@ -304,6 +321,20 @@ All three subcommands operate on a single SQLite database file: in WAL mode until the next scan. `report` writes each row as it reads it, so it is still reading while its output is paused (a pager, a stalled pipe), and a `scan` started then fails after the busy timeout. +- A `scan` writes to the database and the files beside it at these points (its + phases are described under "`scan` mode"): + - As it starts, before the walk: the directory, an empty database file and + the `.lock` file, each only if missing. + - Then, still before the walk: the schema, in a new database, and the `-wal` + and `-shm` files, which stay beside the database while the scan runs. + - While it loads the existing records (`load`) and walks: no records. A + first scan's database holds none, however long its walk has run. + - During the hash phase: the first records, committed 10,000 at a time. + - In the update phase: the rest of the hashed records, the records for + size-unique files, and the deletions. + - In the content phase: content hashes, again committed 10,000 at a time. + - At the end, as it closes the database: the `-wal` and `-shm` files go + away, unless a report still has the database open (see above). - `report` and `trees` open the database read-only and need only read access to the database file, and no write access to its directory. While the database is in WAL mode they also read the `-wal` and `-shm` files beside it, which SQLite diff --git a/TODO.md b/TODO.md index 0c37f2a..0c6ab0c 100644 --- a/TODO.md +++ b/TODO.md @@ -29,6 +29,12 @@ # Completed Steps +- `scan`, `report` and `trees` first print a line on stderr naming the database + they use, as an absolute path, and whether that path is the default or comes + from `SFDUPES_DATABASE`; `scan` says when the database is new. README + "Database" says when `scan` writes each file (2026-10-16, + https://git.eeqj.de/sneak/sfdupes/issues/111) + - re-vendor the canonical files and model scripts from `sneak/prompts` `next` at `e3f79c4`: golangci-lint v2.14.0; lint and test are phases of the `Dockerfile` that write no image, and `make test` runs the suite under the race detector, diff --git a/cancel_test.go b/cancel_test.go index 556e150..da543fd 100644 --- a/cancel_test.go +++ b/cancel_test.go @@ -344,9 +344,10 @@ func storedPaths(t *testing.T, path string) []string { // TestRunScanInterrupted calls the scan entrypoint with a context that // is already cancelled, as when a signal arrives at once. It must return -// errInterrupted promptly with its one line on stderr and nothing on -// stdout, leave the database valid and as it was, and leave nothing in -// the way of the next scan, which must bring the database up to date. +// errInterrupted promptly, with its line on stderr after the one naming +// the database and nothing on stdout, leave the database valid and as +// it was, and leave nothing in the way of the next scan, which must +// bring the database up to date. func TestRunScanInterrupted(t *testing.T) { path := testDBPath(t) t.Setenv(databaseEnv, path) @@ -389,7 +390,8 @@ func TestRunScanInterrupted(t *testing.T) { err, errInterrupted) } - want := "scan: interrupted after 0 files\n" + want := "scan: database " + path + " (from " + databaseEnv + ")\n" + + "scan: interrupted after 0 files\n" if got := stderr()[shown:]; got != want { t.Errorf("stderr = %q, want %q", got, want) } diff --git a/db.go b/db.go index e0dff42..2bd39b7 100644 --- a/db.go +++ b/db.go @@ -11,6 +11,7 @@ import ( "path/filepath" "slices" "strconv" + "strings" "time" "golang.org/x/sys/unix" @@ -91,13 +92,31 @@ var errSchemaVersion = errors.New("unsupported database schema version") var errScanRunning = errors.New("another scan is running") // databasePath resolves the database location: SFDUPES_DATABASE when -// set and non-empty, the compiled-in default otherwise. -func databasePath() string { +// set and non-empty, the compiled-in default otherwise. It returns the +// path, with the working directory put in front of it when it is +// relative, and where the path came from, "default" or "from +// SFDUPES_DATABASE", for the line each subcommand prints first on +// stderr. +func databasePath() (string, string, error) { + path, source := defaultDatabasePath, "default" if p := os.Getenv(databaseEnv); p != "" { - return p + path, source = p, "from "+databaseEnv } - return defaultDatabasePath + if filepath.IsAbs(path) { + return path, source, nil + } + + wd, err := os.Getwd() + if err != nil { + return "", "", fmt.Errorf("resolve database path %s: %w", path, err) + } + + // Not filepath.Abs or filepath.Join: they clean the path, which + // drops each ".." together with the directory name before it. When + // that directory is a symlink, the cleaned path names another file. + // The working directory ends in "/" only when it is the root. + return strings.TrimSuffix(wd, "/") + "/" + path, source, nil } // scanParams are the connection parameters for scan: read-write, with diff --git a/db_test.go b/db_test.go index e4b8600..f3a0481 100644 --- a/db_test.go +++ b/db_test.go @@ -43,16 +43,42 @@ func openTestDB(t *testing.T) *sql.DB { } func TestDatabasePath(t *testing.T) { - t.Setenv(databaseEnv, "") + // README §Database: SFDUPES_DATABASE when set and non-empty, the + // default otherwise, with the working directory put in front of a + // relative path. Nothing is cleaned, so a ".." stays. + dir := t.TempDir() + t.Chdir(dir) - if got := databasePath(); got != defaultDatabasePath { - t.Errorf("databasePath() = %q, want %q", got, defaultDatabasePath) + fromEnv := "from " + databaseEnv + cases := []struct { + env, path, source string + }{ + {"", defaultDatabasePath, "default"}, + {"/custom/place.sqlite", "/custom/place.sqlite", fromEnv}, + {"/custom/link/../place.sqlite", "/custom/link/../place.sqlite", + fromEnv}, + {"sub/place.sqlite", dir + "/sub/place.sqlite", fromEnv}, + {"link/../place.sqlite", dir + "/link/../place.sqlite", fromEnv}, } - t.Setenv(databaseEnv, "/custom/place.sqlite") + for _, tc := range cases { + t.Setenv(databaseEnv, tc.env) - if got := databasePath(); got != "/custom/place.sqlite" { - t.Errorf("databasePath() = %q, want the env override", got) + path, source, err := databasePath() + if err != nil || path != tc.path || source != tc.source { + t.Errorf("databasePath() with %s=%q = %q, %q, %v; want %q, %q", + databaseEnv, tc.env, path, source, err, tc.path, tc.source) + } + } + + // In the root directory the path gets one slash in front, not two. + t.Chdir("/") + t.Setenv(databaseEnv, "place.sqlite") + + path, _, err := databasePath() + if err != nil || path != "/place.sqlite" { + t.Errorf("databasePath() in / = %q, %v; want %q", + path, err, "/place.sqlite") } } diff --git a/main_test.go b/main_test.go index ca6faf7..92fe72d 100644 --- a/main_test.go +++ b/main_test.go @@ -285,22 +285,29 @@ func assertFatalOutput(t *testing.T, stderr, stdout string) { func TestRunMissingDatabaseIsFatal(t *testing.T) { // README §Database: report and trees need an existing database; a - // missing one exits 1 with a message telling the user to run scan. + // missing one exits 1 with a message telling the user to run scan, + // after the line naming the database. for _, name := range []string{cmdReport, cmdTrees} { t.Run(name, func(t *testing.T) { path := testDBPath(t) t.Setenv(databaseEnv, path) - var stdout, stderr bytes.Buffer + var stdout bytes.Buffer - code := run([]string{name}, &stdout, &stderr) + // run gets os.Stderr, where the subcommand prints its line, + // so the capture shows the two in the order printed. + stderr := captureStderr(t) + + code := run([]string{name}, &stdout, os.Stderr) if code != exitFatal { t.Errorf("run(%s) = %d, want %d", name, code, exitFatal) } - want := "sfdupes: " + path + ": no database (run \"sfdupes " + + want := name + ": database " + path + " (from " + databaseEnv + + ")\n" + + "sfdupes: " + path + ": no database (run \"sfdupes " + "scan\" first, or set " + databaseEnv + ")\n" - if got := stderr.String(); got != want { + if got := stderr(); got != want { t.Errorf("stderr = %q, want %q", got, want) } @@ -311,6 +318,165 @@ func TestRunMissingDatabaseIsFatal(t *testing.T) { } } +func TestRunNamesRelativeDatabase(t *testing.T) { + // README §Database: each subcommand's first line on stderr names the + // database, a relative SFDUPES_DATABASE made absolute against the + // working directory. scan calls it new until a scan has created it. + path := testDBPath(t) + t.Chdir(filepath.Dir(path)) + t.Setenv(databaseEnv, filepath.Base(path)) + + dir := t.TempDir() + from := " (from " + databaseEnv + ")\n" + + want := "scan: new database " + path + from + if got := scanOK(t, dir); !strings.HasPrefix(got, want) { + t.Errorf("first scan stderr = %q, want it to start %q", got, want) + } + + want = "scan: database " + path + from + if got := scanOK(t, dir); !strings.HasPrefix(got, want) { + t.Errorf("second scan stderr = %q, want it to start %q", got, want) + } + + for _, name := range []string{cmdReport, cmdTrees} { + stderr := captureStderr(t) + + code := run([]string{name}, io.Discard, os.Stderr) + + want = name + ": database " + path + from + if got := stderr(); code != exitOK || !strings.HasPrefix(got, want) { + t.Errorf("run(%s) = %d with stderr %q, want %d and it to "+ + "start %q", name, code, got, exitOK, want) + } + } +} + +func TestRunUsesDatabaseThroughSymlinkedDirectory(t *testing.T) { + // README §Database: the path names the file exactly. With link a + // symlink to the directory real/sub, link/../db.sqlite names + // real/db.sqlite, not db.sqlite beside link. Every subcommand must + // use that file, relative or absolute, and its first line must name + // the path as set, with the working directory put in front of a + // relative one. + dir := t.TempDir() + t.Chdir(dir) + + err := os.MkdirAll(filepath.Join(dir, "real", "sub"), 0o750) + if err != nil { + t.Fatal(err) + } + + err = os.Symlink(filepath.Join(dir, "real", "sub"), + filepath.Join(dir, "link")) + if err != nil { + t.Fatal(err) + } + + named := dir + "/link/../db.sqlite" + from := " (from " + databaseEnv + ")\n" + tree := t.TempDir() + + t.Setenv(databaseEnv, "link/../db.sqlite") + + want := "scan: new database " + named + from + if got := scanOK(t, tree); !strings.HasPrefix(got, want) { + t.Errorf("relative scan stderr = %q, want it to start %q", got, want) + } + + // The absolute path names the database the relative one created. + t.Setenv(databaseEnv, named) + + want = "scan: database " + named + from + if got := scanOK(t, tree); !strings.HasPrefix(got, want) { + t.Errorf("absolute scan stderr = %q, want it to start %q", got, want) + } + + for _, name := range []string{cmdReport, cmdTrees} { + stderr := captureStderr(t) + + code := run([]string{name}, io.Discard, os.Stderr) + + want = name + ": database " + named + from + if got := stderr(); code != exitOK || !strings.HasPrefix(got, want) { + t.Errorf("run(%s) = %d with stderr %q, want %d and it to "+ + "start %q", name, code, got, exitOK, want) + } + } + + _, err = os.Stat(filepath.Join(dir, "real", "db.sqlite")) + if err != nil { + t.Errorf("database the path names: %v", err) + } + + // With no file at the cleaned path, report and trees succeeded on + // the database in real. + _, err = os.Stat(filepath.Join(dir, "db.sqlite")) + if !errors.Is(err, fs.ErrNotExist) { + t.Errorf("database at the cleaned path: stat error %v, want it "+ + "missing", err) + } +} + +func TestRunNamesDefaultDatabase(t *testing.T) { + // README §Database: with SFDUPES_DATABASE empty, each subcommand's + // first line on stderr names the default database. A database may + // exist at the default path on the machine running the test, so + // nothing here may create or read one: scan gets an operand that + // does not exist, which stops it before it takes the lock, and + // report and trees a cancelled context, which stops them before + // they read anything. + t.Setenv(databaseEnv, "") + + // Whether scan calls the database new depends on that machine too. + scanWant := "scan: database " + defaultDatabasePath + " (default)\n" + + _, err := os.Stat(defaultDatabasePath) + if errors.Is(err, fs.ErrNotExist) { + scanWant = "scan: new database " + defaultDatabasePath + + " (default)\n" + } + + missing := filepath.Join(t.TempDir(), "nope") + + cases := map[string]struct { + run func() error + want string + }{ + cmdScan: { + run: func() error { + return runScan(t.Context(), []string{missing}, 1, false) + }, + want: scanWant, + }, + cmdReport: { + run: func() error { + return runReport(cancelledContext(t), io.Discard) + }, + want: "report: database " + defaultDatabasePath + " (default)\n", + }, + cmdTrees: { + run: func() error { + return runTrees(cancelledContext(t), io.Discard) + }, + want: "trees: database " + defaultDatabasePath + " (default)\n", + }, + } + + for name, tc := range cases { + stderr := captureStderr(t) + + err = tc.run() + if err == nil { + t.Errorf("%s: error = nil, want it stopped", name) + } + + if got := stderr(); got != tc.want { + t.Errorf("%s stderr = %q, want %q", name, got, tc.want) + } + } +} + func TestRunUsageErrors(t *testing.T) { // Usage errors keep exiting 2 with cobra's own report on stderr. cases := map[string]struct { @@ -661,7 +827,8 @@ func TestRunReportSucceeds(t *testing.T) { t.Errorf("stdout = %q, want %q", got, want) } - want = "report: 2 records read, 1 duplicate groups, 1 dupe files, " + + want = "report: database " + path + " (from " + databaseEnv + ")\n" + + "report: 2 records read, 1 duplicate groups, 1 dupe files, " + "300 B reclaimable\n" if got := stderr(); got != want { t.Errorf("stderr = %q, want %q", got, want) @@ -694,7 +861,8 @@ func TestRunTreesSucceeds(t *testing.T) { t.Errorf("stdout = %q, want %q", got, want) } - want = "trees: 2 records read, 1 duplicate tree groups, 1 dupe trees, " + + want = "trees: database " + path + " (from " + databaseEnv + ")\n" + + "trees: 2 records read, 1 duplicate tree groups, 1 dupe trees, " + "300 B reclaimable\n" if got := stderr(); got != want { t.Errorf("stderr = %q, want %q", got, want) diff --git a/report.go b/report.go index 425f412..fba801c 100644 --- a/report.go +++ b/report.go @@ -34,9 +34,16 @@ type scanRec struct { // records, and each row is written as it is read, so no group is held // in memory. It never touches the scanned filesystem; its only I/O is // the database (with SQLite's temporary sort file), stdout, and stderr. -// Any database problem, including a missing database, is fatal. +// It names the database on stderr before it opens it. Any database +// problem, including a missing database, is fatal. func runReport(ctx context.Context, stdout io.Writer) error { - dbPath := databasePath() + dbPath, source, err := databasePath() + if err != nil { + return err + } + + fmt.Fprintf(os.Stderr, "report: database %s (%s)\n", + escapePath(dbPath), source) db, err := openReportDatabase(ctx, dbPath) if err != nil { diff --git a/scan.go b/scan.go index 0f70378..e3d33ed 100644 --- a/scan.go +++ b/scan.go @@ -91,9 +91,11 @@ type fileMeta struct { // least one other file shares are ever hashed: a size-unique file // cannot be a duplicate. A file of headTailMin or more gets its content // hash only when its size, head, and tail match another file's. Flag -// parsing and the at-least-one-operand check are done by cobra. The -// scan holds the lock on the database for its whole run, so a second -// scan fails before it walks the filesystem or opens the database. +// parsing and the at-least-one-operand check are done by cobra. Before +// anything else, the scan names the database on stderr, saying whether +// it is new. The scan holds the lock on the database for its whole +// run, so a second scan fails before it walks the filesystem or opens +// the database. // Errors are returned rather than exiting, so that the deferred close — // which takes the database out of WAL mode — always runs, and the lock // is released after it. When ctx is cancelled, as by the SIGINT or @@ -104,12 +106,26 @@ type fileMeta struct { func runScan(ctx context.Context, roots []string, workers int, oneFS bool, ) error { - roots, err := resolveRoots(roots) + dbPath, source, err := databasePath() if err != nil { return err } - dbPath := databasePath() + // A database file missing now is created by lockScanDatabase below. + what := "database" + + _, err = os.Stat(dbPath) + if errors.Is(err, fs.ErrNotExist) { + what = "new database" + } + + fmt.Fprintf(os.Stderr, "scan: %s %s (%s)\n", what, escapePath(dbPath), + source) + + roots, err = resolveRoots(roots) + if err != nil { + return err + } lock, err := lockScanDatabase(dbPath) if err != nil { diff --git a/trees.go b/trees.go index 5c1bbf7..6d18564 100644 --- a/trees.go +++ b/trees.go @@ -29,10 +29,16 @@ type treeNode struct { // the record paths, computes a Merkle-style digest per directory, and // prints maximal duplicate-tree groups as TSV on stdout. It never // touches the scanned filesystem; its only I/O is the database, stdout, -// and stderr. Any database problem, including a missing database, is -// fatal. +// and stderr. It names the database on stderr before it opens it. Any +// database problem, including a missing database, is fatal. func runTrees(ctx context.Context, stdout io.Writer) error { - dbPath := databasePath() + dbPath, source, err := databasePath() + if err != nil { + return err + } + + fmt.Fprintf(os.Stderr, "trees: database %s (%s)\n", + escapePath(dbPath), source) db, err := openReportDatabase(ctx, dbPath) if err != nil {