diff --git a/README.md b/README.md index 40d47e1..ef0ff2e 100644 --- a/README.md +++ b/README.md @@ -398,7 +398,10 @@ results do not depend on the working directory. All operands belong to a single scan and are enumerated concurrently: every operand seeds the shared walk worker pool. Overlapping operands are harmless — an operand that duplicates another or lies under another is dropped before walking, so every file is reached exactly -once and produces one database record. +once and produces one database record. With `-x`, though, an operand under +another one is kept and walked on its own when it is a directory on a different +filesystem than the operand covering it, or a regular file in such a directory, +because the walk of the covering operand 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 @@ -451,7 +454,8 @@ operands: directory is on another filesystem than its operand counts as outside the scanned operands: it is not deleted, and its size joins the size census. A filesystem mounted under an operand can therefore be scanned on its own - schedule into the same database. + schedule into the same database. Records below an operand at or inside such a + directory are not kept this way, since that operand's own walk verifies them. `scan` runs **four sequential phases over the whole scan**. Parallelism lives inside each phase; batched database writes begin during the hash phase: diff --git a/TODO.md b/TODO.md index c588fef..318a8c1 100644 --- a/TODO.md +++ b/TODO.md @@ -29,6 +29,12 @@ # Completed Steps +- with `-x`, an operand under another operand is walked on its own instead of + being dropped when it is a directory on a different filesystem than that + operand, or a regular file in such a directory, and the records of files gone + from below it are deleted (2026-10-14, + https://git.eeqj.de/sneak/sfdupes/issues/106) + - with `-x`, `scan` keeps the records below a directory on another filesystem under its operand, and counts their sizes in the size census, instead of deleting them (2026-10-14, https://git.eeqj.de/sneak/sfdupes/issues/99) diff --git a/scan.go b/scan.go index f4a031c..42e1286 100644 --- a/scan.go +++ b/scan.go @@ -203,10 +203,16 @@ func resolveRoots(roots []string) ([]string, error) { } // pruneRoots drops operands already covered by another operand: -// duplicates and any operand lying under another one. Every remaining -// file is then reachable through exactly one root, so walked paths are -// unique without keeping a scan-wide set of every path seen. -func pruneRoots(roots []string) []string { +// duplicates and any operand lying under another one. With -x, an +// operand under another one is kept when device puts it, or for a +// regular file its directory, on a different filesystem than the +// deepest kept operand above it, because that operand's walk then does +// not reach it. Every remaining file is then reachable through exactly +// one root, so walked paths are unique without keeping a scan-wide set +// of every path seen. +func pruneRoots(roots []string, oneFS bool, + device func(path string) (uint64, bool), +) []string { // Shorter-first ordering guarantees an ancestor is kept before any // operand under it is considered. sorted := slices.Clone(roots) @@ -221,7 +227,31 @@ func pruneRoots(roots []string) []string { kept := make([]string, 0, len(sorted)) for _, r := range sorted { - if !underAnyRoot(r, kept) { + above := deepestRoot(r, kept) + if above == "" { + kept = append(kept, r) + + continue + } + + // Without -x, and always for a duplicate, r is dropped. + if !oneFS || above == r { + continue + } + + // The walk of above enters a directory only when it is on + // above's filesystem, and then reaches every regular file in it + // whatever device the file reports, so for a regular-file + // operand its directory decides. + dir := r + if fi, err := os.Lstat(r); err == nil && fi.Mode().IsRegular() { + dir = filepath.Dir(r) + } + + dev, ok := device(dir) + aboveDev, aboveOK := device(above) + + if ok && aboveOK && dev != aboveDev { kept = append(kept, r) } } @@ -229,6 +259,17 @@ func pruneRoots(roots []string) []string { return kept } +// deviceOfPath returns the filesystem device ID of the file at path, +// when lstat succeeds and the platform exposes one. +func deviceOfPath(path string) (uint64, bool) { + fi, err := os.Lstat(path) + if err != nil { + return 0, false + } + + return deviceOfInfo(fi) +} + // scanStats summarizes one scan's database synchronization for the // final stderr summary, or for the line an interrupted scan prints. type scanStats struct { @@ -295,14 +336,15 @@ func (s *scanState) runPhases(ctx context.Context, roots []string, ) error { // Types are checked before pruning so that an operand under a // dropped one is still scanned, not dropped as lying under it. - roots = pruneRoots(s.walkableRoots(roots)) + roots = pruneRoots(s.walkableRoots(roots), oneFS, deviceOfPath) err := s.loadIndex(ctx, roots) if err != nil { return err } - changed, unhashed := s.walkPhase(startWalk(ctx, roots, oneFS, workers)) + changed, unhashed := s.walkPhase( + startWalk(ctx, roots, oneFS, workers), roots) // A cancelled walk stops early, so its size census covers only part // of the roots, and every file it never reached would look vanished @@ -393,9 +435,10 @@ func (s *scanState) loadIndex(ctx context.Context, roots []string) error { // hashes; both remain candidates until the census decides whether // their sizes are shared. A record below a directory -x did not enter // counts as outside the roots: it is not deleted, and its size joins -// the census. +// the census. That does not hold below a root at or inside that +// directory, since that root's own walk verifies the records below it. func (s *scanState) walkPhase( - events <-chan walkEvent, + events <-chan walkEvent, roots []string, ) ([]fileRec, []fileRec) { prog := newProgress("walk", -1) @@ -441,7 +484,12 @@ func (s *scanState) walkPhase( prog.finish() for path, old := range s.existing { - if underAnyRoot(path, otherFSDirs) { + // Kept only when a directory -x did not enter lies below the + // deepest root above the record. A root at or below that + // directory was walked on its own and did not find the file. + // Both contain the record, so the longer path is the deeper. + otherFSDir := deepestRoot(path, otherFSDirs) + if len(otherFSDir) > len(deepestRoot(path, roots)) { delete(s.existing, path) s.sizes = append(s.sizes, old.size) @@ -875,6 +923,20 @@ func underRoot(path, root string) bool { return strings.HasPrefix(path, prefix) } +// deepestRoot returns the longest of the roots that path is or lies +// under, or "" when it lies under none. +func deepestRoot(path string, roots []string) string { + deepest := "" + + for _, root := range roots { + if underRoot(path, root) && len(root) > len(deepest) { + deepest = root + } + } + + return deepest +} + // dirJob is one directory awaiting traversal by the walk workers. It // carries its operand's filesystem device so -x can stop at // filesystem boundaries. @@ -898,10 +960,10 @@ type walkEvent struct { // startWalk seeds every root into the shared walk worker pool and // returns the event stream: one record per regular file, one warning // event per per-path error, and one event per directory -x did not -// enter. The channel is closed when the walk -// completes, and also when ctx is cancelled — every goroutine in the -// pool abandons its blocking send in that case, so the consumer sees a -// truncated but properly terminated stream instead of a stalled one. +// enter. The channel is closed when the walk completes, and also when +// ctx is cancelled — every goroutine in the pool abandons its blocking +// send in that case, so the consumer sees a truncated but properly +// terminated stream instead of a stalled one. func startWalk(ctx context.Context, roots []string, oneFS bool, workers int, ) <-chan walkEvent { diff --git a/scan_test.go b/scan_test.go index 56880ab..67323f3 100644 --- a/scan_test.go +++ b/scan_test.go @@ -1135,14 +1135,71 @@ func TestSubdirJobStatError(t *testing.T) { } } +// syncOneFilesystem scans roots into db with -x and returns the scan +// stats, taking the device of each operand, and of a regular-file +// operand's directory, from devices instead of from lstat, so a test +// can put an operand on a filesystem that no directory here is on. It +// runs the steps of runPhases except the content phase. +func syncOneFilesystem(t *testing.T, db *sql.DB, roots []string, + devices map[string]uint64, +) scanStats { + t.Helper() + + device := func(path string) (uint64, bool) { + dev, ok := devices[path] + + return dev, ok + } + + roots = pruneRoots(roots, true, device) + + ctx := t.Context() + s := &scanState{db: db} + + err := s.loadIndex(ctx, roots) + if err != nil { + t.Fatal(err) + } + + jobs, subdirs, events := startWalkWorkers(ctx, 2, true) + + // seedRoot emits a regular-file operand itself and returns a job + // only for a directory operand. + var initial []dirJob + + for _, root := range roots { + for _, job := range seedRoot(ctx, root, events) { + job.rootDev = devices[root] + initial = append(initial, job) + } + } + + dispatchDirs(ctx, initial, jobs, subdirs) + + changed, unhashed := s.walkPhase(events, roots) + s.partition(changed, unhashed) + + err = s.hashPhase(ctx, 2) + if err != nil { + t.Fatal(err) + } + + err = s.updatePhase(ctx) + if err != nil { + t.Fatal(err) + } + + return s.st +} + // TestScanOneFilesystemKeepsOtherFilesystemRecords checks that with -x a // record below a directory the walk does not enter, because it is on // another filesystem than its operand, counts as outside the operands: // it is not deleted, and its size joins the size census. The record of // a gone file that is not below that directory is still deleted, even -// when its path begins with the directory's name. The walk starts from -// an operand device that no directory here is on, so no second real -// filesystem is needed; the phases after it run as runPhases runs them. +// when its path begins with the directory's name. The operand is given +// a device that no directory here is on, so no second real filesystem +// is needed. func TestScanOneFilesystemKeepsOtherFilesystemRecords(t *testing.T) { t.Parallel() @@ -1174,30 +1231,7 @@ func TestScanOneFilesystemKeepsOtherFilesystemRecords(t *testing.T) { t.Skip("platform exposes no device id") } - ctx := t.Context() - s := &scanState{db: db} - - err = s.loadIndex(ctx, []string{dir}) - if err != nil { - t.Fatal(err) - } - - jobs, subdirs, events := startWalkWorkers(ctx, 2, true) - root := dirJob{path: dir, rootDev: dev + 1, rootDevOK: true} - dispatchDirs(ctx, []dirJob{root}, jobs, subdirs) - - changed, unhashed := s.walkPhase(events) - s.partition(changed, unhashed) - - err = s.hashPhase(ctx, 2) - if err != nil { - t.Fatal(err) - } - - err = s.updatePhase(ctx) - if err != nil { - t.Fatal(err) - } + syncOneFilesystem(t, db, []string{dir}, map[string]uint64{dir: dev + 1}) recs := dbRecords(t, db) recordByPath(t, recs, mounted) @@ -1213,6 +1247,93 @@ func TestScanOneFilesystemKeepsOtherFilesystemRecords(t *testing.T) { } } +// TestScanOneFilesystemWalksNestedOperand checks that with -x an +// operand under another one, on a different filesystem than it, is +// walked on its own: a file added below it is recorded, and the record +// of a file gone from below it is deleted. A record below a directory +// that its own walk does not enter is still kept. Each operand is given +// a device that no directory here is on, so neither walk enters a +// subdirectory. +func TestScanOneFilesystemWalksNestedOperand(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + db := openTestDB(t) + + // A scan without -x records both files below mnt. + gone := writeFile(t, dir, "mnt/gone", pattern(1, 10)) + kept := writeFile(t, dir, "mnt/sub/kept", pattern(2, 20)) + syncTree(t, db, dir) + + err := os.Remove(gone) + if err != nil { + t.Fatal(err) + } + + added := writeFile(t, dir, "mnt/added", pattern(3, 30)) + + info, err := os.Lstat(dir) + if err != nil { + t.Fatal(err) + } + + dev, ok := deviceOfInfo(info) + if !ok { + t.Skip("platform exposes no device id") + } + + mnt := filepath.Join(dir, "mnt") + syncOneFilesystem(t, db, []string{dir, mnt}, + map[string]uint64{dir: dev + 1, mnt: dev + 2}) + + paths := recordPaths(dbRecords(t, db)) + + if !slices.Contains(paths, added) { + t.Errorf("no record for %s: the walk did not reach %s", added, mnt) + } + + if slices.Contains(paths, gone) { + t.Errorf("record for %s kept: the file is gone", gone) + } + + if !slices.Contains(paths, kept) { + t.Errorf("record for %s deleted: the walk of %s did not enter %s", + kept, mnt, filepath.Dir(kept)) + } +} + +// TestScanOneFilesystemRecordsNestedFileOnce checks that with -x a +// regular-file operand under another operand is dropped when the file +// reports a different device than that operand but its directory does +// not, as a file bind-mounted into the tree does: the walk of the other +// operand enters the directory and reaches the file, so the file is +// recorded once. +func TestScanOneFilesystemRecordsNestedFileOnce(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + db := openTestDB(t) + file := writeFile(t, dir, "sub/f", pattern(1, 10)) + + info, err := os.Lstat(dir) + if err != nil { + t.Fatal(err) + } + + dev, ok := deviceOfInfo(info) + if !ok { + t.Skip("platform exposes no device id") + } + + st := syncOneFilesystem(t, db, []string{dir, file}, map[string]uint64{ + dir: dev, filepath.Dir(file): dev, file: dev + 1, + }) + + if st.added != 1 { + t.Errorf("%s recorded %d times, want once", file, st.added) + } +} + // buildSmokeTree recreates the README smoke-test filesystem layout // with deterministic content and returns the tree root. func buildSmokeTree(t *testing.T) string { @@ -2123,14 +2244,43 @@ func TestHashRuns(t *testing.T) { func TestPruneRoots(t *testing.T) { t.Parallel() + const pqRoot = "/p/q" + + // /p/q is on another filesystem than /p; /p/q/r is on the same one as + // /p/q, /p/s as /p, and /p/t has no known device. + devices := map[string]uint64{"/p": 1, pqRoot: 2, "/p/q/r": 2, "/p/s": 1} + device := func(path string) (uint64, bool) { + dev, ok := devices[path] + + return dev, ok + } + // Duplicates and operands under other operands are dropped; /cc is // not under /c (sibling with a shared prefix). - got := pruneRoots([]string{"/a/b", "/a", "/c", "/a", "/a/b/c", "/cc"}) + got := pruneRoots([]string{"/a/b", "/a", "/c", "/a", "/a/b/c", "/cc"}, + false, device) want := []string{"/a", "/c", "/cc"} if !slices.Equal(got, want) { t.Fatalf("pruneRoots = %q, want %q", got, want) } + + nested := []string{"/p/q/r", pqRoot, "/p", "/p", "/p/s", "/p/t"} + + got = pruneRoots(nested, false, device) + + want = []string{"/p"} + if !slices.Equal(got, want) { + t.Fatalf("pruneRoots without -x = %q, want %q", got, want) + } + + // With -x, /p/q, on another filesystem than /p, is kept as well. + got = pruneRoots(nested, true, device) + + want = []string{"/p", pqRoot} + if !slices.Equal(got, want) { + t.Fatalf("pruneRoots with -x = %q, want %q", got, want) + } } func TestReportsNeverTouchFilesystem(t *testing.T) {