diff --git a/README.md b/README.md index 148bb72..1e1dc06 100644 --- a/README.md +++ b/README.md @@ -575,6 +575,15 @@ Rules for the walk: - Never descend into a directory named `.zfs` (ZFS snapshot pseudo-dirs; walking them would list every file once per snapshot), not even when it is an operand; such an operand is dropped the same way. +- Never record the scan's own database. Once the database and its `.lock` file + are open, `scan` notes the device and inode of the database file, its `-wal` + and `-shm` files and its `.lock` file, and the walk passes over a file with + one of them, however it reaches it: as an operand, below one, or through a + symlinked directory. It prints no line for such a file and does not count it + as skipped. A record that an earlier scan left for one of them is deleted, + like the record of any file the scan no longer verifies. There is no + `-journal` file during the walk, because the database is in WAL mode then (see + "Database"). - Filesystem boundaries are crossed by default. With `-x` (long form `--one-file-system`, following the GNU `du`/`rsync` convention), never descend into a directory on a different filesystem than its `PATH` operand; each @@ -1161,10 +1170,6 @@ All of the following, run in this directory, must pass: ./sfdupes report ``` - (The database lives in a temp directory of its own: inside `$d`, the scan - would record it, and its empty lock file would join the `empty1`/`empty2` - group.) - Expected from the first `report`: `one.bin`/`copy.bin`/`copy2.bin` form one group (two dupe rows, `first` is the lexicographically smallest path); `t1/f1`/`t2/f1`/`t3/f1` form one group; `t1/sub/f2`/ diff --git a/TODO.md b/TODO.md index 37fc9a6..9f83b8a 100644 --- a/TODO.md +++ b/TODO.md @@ -29,6 +29,13 @@ # Completed Steps +- `scan` never records its own database: once the database and its `.lock` file + are open, it notes the device and inode of the database file, its `-wal` and + `-shm` files and its `.lock` file, and the walk passes over a file with one of + them wherever the operands reach it, through a symlinked directory included, + printing no line and counting nothing. A record an earlier scan left for one + of them is deleted (2026-10-17, https://git.eeqj.de/sneak/sfdupes/issues/115) + - `scan` never records or reads anything that is not on a persistent filesystem (`tmpfs`, `proc`, `sysfs` and the other kernel or memory-backed types), matched by the device ID of each such filesystem, listed as the scan starts diff --git a/db.go b/db.go index 9404dc1..4f15621 100644 --- a/db.go +++ b/db.go @@ -245,6 +245,42 @@ func openScanDatabase(ctx context.Context, path string) (*sql.DB, error) { return db, nil } +// databaseFiles returns, by device and inode, the files of the database +// at path that the scan keeps open: the database file, its -wal and +// -shm files and its .lock file, each named from the database's path +// with symlinks resolved, as SQLite and lockScanDatabase name them. The +// scan calls it once the database and the lock file are open, so that +// its walk passes over them however it reaches them, through a +// symlinked directory included. A file that does not exist then is left +// out, since no walk can reach it. There is no -journal file while the +// database is in WAL mode. +func databaseFiles(path string) ([]fileRec, error) { + resolved, err := filepath.EvalSymlinks(path) + if err != nil { + return nil, fmt.Errorf("resolve database path: %w", err) + } + + var files []fileRec + + for _, p := range []string{ + resolved, resolved + "-wal", resolved + "-shm", resolved + ".lock", + } { + fi, err := os.Lstat(p) + if errors.Is(err, fs.ErrNotExist) { + continue + } + + if err != nil { + return nil, fmt.Errorf("database: %w", err) + } + + dev, ino := inodeOfInfo(fi) + files = append(files, fileRec{path: p, dev: dev, ino: ino}) + } + + return files, nil +} + // closeScanDatabase switches the database at path from WAL back to // rollback-journal mode and closes it. Out of WAL mode the database // file alone holds the whole database, so a reader needs no -wal or diff --git a/db_test.go b/db_test.go index 7346372..fc84584 100644 --- a/db_test.go +++ b/db_test.go @@ -118,6 +118,44 @@ func TestOpenScanDatabaseCreates(t *testing.T) { } } +// TestDatabaseFiles opens a database through a symlink to its file: +// databaseFiles finds the database file and its -wal and -shm files +// beside the symlink's target, where SQLite keeps them, and leaves out +// the .lock file, which only a scan creates. +func TestDatabaseFiles(t *testing.T) { + t.Parallel() + + path := testDBPath(t) + link := filepath.Join(t.TempDir(), "link.sqlite") + + err := os.Symlink(path, link) + if err != nil { + t.Fatal(err) + } + + db, err := openScanDatabase(t.Context(), link) + if err != nil { + t.Fatal(err) + } + + defer func() { _ = db.Close() }() + + files, err := databaseFiles(link) + if err != nil { + t.Fatal(err) + } + + got := make([]string, 0, len(files)) + for _, f := range files { + got = append(got, f.path) + } + + want := []string{path, path + "-wal", path + "-shm"} + if !slices.Equal(got, want) { + t.Errorf("databaseFiles(%s) = %q, want %q", link, got, want) + } +} + func TestOpenDatabaseUnversionedForeign(t *testing.T) { t.Parallel() diff --git a/exclude.go b/exclude.go index 9700258..b023d16 100644 --- a/exclude.go +++ b/exclude.go @@ -43,14 +43,17 @@ const typesPerHelpLine = 6 // readMountInfo reads. var errMountInfoLine = errors.New("unexpected line") -// walkRules holds the two rules for the walk that a scan sets up as it +// walkRules holds the rules for the walk that a scan sets up as it // starts. excludes are the default excludes, or none with // --no-default-excludes. nonPersistent maps the device ID of every // mounted filesystem that is not persistent to its type; the scan // records and reads nothing on one of those, whatever the flags. +// databaseFiles are the scan's own database files, which the walk +// passes over (see databaseFiles in db.go). type walkRules struct { excludes []string nonPersistent map[uint64]string + databaseFiles []fileRec } // defaultExcludes returns the default excludes of the system goos diff --git a/scan.go b/scan.go index a10984f..587276f 100644 --- a/scan.go +++ b/scan.go @@ -118,7 +118,8 @@ func scanPhase(name string) string { // rejects anything less. Before the lock is taken, the scan lists the // filesystems that are not persistent, on which the walk records // nothing; unless noDefaultExcludes is set, the walk does not enter the -// default excludes either. +// default excludes either. Once the database is open, the scan notes +// its files, which the walk passes over. func runScan(ctx context.Context, roots []string, workers int, oneFS, noDefaultExcludes bool, ) error { @@ -173,6 +174,11 @@ func runScan(ctx context.Context, roots []string, workers int, defer closeScanDatabase(ctx, db, dbPath) + rules.databaseFiles, err = databaseFiles(dbPath) + if err != nil { + return err + } + st, err := syncScan(ctx, db, roots, workers, oneFS, rules) if errors.Is(err, context.Canceled) { return interrupted(st.walked) @@ -710,6 +716,15 @@ func sameInode(a, b fileRec) bool { return (a.dev != 0 || a.ino != 0) && a.dev == b.dev && a.ino == b.ino } +// isDatabaseFile reports whether f, a file the walk reached, is one of +// the scan's own database files in rules, which the walk passes over +// without a line and without counting it. +func isDatabaseFile(f fileRec, rules walkRules) bool { + return slices.ContainsFunc(rules.databaseFiles, func(d fileRec) bool { + return sameInode(d, f) + }) +} + // hashPhase hashes every queued file with hashSignature — the head and // tail of a file of headTailMin or more, the whole file below that — // committing completed records to the database in batches as results @@ -1143,7 +1158,7 @@ func startWalk(ctx context.Context, roots []string, oneFS bool, go func() { initial := make([]dirJob, 0, len(roots)) for _, root := range roots { - initial = append(initial, seedRoot(ctx, root, events)...) + initial = append(initial, seedRoot(ctx, root, rules, events)...) } dispatchDirs(ctx, initial, jobs, subdirs) @@ -1197,13 +1212,14 @@ func operandWarning(root string, fi fs.FileInfo) string { } // seedRoot turns one PATH operand into the walk's starting state: a -// regular-file operand is statted and emitted directly, and a directory +// regular-file operand is statted and emitted directly, unless it is +// one of the scan's own database files in rules, and a directory // operand becomes an initial job. walkableRoots has already dropped // every other operand. One that has changed into something else since // is warned about and skipped here; it is still a root, so the records // stored beneath it, other than those under another root, are deleted // as unverified. -func seedRoot(ctx context.Context, root string, +func seedRoot(ctx context.Context, root string, rules walkRules, events chan<- walkEvent, ) []dirJob { fi, err := os.Lstat(root) @@ -1230,14 +1246,17 @@ func seedRoot(ctx context.Context, root string, } dev, ino := inodeOfInfo(fi) - - sendEvent(ctx, events, walkEvent{rec: fileRec{ + rec := fileRec{ path: root, size: fi.Size(), mtime: fi.ModTime(), dev: dev, ino: ino, - }}) + } + + if !isDatabaseFile(rec, rules) { + sendEvent(ctx, events, walkEvent{rec: rec}) + } return nil } @@ -1372,7 +1391,8 @@ func walkOneDir(ctx context.Context, job dirJob, oneFS bool, // regular file) between the directory read and the lstat is warned // about and skipped. A file on a filesystem in rules that is not // persistent, as through a bind mount of one file, is not recorded: it -// gets an event naming it instead. +// gets an event naming it instead. One of the scan's own database files +// in rules gets no event at all. func emitFile(ctx context.Context, p string, e fs.DirEntry, rules walkRules, events chan<- walkEvent, ) { @@ -1398,14 +1418,17 @@ func emitFile(ctx context.Context, p string, e fs.DirEntry, } dev, ino := inodeOfInfo(info) - - sendEvent(ctx, events, walkEvent{rec: fileRec{ + rec := fileRec{ path: p, size: info.Size(), mtime: info.ModTime(), dev: dev, ino: ino, - }}) + } + + if !isDatabaseFile(rec, rules) { + sendEvent(ctx, events, walkEvent{rec: rec}) + } } // subdirJob applies the descent rules to directory p: never enter diff --git a/scan_test.go b/scan_test.go index 3f45bae..1d719ef 100644 --- a/scan_test.go +++ b/scan_test.go @@ -1171,7 +1171,7 @@ func syncOneFilesystem(t *testing.T, db *sql.DB, roots []string, var initial []dirJob for _, root := range roots { - for _, job := range seedRoot(ctx, root, events) { + for _, job := range seedRoot(ctx, root, walkRules{}, events) { job.rootDev = devices[root] initial = append(initial, job) } @@ -2501,3 +2501,99 @@ func TestUnderRoot(t *testing.T) { } } } + +// TestRunScanPassesOverDatabase scans a tree that holds the scan's own +// database, reached directly and through a symlinked directory. The +// walk passes over the database file, its -wal and -shm files and its +// .lock file with no line and no count, and the record an earlier scan +// left for the .lock file is deleted. A rescan of the unchanged tree +// updates nothing, and report lists none of the database's files. +func TestRunScanPassesOverDatabase(t *testing.T) { + for _, viaSymlink := range []bool{false, true} { + t.Run(fmt.Sprintf("via symlink %v", viaSymlink), func(t *testing.T) { + base := t.TempDir() + tree := filepath.Join(base, "real", "tree") + operand := tree + + writeFile(t, tree, "a", pattern(1, 100)) + writeFile(t, tree, "b", pattern(1, 100)) + writeFile(t, tree, "empty1", nil) + writeFile(t, tree, "empty2", nil) + + if viaSymlink { + err := os.Symlink(filepath.Join(base, "real"), + filepath.Join(base, "link")) + if err != nil { + t.Fatal(err) + } + + operand = filepath.Join(base, "link", "tree") + } + + dbPath := filepath.Join(tree, "db.sqlite") + t.Setenv(databaseEnv, dbPath) + + db, err := openScanDatabase(t.Context(), dbPath) + if err != nil { + t.Fatal(err) + } + + err = applyChanges(t.Context(), db, []scanRec{ + {path: filepath.Join(operand, "db.sqlite.lock")}, + }, nil, nil) + if err != nil { + t.Fatal(err) + } + + closeScanDatabase(t.Context(), db, dbPath) + + assertScanSummary(t, + "4 files seen (4 added, 0 updated, 1 removed, 0 unchanged)", + operand) + assertScanSummary(t, + "4 files seen (0 added, 0 updated, 0 removed, 4 unchanged)", + operand) + + report := runStdout(t, cmdReport) + if strings.Contains(report, "db.sqlite") || + !strings.Contains(report, filepath.Join(operand, "empty2")) { + t.Errorf("report = %q, want the empty files and none of "+ + "the database's files", report) + } + }) + } +} + +// TestRunScanPassesOverDatabaseOperands scans the database file and its +// .lock file given as operands themselves: neither is recorded or +// counted. +func TestRunScanPassesOverDatabaseOperands(t *testing.T) { + dbPath := testDBPath(t) + t.Setenv(databaseEnv, dbPath) + + // The first scan creates both files. + scanOK(t, t.TempDir()) + + assertScanSummary(t, + "0 files seen (0 added, 0 updated, 0 removed, 0 unchanged)", + dbPath, dbPath+".lock") +} + +// assertScanSummary scans operands and fails the test unless the +// summary line gives counts with nothing skipped, and the line naming +// the database is the only one that names one of its files. +func assertScanSummary(t *testing.T, counts string, operands ...string) { + t.Helper() + + stderr := scanOK(t, operands...) + + want := "scan: " + counts + ", 0 skipped\n" + if !strings.Contains(stderr, want) { + t.Errorf("scan %q: stderr = %q, want %q", operands, stderr, want) + } + + if strings.Count(stderr, "db.sqlite") != 1 { + t.Errorf("scan %q: stderr = %q, want only the line naming the "+ + "database to name one of its files", operands, stderr) + } +}