diff --git a/README.md b/README.md index 6508cbe..0d34f88 100644 --- a/README.md +++ b/README.md @@ -251,6 +251,14 @@ duplicates another or lies under another is dropped before walking, so every file is reached exactly once and produces one database record. +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, so the records stored beneath it are left as +they are. This is not an error: a scan whose every operand is dropped +walks nothing and exits 0. + `scan` synchronizes the database with the filesystem state under the scanned operands: @@ -356,9 +364,12 @@ during the hash phase: Rules for the walk: - Only regular files. Skip directories, symlinks (do not follow, - including symlink operands), sockets, FIFOs, and device nodes. + including symlink operands), sockets, FIFOs, and device nodes. An + operand that is a symlink, socket, FIFO, or device node is dropped + as described in "`scan` mode" above. - Never descend into a directory named `.zfs` (ZFS snapshot pseudo-dirs; - walking them would list every file once per snapshot). + walking them would list every file once per snapshot), not even + when it is an operand; such an operand is dropped the same way. - 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 @@ -369,7 +380,8 @@ Rules for the walk: path, and continue. Per-file errors never abort the run; the final summary reports how many were skipped. As specified above, a skipped path that has a database record from an earlier scan loses - that record, unless it failed only in the content phase; an + that record, unless it failed only in the content phase or is an + operand dropped before the database was read; an unreadable directory subtree likewise loses its records (accepted: the database mirrors what the latest scan could actually verify). diff --git a/TODO.md b/TODO.md index b540d3a..1a753a1 100644 --- a/TODO.md +++ b/TODO.md @@ -29,6 +29,10 @@ # Completed Steps +- warn about and skip symlink, socket, FIFO, device and `.zfs` + operands, keeping the records beneath them (2026-10-03, + https://git.eeqj.de/sneak/sfdupes/issues/9) + - stamp the git tag or short commit in a plain `docker build .` instead of `dev` (2026-10-02, branch `next`, closes https://git.eeqj.de/sneak/sfdupes/issues/67): `.dockerignore` now diff --git a/main_test.go b/main_test.go index b307a82..b1101fb 100644 --- a/main_test.go +++ b/main_test.go @@ -51,16 +51,25 @@ func assertNoSidecars(t *testing.T, path string) { func captureStdout(t *testing.T) func() string { t.Helper() - f, err := os.Create(filepath.Join(t.TempDir(), "stdout")) + return captureStream(t, &os.Stdout) +} + +// captureStream redirects *stream (os.Stdout or os.Stderr) to a file +// for the rest of the test and returns a function reading back +// everything written to it. +func captureStream(t *testing.T, stream **os.File) func() string { + t.Helper() + + f, err := os.Create(filepath.Join(t.TempDir(), "capture")) if err != nil { t.Fatal(err) } - saved := os.Stdout - os.Stdout = f + saved := *stream + *stream = f t.Cleanup(func() { - os.Stdout = saved + *stream = saved _ = f.Close() }) @@ -320,21 +329,30 @@ func scanFixture(t *testing.T) []string { t.Fatal(err) } - var stderr bytes.Buffer + scanOK(t, dir) + + return dupes +} + +// scanOK runs scan over operands, fails the test unless it exits 0 with +// nothing on stdout, and returns everything it printed to stderr. +func scanOK(t *testing.T, operands ...string) string { + t.Helper() stdout := captureStdout(t) + stderr := captureStream(t, &os.Stderr) - code := run([]string{cmdScan, dir}, &stderr) + code := run(append([]string{cmdScan}, operands...), os.Stderr) if code != exitOK { - t.Fatalf("run(scan) = %d, want %d; stderr: %s", - code, exitOK, stderr.String()) + t.Fatalf("run(scan %q) = %d, want %d; stderr: %s", + operands, code, exitOK, stderr()) } if got := stdout(); got != "" { t.Errorf("scan stdout = %q, want nothing (data only)", got) } - return dupes + return stderr() } func TestRunScanSucceedsDespiteWarnings(t *testing.T) { @@ -345,6 +363,75 @@ func TestRunScanSucceedsDespiteWarnings(t *testing.T) { assertNoSidecars(t, path) } +func TestRunScanSkipsSymlinkOperand(t *testing.T) { + path := testDBPath(t) + t.Setenv(databaseEnv, path) + + dir := t.TempDir() + writeFile(t, dir, "target/sub/f", pattern(1, 10)) + + link := filepath.Join(dir, "link") + + err := os.Symlink(filepath.Join(dir, "target"), link) + if err != nil { + t.Fatal(err) + } + + // Scanning a directory through the symlink stores a record beneath + // the symlink's own path for a file beneath its target. + scanOK(t, filepath.Join(link, "sub")) + + assertOperandSkipped(t, path, link, "symlink", + filepath.Join(link, "sub", "f")) +} + +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)) + + // An operand beneath a .zfs directory is walked, because it is not + // itself named .zfs. + scanOK(t, snapshot) + + 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. +func assertOperandSkipped(t *testing.T, dbPath, operand, kind, + kept string, +) { + t.Helper() + + stderr := scanOK(t, operand) + + warning := "walk " + operand + ": skipping " + kind + " operand\n" + if !strings.Contains(stderr, warning) { + t.Errorf("stderr = %q, want %q", stderr, warning) + } + + summary := "scan: 0 files seen (0 added, 0 updated, 0 removed, " + + "0 unchanged), 1 skipped\n" + if !strings.Contains(stderr, summary) { + t.Errorf("stderr = %q, want %q", stderr, summary) + } + + db, err := openDB(dbPath) + if err != nil { + t.Fatal(err) + } + + t.Cleanup(func() { _ = db.Close() }) + + recordByPath(t, dbRecords(t, db), kept) +} + func TestRunReportSucceeds(t *testing.T) { path := testDBPath(t) t.Setenv(databaseEnv, path) diff --git a/scan.go b/scan.go index 6d7aaf4..2837f85 100644 --- a/scan.go +++ b/scan.go @@ -210,14 +210,15 @@ type scanState struct { // in the content hash of every record of headTailMin or more whose // size, head, and tail match another record's). Records outside the // roots are never touched, except that the content phase fills in -// their content hash. +// their content hash. Operands the walk cannot start from are dropped +// first, so the records beneath them count as outside the roots. func syncScan(ctx context.Context, db *sql.DB, roots []string, workers int, oneFS bool, ) (scanStats, error) { - roots = pruneRoots(roots) - s := &scanState{db: db} + roots = pruneRoots(s.walkableRoots(roots)) + err := s.loadIndex(ctx, roots) if err != nil { return s.st, err @@ -253,6 +254,34 @@ func syncScan(ctx context.Context, db *sql.DB, roots []string, return s.st, s.contentPhase(ctx, workers) } +// 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 left as they are +// instead of being deleted as unverified. An operand that fails lstat +// here is kept, and the walk warns about it. +func (s *scanState) walkableRoots(roots []string) []string { + kept := make([]string, 0, len(roots)) + + for _, root := range roots { + fi, err := os.Lstat(root) + if err == nil { + warn := operandWarning(root, fi) + if warn != "" { + s.st.skipped++ + + fmt.Fprintln(os.Stderr, warn) + + continue + } + } + + kept = append(kept, root) + } + + return kept +} + // loadIndex indexes the database records under the scan roots for // change detection and collects the sizes of every record outside // them: out-of-scope records join the size census so a scanned file @@ -789,11 +818,43 @@ 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. +func operandWarning(root string, fi fs.FileInfo) string { + var kind string + + switch mode := fi.Mode(); { + case mode.IsRegular(): + return "" + case mode.IsDir(): + if filepath.Base(root) != ".zfs" { + return "" + } + + kind = ".zfs directory" + case mode&fs.ModeSymlink != 0: + kind = "symlink" + case mode&fs.ModeSocket != 0: + kind = "socket" + case mode&fs.ModeNamedPipe != 0: + kind = "FIFO" + case mode&fs.ModeDevice != 0: + kind = "device node" + default: + kind = "non-regular file" + } + + return fmt.Sprintf("walk %s: skipping %s operand", root, kind) +} + // seedRoot turns one PATH operand into the walk's starting state: a -// regular-file operand is statted and emitted directly, a directory -// operand becomes an initial job, and a symlink or other non-regular -// operand yields nothing (symlinks are never followed, including as -// operands). +// regular-file operand is statted and emitted directly, 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 are deleted as unverified. func seedRoot(ctx context.Context, root string, events chan<- walkEvent, ) []dirJob { @@ -807,30 +868,30 @@ func seedRoot(ctx context.Context, root string, return nil } - switch { - case fi.IsDir(): - if filepath.Base(root) == ".zfs" { - return nil - } + warn := operandWarning(root, fi) + if warn != "" { + sendEvent(ctx, events, walkEvent{warn: warn, fail: true}) + return nil + } + + if fi.IsDir() { dev, ok := deviceOfInfo(fi) return []dirJob{{path: root, rootDev: dev, rootDevOK: ok}} - case fi.Mode().IsRegular(): - dev, ino := inodeOfInfo(fi) - - sendEvent(ctx, events, walkEvent{rec: fileRec{ - path: root, - size: fi.Size(), - mtime: fi.ModTime().Unix(), - dev: dev, - ino: ino, - }}) - - return nil - default: - return nil } + + dev, ino := inodeOfInfo(fi) + + sendEvent(ctx, events, walkEvent{rec: fileRec{ + path: root, + size: fi.Size(), + mtime: fi.ModTime().Unix(), + dev: dev, + ino: ino, + }}) + + return nil } // startWalkWorkers starts the walk worker pool. Each worker processes diff --git a/scan_test.go b/scan_test.go index f47005d..282ba0f 100644 --- a/scan_test.go +++ b/scan_test.go @@ -856,9 +856,11 @@ func TestWalkFileAndSymlinkOperands(t *testing.T) { t.Fatalf("file operand: recs = %+v, errs = %d", recs, errs) } - // A symlink operand is not followed and yields nothing. + // A symlink operand that reaches the walk (it became one after + // walkableRoots checked it) is not followed: it yields a warning and + // no records. recs, errs = collectWalk(t, []string{link}, false, 2) - if errs != 0 || len(recs) != 0 { + if errs != 1 || len(recs) != 0 { t.Fatalf("symlink operand: recs = %+v, errs = %d", recs, errs) } }