From 3864071c3fc39c873d23f5e7291371cd90d03221 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Fri, 16 Oct 2026 19:13:20 +0000 Subject: [PATCH] report: show its phases as N of M, with progress, elapsed time and ETA (closes #114) report now runs in two phases, open and read, under the progress display scan and trees use, until the summary line. open covers a wait on a busy database. read counts the records, then reads them in the same read-only transaction in signature order, the files_signature index read backwards, so SQLite sorts nothing and the read ends at exactly its count. The duplicate groups are formed in Go from runs of equal signatures; only those of the size being read are held, and they are written, ordered by first path, before the next size. The rows go through the display's stdout writer. stdout and the summary line are unchanged. Judgement call: writing is part of read, not a phase of its own, since each size's rows are written while the read goes on. Model: opus-5-5 --- README.md | 80 +++++++++------- TODO.md | 12 +++ db.go | 75 ++++++++------- db_test.go | 127 ++++++++++++++++++------- main_test.go | 23 +++-- progress.go | 4 + progress_test.go | 4 +- report.go | 234 +++++++++++++++++++++++++++++++++++++---------- report_test.go | 207 +++++++++++++++++++++++++++++++++++++---- scan.go | 2 +- trees.go | 2 +- 11 files changed, 590 insertions(+), 180 deletions(-) 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) -- 2.54.0