From e231538bc0dbb3ab4eaf63523bc62040b8f09950 Mon Sep 17 00:00:00 2001 From: sneak Date: Mon, 26 Oct 2026 13:19:09 +0000 Subject: [PATCH] Never scan an operand inside a .zfs directory (closes #142) An operand inside a directory named .zfs was walked, because the operand check looked only at the operand's own name. The check now also looks at every directory above the operand, both as given and with its symlinks resolved. Such an operand gets a warning, counts as skipped and is not walked, whether given alone or under another operand. The check runs before overlapping operands are pruned, so an operand under another one still gets its warning. The test for an operand named .zfs now writes its record beneath .zfs straight into the database, since no scan stores one there any more. Judgement call: a path component named .zfs counts whatever it is, a symlink included. Model: opus-5-5 --- README.md | 31 +++++++----- TODO.md | 5 ++ main_test.go | 132 +++++++++++++++++++++++++++++++++++++++++++++++---- scan.go | 40 +++++++++++----- 4 files changed, 175 insertions(+), 33 deletions(-) diff --git a/README.md b/README.md index 3a6ce3f..6a4b5c3 100644 --- a/README.md +++ b/README.md @@ -461,17 +461,26 @@ exclude, is not on a persistent filesystem (see "Rules for the walk"), or, with `-x`, is on a different filesystem than that operand, because that operand's walk does not enter such a directory and so does not reach it. -An operand that is a symlink (never followed, not even as an operand), socket, -FIFO, or device node, or a directory named `.zfs`, is not scanned. `scan` prints -a one-line warning naming the path and what it is, counts it as skipped, and -drops it from the scanned operands before reading the database. Another operand -beneath it is still scanned. The records stored beneath it are not deleted: they -are treated like any other record outside the scanned operands. If it lies under -another operand, they are under that operand instead, and are treated like any -other record there. This is not an error: a scan whose every operand is dropped -walks nothing and exits 0. An operand that is not on a persistent filesystem -(see "Rules for the walk") is dropped the same way. The line `scan` prints for -it is not a warning, though, and it is not counted as skipped. +An operand is not scanned when it is: + +- a symlink, which `scan` never follows, not even as an operand; +- a socket, FIFO or device node; +- at or inside a directory named `.zfs`. + +An operand reached through a symlink into a `.zfs` directory is not scanned +either. + +This holds even when the operand lies under another operand. `scan` prints a +one-line warning naming the path and what it is, counts it as skipped, and drops +it from the scanned operands before reading the database. An operand beneath a +symlink operand is still scanned. The records stored beneath the dropped operand +are not deleted: they are treated like any other record outside the scanned +operands. If it lies under another operand, they are under that operand instead, +and are treated like any other record there. This is not an error: a scan whose +every operand is dropped walks nothing and exits 0. An operand that is not on a +persistent filesystem (see "Rules for the walk") is dropped the same way. The +line `scan` prints for it is not a warning, though, and it is not counted as +skipped. `scan` synchronizes the database with the filesystem state under the scanned operands: diff --git a/TODO.md b/TODO.md index 039cd5e..def2438 100644 --- a/TODO.md +++ b/TODO.md @@ -29,6 +29,11 @@ # Completed Steps +- an operand at or inside a directory named `.zfs` is never scanned, whether + given alone or under another operand. `scan` warns about it, counts it as + skipped and does not walk it (2026-10-26, + https://git.eeqj.de/sneak/sfdupes/issues/142) + - a file or directory removed while the scan runs is not an error, in the walk or in the hash phase. The scan prints no line for it and does not count it as skipped. When the first of several hard links to a file is gone, the hash diff --git a/main_test.go b/main_test.go index 8e39eab..cca359a 100644 --- a/main_test.go +++ b/main_test.go @@ -928,24 +928,21 @@ func TestRunScanWalksOperandUnderSymlinkOperand(t *testing.T) { } func TestRunScanSkipsZFSOperand(t *testing.T) { - path := testDBPath(t) - t.Setenv(databaseEnv, path) - zfs := filepath.Join(t.TempDir(), ".zfs") - snapshot := filepath.Join(zfs, "snapshot", "hourly") - f := writeFile(t, snapshot, "f", pattern(1, 10)) + f := writeFile(t, zfs, "snapshot/hourly/f", pattern(1, 10)) - // An operand beneath a .zfs directory is walked, because it is not - // itself named .zfs. - scanOK(t, snapshot) + // No scan records a file inside a .zfs directory, so the record + // beneath the operand is written straight into the database. + path := seedDatabase(t, []scanRec{{size: 10, path: f}}) + t.Setenv(databaseEnv, path) assertOperandSkipped(t, path, zfs, ".zfs directory", f) } // assertOperandSkipped scans operand alone and checks that it is skipped // as kind: a warning naming it, one skip in the summary, exit 0, and the -// record for kept, which an earlier scan stored beneath operand, still -// in the database at dbPath. +// record for kept, stored beneath operand before this scan, still in the +// database at dbPath. func assertOperandSkipped(t *testing.T, dbPath, operand, kind, kept string, ) { @@ -974,6 +971,121 @@ func assertOperandSkipped(t *testing.T, dbPath, operand, kind, recordByPath(t, dbRecords(t, db), kept) } +// TestRunScanSkipsOperandInsideZFS scans a directory inside a .zfs +// directory, first alone and then under another operand. Each time it +// is warned about and counted as skipped, and nothing in it is walked. +// The other operand is scanned as usual. +func TestRunScanSkipsOperandInsideZFS(t *testing.T) { + path := testDBPath(t) + t.Setenv(databaseEnv, path) + + dir := t.TempDir() + hourly := filepath.Join(dir, ".zfs", "snapshot", "hourly") + writeFile(t, hourly, "f", pattern(1, 10)) + f := writeFile(t, dir, "f", pattern(2, 10)) + + warning := "walk " + hourly + ": skipping operand inside a .zfs directory\n" + + for _, tc := range []struct { + operands []string + summary string + }{ + { + []string{hourly}, + "scan: 0 files seen (0 added, 0 updated, 0 removed, " + + "0 unchanged), 1 skipped\n", + }, + { + []string{dir, hourly}, + "scan: 1 files seen (1 added, 0 updated, 0 removed, " + + "0 unchanged), 1 skipped\n", + }, + } { + stderr := scanOK(t, tc.operands...) + if !strings.Contains(stderr, warning) || + !strings.Contains(stderr, tc.summary) { + t.Errorf("scan %q: stderr = %q, want %q and %q", tc.operands, + stderr, warning, tc.summary) + } + } + + db, err := openDB(path, reportParams) + if err != nil { + t.Fatal(err) + } + + t.Cleanup(func() { _ = db.Close() }) + + if got := recordPaths(dbRecords(t, db)); !slices.Equal(got, []string{f}) { + t.Errorf("records = %q, want %q", got, []string{f}) + } +} + +// TestRunScanSkipsOperandThroughSymlinkIntoZFS scans snaps/daily, where +// snaps is a symlink to pool/.zfs/snapshot. The operand is warned about +// by the path given, counted as skipped, and not walked. +func TestRunScanSkipsOperandThroughSymlinkIntoZFS(t *testing.T) { + t.Setenv(databaseEnv, testDBPath(t)) + + dir := t.TempDir() + writeFile(t, dir, "pool/.zfs/snapshot/daily/f", pattern(1, 10)) + + snaps := filepath.Join(dir, "snaps") + + err := os.Symlink(filepath.Join(dir, "pool", ".zfs", "snapshot"), snaps) + if err != nil { + t.Fatal(err) + } + + daily := filepath.Join(snaps, "daily") + stderr := scanOK(t, daily) + + warning := "walk " + daily + ": skipping operand inside a .zfs directory\n" + summary := "scan: 0 files seen (0 added, 0 updated, 0 removed, " + + "0 unchanged), 1 skipped\n" + + if !strings.Contains(stderr, warning) || + !strings.Contains(stderr, summary) { + t.Errorf("stderr = %q, want %q and %q", stderr, warning, summary) + } +} + +// TestRunScanWalksOperandThroughSymlink scans data/daily, where data is +// a symlink to a directory outside any .zfs directory. The operand is +// scanned as usual, and its file is recorded by the path given. +func TestRunScanWalksOperandThroughSymlink(t *testing.T) { + path := testDBPath(t) + t.Setenv(databaseEnv, path) + + dir := t.TempDir() + writeFile(t, dir, "pool/data/daily/f", pattern(1, 10)) + + data := filepath.Join(dir, "data") + + err := os.Symlink(filepath.Join(dir, "pool", "data"), data) + if err != nil { + t.Fatal(err) + } + + daily := filepath.Join(data, "daily") + stderr := scanOK(t, daily) + + summary := "scan: 1 files seen (1 added, 0 updated, 0 removed, " + + "0 unchanged), 0 skipped\n" + if !strings.Contains(stderr, summary) { + t.Errorf("stderr = %q, want %q", stderr, summary) + } + + db, err := openDB(path, reportParams) + if err != nil { + t.Fatal(err) + } + + t.Cleanup(func() { _ = db.Close() }) + + recordByPath(t, dbRecords(t, db), filepath.Join(daily, "f")) +} + func TestRunReportSucceeds(t *testing.T) { path := testDBPath(t) t.Setenv(databaseEnv, path) diff --git a/scan.go b/scan.go index 2aed86b..a16dd84 100644 --- a/scan.go +++ b/scan.go @@ -383,7 +383,8 @@ func (s *scanState) runPhases(ctx context.Context, roots []string, ) error { // Types and filesystems are checked before pruning so that an // operand under a dropped one is still scanned, not dropped as lying - // under it. + // under it, and so that an operand inside a .zfs directory gets its + // warning even when it lies under another operand. roots = pruneRoots(s.walkableRoots(roots, rules), oneFS, rules, deviceOfPath) @@ -411,15 +412,15 @@ func (s *scanState) runPhases(ctx context.Context, roots []string, } // walkableRoots returns the operands the walk can start from: regular -// files, and directories not named .zfs. Every other operand is warned -// about, counted as skipped, and dropped. A dropped operand is no -// longer a root, so the records stored beneath it are treated as if it -// had not been given: those under no other root count as outside the -// roots and are not deleted as unverified. An operand on a filesystem -// in rules that is not persistent is dropped the same way, with a line -// saying so that is not a warning, and is not counted as skipped. An -// operand that fails lstat here is kept, and the walk warns about it -// unless it no longer exists. +// files, and directories not named .zfs, none of them inside a +// directory named .zfs. Every other operand is warned about, counted as +// skipped, and dropped. A dropped operand is no longer a root, so the +// records stored beneath it are treated as if it had not been given: +// those under no other root count as outside the roots and are not +// deleted as unverified. An operand on a filesystem in rules that is not +// persistent is dropped the same way, with a line saying so that is not +// a warning, and is not counted as skipped. An operand that fails lstat +// here is kept, and the walk warns about it unless it no longer exists. func (s *scanState) walkableRoots(roots []string, rules walkRules) []string { kept := make([]string, 0, len(roots)) @@ -874,9 +875,24 @@ func sendEvent(ctx context.Context, events chan<- walkEvent, // operandWarning returns the one-line warning for an operand the walk // does not start from, naming the path and what it is, or "" for one it -// does: a regular file, or a directory not named .zfs. Symlinks are -// never followed, including as operands. +// does: a regular file, or a directory not named .zfs, that is not +// inside a directory named .zfs, neither as given nor with its symlinks +// resolved. Symlinks are never followed, including as operands. func operandWarning(root string, fi fs.FileInfo) string { + // The walk never enters a .zfs directory, so it starts from nothing + // inside one either, not even through a symlink. An operand whose + // symlinks cannot be resolved is checked as given. + resolved, err := filepath.EvalSymlinks(root) + if err != nil { + resolved = root + } + + if slices.Contains(strings.Split(filepath.Dir(root), "/"), ".zfs") || + slices.Contains(strings.Split(filepath.Dir(resolved), "/"), ".zfs") { + return fmt.Sprintf("walk %s: skipping operand inside a .zfs directory", + root) + } + var kind string switch mode := fi.Mode(); { -- 2.54.0