From 4b1c3cbf706083c9dd37d6441b4361427d441650 Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 23 Jul 2026 08:55:17 +0700 Subject: [PATCH] Make scan paths required operands; add -x/--one-file-system scan now takes one or more PATH operands (directories or regular files) via cobra flags instead of the -root flag with its /srv default; invoking scan with no operand is a usage error and a nonexistent operand is fatal. Filesystem boundaries are crossed by default; the new -x/--one-file-system flag (GNU du/rsync convention) stops the walk at each operand's filesystem, implemented by comparing lstat device IDs with build-tagged helpers for darwin's int32 Dev. Verified against a real mounted disk image: default crosses, -x does not, --one-file-system is identical to -x. --- README.md | 48 +++++++++----- TODO.md | 4 ++ main.go | 22 +++++-- scan.go | 157 ++++++++++++++++++++++++++++----------------- scan_dev_darwin.go | 14 ++++ scan_dev_other.go | 10 +++ scan_test.go | 91 +++++++++++++++++++++++++- 7 files changed, 262 insertions(+), 84 deletions(-) create mode 100644 scan_dev_darwin.go create mode 100644 scan_dev_other.go diff --git a/README.md b/README.md index 98deca3..fcaed1a 100644 --- a/README.md +++ b/README.md @@ -20,16 +20,17 @@ This README is the complete and authoritative specification. ```sh make build -./sfdupes scan -root /srv > files.dat +./sfdupes scan /srv > files.dat ./sfdupes report files.dat > dupes.tsv ./sfdupes trees files.dat > dupetrees.tsv ``` -`scan` walks a filesystem tree and emits one record per regular file -(path, size, mtime, head hash, tail hash). `report` ingests that stream -and prints the file-level duplicates report. `trees` ingests the same -stream and prints the duplicate-tree report. A missing/invalid -subcommand prints a usage message and exits 2. +`scan` walks one or more filesystem trees and emits one record per +regular file (path, size, mtime, head hash, tail hash). `report` +ingests that stream and prints the file-level duplicates report. +`trees` ingests the same stream and prints the duplicate-tree report. A +missing/invalid subcommand — or a `scan` invocation with no `PATH` +operand — prints a usage message and exits 2. ## Rationale @@ -92,18 +93,26 @@ Three subcommands, all implemented: directory, and report maximal groups of identical trees. ``` -sfdupes scan [-root /srv] [-workers N] > files.dat +sfdupes scan [--workers N] [-x] PATH... > files.dat sfdupes report [files.dat|-] > dupes.tsv sfdupes trees [files.dat|-] > dupetrees.tsv ``` ### `scan` mode +`scan` requires one or more `PATH` operands naming the trees to scan. +There is no default path; invoking `scan` with no operand is a usage +error (usage message on stderr, exit 2). An operand may be a directory +or a regular file; an operand that does not exist is a fatal error +(exit 1). Operands are walked in the order given; overlapping operands +(one containing another) emit their common files once per operand, so +callers should pass disjoint paths. + `scan` runs **three sequential passes**, in this order, so that every expensive pass has an exact total for meaningful progress and ETA: -1. **walk** — recursively enumerate the tree under `-root` (default - `/srv`), collecting the list of regular-file paths. Total unknown +1. **walk** — recursively enumerate the tree under each `PATH` in + turn, collecting the list of regular-file paths. Total unknown while running: show a live count, not a percentage. 2. **stat** — `lstat` every collected path, recording size and mtime. 3. **hash** — for each file, read the first `min(1024, size)` bytes and @@ -113,16 +122,21 @@ expensive pass has an exact total for meaningful progress and ETA: Rules for the walk: -- Only regular files. Skip directories, symlinks (do not follow), - sockets, FIFOs, and device nodes. +- Only regular files. Skip directories, symlinks (do not follow, + including symlink operands), sockets, FIFOs, and device nodes. - Never descend into a directory named `.zfs` (ZFS snapshot pseudo-dirs; walking them would list every file once per snapshot). +- Filesystem boundaries are crossed by default. With `-x` + (long form `--one-file-system`, following the GNU `du`/`rsync` + convention), never descend into a directory on a different + filesystem than its `PATH` operand; each operand is bounded by its + own filesystem. - On any per-path error (permission denied, file vanished between passes, unreadable): print a one-line warning to stderr, skip the path, and continue. Per-file errors never abort the run; the final summary reports how many were skipped. -Concurrency: the stat and hash passes use a worker pool (`-workers`, +Concurrency: the stat and hash passes use a worker pool (`--workers`, default `runtime.NumCPU()`). The main goroutine owns stdout writing and progress rendering; progress display must never block the workers. @@ -278,9 +292,9 @@ Additional requirements: ### Error handling and exit codes - `0`: success, even if individual files were skipped with warnings. -- `1`: fatal error (e.g., `-root` does not exist, cannot read the scan - input, stdout write failure). -- `2`: usage error. +- `1`: fatal error (e.g., a `PATH` operand does not exist, cannot + read the scan input, stdout write failure). +- `2`: usage error (including `scan` with no `PATH` operand). ## Build @@ -325,7 +339,7 @@ All of the following, run in this directory, must pass: cp "$d/t1/sub/f2" "$d/t2/sub/f2" cp "$d/t1/f1" "$d/t3/f1" cp "$d/t1/sub/f2" "$d/t3/sub/f2renamed" - ./sfdupes scan -root "$d" > files.dat + ./sfdupes scan "$d" > files.dat ./sfdupes report files.dat ./sfdupes trees files.dat ``` @@ -336,7 +350,7 @@ All of the following, run in this directory, must pass: `t2/sub/f2`/`t3/sub/f2renamed` form one group; `tiny1`/`tiny2` pair; `empty1`/`empty2` pair; `unique.bin` and `tiny3` appear nowhere; groups ordered by size descending; piping scan directly into report - (`./sfdupes scan -root "$d" | ./sfdupes report`) gives the same + (`./sfdupes scan "$d" | ./sfdupes report`) gives the same rows. Expected from `trees`: exactly one row — `first` `$d/t1`, `dupe` diff --git a/TODO.md b/TODO.md index 73cbc0f..4927066 100644 --- a/TODO.md +++ b/TODO.md @@ -18,6 +18,10 @@ # Completed Steps +- `scan` CLI rework (2026-07-23, branch `scan-required-paths`): required + `PATH...` operands via cobra flags replacing the `/srv` `-root` + default; new `-x`/`--one-file-system` flag (GNU convention) to stop + at filesystem boundaries, which are crossed by default - bring the repo into full policy compliance (2026-07-23, branch `repo-policy-compliance`; checklist below) - `git init` with README-only first commit; code baseline committed on diff --git a/main.go b/main.go index acd0edb..1e0ca25 100644 --- a/main.go +++ b/main.go @@ -6,7 +6,7 @@ // // Usage: // -// sfdupes scan [-root /srv] [-workers N] > files.dat +// sfdupes scan [--workers N] [-x] PATH... > files.dat // sfdupes report [files.dat|-] > dupes.tsv // sfdupes trees [files.dat|-] > dupetrees.tsv // @@ -16,6 +16,7 @@ package main import ( "fmt" "os" + "runtime" "github.com/spf13/cobra" ) @@ -52,16 +53,23 @@ func main() { root.SetErr(os.Stderr) root.CompletionOptions.DisableDefaultCmd = true + var ( + scanWorkers int + scanOneFS bool + ) + scanCmd := &cobra.Command{ - Use: "scan [-root /srv] [-workers N]", - Short: "Walk a tree and emit one record per regular file on stdout", - // The README specifies single-dash flags (-root, -workers); - // parse them with the stdlib flag package inside runScan. - DisableFlagParsing: true, + Use: "scan [--workers N] [-x] PATH...", + Short: "Walk trees and emit one record per regular file on stdout", + Args: cobra.MinimumNArgs(1), Run: func(_ *cobra.Command, args []string) { - runScan(args) + runScan(args, scanWorkers, scanOneFS) }, } + scanCmd.Flags().IntVar(&scanWorkers, "workers", runtime.NumCPU(), + "concurrent workers for the stat and hash passes") + scanCmd.Flags().BoolVarP(&scanOneFS, "one-file-system", "x", false, + "do not cross filesystem boundaries") reportCmd := &cobra.Command{ Use: "report [files.dat|-]", diff --git a/scan.go b/scan.go index 50fd3f9..42d17c2 100644 --- a/scan.go +++ b/scan.go @@ -5,12 +5,11 @@ import ( "crypto/sha256" "encoding/hex" "errors" - "flag" "fmt" "io/fs" "os" "path/filepath" - "runtime" + "syscall" ) // chunk is the number of bytes hashed from each end of a file. @@ -31,63 +30,66 @@ type fileRec struct { mtime int64 } -// runScan implements the scan subcommand: three sequential passes (walk, -// stat, hash) over the tree under -root, emitting one NUL-terminated -// record per regular file on stdout. -func runScan(args []string) { - fl := flag.NewFlagSet("scan", flag.ExitOnError) - root := fl.String("root", "/srv", "directory tree to scan") - workers := fl.Int("workers", runtime.NumCPU(), - "concurrent workers for the stat and hash passes") - // The flag set uses ExitOnError, so Parse cannot return a non-nil - // error; the check keeps the error handled explicitly. - err := fl.Parse(args) - if err != nil { - os.Exit(exitUsage) +// runScan implements the scan subcommand: three sequential passes +// (walk, stat, hash) over the trees named by the PATH operands, +// emitting one NUL-terminated record per regular file on stdout. Flag +// parsing and the at-least-one-operand check are done by cobra. +func runScan(roots []string, workers int, oneFS bool) { + if workers < 1 { + workers = 1 } - if fl.NArg() > 0 { - fmt.Fprintf(os.Stderr, "scan: unexpected argument %q\n", fl.Arg(0)) - os.Exit(exitUsage) + // A nonexistent operand is a fatal error before any scanning. + for _, root := range roots { + _, err := os.Lstat(root) + if err != nil { + fatalf("%v", err) + } } - if *workers < 1 { - *workers = 1 - } - - st, err := os.Stat(*root) - if err != nil { - fatalf("root %s: %v", *root, err) - } - - if !st.IsDir() { - fatalf("root %s: not a directory", *root) - } - - paths, walkErrs := walkPass(*root) - recs, statErrs := statPass(paths, *workers) - emitted, hashErrs := hashPass(recs, *workers) + paths, walkErrs := walkPass(roots, oneFS) + recs, statErrs := statPass(paths, workers) + emitted, hashErrs := hashPass(recs, workers) skipped := walkErrs + statErrs + hashErrs fmt.Fprintf(os.Stderr, "scan: %d files emitted, %d skipped\n", emitted, skipped) } -// walkPass enumerates every regular file under root. It never follows -// symlinks, never descends into directories named .zfs, and warns and -// continues on any per-path error. -func walkPass(root string) ([]string, int) { - var ( - paths []string - errs int - ) +// treeWalker carries the walk-pass state shared by all PATH operands. +type treeWalker struct { + prog *progress + oneFS bool + paths []string + errs int +} + +// walkPass enumerates every regular file under each root operand in +// order. It never follows symlinks, never descends into directories +// named .zfs, and warns and continues on any per-path error. With +// oneFS set it never descends into a directory on a different +// filesystem than its root operand. +func walkPass(roots []string, oneFS bool) ([]string, int) { + w := &treeWalker{prog: newProgress("walk", -1), oneFS: oneFS} + + for _, root := range roots { + w.walkRoot(root) + } + + w.prog.finish() + + return w.paths, w.errs +} + +// walkRoot walks a single PATH operand, appending regular-file paths. +func (w *treeWalker) walkRoot(root string) { + rootDev, rootDevOK := deviceOf(root) - prog := newProgress("walk", -1) walkErr := filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error { if err != nil { - errs++ + w.errs++ - prog.warnf("walk %s: %v", p, err) + w.prog.warnf("walk %s: %v", p, err) if d != nil && d.IsDir() { return filepath.SkipDir @@ -97,13 +99,7 @@ func walkPass(root string) ([]string, int) { } if d.IsDir() { - // ZFS snapshot pseudo-dirs would list every file - // once per snapshot; never descend. - if d.Name() == ".zfs" { - return filepath.SkipDir - } - - return nil + return w.dirAction(p, d, rootDev, rootDevOK) } // Regular files only: skip symlinks, sockets, FIFOs, and // device nodes. @@ -111,20 +107,65 @@ func walkPass(root string) ([]string, int) { return nil } - paths = append(paths, p) + w.paths = append(w.paths, p) - prog.increment() + w.prog.increment() return nil }) - - prog.finish() - if walkErr != nil { fatalf("walk %s: %v", root, walkErr) } +} - return paths, errs +// dirAction decides whether the walk descends into directory p. +func (w *treeWalker) dirAction(p string, d fs.DirEntry, rootDev uint64, rootDevOK bool) error { + // ZFS snapshot pseudo-dirs would list every file once per + // snapshot; never descend. + if d.Name() == ".zfs" { + return filepath.SkipDir + } + + if !w.oneFS || !rootDevOK { + return nil + } + + info, err := d.Info() + if err != nil { + w.errs++ + + w.prog.warnf("walk %s: %v", p, err) + + return filepath.SkipDir + } + + if dev, ok := deviceOfInfo(info); ok && dev != rootDev { + return filepath.SkipDir + } + + return nil +} + +// deviceOf returns the filesystem device ID of path without following +// symlinks. +func deviceOf(path string) (uint64, bool) { + fi, err := os.Lstat(path) + if err != nil { + return 0, false + } + + return deviceOfInfo(fi) +} + +// deviceOfInfo extracts the filesystem device ID from a FileInfo, when +// the platform exposes one. +func deviceOfInfo(fi fs.FileInfo) (uint64, bool) { + st, ok := fi.Sys().(*syscall.Stat_t) + if !ok { + return 0, false + } + + return statDev(st), true } // statPass lstats every collected path in a worker pool, recording size diff --git a/scan_dev_darwin.go b/scan_dev_darwin.go new file mode 100644 index 0000000..a483441 --- /dev/null +++ b/scan_dev_darwin.go @@ -0,0 +1,14 @@ +//go:build darwin + +package main + +import "syscall" + +// statDev returns the filesystem device ID of st as uint64. Device IDs +// are opaque; the conversion only needs to be consistent so equality +// comparisons work, not numerically meaningful. +// +//nolint:gosec // int32 device IDs convert consistently; only equality matters +func statDev(st *syscall.Stat_t) uint64 { + return uint64(st.Dev) +} diff --git a/scan_dev_other.go b/scan_dev_other.go new file mode 100644 index 0000000..bf16da3 --- /dev/null +++ b/scan_dev_other.go @@ -0,0 +1,10 @@ +//go:build !darwin + +package main + +import "syscall" + +// statDev returns the filesystem device ID of st. +func statDev(st *syscall.Stat_t) uint64 { + return st.Dev +} diff --git a/scan_test.go b/scan_test.go index da13854..5eba987 100644 --- a/scan_test.go +++ b/scan_test.go @@ -129,7 +129,7 @@ func TestWalkPass(t *testing.T) { t.Fatal(err) } - paths, errs := walkPass(dir) + paths, errs := walkPass([]string{dir}, false) if errs != 0 { t.Fatalf("errs = %d, want 0", errs) } @@ -141,6 +141,93 @@ func TestWalkPass(t *testing.T) { } } +func TestWalkPassMultipleRoots(t *testing.T) { + t.Parallel() + + rootA := t.TempDir() + rootB := t.TempDir() + want := []string{ + writeFile(t, rootA, "a1", []byte("1")), + writeFile(t, rootA, "sub/a2", []byte("2")), + writeFile(t, rootB, "b1", []byte("3")), + } + + paths, errs := walkPass([]string{rootA, rootB}, false) + if errs != 0 { + t.Fatalf("errs = %d, want 0", errs) + } + + // Operands are walked in the order given. + if !slices.Equal(paths, want) { + t.Fatalf("paths = %q, want %q", paths, want) + } +} + +func TestWalkPassFileAndSymlinkOperands(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + f := writeFile(t, dir, "plain", []byte("data")) + + link := filepath.Join(dir, "link") + + err := os.Symlink(f, link) + if err != nil { + t.Fatal(err) + } + + // A regular-file operand is emitted as itself. + paths, errs := walkPass([]string{f}, false) + if errs != 0 || !slices.Equal(paths, []string{f}) { + t.Fatalf("file operand: paths = %q, errs = %d", paths, errs) + } + + // A symlink operand is not followed and yields nothing. + paths, errs = walkPass([]string{link}, false) + if errs != 0 || len(paths) != 0 { + t.Fatalf("symlink operand: paths = %q, errs = %d", paths, errs) + } +} + +func TestWalkPassOneFilesystemSameFS(t *testing.T) { + t.Parallel() + + // Everything in one filesystem: -x must not skip anything. + dir := t.TempDir() + want := []string{ + writeFile(t, dir, "a", []byte("a")), + writeFile(t, dir, "sub/deep/b", []byte("b")), + } + + paths, errs := walkPass([]string{dir}, true) + if errs != 0 { + t.Fatalf("errs = %d, want 0", errs) + } + + slices.Sort(paths) + + if !slices.Equal(paths, want) { + t.Fatalf("paths = %q, want %q", paths, want) + } +} + +func TestDeviceOf(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + + dev1, ok1 := deviceOf(dir) + + dev2, ok2 := deviceOf(dir) + if !ok1 || !ok2 || dev1 != dev2 { + t.Fatalf("deviceOf unstable: %d/%v vs %d/%v", dev1, ok1, dev2, ok2) + } + + if _, ok := deviceOf(filepath.Join(dir, "missing")); ok { + t.Fatal("deviceOf reported ok for a missing path") + } +} + func TestStatPass(t *testing.T) { t.Parallel() @@ -234,7 +321,7 @@ func buildSmokeTree(t *testing.T) string { func scanToRecords(t *testing.T, dir string) []scanRec { t.Helper() - paths, walkErrs := walkPass(dir) + paths, walkErrs := walkPass([]string{dir}, false) if walkErrs != 0 { t.Fatalf("walk errors: %d", walkErrs) }