From ab15cf32c6900f72540dd3cb033083fc03716e6f Mon Sep 17 00:00:00 2001 From: sneak Date: Wed, 11 Nov 2026 02:29:36 +0000 Subject: [PATCH] Hash a file again when its mtime moved back, not only forward (closes #176) scan treated a recorded file as unchanged when its size matched and its mtime was not newer. A file replaced by other content of the same size with an older mtime, as cp -p, rsync -a or a restore writes it, kept the hash of the file it replaced. Any mtime difference now counts as a change. README.md says so, and a new test covers the older mtime. Model: opus-5-5 --- README.md | 15 ++++++------- TODO.md | 5 +++++ internal/sfdupes/scan.go | 12 +--------- internal/sfdupes/scan_test.go | 41 +++++++++++++++++++++++++++++++++++ 4 files changed, 54 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 6003f8a..8b9bd95 100644 --- a/README.md +++ b/README.md @@ -208,8 +208,8 @@ makes the comparison exact. Reading samples cuts a 150 GiB file to about 150 MiB of reads, at the price of a comparison that is not exact. The hashes live in a database that is kept between runs. A rescan reads only new -files, and files whose size changed or whose modification time (mtime) is newer -than recorded. All analysis then works from the database alone. +files, and files whose size or modification time (mtime) differs from their +record. All analysis then works from the database alone. The goal is whole duplicated trees, not only single files. A download extracted twice, or a project copied once too often, is such a tree. It can be removed as @@ -410,17 +410,16 @@ operands: - A file not yet in the database is hashed and inserted. - A file already in the database is not read again when its size and mtime match - its record. The mtime matches when it is not newer than the recorded one. This - is what makes a daily rescan cheap. + its record. This is what makes a daily rescan cheap. - When such a file's size on disk differs from its recorded `disk_size`, `scan` updates `disk_size` alone. It still does not read the file, and counts it as unchanged. ZFS, for example, counts a new file's blocks up to seconds after the write. A block is the unit in which a filesystem allocates disk space. A file scanned within those seconds gets its real `disk_size` at the next scan. -- `scan` hashes a file again when its size differs or its mtime is newer. The - mtime is compared to the nanosecond, as finely as the filesystem records it. A - same-size rewrite counts as a change whenever the filesystem gives it a later - mtime, even within the same second. +- `scan` hashes a file again when its size or its mtime differs from its record. + An older mtime counts as much as a newer one. A file put back by `cp -p`, + `rsync -a` or a restore from backup often has an older mtime. The mtime is + compared to the nanosecond, as finely as the filesystem records it. - The scan deletes each record under a scanned operand whose file it could not verify. That covers deleted files, and paths the scan could not stat or read this run. To stat a file is to read its details, such as its size, without diff --git a/TODO.md b/TODO.md index 8424492..30ef2bb 100644 --- a/TODO.md +++ b/TODO.md @@ -29,6 +29,11 @@ Newest first. An entry ends with its date and its issue or branch, where those are known. +- `scan` hashes a file again when its mtime differs from its record, older or + newer. Before, an older mtime counted as unchanged, so a file put back by + `cp -p` or a restore kept the hash of the file it replaced (2026-11-11, + https://git.eeqj.de/sneak/sfdupes/issues/176) + - The Go code moved out of the repository root. `cmd/sfdupes/main.go` is the command, and it calls `Main` in the package `internal/sfdupes`. `main.go` became `internal/sfdupes/cli.go`. Every other file kept its name. The install diff --git a/internal/sfdupes/scan.go b/internal/sfdupes/scan.go index ba45468..a90d534 100644 --- a/internal/sfdupes/scan.go +++ b/internal/sfdupes/scan.go @@ -545,7 +545,7 @@ func (s *scanState) walkPhase(events <-chan walkEvent, roots []string) { prog.increment() old, ok := s.existing[ev.rec.path] - if !ok || old.size != ev.rec.size || mtimeAfter(ev.rec.mtime, old.mtime) { + if !ok || old.size != ev.rec.size || !ev.rec.mtime.Equal(old.mtime) { s.toHash = append(s.toHash, ev.rec) continue @@ -780,16 +780,6 @@ func (s *scanState) updatePhase(ctx context.Context) error { return applyChanges(ctx, s.db, nil, deletes, prog) } -// mtimeAfter reports whether mtime a is later than mtime b. -// Not a.After(b): time.Time wraps an mtime past year 292 billion; Unix() undoes it. -func mtimeAfter(a, b time.Time) bool { - if a.Unix() != b.Unix() { - return a.Unix() > b.Unix() - } - - return a.Nanosecond() > b.Nanosecond() -} - // underAnyRoot reports whether path is any of the roots or lies under // one of them. func underAnyRoot(path string, roots []string) bool { diff --git a/internal/sfdupes/scan_test.go b/internal/sfdupes/scan_test.go index dafe42b..c731dd5 100644 --- a/internal/sfdupes/scan_test.go +++ b/internal/sfdupes/scan_test.go @@ -1383,6 +1383,47 @@ func TestSyncScanMtimeBump(t *testing.T) { } } +// TestSyncScanMtimeMovedBack replaces a file with other content of the +// same size and an older mtime, as cp -p or a restore from backup does. +// The next scan must hash the file again. +func TestSyncScanMtimeMovedBack(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + db := openTestDB(t) + a := writeFile(t, dir, "a.bin", pattern(1, 500)) + + recorded := time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC) + + err := os.Chtimes(a, recorded, recorded) + if err != nil { + t.Fatal(err) + } + + syncTree(t, db, dir) + + replaced := pattern(3, 500) + writeFile(t, dir, "a.bin", replaced) + + older := recorded.Add(-time.Hour) + + err = os.Chtimes(a, older, older) + if err != nil { + t.Fatal(err) + } + + st := syncTree(t, db, dir) + if st != (scanStats{walked: 1, updated: 1}) { + t.Fatalf("rescan stats = %+v, want 1 updated", st) + } + + assertWholeFileHashed(t, db, a, replaced) + + if r := recordByPath(t, dbRecords(t, db), a); !r.mtime.Equal(older) { + t.Fatalf("mtime = %v, want %v", r.mtime, older) + } +} + // assertWholeFileHashed fails unless the record for path holds the // whole-file hash of data as its head, tail, and content. func assertWholeFileHashed(t *testing.T, db *sql.DB, path string,