From a179fbaa9c74e78a543d4aa6d1a7df8b32ae6725 Mon Sep 17 00:00:00 2001 From: sneak Date: Wed, 11 Nov 2026 09:26:03 +0000 Subject: [PATCH] Say "skipped" in scan's warnings for skipped paths (closes #170) A warning for a path the summary counts as skipped now reads `scan: skipped PATH (REASON)`, naming the path once. This covers unreadable directories, failed lstats, failed reads and dropped operands. Each path of a hard-linked file that cannot be read gets its own line, since each is counted. Lines for paths left out on purpose (`--exclude`, default excludes, filesystems that are not persistent) now read `scan: left out PATH (...)`, so a search for `scan: skipped` finds only the counted paths. Model: opus-5-5 --- README.md | 19 +++++++++++--- internal/sfdupes/cli_test.go | 11 ++++---- internal/sfdupes/dwim_test.go | 6 ++--- internal/sfdupes/exclude.go | 4 +-- internal/sfdupes/exclude_test.go | 22 ++++++++-------- internal/sfdupes/scan.go | 40 ++++++++++++++++++++--------- internal/sfdupes/scan_test.go | 44 ++++++++++++++++++++++++-------- 7 files changed, 98 insertions(+), 48 deletions(-) diff --git a/README.md b/README.md index 943be3c..fc1e04e 100644 --- a/README.md +++ b/README.md @@ -563,7 +563,7 @@ walk practical on large or busy storage pools. `scan` prints a line on stderr naming the path and its filesystem type: ``` - scan: skipped /dev/shm (tmpfs, not a persistent filesystem) + scan: left out /dev/shm (tmpfs, not a persistent filesystem) ``` The path is not counted as skipped. The records below it are kept, as @@ -573,7 +573,7 @@ walk practical on large or busy storage pools. For each one it reaches, `scan` prints a line on stderr naming it: ``` - scan: skipped /srv/cache (--exclude) + scan: left out /srv/cache (--exclude) ``` The path is not counted as skipped. The scan keeps the records at and below @@ -622,7 +622,7 @@ walk practical on large or busy storage pools. not enter, `scan` prints a line on stderr naming it: ``` - scan: skipped /var/tmp (default exclude) + scan: left out /var/tmp (default exclude) ``` The directory is not counted as skipped. `scan` prints the filesystem line @@ -682,6 +682,19 @@ scan: 123400 files seen (1200 added, 34 updated, 56 removed, 122166 unchanged), `removed` counts deleted records, which are not part of the files seen. +The warning for a skipped path names the path once and gives the reason: + +``` +scan: skipped /srv/a/closed (permission denied) +scan: skipped /srv/a/pipe (a FIFO) +``` + +The summary counts each path named in such a warning. When a file with several +hard links cannot be read, each of its paths gets a warning. A line for a path +left out on purpose starts with `scan: left out` instead, and is not counted. A +search of the log for `scan: skipped` therefore finds the paths the summary +counts. + #### Stopping a scan `scan` stops cleanly on `SIGINT`, the signal Ctrl-C sends, or `SIGTERM`, the diff --git a/internal/sfdupes/cli_test.go b/internal/sfdupes/cli_test.go index 7df15c7..cb7c653 100644 --- a/internal/sfdupes/cli_test.go +++ b/internal/sfdupes/cli_test.go @@ -1013,8 +1013,7 @@ func TestRunScanSucceedsDespiteWarnings(t *testing.T) { stderr := scanOK(t, dir) - warning := "hash " + unreadable + ": open " + unreadable + - ": permission denied\n" + warning := "scan: skipped " + unreadable + " (permission denied)\n" if !strings.Contains(stderr, warning) { t.Errorf("stderr = %q, want %q", stderr, warning) } @@ -1286,7 +1285,7 @@ func TestRunScanSkipsSymlinkToFIFOOperand(t *testing.T) { stderr := scanOK(t, link) - warning := "walk " + fifo + ": skipping FIFO operand\n" + warning := "scan: skipped " + fifo + " (a FIFO)\n" if !strings.Contains(stderr, warning) { t.Errorf("stderr = %q, want %q", stderr, warning) } @@ -1321,7 +1320,7 @@ func assertOperandSkipped(t *testing.T, dbPath, operand, kind, stderr := scanOK(t, operand) - warning := "walk " + operand + ": skipping " + kind + " operand\n" + warning := "scan: skipped " + operand + " (a " + kind + ")\n" if !strings.Contains(stderr, warning) { t.Errorf("stderr = %q, want %q", stderr, warning) } @@ -1355,7 +1354,7 @@ func TestRunScanSkipsOperandInsideZFS(t *testing.T) { writeFile(t, hourly, "f", pattern(1, 10)) f := writeFile(t, dir, "f", pattern(2, 10)) - warning := "walk " + hourly + ": skipping operand inside a .zfs directory\n" + warning := "scan: skipped " + hourly + " (inside a .zfs directory)\n" for _, tc := range []struct { operands []string @@ -1407,7 +1406,7 @@ func TestRunScanSkipsOperandThroughSymlinkIntoZFS(t *testing.T) { stderr := scanOK(t, filepath.Join(snaps, "daily")) daily := filepath.Join(dir, "pool", ".zfs", "snapshot", "daily") - warning := "walk " + daily + ": skipping operand inside a .zfs directory\n" + warning := "scan: skipped " + daily + " (inside a .zfs directory)\n" summary := "scan: 0 files seen (0 added, 0 updated, 0 removed, " + "0 unchanged), 1 skipped\n" diff --git a/internal/sfdupes/dwim_test.go b/internal/sfdupes/dwim_test.go index 9598e9a..d903c45 100644 --- a/internal/sfdupes/dwim_test.go +++ b/internal/sfdupes/dwim_test.go @@ -121,9 +121,9 @@ func TestRunDwimExcludeLeavesTreeOut(t *testing.T) { t.Errorf("group = %q, want it to start %q", lines[1], want) } - skipped := "scan: skipped " + excluded + " (--exclude)\n" - if got := stderr(); !strings.Contains(got, skipped) { - t.Errorf("stderr = %q, want it to contain %q", got, skipped) + leftOut := "scan: left out " + excluded + " (--exclude)\n" + if got := stderr(); !strings.Contains(got, leftOut) { + t.Errorf("stderr = %q, want it to contain %q", got, leftOut) } } diff --git a/internal/sfdupes/exclude.go b/internal/sfdupes/exclude.go index da6a74a..46a0ee4 100644 --- a/internal/sfdupes/exclude.go +++ b/internal/sfdupes/exclude.go @@ -203,7 +203,7 @@ func (rules walkRules) excludePathLine(path string, fi fs.FileInfo) string { return "" } - return fmt.Sprintf("scan: skipped %s (--exclude)", path) + return fmt.Sprintf("scan: left out %s (--exclude)", path) } // nonPersistentFilesystemTypes returns the filesystem types that are @@ -233,7 +233,7 @@ func (rules walkRules) nonPersistentLine(path string, fi fs.FileInfo) string { return "" } - return fmt.Sprintf("scan: skipped %s (%s, not a persistent filesystem)", + return fmt.Sprintf("scan: left out %s (%s, not a persistent filesystem)", path, fsType) } diff --git a/internal/sfdupes/exclude_test.go b/internal/sfdupes/exclude_test.go index 782bf34..062950e 100644 --- a/internal/sfdupes/exclude_test.go +++ b/internal/sfdupes/exclude_test.go @@ -242,7 +242,7 @@ func TestScanDoesNotEnterDefaultExclude(t *testing.T) { t.Errorf("%s recorded: the walk entered %s", unseen, excluded) } - line := "\nscan: skipped " + excluded + " (default exclude)\n" + line := "\nscan: left out " + excluded + " (default exclude)\n" if !strings.Contains(stderr(), line) { t.Errorf("stderr = %q, want %q", stderr(), line) } @@ -311,7 +311,7 @@ func TestScanNamesNoDefaultExcludeThatIsAnOperand(t *testing.T) { recordByPath(t, dbRecords(t, db), f) - if strings.Contains(stderr(), "scan: skipped") { + if strings.Contains(stderr(), "scan: left out") { t.Errorf("stderr = %q, want no line for %s", stderr(), excluded) } } @@ -332,8 +332,8 @@ func TestRunScanTempDirOperand(t *testing.T) { f := writeFile(t, dir, "sub/f", pattern(1, 10)) stderr := scanOK(t, dir) - if strings.Contains(stderr, "scan: skipped") { - t.Errorf("stderr = %q, want no line saying a path was skipped", + if strings.Contains(stderr, "scan: left out") { + t.Errorf("stderr = %q, want no line saying a path was left out", stderr) } @@ -377,7 +377,7 @@ func TestRunScanNoDefaultExcludes(t *testing.T) { path := testDBPath(t) t.Setenv(databaseEnv, path) - line := "scan: skipped /var/tmp (default exclude)\n" + line := "scan: left out /var/tmp (default exclude)\n" if stderr := scanOK(t, "/var"); !strings.Contains(stderr, line) { t.Errorf("scan /var: stderr = %q, want %q", stderr, line) @@ -418,7 +418,7 @@ func TestRunScanExcludeNested(t *testing.T) { stderr := scanOK(t, excludeFlag, excluded, dir) - line := "scan: skipped " + excluded + " (--exclude)\n" + line := "scan: left out " + excluded + " (--exclude)\n" if !strings.Contains(stderr, line) { t.Errorf("stderr = %q, want %q", stderr, line) } @@ -480,8 +480,8 @@ func TestRunScanExcludeReachedByAnotherPath(t *testing.T) { stderr := scanOK(t, excludeFlag, filepath.Join(link, "cache"), excludeFlag, dotDot, dir) - for _, skipped := range []string{filepath.Dir(inDir), file, hardLink} { - line := "scan: skipped " + skipped + " (--exclude)\n" + for _, leftOut := range []string{filepath.Dir(inDir), file, hardLink} { + line := "scan: left out " + leftOut + " (--exclude)\n" if !strings.Contains(stderr, line) { t.Errorf("stderr = %q, want %q", stderr, line) } @@ -615,7 +615,7 @@ func TestRunScanSkipsNonPersistentOperand(t *testing.T) { t.Setenv(databaseEnv, testDBPath(t)) - line := "scan: skipped " + shm + " (" + fsType + + line := "scan: left out " + shm + " (" + fsType + ", not a persistent filesystem)\n" summary := "scan: 0 files seen" @@ -669,7 +669,7 @@ func TestScanKeepsRecordsOnNonPersistent(t *testing.T) { t.Errorf("%s recorded", unseen) } - line := "scan: skipped " + shm + " (" + fsType + + line := "scan: left out " + shm + " (" + fsType + ", not a persistent filesystem)\n" if !strings.Contains(stderr(), line) { t.Errorf("stderr = %q, want %q", stderr(), line) @@ -702,7 +702,7 @@ func TestWalkLeavesOutNonPersistent(t *testing.T) { leftOut = append(leftOut, ev.leftOut) - want := "scan: skipped " + ev.leftOut + " (" + fsType + + want := "scan: left out " + ev.leftOut + " (" + fsType + ", not a persistent filesystem)" if ev.warn != want { t.Errorf("line = %q, want %q", ev.warn, want) diff --git a/internal/sfdupes/scan.go b/internal/sfdupes/scan.go index 13d55a1..f86ed8c 100644 --- a/internal/sfdupes/scan.go +++ b/internal/sfdupes/scan.go @@ -664,8 +664,8 @@ func isDatabaseFile(f fileRec, rules walkRules) bool { // inode run, committing completed records to the database in batches // as results arrive, so a long scan persists its progress as it goes // (an interrupted scan resumes cheaply: the next run skips everything -// already recorded). A run that fails to hash is warned about and -// counted as skipped; stale records for its paths, if any, are deleted +// already recorded). Each path of a run that fails to hash is warned +// about and counted as skipped; stale records for them, if any, are deleted // by the update phase. A run whose file no longer exists gets no record // and no warning, and is not counted as skipped. The total counts actual // reads, so the display's time remaining is meaningful. @@ -706,9 +706,12 @@ func (s *scanState) hashPhase(ctx context.Context, workers int) error { } if r.err != nil { - s.st.skipped += len(r.run) + // Every path in the run is counted, so every path gets a line. + for _, rec := range r.run { + s.st.skipped++ - prog.warnf("hash %s: %v", r.run[0].path, r.err) + prog.warnf("%s", skippedLine(rec.path, r.err)) + } continue } @@ -915,8 +918,7 @@ func operandWarning(root string, fi fs.FileInfo) string { // The walk never enters a .zfs directory, so it starts from nothing // inside one either. if slices.Contains(strings.Split(filepath.Dir(root), "/"), ".zfs") { - return fmt.Sprintf("walk %s: skipping operand inside a .zfs directory", - root) + return fmt.Sprintf("scan: skipped %s (inside a .zfs directory)", root) } var kind string @@ -942,7 +944,21 @@ func operandWarning(root string, fi fs.FileInfo) string { kind = "non-regular file" } - return fmt.Sprintf("walk %s: skipping %s operand", root, kind) + return fmt.Sprintf("scan: skipped %s (a %s)", root, kind) +} + +// skippedLine returns the warning for a path the scan skips because of +// err. The reason is the error without its path, which the line already +// names. +func skippedLine(path string, err error) string { + reason := err.Error() + + var pathErr *fs.PathError + if errors.As(err, &pathErr) { + reason = pathErr.Err.Error() + } + + return fmt.Sprintf("scan: skipped %s (%s)", path, reason) } // seedRoot turns one PATH operand into the walk's starting state: a @@ -965,7 +981,7 @@ func seedRoot(ctx context.Context, root string, rules walkRules, if err != nil { sendEvent(ctx, events, walkEvent{ - warn: fmt.Sprintf("walk %s: %v", root, err), + warn: skippedLine(root, err), fail: true, }) @@ -1094,7 +1110,7 @@ func walkOneDir(ctx context.Context, job dirJob, oneFS bool, if err != nil { sendEvent(ctx, events, walkEvent{ - warn: fmt.Sprintf("walk %s: %v", job.path, err), + warn: skippedLine(job.path, err), fail: true, }) @@ -1153,7 +1169,7 @@ func emitFile(ctx context.Context, p string, e fs.DirEntry, if err != nil { sendEvent(ctx, events, walkEvent{ - warn: fmt.Sprintf("stat %s: %v", p, err), + warn: skippedLine(p, err), fail: true, }) @@ -1214,7 +1230,7 @@ func subdirJob(ctx context.Context, p string, e fs.DirEntry, if err != nil { sendEvent(ctx, events, walkEvent{ - warn: fmt.Sprintf("walk %s: %v", p, err), + warn: skippedLine(p, err), fail: true, }) @@ -1235,7 +1251,7 @@ func subdirJob(ctx context.Context, p string, e fs.DirEntry, if isDefaultExclude(p, rules.defaultExcludes) { sendEvent(ctx, events, walkEvent{ leftOut: p, - warn: fmt.Sprintf("scan: skipped %s (default exclude)", p), + warn: fmt.Sprintf("scan: left out %s (default exclude)", p), }) return dirJob{}, false diff --git a/internal/sfdupes/scan_test.go b/internal/sfdupes/scan_test.go index d41e5bb..58e6429 100644 --- a/internal/sfdupes/scan_test.go +++ b/internal/sfdupes/scan_test.go @@ -752,8 +752,9 @@ func TestWalkPassesOverRemovedPaths(t *testing.T) { // TestScanWarnsAboutUnreadablePaths checks that a permission error is // still warned about and counted as skipped, unlike a removed path. The -// walk reaches an unreadable directory, and a file in a directory that -// can be read but not searched. Another such file is an operand. +// walk reaches an unreadable directory. It also reaches a file and a +// subdirectory in a directory that can be read but not searched. Another +// such file is an operand. // //nolint:paralleltest // captureStderr replaces the process-wide os.Stderr func TestScanWarnsAboutUnreadablePaths(t *testing.T) { @@ -765,6 +766,7 @@ func TestScanWarnsAboutUnreadablePaths(t *testing.T) { dir := t.TempDir() tree := filepath.Join(dir, "tree") walked := writeFile(t, tree, "unsearchable/f", []byte("x")) + subdir := filepath.Join(filepath.Dir(walked), "d") operand := writeFile(t, dir, "unsearchable/f", []byte("y")) unreadable := filepath.Join(tree, "unreadable") @@ -773,6 +775,11 @@ func TestScanWarnsAboutUnreadablePaths(t *testing.T) { t.Fatal(err) } + err = os.Mkdir(subdir, 0o750) + if err != nil { + t.Fatal(err) + } + modes := map[string]fs.FileMode{ unreadable: 0, filepath.Dir(walked): 0o400, @@ -793,15 +800,16 @@ func TestScanWarnsAboutUnreadablePaths(t *testing.T) { } st := syncTree(t, openTestDB(t), tree, operand) - if st != (scanStats{skipped: 3}) { - t.Errorf("stats = %+v, want 3 skipped and nothing else", st) + if st != (scanStats{skipped: 4}) { + t.Errorf("stats = %+v, want 4 skipped and nothing else", st) } got := stderr() for _, warning := range []string{ - "walk " + unreadable + ": open " + unreadable + ": permission denied\n", - "stat " + walked + ": lstat " + walked + ": permission denied\n", - "walk " + operand + ": lstat " + operand + ": permission denied\n", + "scan: skipped " + unreadable + " (permission denied)\n", + "scan: skipped " + walked + " (permission denied)\n", + "scan: skipped " + subdir + " (permission denied)\n", + "scan: skipped " + operand + " (permission denied)\n", } { if !strings.Contains(got, warning) { t.Errorf("stderr = %q, want %q", got, warning) @@ -1941,14 +1949,19 @@ func TestScanHardlinksReadOnce(t *testing.T) { } } +// TestScanHardlinkRunFailsTogether makes a file with two hard links +// unreadable. Both paths are counted as skipped, and each gets its own +// warning. +// +//nolint:paralleltest // captureStderr replaces the process-wide os.Stderr func TestScanHardlinkRunFailsTogether(t *testing.T) { - t.Parallel() - + stderr := captureStderr(t) dir := t.TempDir() db := openTestDB(t) a := writeFile(t, dir, "a.bin", pattern(1, 300)) + b := filepath.Join(dir, "b.bin") - err := os.Link(a, filepath.Join(dir, "b.bin")) + err := os.Link(a, b) if err != nil { t.Fatal(err) } @@ -1964,6 +1977,15 @@ func TestScanHardlinkRunFailsTogether(t *testing.T) { if st.skipped != 2 || st.added != 0 { t.Fatalf("stats = %+v, want both hardlink paths skipped", st) } + + got := stderr() + + for _, p := range []string{a, b} { + warning := "scan: skipped " + p + " (permission denied)\n" + if !strings.Contains(got, warning) { + t.Errorf("stderr = %q, want %q", got, warning) + } + } } // TestHashPhaseSkipsFileReplacedByFIFO replaces a walked regular file @@ -2001,7 +2023,7 @@ func TestHashPhaseSkipsFileReplacedByFIFO(t *testing.T) { s.st.skipped, len(s.batch)) } - warning := "hash " + p + ": not a regular file\n" + warning := "scan: skipped " + p + " (not a regular file)\n" if got := stderr(); strings.Count(got, warning) != 1 { t.Errorf("stderr = %q, want %q once", got, warning) }