From f0b679e9fa64bea50f82a266b1c0137cbc15e4ba Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Wed, 11 Nov 2026 06:07:31 +0100 Subject: [PATCH] Leave removed copies out of report and trees (closes #173) (#184) --- README.md | 81 +++++++++------ TODO.md | 5 + internal/sfdupes/progress_test.go | 4 +- internal/sfdupes/report.go | 59 ++++++++--- internal/sfdupes/report_test.go | 159 ++++++++++++++++++++++++++++-- internal/sfdupes/scan_test.go | 83 ++++++++++++++-- internal/sfdupes/trees.go | 39 +++++--- internal/sfdupes/trees_test.go | 132 ++++++++++++++++++++++--- 8 files changed, 466 insertions(+), 96 deletions(-) diff --git a/README.md b/README.md index f82360b..f70186e 100644 --- a/README.md +++ b/README.md @@ -21,9 +21,10 @@ file takes too long. It has five subcommands. `scan` walks the filesystem and records the files it finds in an SQLite database. SQLite keeps a whole database in one file. The database is kept between runs. A rescan reads only new and changed files. That makes `scan` cheap enough to run every day from `cron`, the -system's job scheduler. `report`, `trees` and `info` read the database alone and -can run at any time. `dwim` scans `/` and then prints the `trees` report, for a -first look at a new machine with one command. +system's job scheduler. `report`, `trees` and `info` work from the database and +can run at any time. `report` and `trees` leave out a copy that was deleted +after the scan. `dwim` scans `/` and then prints the `trees` report, for a first +look at a new machine with one command. ## Getting Started @@ -98,13 +99,15 @@ Each `-v` flag below makes a bind mount, which shows a host directory at a path inside the container. Bind-mount `/var/lib/sfdupes` to keep the database between runs. Mount the tree to scan at the same path inside the container as on the host. Read-only access is enough. The database then records the host's paths, -and so do the reports. +and so do the reports. `report` and `trees` need that mount too. Without it, +every recorded path counts as gone, and each report holds only its header line. ```sh make docker docker run --rm -v /srv:/srv:ro -v /var/lib/sfdupes:/var/lib/sfdupes \ sfdupes scan /srv -docker run --rm -v /var/lib/sfdupes:/var/lib/sfdupes sfdupes report > dupes.tsv +docker run --rm -v /srv:/srv:ro -v /var/lib/sfdupes:/var/lib/sfdupes \ + sfdupes report > dupes.tsv ``` ### Daily scan from `cron` @@ -159,8 +162,10 @@ to keep is your choice. A row is a candidate, not proof: -- The reports show each file as it was when a scan last recorded it. A file may - have changed or gone since. +- The reports show each file as it was when a scan last recorded it. They check + only that each copy still exists: the file, or the directory at the top of a + tree. A file may have changed since, and a file inside a tree may be gone. Run + `scan` again to bring the reports up to date. - A file of 100 MiB or more is compared only on samples of its content (see "Duplicate detection"). - Two paths that are hard links, two names for one file, are listed as @@ -209,7 +214,8 @@ of reads, at the price of a comparison that is not exact. The hashes live in a database that is kept between runs. A rescan reads only new files, and files whose size or modification time (mtime) differs from their -record. All analysis then works from the database alone. +record. All analysis then works from the database. The reports only check that +each copy they name still exists. The goal is whole duplicated trees, not only single files. A download extracted twice, or a project copied once too often, is such a tree. It can be removed as @@ -235,9 +241,10 @@ In order: the files of the directories it is reading. 3. **Rescan cheaply, analyze from the database.** The scan keeps its database between runs, and an unchanged file is never read again. `report`, `trees` - and `info` work from the database alone and never touch the scanned - filesystem. `scan` is built to run from `cron`. The reports run at any time, - against what the database holds. + and `info` work from the database and never read a scanned file. `report` and + `trees` check only that each copy they name still exists, and leave out a + copy that is gone. `scan` is built to run from `cron`. The reports run at any + time, against what the database holds. 4. **Keep the streams apart.** Everything on stdout is machine-readable data. Everything for a human reader, such as progress and warnings, goes to stderr. @@ -654,21 +661,29 @@ and `scan` leaves it ignored. `report` reads every record from the database. It takes no positional arguments. A positional argument is anything on the command line that is not a flag, such -as the paths `scan` takes. `report` never touches the scanned filesystem. It -neither stats nor opens any path in the records. It reads the database and -writes to stdout and stderr, and nothing else. Its output is the same whether or -not the scanned filesystem is still mounted. +as the paths `scan` takes. `report` never opens a scanned file. It checks that +each path in a group still exists, and looks at no other path. The check is +`lstat`, the system call that looks up a path without following a symlink: + +- A path is gone when `lstat` finds that it, or a directory above it, does not + exist. +- Any other error, such as a permission error, counts as present. So a user who + may not look inside a directory still sees its copies. + +The check does not notice a file that changed after the scan. Run `scan` again +to bring the report up to date. Processing: - Records are grouped by signature, the four values `(size, head, tail, content)`. -- Every group with two or more paths is a duplicate group. +- In a group with two or more paths, a path that is gone is left out. A group + that still has two or more paths is a duplicate group. - Empty files form no group, because removing one frees nothing. - Within a group, paths are sorted in byte order. The first is `first`, and every other path is a `dupe`. - Groups are ordered by size, largest first, then by `first` path. The output is - the same for the same database contents. + the same for the same database contents and the same paths on disk. `report` has two phases, each with its own progress display on stderr (see "Progress"). `open` opens the database. `read` gets the number of records from @@ -721,8 +736,9 @@ report: 14 records read, 4 duplicate groups, 7 dupe files, 10.0 KiB reclaimable `trees` reads the same database as `report` and takes no positional arguments. It reports whole duplicate directory trees. Two directories are duplicate trees when they hold the same relative paths, and each path holds a file with the same -signature in both. Like `report`, it never touches the scanned filesystem. It -rebuilds the directory tree from the paths in the records, split on `/`. +signature in both. It rebuilds the directory tree from the paths in the records, +split on `/`. Like `report`, it never opens a scanned file. It checks only that +the directory at the top of each copy still exists, as `report` checks a path. Definitions: @@ -749,10 +765,13 @@ Processing: directories that all have the same digest. The parents' group, or an ancestor's, already implies it. A group is always reported when it holds sibling directories, or when its members' parents have different digests. +- A directory that is gone is dropped from its group. A group left with fewer + than two directories is not reported. - Within a group, paths are sorted in byte order. The first is `first`, and every other path is a `dupe`. - Groups are ordered by total tree size, largest first, then by `first` path. - The output is the same for the same database contents. + The output is the same for the same database contents and the same paths on + disk. Two limitations are accepted: @@ -766,8 +785,9 @@ Two limitations are accepted: `trees` has four phases, each with its own progress display on stderr (see "Progress"). `open` opens the database. `read` gets the number of records from the `totals` table, then reads them in path order. It builds the directory tree -and computes the digests as it goes. `group` groups the directories by digest -and picks the groups to report. `write` writes them to stdout. +and computes the digests as it goes. `group` groups the directories by digest, +picks the groups to report and checks that their directories still exist. +`write` writes the groups to stdout. #### Trees output format @@ -825,8 +845,8 @@ The report goes to stdout, exactly as `trees` prints it. On stderr comes what dwim: run "sfdupes trees" to print this report again ``` -The database keeps the scan, so `sfdupes trees` prints the same report later, -even after the terminal is gone. +The database keeps the scan, so `sfdupes trees` prints the report again later, +even after the terminal is gone. It then leaves out a copy deleted in between. `SIGINT` or `SIGTERM` stops the scan as it stops `scan` (see "Stopping a scan"). `dwim` then prints no report and exits 1. A fatal scan error also ends it before @@ -1118,9 +1138,10 @@ in, and a phase that may wait in two places names both, as `disk/db`: - `hash` says `disk/db`, because it reads files and writes their records to the database between them. - `load`, `update` and the `read` phase of `trees` say `db`. -- The `read` phase of `report` says `db/out`, because it writes the report to - stdout while it reads. -- `group` of `trees` says `cpu`, for work in memory, and its `write` says `out`. +- The `read` phase of `report` says `db/disk/out`. It checks that the paths of + each group still exist, and writes the report to stdout, while it reads. +- `group` of `trees` says `cpu/disk`, for work in memory and the check that each + copy still exists. Its `write` says `out`. - A phase still waiting for its total says what it waits for, and nothing more. On a terminal: @@ -1360,8 +1381,8 @@ commands of their own: not a goal. `modernc.org/sqlite` is pure Go too, so the binary builds without cgo. The `Makefile` exports `CGO_ENABLED=0`. The test build is the one exception, because the race detector needs cgo (see `script/test`). -- `report` and `trees` give the same output for the same database contents, - whatever order the records were inserted in. +- `report` and `trees` give the same output for the same database contents and + the same paths on disk, whatever order the records were inserted in. ### Definition of done @@ -1426,7 +1447,7 @@ All of the following, run in this directory, must pass: The test suite automates this scenario (see `internal/sfdupes/scan_test.go`). It also checks that `report` and `trees` - never touch the scanned filesystem. + leave out a copy removed after the scan. ## TODO diff --git a/TODO.md b/TODO.md index 94e5c67..b98640a 100644 --- a/TODO.md +++ b/TODO.md @@ -29,6 +29,11 @@ Newest first. An entry ends with its date and its issue or branch, where those are known. +- `report` and `trees` leave out a copy that was deleted after the scan. Each + checks only the copy itself with `lstat`: the file, or the directory at the + top of a tree. A group left with one copy is not reported. A missing `scan` + operand is still a fatal error (2026-11-11, + https://git.eeqj.de/sneak/sfdupes/issues/173) - The usage line of `scan` no longer ends in `[flags]`. It is the line `README.md` shows under "Subcommands" (2026-11-11, https://git.eeqj.de/sneak/sfdupes/issues/169) diff --git a/internal/sfdupes/progress_test.go b/internal/sfdupes/progress_test.go index 91fb275..c2a6401 100644 --- a/internal/sfdupes/progress_test.go +++ b/internal/sfdupes/progress_test.go @@ -642,9 +642,9 @@ func TestTerminalStalledLines(t *testing.T) { {label: scanPhase("hash"), stalledOn: hashStalledOn}, {label: scanPhase("update"), stalledOn: "db"}, {label: treesPhase("read"), stalledOn: "db"}, - {label: treesPhase("group"), stalledOn: "cpu"}, + {label: treesPhase("group"), stalledOn: "cpu/disk"}, {label: treesPhase("write"), stalledOn: "out"}, - {label: reportPhase("read"), stalledOn: "db/out"}, + {label: reportPhase("read"), stalledOn: "db/disk/out"}, } for _, params := range phases { diff --git a/internal/sfdupes/report.go b/internal/sfdupes/report.go index 949b38a..84eb53f 100644 --- a/internal/sfdupes/report.go +++ b/internal/sfdupes/report.go @@ -4,11 +4,14 @@ import ( "context" "crypto/sha256" "database/sql" + "errors" "fmt" "io" + "io/fs" "os" "slices" "strings" + "syscall" "time" ) @@ -41,10 +44,10 @@ func reportPhase(name string) string { } // runReport implements the report subcommand: it prints the file-level -// duplicates report as TSV on stdout. It never touches the scanned -// filesystem; its only I/O is the database, stdout, and stderr. It -// names the database on stderr before it opens it. Any database -// problem, including a missing database, is fatal. +// duplicates report as TSV on stdout. Of the scanned filesystem it +// only checks, with pathExists, that each path in a duplicate group is +// still there. It names the database on stderr before it opens it. Any +// database problem, including a missing database, is fatal. // // The work runs in two phases, each under a progress display of its // own on stderr: open (open the database) and read (read the number of @@ -77,13 +80,14 @@ func runReport(ctx context.Context, stdout io.Writer) error { defer func() { _ = db.Close() }() - // The read phase writes the report to stdout while it reads the - // records, so it may wait on either. + // The read phase checks the paths of each duplicate group on disk + // and writes the report to stdout while it reads the records, so it + // may wait on any of the three. prog = newProgress(progressParams{ label: reportPhase("read"), unit: "records", total: -1, - waitingFor: countingRecords, stalledOn: "db/out", + waitingFor: countingRecords, stalledOn: "db/disk/out", }) - report, err := readReport(ctx, db, dbPath, stdout, prog) + report, err := readReport(ctx, db, dbPath, stdout, pathExists, prog) prog.finish() @@ -104,14 +108,15 @@ func runReport(ctx context.Context, stdout io.Writer) error { // dbPath, as its total, then reads every record in signature order, // counting each on prog, and writes the report to stdout: the header // line, then the rows of each size's duplicate groups once that size is -// read. stdout may be the terminal prog is drawn on, so the lines go -// through prog's writer. It returns what it wrote, with the totals for -// the summary line. +// read. A path for which exists returns false is left out of its group. +// stdout may be the terminal prog is drawn on, so the lines go through +// prog's writer. It returns what it wrote, with the totals for the +// summary line. func readReport(ctx context.Context, db *sql.DB, dbPath string, - stdout io.Writer, prog *progress, + stdout io.Writer, exists func(path string) bool, prog *progress, ) (*reportWriter, error) { out := prog.writer(stdout) - report := &reportWriter{out: out} + report := &reportWriter{out: out, exists: exists} _, err := fmt.Fprintln(out, "first\tdupe\tsize") if err != nil { @@ -158,6 +163,10 @@ func readReport(ctx context.Context, db *sql.DB, dbPath string, type reportWriter struct { out io.Writer + // exists reports whether a path is still on disk. A path that is + // gone is left out of its group. + exists func(path string) bool + sig scanRec // the signature being read; its path is not used paths []string // the paths read so far with sig groups [][]string // the duplicate groups of sig.size, each sorted @@ -194,10 +203,18 @@ func (w *reportWriter) add(r scanRec) error { return nil } -// endGroup ends the paths read with w.sig: two or more are a duplicate -// group, held with its paths sorted until writeGroups writes it. Empty -// files form no group, since removing one frees nothing. +// endGroup ends the paths read with w.sig: two or more that are still +// on disk are a duplicate group, held with its paths sorted until +// writeGroups writes it. Empty files form no group, since removing one +// frees nothing. Only the paths of a possible group are checked, never +// a path with no duplicate or an empty file. func (w *reportWriter) endGroup() { + if len(w.paths) >= minGroupSize && w.sig.size > 0 { + w.paths = slices.DeleteFunc(w.paths, func(p string) bool { + return !w.exists(p) + }) + } + if len(w.paths) < minGroupSize || w.sig.size == 0 { // The paths are dropped, and their slice reused. w.paths = w.paths[:0] @@ -246,6 +263,16 @@ func (w *reportWriter) finish() error { return w.writeGroups() } +// pathExists reports whether path is still on disk, for report and +// trees. It is false only when lstat finds that the path, or a +// directory above it, is gone. Any other error counts as present, so +// a user who may not look inside a directory still sees its copies. +func pathExists(path string) bool { + _, err := os.Lstat(path) + + return !errors.Is(err, fs.ErrNotExist) && !errors.Is(err, syscall.ENOTDIR) +} + // escapePath returns a path as it is written in a report column (README // "Report output format"): a backslash, tab, newline or carriage return // becomes \\, \t, \n or \r, and every other byte is kept as it is. diff --git a/internal/sfdupes/report_test.go b/internal/sfdupes/report_test.go index 5aff75b..e976688 100644 --- a/internal/sfdupes/report_test.go +++ b/internal/sfdupes/report_test.go @@ -72,15 +72,37 @@ type dupeGroup struct { paths []string } +// everyPathExists stands in for pathExists where a test's records name +// made-up paths that are not on disk. +func everyPathExists(string) bool { return true } + +// recsOnDisk returns a new directory and recs with each path moved +// under it, after creating an empty file at each moved path: report and +// trees then find every path still on disk. +func recsOnDisk(t *testing.T, recs []scanRec) (string, []scanRec) { + t.Helper() + + dir := realTempDir(t) + moved := make([]scanRec, len(recs)) + + for i, r := range recs { + r.path = writeFile(t, dir, r.path, nil) + moved[i] = r + } + + return dir, moved +} + // dupeGroups returns the duplicate groups report reads from db, in // report order, from the rows it writes: a group's rows come together, -// each naming the group's first path. +// each naming the group's first path. Every path counts as still on +// disk. func dupeGroups(t *testing.T, db *sql.DB) []dupeGroup { t.Helper() var out bytes.Buffer - _, err := readReport(t.Context(), db, "", &out, nil) + _, err := readReport(t.Context(), db, "", &out, everyPathExists, nil) if err != nil { t.Fatal(err) } @@ -127,7 +149,8 @@ func dupeGroupsOf(t *testing.T, recs []scanRec) []dupeGroup { } func TestRunReportEscapesPaths(t *testing.T) { - t.Setenv(databaseEnv, seedDatabase(t, awkwardPairRecs())) + dir, recs := recsOnDisk(t, awkwardPairRecs()) + t.Setenv(databaseEnv, seedDatabase(t, recs)) var stdout, stderr bytes.Buffer @@ -138,7 +161,7 @@ func TestRunReportEscapesPaths(t *testing.T) { } want := "first\tdupe\tsize\n" + - `/d/\tone\ntwo\rthree\\four/f` + "\t/d/A/f\t5\n" + dir + `/d/\tone\ntwo\rthree\\four/f` + "\t" + dir + "/d/A/f\t5\n" if got := stdout.String(); got != want { t.Errorf("stdout = %q, want %q", got, want) } @@ -172,7 +195,8 @@ func TestReportStdoutFailsWhileReading(t *testing.T) { // Counts the records read, and draws nothing. prog := &progress{now: time.Now} - _, err = readReport(t.Context(), db, "", failingWriter{}, prog) + _, err = readReport(t.Context(), db, "", failingWriter{}, + everyPathExists, prog) if !errors.Is(err, errWriteFailed) || !strings.HasPrefix(err.Error(), "write stdout: ") { t.Errorf("error = %v, want write stdout: %v", err, errWriteFailed) @@ -203,7 +227,8 @@ const smokeReport = "first\tdupe\tsize\n" + // it ends; read counts the 6 records before it reads them. The summary // comes last, and stdout holds the report alone. func TestRunReportProgressNamesEachPhase(t *testing.T) { - path := seedDatabase(t, smokeTreeRecs()) + dir, recs := recsOnDisk(t, smokeTreeRecs()) + path := seedDatabase(t, recs) t.Setenv(databaseEnv, path) stderr := captureStderr(t) @@ -216,8 +241,9 @@ func TestRunReportProgressNamesEachPhase(t *testing.T) { code, exitOK, stderr()) } - if got := stdout.String(); got != smokeReport { - t.Errorf("stdout = %q, want %q", got, smokeReport) + report := strings.ReplaceAll(smokeReport, "/d/", dir+"/d/") + if got := stdout.String(); got != report { + t.Errorf("stdout = %q, want %q", got, report) } // How each line starts; the rate at the end of a phase varies. @@ -266,7 +292,7 @@ func TestReadReportOnTerminal(t *testing.T) { waitingFor: countingRecords, }, terminalWidth, time.Hour, time.Now) - _, err = readReport(t.Context(), db, "", os.Stderr, p) + _, err = readReport(t.Context(), db, "", os.Stderr, everyPathExists, p) p.finish() @@ -301,6 +327,8 @@ func TestRunReportsIgnoreInsertionOrder(t *testing.T) { scanRec{size: 50, head: a, tail: a, content: a, path: "/x/1"}, ) + _, recs = recsOnDisk(t, recs) + reversed := slices.Clone(recs) slices.Reverse(reversed) @@ -455,6 +483,119 @@ func TestDupeGroupsLeavesOutEmptyFiles(t *testing.T) { } } +// TestReadReportChecksOnlyPossibleDuplicates checks that report asks +// whether a path is on disk only when its signature has two or more +// records, and never for an empty file. +func TestReadReportChecksOnlyPossibleDuplicates(t *testing.T) { + t.Parallel() + + a, u, e := testHash("a"), testHash("u"), sha256.Sum256(nil) + recs := []scanRec{ + {size: 9, head: a, tail: a, content: a, path: "/copy/1"}, + {size: 9, head: a, tail: a, content: a, path: "/copy/2"}, + {size: 9, head: a, tail: a, content: a, path: "/copy/3"}, + {size: 9, head: u, tail: u, content: u, path: "/lonely"}, + {size: 0, head: e, tail: e, content: e, path: "/empty1"}, + {size: 0, head: e, tail: e, content: e, path: "/empty2"}, + } + + db := openTestDB(t) + + err := applyChanges(t.Context(), db, recs, nil, nil) + if err != nil { + t.Fatal(err) + } + + var checked []string + + exists := func(path string) bool { + checked = append(checked, path) + + return true + } + + _, err = readReport(t.Context(), db, "", io.Discard, exists, nil) + if err != nil { + t.Fatal(err) + } + + slices.Sort(checked) + + want := []string{"/copy/1", "/copy/2", "/copy/3"} + if !slices.Equal(checked, want) { + t.Errorf("paths checked on disk = %q, want %q", checked, want) + } +} + +// TestReadReportDropsGroupLeftWithOnePath checks that a group of two +// paths, one of them gone, is no duplicate group. If kept, it would +// write no row, but the summary line would count it. +func TestReadReportDropsGroupLeftWithOnePath(t *testing.T) { + t.Parallel() + + a := testHash("a") + recs := []scanRec{ + {size: 9, head: a, tail: a, content: a, path: "/a/kept"}, + {size: 9, head: a, tail: a, content: a, path: "/a/gone"}, + } + + db := openTestDB(t) + + err := applyChanges(t.Context(), db, recs, nil, nil) + if err != nil { + t.Fatal(err) + } + + exists := func(path string) bool { return path != "/a/gone" } + + report, err := readReport(t.Context(), db, "", io.Discard, exists, nil) + if err != nil { + t.Fatal(err) + } + + if report.dupeGroups != 0 { + t.Errorf("duplicate groups = %d, want 0", report.dupeGroups) + } +} + +// TestReadReportKeepsGroupLeftWithTwoPaths checks that a group of three +// paths, one of them gone, is a duplicate group of the other two. +func TestReadReportKeepsGroupLeftWithTwoPaths(t *testing.T) { + t.Parallel() + + a := testHash("a") + recs := []scanRec{ + {size: 9, head: a, tail: a, content: a, path: "/b/kept1"}, + {size: 9, head: a, tail: a, content: a, path: "/b/gone"}, + {size: 9, head: a, tail: a, content: a, path: "/b/kept2"}, + } + + db := openTestDB(t) + + err := applyChanges(t.Context(), db, recs, nil, nil) + if err != nil { + t.Fatal(err) + } + + exists := func(path string) bool { return path != "/b/gone" } + + var out bytes.Buffer + + report, err := readReport(t.Context(), db, "", &out, exists, nil) + if err != nil { + t.Fatal(err) + } + + if report.dupeGroups != 1 { + t.Errorf("duplicate groups = %d, want 1", report.dupeGroups) + } + + want := "first\tdupe\tsize\n/b/kept1\t/b/kept2\t9\n" + if got := out.String(); got != want { + t.Errorf("stdout = %q, want %q", got, want) + } +} + func TestDupeGroupsContentSeparates(t *testing.T) { t.Parallel() diff --git a/internal/sfdupes/scan_test.go b/internal/sfdupes/scan_test.go index ba1fdcf..e9e78b5 100644 --- a/internal/sfdupes/scan_test.go +++ b/internal/sfdupes/scan_test.go @@ -1215,7 +1215,7 @@ func assertSmokeTreeGroups(t *testing.T, dir string, db *sql.DB) { super, dirs := dbTree(t, db) - tg := collectTreeGroups(dirs, super, nil) + tg := collectTreeGroups(dirs, super, everyPathExists, nil) if len(tg) != 1 { t.Fatalf("len(tree groups) = %d, want 1", len(tg)) } @@ -2307,23 +2307,86 @@ func TestPruneRoots(t *testing.T) { } } -func TestReportsNeverTouchFilesystem(t *testing.T) { - t.Parallel() +// TestRunReportsLeaveOutRemovedCopies scans the smoke tree, then +// removes copies the reports name. report leaves out each removed file, +// and a group left with one path. trees checks only the root of each +// duplicate tree: with a file under t2 removed it still names t2, and +// with t2 removed it names nothing. +func TestRunReportsLeaveOutRemovedCopies(t *testing.T) { + t.Setenv(databaseEnv, testDBPath(t)) dir := buildSmokeTree(t) - db := openTestDB(t) + p := func(name string) string { return filepath.Join(dir, name) } - syncTree(t, db, dir) + scanOK(t, dir) - // Remove the scanned tree entirely; the analysis must be - // unaffected because it reads the database alone. - err := os.RemoveAll(dir) + for _, name := range []string{"b/copy2.bin", "tiny2", "t2/sub/f2"} { + err := os.Remove(p(name)) + if err != nil { + t.Fatal(err) + } + } + + want := "first\tdupe\tsize\n" + + p("t1/f1") + "\t" + p("t2/f1") + "\t3000\n" + + p("t1/f1") + "\t" + p("t3/f1") + "\t3000\n" + + p("a/one.bin") + "\t" + p("b/copy.bin") + "\t2000\n" + + p("t1/sub/f2") + "\t" + p("t3/sub/f2renamed") + "\t100\n" + if got := runStdout(t, cmdReport); got != want { + t.Errorf("report stdout = %q, want %q", got, want) + } + + // disk_size, the last column, depends on the filesystem. + header := "first\tdupe\tfiles\tsize\tdisk_size\n" + + want = header + p("t1") + "\t" + p("t2") + "\t2\t3100\t" + if got := runStdout(t, cmdTrees); !strings.HasPrefix(got, want) || + strings.Count(got, "\n") != 2 { + t.Errorf("trees stdout = %q, want one row starting %q", got, want) + } + + err := os.RemoveAll(p("t2")) if err != nil { t.Fatal(err) } - assertSmokeDupeGroups(t, dir, db) - assertSmokeTreeGroups(t, dir, db) + if got := runStdout(t, cmdTrees); got != header { + t.Errorf("trees stdout after removing t2 = %q, want %q", got, header) + } +} + +func TestPathExists(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + file := writeFile(t, dir, "file", nil) + locked := writeFile(t, dir, "locked/file", nil) + + // Without search permission on its directory, a file cannot be + // looked at by anyone but root. + err := os.Chmod(filepath.Dir(locked), 0) + if err != nil { + t.Fatal(err) + } + + // Runs before t.TempDir's own cleanup, which must delete the files. + t.Cleanup(func() { + //nolint:gosec // removing the directory needs its search bit back + _ = os.Chmod(filepath.Dir(locked), 0o700) + }) + + cases := map[string]bool{ + dir: true, + file: true, + locked: true, + filepath.Join(dir, "gone"): false, + filepath.Join(file, "under-a-file"): false, + } + for path, want := range cases { + if got := pathExists(path); got != want { + t.Errorf("pathExists(%q) = %v, want %v", path, got, want) + } + } } func TestUnderRoot(t *testing.T) { diff --git a/internal/sfdupes/trees.go b/internal/sfdupes/trees.go index 873da23..36bfb12 100644 --- a/internal/sfdupes/trees.go +++ b/internal/sfdupes/trees.go @@ -38,10 +38,11 @@ func treesPhase(name string) string { // runTrees implements the trees subcommand: it reads every record from // the database in path order, reconstructs the directory hierarchy from // 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. It names the database on stderr before it opens it. Any -// database problem, including a missing database, is fatal. +// prints maximal duplicate-tree groups as TSV on stdout. Of the scanned +// filesystem it only checks, with pathExists, that the root of each +// duplicate tree is still there, never the files below it. It names the +// database on stderr before it opens it. Any database problem, +// including a missing database, is fatal. // // The work runs in four phases, each under a progress display of its // own on stderr: open (open the database), read (read the number of @@ -86,13 +87,13 @@ func runTrees(ctx context.Context, stdout io.Writer) error { return fmt.Errorf("database %s: %w", dbPath, err) } - // Grouping works in memory, so it waits on nothing but the - // processor. + // Grouping works in memory, then checks the root of each duplicate + // tree on disk, so it waits on the processor or the disk. prog = newProgress(progressParams{ label: treesPhase("group"), unit: "directories", - total: int64(len(allDirs)), stalledOn: "cpu", + total: int64(len(allDirs)), stalledOn: "cpu/disk", }) - dupes := collectTreeGroups(allDirs, super, prog) + dupes := collectTreeGroups(allDirs, super, pathExists, prog) prog.finish() @@ -300,12 +301,14 @@ func fileEntry(name string, r scanRec) string { // collectTreeGroups groups directories by digest, counting each one on // prog as it is grouped, and returns every maximal group with two or -// more members, each group's members sorted by path, groups ordered by -// tree size descending then by first path ascending. A group whose -// trees hold 0 bytes is left out, since removing one frees nothing. The -// groups it implies hold 0 bytes too, so leaving it out hides nothing. +// more members for which exists returns true, each group's members +// sorted by path, groups ordered by tree size descending then by first +// path ascending. A group whose trees hold 0 bytes is left out, since +// removing one frees nothing. The groups it implies hold 0 bytes too, +// so leaving it out hides nothing. exists is called only on the members +// of maximal groups. func collectTreeGroups(allDirs []*treeNode, super *treeNode, - prog *progress, + exists func(path string) bool, prog *progress, ) [][]*treeNode { groups := make(map[[sha256.Size]byte][]*treeNode) for _, d := range allDirs { @@ -323,6 +326,16 @@ func collectTreeGroups(allDirs []*treeNode, super *treeNode, continue } + // A member that is gone leaves the group. A suppressed group is + // still implied: its members still on disk have parents still + // on disk, and those parents' group is checked the same way. + g = slices.DeleteFunc(g, func(n *treeNode) bool { + return !exists(n.path) + }) + if len(g) < minGroupSize { + continue + } + slices.SortFunc(g, func(a, b *treeNode) int { return strings.Compare(a.path, b.path) }) diff --git a/internal/sfdupes/trees_test.go b/internal/sfdupes/trees_test.go index 356d48b..6a4d722 100644 --- a/internal/sfdupes/trees_test.go +++ b/internal/sfdupes/trees_test.go @@ -6,6 +6,7 @@ import ( "database/sql" "os" "slices" + "strconv" "strings" "testing" "time" @@ -179,7 +180,7 @@ func TestTreeNamesSortingBeforeSlash(t *testing.T) { t.Fatalf("directory paths = %q, want %q", got, want) } - groups := collectTreeGroups(dirs, super, nil) + groups := collectTreeGroups(dirs, super, everyPathExists, nil) gotGroups := groupPaths(groups) wantGroups := [][]string{{"/a", "/c"}} @@ -195,7 +196,8 @@ func TestTreeNamesSortingBeforeSlash(t *testing.T) { } func TestRunTreesEscapesPaths(t *testing.T) { - t.Setenv(databaseEnv, seedDatabase(t, awkwardPairRecs())) + dir, recs := recsOnDisk(t, awkwardPairRecs()) + t.Setenv(databaseEnv, seedDatabase(t, recs)) var stdout, stderr bytes.Buffer @@ -206,7 +208,7 @@ func TestRunTreesEscapesPaths(t *testing.T) { } want := "first\tdupe\tfiles\tsize\tdisk_size\n" + - `/d/\tone\ntwo\rthree\\four` + "\t/d/A\t1\t5\t4096\n" + dir + `/d/\tone\ntwo\rthree\\four` + "\t" + dir + "/d/A\t1\t5\t4096\n" if got := stdout.String(); got != want { t.Errorf("stdout = %q, want %q", got, want) } @@ -224,9 +226,11 @@ func TestRunTreesDiskSize(t *testing.T) { {size: 20, diskSize: 1024, head: g, tail: g, content: g, path: "/b/sub/g"}, } + dir, recs := recsOnDisk(t, recs) t.Setenv(databaseEnv, seedDatabase(t, recs)) - want := "first\tdupe\tfiles\tsize\tdisk_size\n/a\t/b\t2\t30\t1536\n" + want := "first\tdupe\tfiles\tsize\tdisk_size\n" + + dir + "/a\t" + dir + "/b\t2\t30\t1536\n" if got := runStdout(t, cmdTrees); got != want { t.Errorf("stdout = %q, want %q", got, want) } @@ -237,10 +241,11 @@ func TestRunTreesDiskSize(t *testing.T) { // there. After the line naming the database, each phase prints a line // naming it with its place among the four as it starts and another as // it ends. read counts the 6 records before it reads them, group counts -// the 8 directories, and write the 1 group. The summary comes last, and +// the directories, and write the 1 group. The summary comes last, and // stdout holds the report alone. func TestRunTreesProgressNamesEachPhase(t *testing.T) { - path := seedDatabase(t, smokeTreeRecs()) + dir, recs := recsOnDisk(t, smokeTreeRecs()) + path := seedDatabase(t, recs) t.Setenv(databaseEnv, path) stderr := captureStderr(t) @@ -254,11 +259,15 @@ func TestRunTreesProgressNamesEachPhase(t *testing.T) { } report := "first\tdupe\tfiles\tsize\tdisk_size\n" + - "/d/t1\t/d/t2\t2\t3100\t8192\n" + dir + "/d/t1\t" + dir + "/d/t2\t2\t3100\t8192\n" if got := stdout.String(); got != report { t.Errorf("stdout = %q, want %q", got, report) } + // The smoke-test records name 8 directories, / included. Moved under + // dir, they name one more for each slash in dir. + dirs := strconv.Itoa(8 + strings.Count(dir, "/")) + // How each line starts; the rate at the end of a phase varies. want := []string{ "trees: database " + path + " (from " + databaseEnv + ")", @@ -266,8 +275,9 @@ func TestRunTreesProgressNamesEachPhase(t *testing.T) { treesOpenLine, "[2/4] read: counting the records, elapsed 0s, remain unknown", "[2/4] read: [6/6] 100%, ", - "[3/4] group: [0/8] 0%, 0 directories/s, elapsed 0s, remain unknown", - "[3/4] group: [8/8] 100%, ", + "[3/4] group: [0/" + dirs + "] 0%, 0 directories/s, elapsed 0s, " + + "remain unknown", + "[3/4] group: [" + dirs + "/" + dirs + "] 100%, ", "[4/4] write: [0/1] 0%, 0 groups/s, elapsed 0s, remain unknown", "[4/4] write: [1/1] 100%, ", "trees: 6 records read, 1 duplicate tree groups, 1 dupe trees, ", @@ -296,7 +306,7 @@ func TestRunTreesProgressNamesEachPhase(t *testing.T) { func TestWriteTreeGroupsOnTerminal(t *testing.T) { stderr := captureStderr(t) super, dirs := treeOf(t, smokeTreeRecs()) - dupes := collectTreeGroups(dirs, super, nil) + dupes := collectTreeGroups(dirs, super, everyPathExists, nil) // No timed draws: the display is drawn as it starts and as it ends. p := startProgress(progressParams{ @@ -377,7 +387,7 @@ func TestCollectTreeGroupsMaximal(t *testing.T) { super, dirs := treeOf(t, smokeTreeRecs()) - groups := collectTreeGroups(dirs, super, nil) + groups := collectTreeGroups(dirs, super, everyPathExists, nil) want := [][]string{{"/d/t1", "/d/t2"}} if got := groupPaths(groups); !slices.EqualFunc(got, want, @@ -401,14 +411,16 @@ func TestCollectTreeGroupsDeterministic(t *testing.T) { super, dirs := treeOf(t, recs) - forward := groupPaths(collectTreeGroups(dirs, super, nil)) + forward := groupPaths(collectTreeGroups(dirs, super, everyPathExists, + nil)) reversed := slices.Clone(recs) slices.Reverse(reversed) superR, dirsR := treeOf(t, reversed) - backward := groupPaths(collectTreeGroups(dirsR, superR, nil)) + backward := groupPaths(collectTreeGroups(dirsR, superR, everyPathExists, + nil)) if !slices.EqualFunc(forward, backward, slices.Equal) { t.Fatalf("output depends on record order: %v vs %v", forward, backward) @@ -428,7 +440,7 @@ func TestCollectTreeGroupsSiblings(t *testing.T) { super, dirs := treeOf(t, recs) - got := groupPaths(collectTreeGroups(dirs, super, nil)) + got := groupPaths(collectTreeGroups(dirs, super, everyPathExists, nil)) want := [][]string{{"/p/x1", "/p/x2"}} if !slices.EqualFunc(got, want, slices.Equal) { @@ -457,7 +469,7 @@ func TestCollectTreeGroupsLeavesOutEmptyTrees(t *testing.T) { super, dirs := treeOf(t, recs) - got := groupPaths(collectTreeGroups(dirs, super, nil)) + got := groupPaths(collectTreeGroups(dirs, super, everyPathExists, nil)) want := [][]string{{"/p/x1", "/p/x2"}} if !slices.EqualFunc(got, want, slices.Equal) { @@ -480,10 +492,98 @@ func TestCollectTreeGroupsDifferingParents(t *testing.T) { super, dirs := treeOf(t, recs) - got := groupPaths(collectTreeGroups(dirs, super, nil)) + got := groupPaths(collectTreeGroups(dirs, super, everyPathExists, nil)) want := [][]string{{"/p/a/x", "/q/b/x"}} if !slices.EqualFunc(got, want, slices.Equal) { t.Fatalf("groups = %v, want %v", got, want) } } + +// TestCollectTreeGroupsChecksOnlyReportedCopies checks that trees asks +// whether a directory is on disk only for the members of the groups it +// returns. It never asks about /a/proj/sub, which lies inside a copy +// and whose group the group of /a/proj implies. +func TestCollectTreeGroupsChecksOnlyReportedCopies(t *testing.T) { + t.Parallel() + + f, g, u := testHash("f"), testHash("g"), testHash("u") + recs := []scanRec{ + {size: 10, head: f, tail: f, content: f, path: "/a/proj/f"}, + {size: 20, head: g, tail: g, content: g, path: "/a/proj/sub/g"}, + {size: 10, head: f, tail: f, content: f, path: "/b/proj/f"}, + {size: 20, head: g, tail: g, content: g, path: "/b/proj/sub/g"}, + // /a lacks this file, so /a and /b are no copies of each other. + {size: 30, head: u, tail: u, content: u, path: "/b/lonely"}, + } + + super, dirs := treeOf(t, recs) + + var checked []string + + exists := func(path string) bool { + checked = append(checked, path) + + return true + } + + got := groupPaths(collectTreeGroups(dirs, super, exists, nil)) + + want := [][]string{{"/a/proj", "/b/proj"}} + if !slices.EqualFunc(got, want, slices.Equal) { + t.Fatalf("groups = %v, want %v", got, want) + } + + slices.Sort(checked) + + if !slices.Equal(checked, want[0]) { + t.Errorf("directories checked on disk = %q, want %q", + checked, want[0]) + } +} + +// TestCollectTreeGroupsDropsGroupLeftWithOneCopy checks that a group of +// two copies, one of them gone, is not returned. If kept, it would +// write no row, but the summary line would count it. +func TestCollectTreeGroupsDropsGroupLeftWithOneCopy(t *testing.T) { + t.Parallel() + + f := testHash("f") + recs := []scanRec{ + {size: 10, head: f, tail: f, content: f, path: "/c/kept/f"}, + {size: 10, head: f, tail: f, content: f, path: "/c/gone/f"}, + } + + super, dirs := treeOf(t, recs) + + exists := func(path string) bool { return path != "/c/gone" } + + got := groupPaths(collectTreeGroups(dirs, super, exists, nil)) + if len(got) != 0 { + t.Errorf("groups = %v, want none", got) + } +} + +// TestCollectTreeGroupsKeepsGroupLeftWithTwoCopies checks that a group +// of three copies, one of them gone, is returned with the other two. +func TestCollectTreeGroupsKeepsGroupLeftWithTwoCopies(t *testing.T) { + t.Parallel() + + f := testHash("f") + recs := []scanRec{ + {size: 10, head: f, tail: f, content: f, path: "/d/kept1/f"}, + {size: 10, head: f, tail: f, content: f, path: "/d/gone/f"}, + {size: 10, head: f, tail: f, content: f, path: "/d/kept2/f"}, + } + + super, dirs := treeOf(t, recs) + + exists := func(path string) bool { return path != "/d/gone" } + + got := groupPaths(collectTreeGroups(dirs, super, exists, nil)) + + want := [][]string{{"/d/kept1", "/d/kept2"}} + if !slices.EqualFunc(got, want, slices.Equal) { + t.Errorf("groups = %v, want %v", got, want) + } +}