diff --git a/README.md b/README.md index d48f695..527b3fd 100644 --- a/README.md +++ b/README.md @@ -573,6 +573,18 @@ Additional requirements: - `2`: usage error (including `scan` with no `PATH` operand and `report`/`trees` with any positional argument). +A stdout write failure, such as a full disk, is reported in one line on +stderr and exits 1. Two cases never reach sfdupes as a failed write: + +- When the reader of a stdout pipe exits early, as in + `sfdupes report | head`, the next write ends sfdupes with `SIGPIPE`, + quietly and without a summary, the way `cat` or `sort` end. The + shell reports the signal (status 141 in most shells), not exit 1. +- When stdout is closed outright (`sfdupes report >&-`), the Go + runtime opens `/dev/null` in its place before sfdupes starts, so + the output is discarded and the run succeeds, as with + `> /dev/null`. + ## Entrypoints This repository adheres to the diff --git a/TODO.md b/TODO.md index c6de6e9..5928fe5 100644 --- a/TODO.md +++ b/TODO.md @@ -29,6 +29,10 @@ # Completed Steps +- test stdout write failures in `report` and `trees`; README states that + `| head` ends sfdupes by `SIGPIPE` and `>&-` writes to `/dev/null` + (2026-10-03, https://git.eeqj.de/sneak/sfdupes/issues/30) + - `report` and `trees` open the database read-only, and `scan` leaves it out of WAL mode, so reading needs only read access (2026-10-03, closes https://git.eeqj.de/sneak/sfdupes/issues/8) diff --git a/main.go b/main.go index d2160c1..3a8ce57 100644 --- a/main.go +++ b/main.go @@ -57,22 +57,27 @@ var errNoSubcommand = errors.New("no subcommand") var Version = "dev" func main() { - os.Exit(run(os.Args[1:], os.Stderr)) + // Once the reader of a stdout pipe has gone, as in "sfdupes report | + // head", the Go runtime ends the process with SIGPIPE on the next + // write instead of returning an error (README "Error handling"). + // Registering for SIGPIPE with os/signal would change that. + os.Exit(run(os.Args[1:], os.Stdout, os.Stderr)) } // run executes args against the command tree and returns the process // exit code. It is the program's single exit point: the subcommands // return their errors instead of exiting, so every deferred cleanup — // above all closing the database, which checkpoints the SQLite WAL — -// runs before the process ends. -func run(args []string, stderr io.Writer) int { +// runs before the process ends. The report and trees subcommands write +// their data to stdout. +func run(args []string, stdout, stderr io.Writer) int { // A nil slice makes cobra fall back to os.Args, which would let a // test binary's own flags reach the command tree. if args == nil { args = []string{} } - root := newRootCommand(stderr) + root := newRootCommand(stdout, stderr) root.SetArgs(args) err := root.Execute() @@ -98,7 +103,7 @@ func run(args []string, stderr io.Writer) int { // newRootCommand builds the command tree. Everything on stdout is // machine-readable data; all human-facing output (help, usage, errors) // goes to stderr. -func newRootCommand(stderr io.Writer) *cobra.Command { +func newRootCommand(stdout, stderr io.Writer) *cobra.Command { root := &cobra.Command{ Use: "sfdupes", Short: "Find candidate duplicate files by size and head/tail/content SHA-256", @@ -140,7 +145,7 @@ func newRootCommand(stderr io.Writer) *cobra.Command { Short: "Read the scan database and print the file-level duplicates report", Args: cobra.NoArgs, RunE: runE(func(ctx context.Context, _ []string) error { - return runReport(ctx) + return runReport(ctx, stdout) }), } @@ -149,7 +154,7 @@ func newRootCommand(stderr io.Writer) *cobra.Command { Short: "Read the scan database and print the duplicate-tree report", Args: cobra.NoArgs, RunE: runE(func(ctx context.Context, _ []string) error { - return runTrees(ctx) + return runTrees(ctx, stdout) }), } diff --git a/main_test.go b/main_test.go index 432e9e4..f04801b 100644 --- a/main_test.go +++ b/main_test.go @@ -82,50 +82,6 @@ func makeReadOnly(t *testing.T, path string) { }) } -// captureStdout redirects os.Stdout to a file for the rest of the test -// and returns a function reading back everything written to it. Only -// machine-readable data belongs on stdout (README design goal 4), so -// the tests assert on it directly. -func captureStdout(t *testing.T) func() string { - t.Helper() - - f, err := os.Create(filepath.Join(t.TempDir(), "stdout")) - if err != nil { - t.Fatal(err) - } - - saved := os.Stdout - os.Stdout = f - - t.Cleanup(func() { - os.Stdout = saved - - _ = f.Close() - }) - - return func() string { - // Read what has been written without disturbing the write - // offset, so the capture can be inspected more than once. - size, err := f.Seek(0, io.SeekCurrent) - if err != nil { - t.Fatal(err) - } - - if size == 0 { - return "" - } - - b := make([]byte, size) - - _, err = f.ReadAt(b, 0) - if err != nil { - t.Fatal(err) - } - - return string(b) - } -} - // brokenDatabase writes a database that opens cleanly and passes the // schema-version check but has no files table, so the first query // fails with the database already open: a fatal error on a path that @@ -198,17 +154,15 @@ func TestRunFatalAfterOpenClosesDatabase(t *testing.T) { args = append(args, t.TempDir()) } - var stderr bytes.Buffer + var stdout, stderr bytes.Buffer - stdout := captureStdout(t) - - code := run(args, &stderr) + code := run(args, &stdout, &stderr) if code != exitFatal { t.Errorf("run(%v) = %d, want %d", args, code, exitFatal) } assertNoSidecars(t, path) - assertFatalOutput(t, stderr.String(), stdout()) + assertFatalOutput(t, stderr.String(), stdout.String()) // Proof that the failure happened after the open: only a // query against the opened database can report this. @@ -226,18 +180,16 @@ func TestRunMissingOperandIsFatalNotUsage(t *testing.T) { // must not dump the usage text. t.Setenv(databaseEnv, testDBPath(t)) - var stderr bytes.Buffer - - stdout := captureStdout(t) + var stdout, stderr bytes.Buffer missing := filepath.Join(t.TempDir(), "nope") - code := run([]string{cmdScan, missing}, &stderr) + code := run([]string{cmdScan, missing}, &stdout, &stderr) if code != exitFatal { t.Errorf("run(scan %s) = %d, want %d", missing, code, exitFatal) } - assertFatalOutput(t, stderr.String(), stdout()) + assertFatalOutput(t, stderr.String(), stdout.String()) } // assertFatalOutput checks that a fatal error was reported the way @@ -281,11 +233,9 @@ func TestRunUsageErrors(t *testing.T) { // path that does not exist. t.Setenv(databaseEnv, testDBPath(t)) - var stderr bytes.Buffer + var stdout, stderr bytes.Buffer - stdout := captureStdout(t) - - code := run(tc.args, &stderr) + code := run(tc.args, &stdout, &stderr) if code != exitUsage { t.Errorf("run(%v) = %d, want %d", tc.args, code, exitUsage) } @@ -294,7 +244,7 @@ func TestRunUsageErrors(t *testing.T) { t.Errorf("stderr = %q, want %q", stderr.String(), tc.want) } - if got := stdout(); got != "" { + if got := stdout.String(); got != "" { t.Errorf("stdout = %q, want nothing (data only)", got) } }) @@ -303,9 +253,9 @@ func TestRunUsageErrors(t *testing.T) { // TestRunHelpAndVersionSucceed checks that the two informational flags // exit 0 and keep their human-facing output on stderr. -// -//nolint:paralleltest // captureStdout replaces the process-wide os.Stdout func TestRunHelpAndVersionSucceed(t *testing.T) { + t.Parallel() + assertHumanOutput(t, "--help") assertHumanOutput(t, "--version") } @@ -316,11 +266,9 @@ func TestRunHelpAndVersionSucceed(t *testing.T) { func assertHumanOutput(t *testing.T, arg string) { t.Helper() - var stderr bytes.Buffer + var stdout, stderr bytes.Buffer - stdout := captureStdout(t) - - code := run([]string{arg}, &stderr) + code := run([]string{arg}, &stdout, &stderr) if code != exitOK { t.Errorf("run(%s) = %d, want %d", arg, code, exitOK) } @@ -329,7 +277,7 @@ func assertHumanOutput(t *testing.T, arg string) { t.Errorf("run(%s) wrote nothing to stderr", arg) } - if got := stdout(); got != "" { + if got := stdout.String(); got != "" { t.Errorf("stdout = %q, want nothing (data only)", got) } } @@ -358,17 +306,15 @@ func scanFixture(t *testing.T) []string { t.Fatal(err) } - var stderr bytes.Buffer + var stdout, stderr bytes.Buffer - stdout := captureStdout(t) - - code := run([]string{cmdScan, dir}, &stderr) + code := run([]string{cmdScan, dir}, &stdout, &stderr) if code != exitOK { t.Fatalf("run(scan) = %d, want %d; stderr: %s", code, exitOK, stderr.String()) } - if got := stdout(); got != "" { + if got := stdout.String(); got != "" { t.Errorf("scan stdout = %q, want nothing (data only)", got) } @@ -389,18 +335,16 @@ func TestRunReportSucceeds(t *testing.T) { dupes := scanFixture(t) - var stderr bytes.Buffer + var stdout, stderr bytes.Buffer - stdout := captureStdout(t) - - code := run([]string{cmdReport}, &stderr) + code := run([]string{cmdReport}, &stdout, &stderr) if code != exitOK { t.Fatalf("run(report) = %d, want %d; stderr: %s", code, exitOK, stderr.String()) } want := "first\tdupe\tsize\n" + dupes[0] + "\t" + dupes[1] + "\t300\n" - if got := stdout(); got != want { + if got := stdout.String(); got != want { t.Errorf("stdout = %q, want %q", got, want) } @@ -413,11 +357,9 @@ func TestRunTreesSucceeds(t *testing.T) { dupes := scanFixture(t) - var stderr bytes.Buffer + var stdout, stderr bytes.Buffer - stdout := captureStdout(t) - - code := run([]string{cmdTrees}, &stderr) + code := run([]string{cmdTrees}, &stdout, &stderr) if code != exitOK { t.Fatalf("run(trees) = %d, want %d; stderr: %s", code, exitOK, stderr.String()) @@ -427,7 +369,7 @@ func TestRunTreesSucceeds(t *testing.T) { // trees of each other. want := "first\tdupe\tfiles\tsize\n" + filepath.Dir(dupes[0]) + "\t" + filepath.Dir(dupes[1]) + "\t1\t300\n" - if got := stdout(); got != want { + if got := stdout.String(); got != want { t.Errorf("stdout = %q, want %q", got, want) } @@ -454,11 +396,9 @@ func TestRunReportsNeedOnlyReadAccess(t *testing.T) { } for name, want := range cases { - var stderr bytes.Buffer + var stdout, stderr bytes.Buffer - stdout := captureStdout(t) - - code := run([]string{name}, &stderr) + code := run([]string{name}, &stdout, &stderr) if code != exitOK { t.Errorf("run(%s) = %d, want %d; stderr: %s", name, code, exitOK, stderr.String()) @@ -466,8 +406,71 @@ func TestRunReportsNeedOnlyReadAccess(t *testing.T) { continue } - if got := stdout(); got != want { + if got := stdout.String(); got != want { t.Errorf("%s stdout = %q, want %q", name, got, want) } } } + +func TestRunStdoutClosedIsFatal(t *testing.T) { + // README §Error handling: a stdout write failure exits 1, reported + // in one line on stderr. + for _, name := range []string{cmdReport, cmdTrees} { + t.Run(name, func(t *testing.T) { + t.Setenv(databaseEnv, testDBPath(t)) + + scanFixture(t) + + stdout, err := os.Create(filepath.Join(t.TempDir(), "stdout")) + if err != nil { + t.Fatal(err) + } + + err = stdout.Close() + if err != nil { + t.Fatal(err) + } + + var stderr bytes.Buffer + + code := run([]string{name}, stdout, &stderr) + if code != exitFatal { + t.Errorf("run(%s) = %d, want %d", name, code, exitFatal) + } + + got := stderr.String() + if !strings.HasPrefix(got, "sfdupes: write stdout: ") || + !strings.Contains(got, os.ErrClosed.Error()) || + strings.Count(got, "\n") != 1 { + t.Errorf("stderr = %q, want one line reporting the "+ + "failed stdout write", got) + } + }) + } +} + +// errWriteFailed is the error failingWriter returns. +var errWriteFailed = errors.New("write failed") + +// failingWriter is a stdout that fails every write. +type failingWriter struct{} + +func (failingWriter) Write([]byte) (int, error) { return 0, errWriteFailed } + +func TestStdoutWriteErrorPropagates(t *testing.T) { + t.Setenv(databaseEnv, testDBPath(t)) + + scanFixture(t) + + cases := map[string]func(context.Context, io.Writer) error{ + cmdReport: runReport, + cmdTrees: runTrees, + } + + for name, fn := range cases { + err := fn(t.Context(), failingWriter{}) + if !errors.Is(err, errWriteFailed) { + t.Errorf("%s: error = %v, want %v", name, err, errWriteFailed) + } + } +} diff --git a/report.go b/report.go index 00b7b01..68b4ecb 100644 --- a/report.go +++ b/report.go @@ -4,6 +4,7 @@ import ( "bufio" "context" "fmt" + "io" "os" "slices" "strings" @@ -65,7 +66,7 @@ type dupeGroup struct { // from the database and 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. -func runReport(ctx context.Context) error { +func runReport(ctx context.Context, stdout io.Writer) error { recs, err := loadRecords(ctx) if err != nil { return err @@ -73,7 +74,7 @@ func runReport(ctx context.Context) error { dupes := collectDupeGroups(recs) - out := bufio.NewWriterSize(os.Stdout, ioBufSize) + out := bufio.NewWriterSize(stdout, ioBufSize) _, err = fmt.Fprintln(out, "first\tdupe\tsize") if err != nil { diff --git a/report_test.go b/report_test.go index d1aa290..9aa092d 100644 --- a/report_test.go +++ b/report_test.go @@ -50,11 +50,9 @@ func seedDatabase(t *testing.T, recs []scanRec) string { func TestRunReportEscapesPaths(t *testing.T) { t.Setenv(databaseEnv, seedDatabase(t, awkwardPairRecs())) - var stderr bytes.Buffer + var stdout, stderr bytes.Buffer - stdout := captureStdout(t) - - code := run([]string{cmdReport}, &stderr) + code := run([]string{cmdReport}, &stdout, &stderr) if code != exitOK { t.Fatalf("run(report) = %d, want %d; stderr: %s", code, exitOK, stderr.String()) @@ -62,7 +60,7 @@ func TestRunReportEscapesPaths(t *testing.T) { want := "first\tdupe\tsize\n" + `/d/\tone\ntwo\rthree\\four/f` + "\t/d/A/f\t5\n" - if got := stdout(); got != want { + if got := stdout.String(); got != want { t.Errorf("stdout = %q, want %q", got, want) } } diff --git a/scan_test.go b/scan_test.go index f47005d..5e2a259 100644 --- a/scan_test.go +++ b/scan_test.go @@ -7,6 +7,7 @@ import ( "database/sql" "encoding/hex" "fmt" + "io" "os" "path/filepath" "runtime" @@ -1548,7 +1549,7 @@ func TestScanHashWriteFailureUnwindsPool(t *testing.T) { code := run([]string{ cmdScan, "--workers", strconv.Itoa(hashLeakWorkers), dir, - }, &stderr) + }, io.Discard, &stderr) if code != exitFatal { t.Fatalf("run(scan) = %d, want %d; stderr: %s", code, exitFatal, stderr.String()) diff --git a/trees.go b/trees.go index f43374c..735c5b5 100644 --- a/trees.go +++ b/trees.go @@ -5,6 +5,7 @@ import ( "context" "crypto/sha256" "fmt" + "io" "os" "slices" "strconv" @@ -36,7 +37,7 @@ type treeNode struct { // maximal duplicate-tree groups as TSV on stdout. It never touches the // scanned filesystem; its only I/O is the database, stdout, and // stderr. -func runTrees(ctx context.Context) error { +func runTrees(ctx context.Context, stdout io.Writer) error { recs, err := loadRecords(ctx) if err != nil { return err @@ -47,7 +48,7 @@ func runTrees(ctx context.Context) error { dupes := collectTreeGroups(allDirs, super) - out := bufio.NewWriterSize(os.Stdout, ioBufSize) + out := bufio.NewWriterSize(stdout, ioBufSize) _, err = fmt.Fprintln(out, "first\tdupe\tfiles\tsize") if err != nil { diff --git a/trees_test.go b/trees_test.go index abf61f1..5cc8817 100644 --- a/trees_test.go +++ b/trees_test.go @@ -108,11 +108,9 @@ func TestBuildHierarchyRootPath(t *testing.T) { func TestRunTreesEscapesPaths(t *testing.T) { t.Setenv(databaseEnv, seedDatabase(t, awkwardPairRecs())) - var stderr bytes.Buffer + var stdout, stderr bytes.Buffer - stdout := captureStdout(t) - - code := run([]string{cmdTrees}, &stderr) + code := run([]string{cmdTrees}, &stdout, &stderr) if code != exitOK { t.Fatalf("run(trees) = %d, want %d; stderr: %s", code, exitOK, stderr.String()) @@ -120,7 +118,7 @@ func TestRunTreesEscapesPaths(t *testing.T) { want := "first\tdupe\tfiles\tsize\n" + `/d/\tone\ntwo\rthree\\four` + "\t/d/A\t1\t5\n" - if got := stdout(); got != want { + if got := stdout.String(); got != want { t.Errorf("stdout = %q, want %q", got, want) } }