Skip removed paths silently and fix the progress line (closes #133)
check / check (push) Canceled after 0s

A path removed while the scan runs gets no warning and no skipped
count, in the walk or the hash phase. If the first of several hard
links is gone, the next is read. Any other error is still warned about.

On a terminal every display is the library's spinner, then the line. A
phase with an exact total ends the padded line with its percent and a
bar at a fixed column. sfdupes draws that bar: the library shows no
spinner beside its own.

The time remaining is labelled remain, not eta.

Judgement call: an operand removed after the scan starts is passed over
silently too.
Deviation: README's 80-column promise for stalled lines is narrowed to
its example.

Model: opus-5-5
This commit is contained in:
2026-10-26 04:06:11 +00:00
parent 68881f04f2
commit e80c6541d2
9 changed files with 522 additions and 230 deletions
+155 -4
View File
@@ -683,6 +683,108 @@ func TestSubdirJobStatError(t *testing.T) {
}
}
// TestWalkPassesOverRemovedPaths removes a file and a directory after
// their parent was read, as when they are deleted while a scan runs.
// The walk then reaches them, the directory also as one to read and as
// an operand. None of that is an error: the walk sends no event.
func TestWalkPassesOverRemovedPaths(t *testing.T) {
t.Parallel()
dir := t.TempDir()
file := writeFile(t, dir, "file", []byte("x"))
writeFile(t, dir, "sub/f", []byte("y"))
sub := filepath.Join(dir, "sub")
fileEntry := dirEntryFor(t, dir, "file")
subEntry := dirEntryFor(t, dir, "sub")
for _, p := range []string{file, sub} {
err := os.RemoveAll(p)
if err != nil {
t.Fatal(err)
}
}
ctx := t.Context()
events := make(chan walkEvent, 4)
emitFile(ctx, file, fileEntry, walkRules{}, events)
_, descend := subdirJob(ctx, sub, subEntry, dirJob{path: dir}, false,
walkRules{}, events)
subs := walkOneDir(ctx, dirJob{path: sub}, false, walkRules{}, events)
jobs := seedRoot(ctx, sub, walkRules{}, events)
close(events)
for ev := range events {
t.Errorf("event %+v for a removed path, want none", ev)
}
if descend || len(subs) != 0 || len(jobs) != 0 {
t.Errorf("descend = %t, subdirectories %+v, jobs %+v, want none",
descend, subs, jobs)
}
}
// 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.
//
//nolint:paralleltest // captureStderr replaces the process-wide os.Stderr
func TestScanWarnsAboutUnreadablePaths(t *testing.T) {
if os.Geteuid() == 0 {
t.Skip("root ignores file permissions")
}
stderr := captureStderr(t)
dir := t.TempDir()
tree := filepath.Join(dir, "tree")
walked := writeFile(t, tree, "unsearchable/f", []byte("x"))
operand := writeFile(t, dir, "unsearchable/f", []byte("y"))
unreadable := filepath.Join(tree, "unreadable")
err := os.Mkdir(unreadable, 0o750)
if err != nil {
t.Fatal(err)
}
modes := map[string]fs.FileMode{
unreadable: 0,
filepath.Dir(walked): 0o400,
filepath.Dir(operand): 0o400,
}
for p, mode := range modes {
err = os.Chmod(p, mode)
if err != nil {
t.Fatal(err)
}
// Runs before t.TempDir's own cleanup, which must delete the files.
t.Cleanup(func() {
//nolint:gosec // removing the files needs the search bit back
_ = os.Chmod(p, 0o700)
})
}
st := syncTree(t, openTestDB(t), tree, operand)
if st != (scanStats{skipped: 3}) {
t.Errorf("stats = %+v, want 3 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",
} {
if !strings.Contains(got, warning) {
t.Errorf("stderr = %q, want %q", got, warning)
}
}
}
// syncOneFilesystem scans roots into db with -x and returns the scan
// stats. Every device pruneRoots looks up, and the device each
// operand's walk compares its subdirectories with, comes from devices
@@ -1128,7 +1230,7 @@ func TestScanPipeline(t *testing.T) {
// database's records. Each phase's first line names it with its place
// among the four: load counts the records before reading them, and the
// walk estimates its total from the records under its operand, or says
// its ETA is unknown when there are none.
// its time remaining is unknown when there are none.
//
//nolint:paralleltest // captureStderr replaces the process-wide os.Stderr
func TestScanProgressNamesEachPhase(t *testing.T) {
@@ -1143,14 +1245,14 @@ func TestScanProgressNamesEachPhase(t *testing.T) {
syncTree(t, db, filepath.Join(dir, "a"))
rescan := stderr()[len(first):]
load := "[1/4] load: counting the records, elapsed 0s, eta unknown"
load := "[1/4] load: counting the records, elapsed 0s, remain unknown"
cases := map[string][]string{
first: {
load, "[2/4] walk: 0 files, 0 files/s, elapsed 0s, eta unknown",
load, "[2/4] walk: 0 files, 0 files/s, elapsed 0s, remain unknown",
"[3/4] hash: [0/", "[4/4] update: [0/",
},
rescan: {
load, "[2/4] walk: [0/~2] 0%, 0 files/s, elapsed 0s, eta unknown",
load, "[2/4] walk: [0/~2] 0%, 0 files/s, elapsed 0s, remain unknown",
"[3/4] hash: [0/", "[4/4] update: [0/",
},
}
@@ -1685,6 +1787,55 @@ func TestHashPhaseSkipsFileReplacedByFIFO(t *testing.T) {
}
}
// TestHashPhasePassesOverRemovedFiles removes two walked files before
// the hash phase reads them: one with no other path, and the first of
// two hard links to one file. Neither is an error. The phase prints
// nothing but its progress, counts nothing as skipped, and records only
// the hard link still there.
//
//nolint:paralleltest // captureStderr replaces the process-wide os.Stderr
func TestHashPhasePassesOverRemovedFiles(t *testing.T) {
stderr := captureStderr(t)
dir := t.TempDir()
alone := writeFile(t, dir, "alone.bin", pattern(1, 300))
a := writeFile(t, dir, "a.bin", pattern(2, 300))
b := filepath.Join(dir, "b.bin")
err := os.Link(a, b)
if err != nil {
t.Fatal(err)
}
walked, _ := collectWalk(t, []string{dir}, false, 1)
// a.bin sorts before b.bin, so the run of the two reads it first.
for _, p := range []string{alone, a} {
err = os.Remove(p)
if err != nil {
t.Fatal(err)
}
}
s := &scanState{db: openTestDB(t), toHash: walked}
err = s.hashPhase(t.Context(), 1)
if err != nil {
t.Fatalf("hashPhase = %v, want nil", err)
}
if s.st.skipped != 0 || len(s.batch) != 1 || s.batch[0].path != b {
t.Errorf("skipped = %d, records %+v, want none skipped and a "+
"record for %s alone", s.st.skipped, s.batch, b)
}
for line := range strings.Lines(stderr()) {
if !strings.HasPrefix(line, scanPhase("hash")+": ") {
t.Errorf("stderr holds %q, want the hash phase's progress alone",
line)
}
}
}
// injectedWriteFailure is the message the injected database trigger
// aborts with, so the test can recognize its own failure in the error
// the scan reports.