diff --git a/README.md b/README.md index 9f18537..ac3d4cc 100644 --- a/README.md +++ b/README.md @@ -199,12 +199,13 @@ Goals, in order: under resilver). Holding one small record (path, size, mtime) per file in memory during a scan is acceptable; holding every file's hashes is not (they stay in the database). The reporting commands do not hold every file's - hashes either: `report` lets SQLite group and order the records and writes - each row as it reads it, so its memory does not grow with the database, and - `trees` reads the records in path order and keeps each directory's path, - digest and totals, plus the hashes of only the files in the directories - holding the record being read, so its memory grows with the number of - directories and with the size of the largest directory. + hashes either: `report` reads the records in signature order and holds the + duplicate paths of one size at a time, so its memory grows with the + duplicates of the size that has the most of them, and `trees` reads the + records in path order and keeps each directory's path, digest and totals, + plus the hashes of only the files in the directories holding the record + being read, so its memory grows with the number of directories and with the + size of the largest directory. 3. **Scan incrementally, analyze offline.** The expensive filesystem scan maintains a persistent database; an unchanged file is never read again on a rescan, except to compute its content hash once a file of 10 MiB or more @@ -323,9 +324,9 @@ All three subcommands operate on a single SQLite database file: needs the database to itself: a `scan` that starts while a report is still reading waits for it up to the 10-second busy timeout, then fails; a `scan` that ends while a report has the database open warns and leaves the database - in WAL mode until the next scan. `report` writes each row as it reads it, so - it is still reading while its output is paused (a pager, a stalled pipe), and - a `scan` started then fails after the busy timeout. + in WAL mode until the next scan. `report` writes its rows while it reads the + records, so it is still reading while its output is paused (a pager, a stalled + pipe), and a `scan` started then fails after the busy timeout. - A `scan` writes to the database and the files beside it at these points (its phases are described under "`scan` mode"): - As it starts, before the walk: the directory, an empty database file and @@ -379,8 +380,8 @@ All three subcommands operate on a single SQLite database file: a file of 10 MiB or more, `content` stays empty until the content phase of a scan (see "`scan` mode" below) has read the file. A record with an empty `content` is never part of a duplicate group, though it still defines the - file for tree reconstruction. The `files_signature` index lets SQLite group - the records by signature for `report` without sorting the whole table. + file for tree reconstruction. The `files_signature` index lets `report` read + the records in signature order without sorting the table. ### Duplicate detection @@ -604,11 +605,8 @@ total.) **`report` must never touch the filesystem being analyzed.** It does not stat, open, or otherwise access any path that appears in the records; its only I/O is -reading the database, writing stdout/stderr, and the temporary file SQLite sorts -in when the duplicate rows do not fit in memory. SQLite puts that file in -`$SQLITE_TMPDIR` or `$TMPDIR` when set, otherwise in `/var/tmp` (or `/tmp`), and -deletes it as soon as it has opened it. `report` must produce identical output -whether or not the scanned filesystem is still mounted. +reading the database and writing stdout/stderr. `report` must produce identical +output whether or not the scanned filesystem is still mounted. Processing: @@ -622,6 +620,16 @@ Processing: by `first` path ascending. Output must be fully deterministic for a given database state. +`report` runs in two phases, each with its own progress display on stderr (see +"Progress"): `open` opens the database; `read` counts the records, then reads +them in signature order, largest size first. That is the order of the +`files_signature` index read backwards, so SQLite sorts nothing and returns the +first record at once. The records sharing a signature come together, and so do +the records of one size, so `report` holds only the duplicate groups of the size +it is reading, and writes them, ordered by `first` path, once it reaches a +smaller size. Its memory holds at most the duplicate paths of one size at a +time, never every record. + #### Report output format TSV on stdout: a header line, then one row per duplicate file (N-1 rows for a @@ -721,13 +729,13 @@ units. ### Progress -Use the progress-bar library for all progress of `scan` and `trees`; rendering -in the style of `pv` is the model. All progress goes to stderr. +Use the progress-bar library for all progress of `scan`, `report` and `trees`; +rendering in the style of `pv` is the model. All progress goes to stderr. -`scan` and `trees` must always show that they are working and not hung. Each -phase gets its own display from the moment it starts, naming the phase and its -place among the command's phases and showing its progress, its elapsed time and -its estimated time remaining (ETA). +`scan`, `report` and `trees` must always show that they are working and not +hung. Each phase gets its own display from the moment it starts, naming the +phase and its place among the command's phases and showing its progress, its +elapsed time and its estimated time remaining (ETA). A scan has five phases (`[1/5] load`, `[2/5] walk`, `[3/5] hash`, `[4/5] update`, `[5/5] content`): @@ -757,6 +765,14 @@ A scan has five phases (`[1/5] load`, `[2/5] walk`, `[3/5] hash`, - `group` has the directories `read` found as its total, and `write` the duplicate-tree groups. +`report` has two phases (`[1/2] open`, `[2/2] read`, see "`report` mode"): + +- `open` has no total. While it opens the database, waiting on a busy one + included, the display says it is opening the database. +- `read` counts the records first, then reads them against that count, writing + the rows to stdout as it goes. While the count runs, the display says it is + counting the records. + Each display is one line holding: - the phase and its place among the phases @@ -777,12 +793,12 @@ Example shape (exact layout is flexible, content is not): Additional requirements: - On a terminal, a phase with an exact total (every phase but `scan`'s `walk` - and `trees`' `open`, once it has counted what it will do) is drawn as its line - followed by the progress-bar library's bar, 10 columns wide, which shows the - percent in place of the line's own. Any other phase, and a phase whose bar has - no room, is drawn as the library's spinner followed by its line. Each display - is drawn again several times a second, so its elapsed time keeps moving while - a phase waits for its next item. + and the `open` phase of `report` and `trees`, once it has counted what it will + do) is drawn as its line followed by the progress-bar library's bar, 10 + columns wide, which shows the percent in place of the line's own. Any other + phase, and a phase whose bar has no room, is drawn as the library's spinner + followed by its line. Each display is drawn again several times a second, so + its elapsed time keeps moving while a phase waits for its next item. - On a terminal no line reaches the terminal's last column, however narrow the terminal is. The phase and its place, the progress, the elapsed time and the ETA are kept whole: a line too wide for the terminal leaves out its rate first @@ -803,12 +819,12 @@ Additional requirements: - A warning printed during a phase always lands on a line of its own, never inside the progress display. So does every line written to stdout when stdout and stderr are the same file (the same device and inode): one terminal, as in - a plain `sfdupes trees`, or one pipe or file that both are sent to. When they - are not, stdout is written without waiting on the display, and the display - keeps being drawn while a slow reader of stdout holds up a write. + a plain `sfdupes report` or `sfdupes trees`, or one pipe or file that both are + sent to. When they are not, stdout is written without waiting on the display, + and the display keeps being drawn while a slow reader of stdout holds up a + write. - A phase that stops short of its total, as an interrupted one does, is left showing the count it stopped at rather than filled up. -- `report` mode needs no progress display, only its stderr summary. ### Error handling and exit codes diff --git a/TODO.md b/TODO.md index 0b3ee31..1690a99 100644 --- a/TODO.md +++ b/TODO.md @@ -29,6 +29,18 @@ # Completed Steps +- `report` shows its work on stderr in two phases, `[1/2] open` and + `[2/2] read`, with the progress display `scan` and `trees` use, from the + moment it starts until the summary line: `open` says it is opening the + database, also while it waits on a busy one; `read` counts the records, then + reads them against that count in signature order, which SQLite takes from the + `files_signature` index without sorting, so the first record comes at once. + `report` forms the duplicate groups itself, holding only those of the size it + is reading, and writes them ordered by first path before it reads the next + size. stdout and the summary line are unchanged, and SQLite no longer needs a + temporary file to sort in (2026-10-16, + https://git.eeqj.de/sneak/sfdupes/issues/114) + - `trees` shows its work on stderr in four phases, `[1/4] open`, `[2/4] read`, `[3/4] group` and `[4/4] write`, each with the progress display `scan` uses, from the moment it starts until the summary line: `open` says it is opening diff --git a/db.go b/db.go index fc23c6a..40cad40 100644 --- a/db.go +++ b/db.go @@ -61,8 +61,8 @@ CREATE TABLE files ( ) WITHOUT ROWID ` -// createIndexSQL indexes the records by signature, so report can have -// SQLite group them without sorting the whole table. +// createIndexSQL indexes the records by signature, so report can read +// them in signature order without sorting the table. const createIndexSQL = ` CREATE INDEX files_signature ON files (size, head, tail, content) ` @@ -463,77 +463,74 @@ func loadFileRows(ctx context.Context, db *sql.DB, total func(n int64), return nil } -// dupeRowsSQL selects every record in a duplicate group, with the -// group's first path. A group is the records with a content hash that -// share a size, head, tail, and content, when there are two or more of -// them. The rows come in report order: groups by size descending, then -// by first path, and each group's paths ascending. -const dupeRowsSQL = ` -SELECT g.first, f.path, f.size -FROM files AS f -JOIN ( - SELECT size, head, tail, content, MIN(path) AS first - FROM files - WHERE content <> '' - GROUP BY size, head, tail, content - HAVING COUNT(*) > 1 -) AS g USING (size, head, tail, content) -ORDER BY f.size DESC, g.first, f.path +// signatureRowsSQL selects every record's signature and path in +// signature order, largest size first. That is the order of the +// files_signature index read backwards, so SQLite reads the index alone +// and returns the first row at once, with nothing to sort. +const signatureRowsSQL = ` +SELECT size, head, tail, content, path +FROM files +ORDER BY size DESC, head DESC, tail DESC, content DESC ` -// loadDupeRows streams the rows of dupeRowsSQL to fn and returns the -// number of records in the database. The count and the rows are read -// in one transaction, so they agree while a scan is committing. An -// error from fn stops the reading and is returned as it is. -func loadDupeRows(ctx context.Context, db *sql.DB, - fn func(first, path string, size int64) error, -) (int, error) { +// loadSignatureRows gives total the number of records, then streams +// every record's signature and path to fn, without its mtime, in the +// order of signatureRowsSQL. The count and the records are read in one +// transaction, so they agree while a scan is committing. An error from +// fn stops the reading and is returned as it is. +func loadSignatureRows(ctx context.Context, db *sql.DB, + total func(n int64), fn func(r scanRec) error, +) error { // Everything goes through tx: the report connection is the only // one, so a query on db would wait for tx forever. tx, err := db.BeginTx(ctx, &sql.TxOptions{ReadOnly: true}) if err != nil { - return 0, fmt.Errorf("read records: %w", err) + return fmt.Errorf("read records: %w", err) } defer func() { _ = tx.Rollback() }() - var records int + var n int64 - err = tx.QueryRowContext(ctx, "SELECT COUNT(*) FROM files").Scan(&records) + err = tx.QueryRowContext(ctx, "SELECT COUNT(*) FROM files").Scan(&n) if err != nil { - return 0, fmt.Errorf("read records: %w", err) + return fmt.Errorf("count records: %w", err) } - rows, err := tx.QueryContext(ctx, dupeRowsSQL) + total(n) + + rows, err := tx.QueryContext(ctx, signatureRowsSQL) if err != nil { - return 0, fmt.Errorf("read records: %w", err) + return fmt.Errorf("read records: %w", err) } defer func() { _ = rows.Close() }() for rows.Next() { var ( - first, path []byte - size int64 + path []byte + r scanRec ) - err = rows.Scan(&first, &path, &size) + err = rows.Scan(&r.size, &r.head, &r.tail, &r.content, &path) if err != nil { - return 0, fmt.Errorf("read record: %w", err) + return fmt.Errorf("read record: %w", err) } - err = fn(string(first), string(path), size) + r.path = string(path) + + err = fn(r) if err != nil { - return 0, err + return err } } err = rows.Err() if err != nil { - return 0, fmt.Errorf("read records: %w", err) + return fmt.Errorf("read records: %w", err) } - return records, nil + return nil } // countRecords returns the number of records in the database. diff --git a/db_test.go b/db_test.go index fab2369..7346372 100644 --- a/db_test.go +++ b/db_test.go @@ -291,53 +291,118 @@ func TestCloseScanDatabaseWhileReportOpen(t *testing.T) { } } -// TestLoadFileRowsCountAgreesDuringScan has a scan commit records after -// loadFileRows has counted the records and before it reads them, as -// when trees runs during a scan: it must read exactly as many records -// as it counted. -func TestLoadFileRowsCountAgreesDuringScan(t *testing.T) { +// TestLoadRowsCountAgreeDuringScan has a scan commit records after +// loadFileRows, as trees calls it, or loadSignatureRows, as report +// calls it, has counted the records and before it reads them, as when +// trees or report runs during a scan: each must read exactly as many +// records as it counted. +func TestLoadRowsCountAgreeDuringScan(t *testing.T) { t.Parallel() - path := testDBPath(t) + loads := map[string]func(context.Context, *sql.DB, func(int64), + func(scanRec)) error{ + "loadFileRows": loadFileRows, + "loadSignatureRows": func(ctx context.Context, db *sql.DB, + total func(int64), fn func(scanRec), + ) error { + return loadSignatureRows(ctx, db, total, func(r scanRec) error { + fn(r) - scanDB, err := openScanDatabase(t.Context(), path) + return nil + }) + }, + } + + for name, load := range loads { + t.Run(name, func(t *testing.T) { + t.Parallel() + + path := testDBPath(t) + + scanDB, err := openScanDatabase(t.Context(), path) + if err != nil { + t.Fatal(err) + } + + defer func() { _ = scanDB.Close() }() + + recs := smokeTreeRecs() + + err = applyChanges(t.Context(), scanDB, recs[:3], nil, nil) + if err != nil { + t.Fatal(err) + } + + reportDB, err := openReportDatabase(t.Context(), path) + if err != nil { + t.Fatal(err) + } + + defer func() { _ = reportDB.Close() }() + + var counted, read int64 + + err = load(t.Context(), reportDB, func(n int64) { + counted = n + + err := applyChanges(t.Context(), scanDB, recs[3:], nil, nil) + if err != nil { + t.Error(err) + } + }, func(scanRec) { read++ }) + if err != nil { + t.Fatal(err) + } + + if counted != 3 || read != counted { + t.Errorf("counted %d records and read %d, want 3 and 3", + counted, read) + } + }) + } +} + +// TestSignatureRowsNeedNoSort checks that SQLite reads signatureRowsSQL +// from the files_signature index alone, with nothing to sort: the +// first record comes at once, so report's read phase counts each +// record as it comes rather than waiting on SQLite for all of them. +func TestSignatureRowsNeedNoSort(t *testing.T) { + t.Parallel() + + db := openTestDB(t) + + rows, err := db.QueryContext(t.Context(), + "EXPLAIN QUERY PLAN "+signatureRowsSQL) if err != nil { t.Fatal(err) } - defer func() { _ = scanDB.Close() }() + defer func() { _ = rows.Close() }() - recs := smokeTreeRecs() + var plan []string - err = applyChanges(t.Context(), scanDB, recs[:3], nil, nil) - if err != nil { - t.Fatal(err) - } + for rows.Next() { + var ( + id, parent, unused int + detail string + ) - reportDB, err := openReportDatabase(t.Context(), path) - if err != nil { - t.Fatal(err) - } - - defer func() { _ = reportDB.Close() }() - - var counted, read int64 - - err = loadFileRows(t.Context(), reportDB, func(n int64) { - counted = n - - err := applyChanges(t.Context(), scanDB, recs[3:], nil, nil) + err = rows.Scan(&id, &parent, &unused, &detail) if err != nil { - t.Error(err) + t.Fatal(err) } - }, func(scanRec) { read++ }) + + plan = append(plan, detail) + } + + err = rows.Err() if err != nil { t.Fatal(err) } - if counted != 3 || read != counted { - t.Errorf("counted %d records and read %d, want 3 and 3", - counted, read) + want := []string{"SCAN files USING COVERING INDEX files_signature"} + if !slices.Equal(plan, want) { + t.Errorf("query plan = %q, want %q", plan, want) } } diff --git a/main_test.go b/main_test.go index a2c09a4..9e4db36 100644 --- a/main_test.go +++ b/main_test.go @@ -303,9 +303,9 @@ func TestRunMissingDatabaseIsFatal(t *testing.T) { t.Errorf("run(%s) = %d, want %d", name, code, exitFatal) } - // trees shows its open phase, as it starts and as it ends, + // Each shows its open phase, as it starts and as it ends, // before the error. - opening := "" + opening := reportOpenLine + "\n" + reportOpenLine + "\n" if name == cmdTrees { opening = treesOpenLine + "\n" + treesOpenLine + "\n" } @@ -513,7 +513,8 @@ func TestRunNamesDefaultDatabase(t *testing.T) { // nothing here may create or read one: scan gets an operand that // does not exist, which stops it before it takes the lock, and // report and trees a cancelled context, which stops them before - // they read anything. trees shows its open phase after the line. + // they read anything. report and trees show their open phase after + // the line. t.Setenv(databaseEnv, "") // Whether scan calls the database new depends on that machine too. @@ -541,7 +542,8 @@ func TestRunNamesDefaultDatabase(t *testing.T) { run: func() error { return runReport(cancelledContext(t), io.Discard) }, - want: "report: database " + defaultDatabasePath + " (default)\n", + want: "report: database " + defaultDatabasePath + " (default)\n" + + reportOpenLine + "\n" + reportOpenLine + "\n", }, cmdTrees: { run: func() error { @@ -916,11 +918,16 @@ func TestRunReportSucceeds(t *testing.T) { t.Errorf("stdout = %q, want %q", got, want) } - want = "report: database " + path + " (from " + databaseEnv + ")\n" + - "report: 2 records read, 1 duplicate groups, 1 dupe files, " + + // Its phases come between the two lines; see + // TestRunReportProgressNamesEachPhase. + first := "report: database " + path + " (from " + databaseEnv + ")\n" + last := "\nreport: 2 records read, 1 duplicate groups, 1 dupe files, " + "300 B reclaimable\n" - if got := stderr(); got != want { - t.Errorf("stderr = %q, want %q", got, want) + + if got := stderr(); !strings.HasPrefix(got, first) || + !strings.HasSuffix(got, last) { + t.Errorf("stderr = %q, want it to start %q and end %q", + got, first, last) } assertNoSidecars(t, path) diff --git a/progress.go b/progress.go index b347aa6..7b0f236 100644 --- a/progress.go +++ b/progress.go @@ -47,6 +47,10 @@ const spinnerMargin = 5 // around them, and the terminal's last column. const barMargin = barWidth + 11 +// countingRecords is what the display of a phase that reads the records +// says while it counts them, before it has its total. +const countingRecords = "counting the records" + // stderrIsTTY reports whether stderr is attached to a terminal. func stderrIsTTY() bool { return term.IsTerminal(int(os.Stderr.Fd())) diff --git a/progress_test.go b/progress_test.go index 1f6f58d..3cda760 100644 --- a/progress_test.go +++ b/progress_test.go @@ -84,7 +84,7 @@ func TestProgressLine(t *testing.T) { }{{ p: &progress{ label: scanPhase("load"), total: -1, - waitingFor: "counting the records", + waitingFor: countingRecords, }, want: "[1/5] load: counting the records, elapsed 10s, eta unknown", }, { @@ -271,7 +271,7 @@ func TestPlainProgressPrintsWhileWaiting(t *testing.T) { stderr := captureStderr(t) p := startProgress(progressParams{ label: scanPhase("load"), total: -1, - waitingFor: "counting the records", + waitingFor: countingRecords, }, nil, testRedraw) awaitOutput(t, stderr, diff --git a/report.go b/report.go index fba801c..2555c2a 100644 --- a/report.go +++ b/report.go @@ -1,11 +1,12 @@ package main import ( - "bufio" "context" + "database/sql" "fmt" "io" "os" + "slices" "strings" "time" ) @@ -29,13 +30,25 @@ type scanRec struct { path string } +// reportPhase names one phase of report with its place among all of +// them, as "[2/2] read", for the phase's progress display. The list +// here is the one place the phases are counted. +func reportPhase(name string) string { + return phaseLabel([]string{"open", "read"}, name) +} + // runReport implements the report subcommand: it prints the file-level -// duplicates report as TSV on stdout. SQLite groups and orders the -// records, and each row is written as it is read, so no group is held -// in memory. It never touches the scanned filesystem; its only I/O is -// the database (with SQLite's temporary sort file), stdout, and stderr. -// It names the database on stderr before it opens it. Any database +// 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. +// +// The work runs in two phases, each under a progress display of its +// own on stderr: open (open the database) and read (count the records, +// then read them in signature order, writing each size's duplicate +// groups to stdout once that size is read; see readReport). Each +// display ends before the next phase starts, and before an error is +// returned to be printed. func runReport(ctx context.Context, stdout io.Writer) error { dbPath, source, err := databasePath() if err != nil { @@ -45,65 +58,194 @@ func runReport(ctx context.Context, stdout io.Writer) error { fmt.Fprintf(os.Stderr, "report: database %s (%s)\n", escapePath(dbPath), source) + // Opening can wait on a busy database for as long as the busy + // timeout. + prog := newProgress(progressParams{ + label: reportPhase("open"), total: -1, + waitingFor: "opening the database", + }) db, err := openReportDatabase(ctx, dbPath) + + prog.finish() + if err != nil { return err } defer func() { _ = db.Close() }() - out := bufio.NewWriterSize(stdout, ioBufSize) + prog = newProgress(progressParams{ + label: reportPhase("read"), unit: "records", total: -1, + waitingFor: countingRecords, + }) + report, err := readReport(ctx, db, dbPath, stdout, prog) - _, err = fmt.Fprintln(out, "first\tdupe\tsize") - if err != nil { - return fmt.Errorf("write stdout: %w", err) - } - - var ( - groups, dupeFiles int - reclaimable int64 - writeErr error - ) - - records, err := loadDupeRows(ctx, db, - func(first, path string, size int64) error { - // A group's first path is its first row; every other - // path is a dupe. - if path == first { - groups++ - - return nil - } - - _, writeErr = fmt.Fprintf(out, "%s\t%s\t%d\n", - escapePath(first), escapePath(path), size) - dupeFiles++ - reclaimable += size - - return writeErr - }) - - if writeErr != nil { - return fmt.Errorf("write stdout: %w", writeErr) - } + prog.finish() if err != nil { - return fmt.Errorf("database %s: %w", dbPath, err) - } - - err = out.Flush() - if err != nil { - return fmt.Errorf("write stdout: %w", err) + return err } fmt.Fprintf(os.Stderr, "report: %d records read, %d duplicate groups, %d dupe files, "+ "%s reclaimable\n", - records, groups, dupeFiles, humanBytes(reclaimable)) + report.records, report.dupeGroups, report.dupeFiles, + humanBytes(report.reclaimable)) return nil } +// readReport gives prog the number of records in db, the database at +// 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. +func readReport(ctx context.Context, db *sql.DB, dbPath string, + stdout io.Writer, prog *progress, +) (*reportWriter, error) { + out := prog.writer(stdout) + report := &reportWriter{out: out} + + _, err := fmt.Fprintln(out, "first\tdupe\tsize") + if err != nil { + return nil, fmt.Errorf("write stdout: %w", err) + } + + var writeErr error + + err = loadSignatureRows(ctx, db, prog.setTotal, func(r scanRec) error { + prog.increment() + + writeErr = report.add(r) + + return writeErr + }) + + if writeErr != nil { + return nil, fmt.Errorf("write stdout: %w", writeErr) + } + + if err != nil { + return nil, fmt.Errorf("database %s: %w", dbPath, err) + } + + err = report.finish() + if err == nil { + err = out.Flush() + } + + if err != nil { + return nil, fmt.Errorf("write stdout: %w", err) + } + + return report, nil +} + +// reportWriter writes the report's rows to out from the records, added +// in signature order, largest size first, as loadSignatureRows reads +// them. The records sharing a signature come together, and so do the +// records of one size, so it holds only the duplicate groups of the +// size being read, and writes them, ordered by first path, once a +// record of a smaller size comes. It counts what the summary line +// reports. +type reportWriter struct { + out io.Writer + + 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 + + records, dupeGroups, dupeFiles int + reclaimable int64 +} + +// add takes the next record. One with the signature being read joins +// its paths; any other ends them, and one of a smaller size first +// writes the groups of the size before. +func (w *reportWriter) add(r scanRec) error { + w.records++ + + // A record without a content hash has unknown content (README + // "Database"), so it is never a duplicate. + if r.content == "" { + return nil + } + + if r.size == w.sig.size && r.head == w.sig.head && + r.tail == w.sig.tail && r.content == w.sig.content { + w.paths = append(w.paths, r.path) + + return nil + } + + w.endGroup() + + if r.size != w.sig.size { + err := w.writeGroups() + if err != nil { + return err + } + } + + w.sig = r + w.paths = append(w.paths, r.path) + + 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. +func (w *reportWriter) endGroup() { + if len(w.paths) < minGroupSize { + // A path with no duplicate is dropped, and its slice reused. + w.paths = w.paths[:0] + + return + } + + slices.Sort(w.paths) + w.groups = append(w.groups, w.paths) + w.paths = nil +} + +// writeGroups writes the duplicate groups held, all of size w.sig.size, +// ordered by first path: for each group, a row for every path but the +// first. No two groups share a first path, since no two records share +// a path. +func (w *reportWriter) writeGroups() error { + slices.SortFunc(w.groups, func(a, b []string) int { + return strings.Compare(a[0], b[0]) + }) + + for _, g := range w.groups { + for _, path := range g[1:] { + _, err := fmt.Fprintf(w.out, "%s\t%s\t%d\n", + escapePath(g[0]), escapePath(path), w.sig.size) + if err != nil { + return err + } + + w.dupeFiles++ + w.reclaimable += w.sig.size + } + + w.dupeGroups++ + } + + w.groups = nil + + return nil +} + +// finish writes the groups of the last size read. +func (w *reportWriter) finish() error { + w.endGroup() + + return w.writeGroups() +} + // 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/report_test.go b/report_test.go index 82f8b03..994564a 100644 --- a/report_test.go +++ b/report_test.go @@ -9,6 +9,7 @@ import ( "os" "path/filepath" "slices" + "strconv" "strings" "testing" "time" @@ -60,27 +61,41 @@ type dupeGroup struct { } // dupeGroups returns the duplicate groups report reads from db, in -// report order. +// report order, from the rows it writes: a group's rows come together, +// each naming the group's first path. func dupeGroups(t *testing.T, db *sql.DB) []dupeGroup { t.Helper() - var groups []dupeGroup + var out bytes.Buffer - _, err := loadDupeRows(t.Context(), db, - func(first, path string, size int64) error { - if path == first { - groups = append(groups, dupeGroup{size: size}) - } - - g := &groups[len(groups)-1] - g.paths = append(g.paths, path) - - return nil - }) + _, err := readReport(t.Context(), db, "", &out, nil) if err != nil { t.Fatal(err) } + var groups []dupeGroup + + rows := strings.Split(strings.TrimSuffix(out.String(), "\n"), "\n") + for _, row := range rows[1:] { // after the header + fields := strings.Split(row, "\t") + if len(fields) != 3 { + t.Fatalf("row %q, want first, dupe and size", row) + } + + size, err := strconv.ParseInt(fields[2], 10, 64) + if err != nil { + t.Fatal(err) + } + + if len(groups) == 0 || groups[len(groups)-1].paths[0] != fields[0] { + groups = append(groups, + dupeGroup{size: size, paths: []string{fields[0]}}) + } + + g := &groups[len(groups)-1] + g.paths = append(g.paths, fields[1]) + } + return groups } @@ -118,26 +133,147 @@ func TestRunReportEscapesPaths(t *testing.T) { } func TestReportStdoutFailsWhileReading(t *testing.T) { - // Each row holds two paths longer than dir, so the report is more - // than twice the stdout buffer and stdout fails while rows are - // still being read, not at the final flush. + t.Parallel() + + // Every two records are a duplicate pair of a size of their own, and + // a pair's row is written once a smaller size is read. Each row + // holds two paths longer than dir, so the rows come to more than + // twice the stdout buffer, and stdout fails while records are still + // being read, not once every record is. dir := "/" + strings.Repeat("d", 4096) - recs := make([]scanRec, ioBufSize/len(dir)) + recs := make([]scanRec, 2*ioBufSize/len(dir)) for i := range recs { recs[i] = scanRec{ - size: 1, head: "h", tail: "t", content: "c", + size: int64(i/2) + 1, head: "h", tail: "t", content: "c", path: fmt.Sprintf("%s/%d", dir, i), } } - t.Setenv(databaseEnv, seedDatabase(t, recs)) + db := openTestDB(t) - err := runReport(t.Context(), failingWriter{}) + err := applyChanges(t.Context(), db, recs, nil, nil) + if err != nil { + t.Fatal(err) + } + + // Counts the records read, and draws nothing. + prog := &progress{} + + _, err = readReport(t.Context(), db, "", failingWriter{}, prog) if !errors.Is(err, errWriteFailed) || !strings.HasPrefix(err.Error(), "write stdout: ") { t.Errorf("error = %v, want write stdout: %v", err, errWriteFailed) } + + if prog.count >= int64(len(recs)) { + t.Errorf("stdout failed after all %d records were read, want it "+ + "to stop the read", prog.count) + } +} + +// reportOpenLine is the plain line of report's open phase when it opens +// the database at once; it prints it as the phase starts and as it ends. +const reportOpenLine = "[1/2] open: opening the database, elapsed 0s, " + + "eta unknown" + +// smokeReport is report's stdout for the smoke-test records. +const smokeReport = "first\tdupe\tsize\n" + + "/d/t1/f1\t/d/t2/f1\t3000\n" + + "/d/t1/f1\t/d/t3/f1\t3000\n" + + "/d/t1/sub/f2\t/d/t2/sub/f2\t100\n" + + "/d/t1/sub/f2\t/d/t3/sub/f2renamed\t100\n" + +// TestRunReportProgressNamesEachPhase runs report on the smoke-test +// records with stderr not a terminal, and checks every line it prints +// there. After the line naming the database, each phase prints a line +// naming it with its place among the two as it starts and another as +// 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()) + t.Setenv(databaseEnv, path) + + stderr := captureStderr(t) + + var stdout bytes.Buffer + + code := run([]string{cmdReport}, &stdout, os.Stderr) + if code != exitOK { + t.Fatalf("run(report) = %d, want %d; stderr: %s", + code, exitOK, stderr()) + } + + if got := stdout.String(); got != smokeReport { + t.Errorf("stdout = %q, want %q", got, smokeReport) + } + + // How each line starts; the rate at the end of a phase varies. + want := []string{ + "report: database " + path + " (from " + databaseEnv + ")", + reportOpenLine, + reportOpenLine, + "[2/2] read: counting the records, elapsed 0s, eta unknown", + "[2/2] read: [6/6] 100%, ", + "report: 6 records read, 2 duplicate groups, 4 dupe files, " + + "6.1 KiB reclaimable", + } + + lines := strings.Split(strings.TrimSuffix(stderr(), "\n"), "\n") + if len(lines) != len(want) { + t.Fatalf("stderr lines = %q, want %d lines starting %q", + lines, len(want), want) + } + + for i := range want { + if !strings.HasPrefix(lines[i], want[i]) { + t.Errorf("stderr line %d = %q, want it to start %q", + i+1, lines[i], want[i]) + } + } +} + +// TestReadReportOnTerminal writes the smoke-test report to the terminal +// its read phase's display is drawn on, as a plain "sfdupes report" +// does: the terminal must show the header and every row whole, each on +// a line of its own. +// +//nolint:paralleltest // captureStderr replaces the process-wide os.Stderr +func TestReadReportOnTerminal(t *testing.T) { + stderr := captureStderr(t) + db := openTestDB(t) + + err := applyChanges(t.Context(), db, smokeTreeRecs(), nil, nil) + if err != nil { + t.Fatal(err) + } + + // No timed draws: the display is drawn as it starts and as it ends. + p := startProgress(progressParams{ + label: reportPhase("read"), unit: "records", total: -1, + waitingFor: countingRecords, + }, terminalWidth, time.Hour) + + _, err = readReport(t.Context(), db, "", os.Stderr, p) + + p.finish() + + if err != nil { + t.Fatal(err) + } + + // A terminal shows each line as the text after its last carriage + // return. + var shown []string + + for line := range strings.SplitSeq(stderr(), "\n") { + shown = append(shown, line[strings.LastIndex(line, "\r")+1:]) + } + + want := strings.Split(strings.TrimSuffix(smokeReport, "\n"), "\n") + if len(shown) < len(want) || !slices.Equal(shown[:len(want)], want) { + t.Errorf("terminal shows lines %q, want %q first", shown, want) + } } func TestRunReportsIgnoreInsertionOrder(t *testing.T) { @@ -354,6 +490,37 @@ func TestDupeGroupsTieBreak(t *testing.T) { } } +func TestDupeGroupsOrderedByFirstPath(t *testing.T) { + t.Parallel() + + // report reads the groups of size 50 in signature order, here /a, + // /z, /k, which is neither the order of their first paths nor its + // reverse, and must write them ordered by first path. The group of + // size 7 comes after them all, though its first path sorts first. + recs := []scanRec{ + {size: 50, head: "a", tail: "a", content: "a", path: "/k/2"}, + {size: 50, head: "a", tail: "a", content: "a", path: "/k/1"}, + {size: 50, head: "b", tail: "b", content: "b", path: "/z/1"}, + {size: 50, head: "b", tail: "b", content: "b", path: "/z/2"}, + {size: 50, head: "c", tail: "c", content: "c", path: "/a/2"}, + {size: 50, head: "c", tail: "c", content: "c", path: "/a/1"}, + {size: 7, head: "d", tail: "d", content: "d", path: "/0/2"}, + {size: 7, head: "d", tail: "d", content: "d", path: "/0/1"}, + } + + groups := dupeGroupsOf(t, recs) + + got := make([]string, 0, len(groups)) + for _, g := range groups { + got = append(got, g.paths[0]) + } + + want := []string{"/a/1", "/k/1", "/z/1", "/0/1"} + if !slices.Equal(got, want) { + t.Errorf("groups' first paths = %q, want %q", got, want) + } +} + func TestDupeGroupsDeterministic(t *testing.T) { t.Parallel() diff --git a/scan.go b/scan.go index 101bd45..56674b0 100644 --- a/scan.go +++ b/scan.go @@ -446,7 +446,7 @@ func (s *scanState) loadIndex(ctx context.Context, roots []string) error { // first gives the display its total. prog := newProgress(progressParams{ label: scanPhase("load"), unit: scanUnit, total: -1, - waitingFor: "counting the records", + waitingFor: countingRecords, }) defer prog.finish() diff --git a/trees.go b/trees.go index 1ff6b76..0944b87 100644 --- a/trees.go +++ b/trees.go @@ -72,7 +72,7 @@ func runTrees(ctx context.Context, stdout io.Writer) error { prog = newProgress(progressParams{ label: treesPhase("read"), unit: "records", total: -1, - waitingFor: "counting the records", + waitingFor: countingRecords, }) records, super, allDirs, err := readTree(ctx, db, prog)