From bcd373da7a746528c97f61f4631635c51a38b1c3 Mon Sep 17 00:00:00 2001 From: sneak Date: Tue, 10 Nov 2026 23:45:17 +0000 Subject: [PATCH] Add scan --exclude PATH (closes #163) The walk neither enters nor records an --exclude path or anything below it, and prints "scan: skipped PATH (--exclude)" for each one it reaches. The flag repeats. The walk knows each --exclude path by device ID and inode number, as it knows the database files, so it also leaves one out when it reaches it by another path, such as a hard link. The records below an excluded path are kept, as for a default exclude. An empty value is a usage error. An --exclude path whose real path scan cannot find, or an operand at or below one, is a fatal error before the scan starts. walkRules names its two lists excludePaths and defaultExcludes. printDatabasePath, split out of runScan, keeps runScan under the lint length limit. Model: opus-5-5 --- README.md | 41 +++++- TODO.md | 5 + internal/sfdupes/cancel_test.go | 8 +- internal/sfdupes/cli.go | 43 +++++- internal/sfdupes/cli_test.go | 16 ++- internal/sfdupes/dwim.go | 3 +- internal/sfdupes/exclude.go | 114 ++++++++++++++-- internal/sfdupes/exclude_test.go | 220 ++++++++++++++++++++++++++++++- internal/sfdupes/scan.go | 124 ++++++++++------- internal/sfdupes/scan_test.go | 2 +- 10 files changed, 495 insertions(+), 81 deletions(-) diff --git a/README.md b/README.md index f70186e..dc51fc8 100644 --- a/README.md +++ b/README.md @@ -259,7 +259,7 @@ In order: report. ``` -sfdupes scan [--workers N] [-x] [--no-default-excludes] PATH... +sfdupes scan [--workers N] [-x] [--no-default-excludes] [--exclude PATH]... PATH... sfdupes report > dupes.tsv sfdupes trees > dupetrees.tsv sfdupes info @@ -304,6 +304,16 @@ The reference below is also the program's specification. - `--no-default-excludes` (`scan`) lets the walk enter the default excludes. Those are the system and temporary directories the walk otherwise leaves out, such as `/var/tmp` on Linux (see "Rules for the walk"). +- `--exclude PATH` (`scan`) keeps the walk out of `PATH` and everything below + it. `PATH` may be a directory or a file. The flag may be given more than once. + The walk knows `PATH` by its device ID and inode number, as it knows the + database files (see "Rules for the walk"). So the walk leaves `PATH` out + whatever path leads to it. That includes a hard link to a file given as + `PATH`. A symlink given as `PATH` excludes its target. An empty `PATH`, as in + `--exclude ""`, is a usage error, reported in one line on stderr with exit 2 + before anything is scanned. A `PATH` that does not exist, or whose real path + `scan` cannot find, is a fatal error. So is an operand at or below one (see + "Operands"). - `--version`, `-v` prints `sfdupes VERSION` (see "Install") to stdout and exits 0. It writes nothing to stderr. - `--help`, `-h`, alone or after a subcommand, prints the help text to stderr @@ -362,6 +372,10 @@ default. An operand may be a directory or a regular file. - An operand that does not exist is a fatal error, and so is a symlink whose target does not exist. `scan` then stops before it walks anything, and exits with code 1. +- An operand at or below an `--exclude` path is a fatal error too. The message + names the operand and the `--exclude` path. `scan` compares the operand and + each directory above it with the `--exclude` paths by device ID and inode + number, not by name. - Each operand is resolved to its real path before the walk, as `realpath` prints it. A relative operand is joined to the working directory as `pwd -P` prints it. Every symlink in the path is then followed. A `..` after a symlink @@ -437,15 +451,16 @@ operands: scanned on different schedules into one database. - The scan keeps every record at or below a path that the walk leaves out for one of these reasons: + - it is an `--exclude` path; - it is a default exclude; - it is not on a persistent filesystem; - with `-x`, it is a directory on another filesystem than its operand. Such records count as outside the scanned operands. So with `-x`, a filesystem mounted under an operand can be scanned on its own schedule into - the same database. An operand may itself lie at or inside such a path, as - "Operands" describes. Its walk verifies the records below it, so they are - deleted when their files are gone. + the same database. An operand may itself lie at or inside such a path, other + than an `--exclude` path (see "Operands"). Its walk verifies the records + below it, so they are deleted when their files are gone. #### Phases @@ -544,6 +559,17 @@ walk practical on large or busy storage pools. The path is not counted as skipped. The records below it are kept, as described under "What a scan does to the database". +- The walk never enters or records an `--exclude` path, nor anything below it. + For each one it reaches, `scan` prints a line on stderr naming it: + + ``` + scan: skipped /srv/cache (--exclude) + ``` + + The path is not counted as skipped. The scan keeps the records at and below + it, as described under "What a scan does to the database". + `--no-default-excludes` does not change this rule. + - Below an operand, the walk does not enter a default exclude. On Linux, the default excludes hold temporary files and the kernel's view of the running system. They depend on the system, and `sfdupes scan --help` lists those of @@ -618,7 +644,8 @@ from a thread of its own and never blocks the workers. it repeats another or lies under another; - the progress display of each phase (see "Progress"). `scan` prints each warning for a skipped path as it happens. It prints a line the same way for - each path left out as a default exclude or as not on a persistent filesystem; + each path left out as an `--exclude` path, as a default exclude or as not on a + persistent filesystem; - the summary line; - a warning when a report still has the database open as the scan ends (see "Concurrent reads"). @@ -1199,6 +1226,9 @@ showing the count it stopped at. - `1`: a fatal error, a `scan` stopped by `SIGINT` or `SIGTERM`, or a `dwim` stopped by either during its scan. A fatal error is one of these: - an operand that does not exist, or whose real path `scan` cannot find; + - an `--exclude` path that does not exist, or whose real path `scan` cannot + find; + - an operand at or below an `--exclude` path; - a list of mounted filesystems that `scan` cannot read; - another `scan` holding the lock (see "The scan lock"); - a database, its directory or its lock file that cannot be created or @@ -1218,6 +1248,7 @@ showing the count it stopped at. `--workers abc`; - `scan` with no operand; - `--workers` below 1; + - an empty `--exclude` value, as in `--exclude ""`; - `report`, `trees`, `info` or `dwim` with a positional argument. A fatal error is reported on stderr, starting with `sfdupes:`. A failed write to diff --git a/TODO.md b/TODO.md index b98640a..8fb200d 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. +- New `scan` flag `--exclude PATH`, which may be repeated. The walk neither + enters nor records an excluded path, and keeps the records below it. An + operand at or below one is a fatal error (2026-11-11, + https://git.eeqj.de/sneak/sfdupes/issues/163) + - `report` and `trees` leave out a copy that was deleted after the scan. Each checks only the copy itself with `lstat`: the file, or the directory at the top of a tree. A group left with one copy is not reported. A missing `scan` diff --git a/internal/sfdupes/cancel_test.go b/internal/sfdupes/cancel_test.go index a16b4e6..fb44f82 100644 --- a/internal/sfdupes/cancel_test.go +++ b/internal/sfdupes/cancel_test.go @@ -359,7 +359,7 @@ func TestRunScanInterrupted(t *testing.T) { dir := buildSmokeTree(t) err := runScan(t.Context(), []string{dir}, walkCancelWorkers, false, - false) + false, nil) if err != nil { t.Fatal(err) } @@ -383,7 +383,7 @@ func TestRunScanInterrupted(t *testing.T) { defer close(done) err = runScan(cancelledContext(t), []string{dir}, walkCancelWorkers, - false, false) + false, false, nil) }() awaitReturn(t, done, "runScan") @@ -411,7 +411,7 @@ func TestRunScanInterrupted(t *testing.T) { } err = runScan(t.Context(), []string{dir}, walkCancelWorkers, false, - false) + false, nil) if err != nil { t.Fatal(err) } @@ -437,7 +437,7 @@ func TestRunScanInterruptedMidHash(t *testing.T) { dir := buildWalkCancelTree(t) err := runScan(newWalkClock(hashCancelAtDone), []string{dir}, - walkCancelWorkers, false, false) + walkCancelWorkers, false, false, nil) if !errors.Is(err, errInterrupted) { t.Fatalf("runScan interrupted mid-hash = %v, want %v", err, errInterrupted) diff --git a/internal/sfdupes/cli.go b/internal/sfdupes/cli.go index 387a0b5..d37754d 100644 --- a/internal/sfdupes/cli.go +++ b/internal/sfdupes/cli.go @@ -10,6 +10,7 @@ import ( "os" "runtime" "runtime/debug" + "slices" "github.com/spf13/cobra" ) @@ -40,6 +41,11 @@ var errNoSubcommand = errors.New("no subcommand") // below 1. var errWorkersBelowOne = errors.New("--workers must be at least 1") +// errEmptyExcludeValue is the usage error for an empty scan --exclude +// value, as from --exclude "$DIR" with DIR unset. Taken as a path, it +// would name the working directory. +var errEmptyExcludeValue = errors.New("--exclude must not be empty") + // Version is the build version, injected at link time via -ldflags // (see the Makefile); empty when the linker set none, as for a binary // built by go install. buildVersion turns it into what --version prints. @@ -201,21 +207,23 @@ func newRootCommand(stdout, stderr io.Writer) *cobra.Command { return root } -// newScanCommand builds the scan subcommand, with its --workers, -x and -// --no-default-excludes flags. Its help lists the default excludes of -// the system it runs on, and the filesystem types it never records -// anything on. +// newScanCommand builds the scan subcommand, with its --workers, -x, +// --no-default-excludes and --exclude flags. Its help lists the default +// excludes of the system it runs on, and the filesystem types it never +// records anything on. func newScanCommand() *cobra.Command { var ( scanWorkers int scanOneFS bool scanNoDefaultExcludes bool + scanExclude []string ) short := "Walk trees and synchronize the scan database" scanCmd := &cobra.Command{ - Use: cmdScan + " [--workers N] [-x] [--no-default-excludes] PATH...", + Use: cmdScan + " [--workers N] [-x] [--no-default-excludes] " + + "[--exclude PATH]... PATH...", // The Use line already names every flag, so cobra must not // append its generic "[flags]" to it. DisableFlagsInUseLine: true, @@ -223,14 +231,19 @@ func newScanCommand() *cobra.Command { Long: short + ".\n\n" + defaultExcludesHelp(runtime.GOOS), Args: cobra.MinimumNArgs(1), PreRunE: func(cmd *cobra.Command, _ []string) error { - return checkScanWorkers(cmd, scanWorkers) + err := checkScanWorkers(cmd, scanWorkers) + if err != nil { + return err + } + + return rejectEmptyExcludeValue(cmd, scanExclude) }, RunE: runE(func(ctx context.Context, args []string) error { ctx, stop := interruptContext(ctx) defer stop() return runScan(ctx, args, scanWorkers, scanOneFS, - scanNoDefaultExcludes) + scanNoDefaultExcludes, scanExclude) }), } scanCmd.Flags().IntVar(&scanWorkers, "workers", runtime.NumCPU(), @@ -239,6 +252,10 @@ func newScanCommand() *cobra.Command { "do not cross filesystem boundaries") scanCmd.Flags().BoolVar(&scanNoDefaultExcludes, "no-default-excludes", false, "enter the default excludes, the paths listed above") + // A string array, not a slice, so a comma in a path is not a + // separator. + scanCmd.Flags().StringArrayVar(&scanExclude, "exclude", nil, + "never enter or record `PATH` or anything below it; repeatable") return scanCmd } @@ -256,6 +273,18 @@ func checkScanWorkers(cmd *cobra.Command, workers int) error { return fmt.Errorf("%w, got %d", errWorkersBelowOne, workers) } +// rejectEmptyExcludeValue rejects an empty scan --exclude value. That +// is a usage error reported in one line, as for --workers below 1. +func rejectEmptyExcludeValue(cmd *cobra.Command, excludePaths []string) error { + if !slices.Contains(excludePaths, "") { + return nil + } + + cmd.SilenceUsage = true + + return errEmptyExcludeValue +} + // runE adapts a subcommand implementation, or the version print, to // cobra's RunE. Cobra prints the error and the command's usage text for // every error RunE returns, but a subcommand that ran and failed has no diff --git a/internal/sfdupes/cli_test.go b/internal/sfdupes/cli_test.go index a56321e..dc1bf26 100644 --- a/internal/sfdupes/cli_test.go +++ b/internal/sfdupes/cli_test.go @@ -570,7 +570,8 @@ func TestRunNamesDefaultDatabase(t *testing.T) { }{ cmdScan: { run: func() error { - return runScan(t.Context(), []string{missing}, 1, false, false) + return runScan(t.Context(), []string{missing}, 1, false, false, + nil) }, want: scanWant, }, @@ -712,6 +713,16 @@ func TestRunHelp(t *testing.T) { args, got) } } + + // The scan help lists --exclude with its argument. + var stdout, stderr bytes.Buffer + + run([]string{cmdScan, "-h"}, &stdout, &stderr) + + if !strings.Contains(stderr.String(), excludeFlag+" PATH") { + t.Errorf("scan -h stderr = %q, want %s PATH", stderr.String(), + excludeFlag) + } } func TestRunScanUsageLine(t *testing.T) { @@ -719,7 +730,8 @@ func TestRunScanUsageLine(t *testing.T) { // README §Subcommands shows this usage line for scan, and the help // and the usage error both print it. - const want = "\n sfdupes scan [--workers N] [-x] [--no-default-excludes] PATH...\n" + const want = "\n sfdupes scan [--workers N] [-x] [--no-default-excludes] " + + "[--exclude PATH]... PATH...\n" cases := map[string][]string{ "help": {cmdScan, "-h"}, diff --git a/internal/sfdupes/dwim.go b/internal/sfdupes/dwim.go index 5158a46..7225a00 100644 --- a/internal/sfdupes/dwim.go +++ b/internal/sfdupes/dwim.go @@ -17,7 +17,8 @@ import ( // ends it at once, as it ends trees. func runDwim(ctx context.Context, root string, stdout io.Writer) error { scanCtx, stop := interruptContext(ctx) - err := runScan(scanCtx, []string{root}, runtime.NumCPU(), false, false) + err := runScan(scanCtx, []string{root}, runtime.NumCPU(), false, false, + nil) stop() diff --git a/internal/sfdupes/exclude.go b/internal/sfdupes/exclude.go index 743d4a8..8d57da1 100644 --- a/internal/sfdupes/exclude.go +++ b/internal/sfdupes/exclude.go @@ -1,8 +1,9 @@ package sfdupes -// This file holds two rules for the walk (README "Rules for the walk"): +// This file holds three rules for the walk (README "Rules for the +// walk"): the paths given with --exclude, which the scan never enters; // the default excludes, the paths a scan's walk does not enter below an -// operand unless scan is given --no-default-excludes, and the +// operand unless scan is given --no-default-excludes; and the // filesystems that are not persistent, on which the scan never records // or reads anything. @@ -12,6 +13,7 @@ import ( "fmt" "io" "io/fs" + "os" "path/filepath" "slices" "strings" @@ -43,17 +45,25 @@ const typesPerHelpLine = 6 // readMountInfo reads. var errMountInfoLine = errors.New("unexpected line") +// errExcludedOperand reports an operand at or below a path given with +// --exclude. +var errExcludedOperand = errors.New("operand excluded by --exclude") + // walkRules holds the rules for the walk that a scan sets up as it -// starts. excludes are the default excludes, or none with -// --no-default-excludes. nonPersistent maps the device ID of every -// mounted filesystem that is not persistent to its type; the scan -// records and reads nothing on one of those, whatever the flags. -// databaseFiles are the scan's own database files, which the walk -// passes over (see databaseFiles in db.go). +// starts. excludePaths are the paths given with --exclude, each with +// the device ID and inode number of the file it names (see +// excludePathInodes), which the walk neither enters nor records, +// whatever path leads to them. defaultExcludes are the default +// excludes, or none with --no-default-excludes. nonPersistent maps the +// device ID of every mounted filesystem that is not persistent to its +// type; the scan records and reads nothing on one of those, whatever +// the flags. databaseFiles are the scan's own database files, which the +// walk passes over (see databaseFiles in db.go). type walkRules struct { - excludes []string - nonPersistent map[uint64]string - databaseFiles []fileRec + excludePaths []fileRec + defaultExcludes []string + nonPersistent map[uint64]string + databaseFiles []fileRec } // defaultExcludes returns the default excludes of the system goos @@ -97,6 +107,88 @@ func isDefaultExclude(path string, excludes []string) bool { return false } +// excludePathInodes returns each of paths, the --exclude values, as its +// real path (see realPaths) with the device ID and inode number of the +// file there. The walk compares what it reaches with those, as it does +// with the database files, so it also leaves out an --exclude path +// spelled another way, such as in another letter case on macOS. +func excludePathInodes(paths []string) ([]fileRec, error) { + resolved, err := realPaths(paths) + if err != nil { + return nil, err + } + + files := make([]fileRec, 0, len(resolved)) + + for _, path := range resolved { + fi, err := os.Lstat(path) + if err != nil { + return nil, err + } + + dev, ino := inodeOfInfo(fi) + files = append(files, fileRec{path: path, dev: dev, ino: ino}) + } + + return files, nil +} + +// matchingExcludePath returns the path of the one of excludePaths with +// the device ID and inode number of fi, an lstat result, or "" when +// there is none. +func matchingExcludePath(fi fs.FileInfo, excludePaths []fileRec) string { + dev, ino := inodeOfInfo(fi) + f := fileRec{dev: dev, ino: ino} + + for _, exclude := range excludePaths { + if sameInode(exclude, f) { + return exclude.path + } + } + + return "" +} + +// checkOperandsNotExcluded returns an error when one of roots, the +// operands, or a directory above it is one of excludePaths, the +// --exclude paths, by device ID and inode number. The error names the +// operand and the --exclude path. The roots are absolute, so the +// directories above each one end at /. A path here that lstat cannot +// read is an error too. +func checkOperandsNotExcluded(roots []string, excludePaths []fileRec) error { + for _, root := range roots { + for path := root; ; path = filepath.Dir(path) { + fi, err := os.Lstat(path) + if err != nil { + return fmt.Errorf("scan %s: %w", root, err) + } + + exclude := matchingExcludePath(fi, excludePaths) + if exclude != "" { + return fmt.Errorf("scan %s: %w %s", root, errExcludedOperand, + exclude) + } + + if path == "/" { + break + } + } + } + + return nil +} + +// excludePathLine returns the line the scan prints for path when fi, +// its lstat, makes it one of rules.excludePaths, and "" when it does +// not. +func (rules walkRules) excludePathLine(path string, fi fs.FileInfo) string { + if matchingExcludePath(fi, rules.excludePaths) == "" { + return "" + } + + return fmt.Sprintf("scan: skipped %s (--exclude)", path) +} + // nonPersistentFilesystemTypes returns the filesystem types that are // not persistent: filesystems that the kernel makes up or that are kept // in memory. devfs is the macOS filesystem of /dev; the others are diff --git a/internal/sfdupes/exclude_test.go b/internal/sfdupes/exclude_test.go index e40a4f1..f3262f8 100644 --- a/internal/sfdupes/exclude_test.go +++ b/internal/sfdupes/exclude_test.go @@ -14,6 +14,9 @@ import ( "golang.org/x/sys/unix" ) +// excludeFlag is the scan flag that names a path the walk never enters. +const excludeFlag = "--exclude" + func TestDefaultExcludes(t *testing.T) { t.Parallel() @@ -188,7 +191,7 @@ func TestScanDoesNotEnterDefaultExclude(t *testing.T) { unseen := writeFile(t, excluded, "new", pattern(2, 10)) st, err := syncScan(t.Context(), db, []string{dir}, 1, false, - walkRules{excludes: []string{excluded}}) + walkRules{defaultExcludes: []string{excluded}}) if err != nil { t.Fatal(err) } @@ -234,7 +237,7 @@ func TestScanWalksOperandInDefaultExclude(t *testing.T) { db := openTestDB(t) st, err := syncScan(t.Context(), db, roots, 1, false, - walkRules{excludes: []string{excluded}}) + walkRules{defaultExcludes: []string{excluded}}) if err != nil { t.Fatal(err) } @@ -262,7 +265,7 @@ func TestScanNamesNoDefaultExcludeThatIsAnOperand(t *testing.T) { f := writeFile(t, excluded, "f", pattern(1, 10)) st, err := syncScan(t.Context(), db, []string{dir, excluded}, 1, false, - walkRules{excludes: []string{excluded}}) + walkRules{defaultExcludes: []string{excluded}}) if err != nil { t.Fatal(err) } @@ -356,6 +359,217 @@ func TestRunScanNoDefaultExcludes(t *testing.T) { assertRecorded(t, path, f, true) } +// TestRunScanExcludeNested scans a tree through the command with an +// --exclude path two levels below the operand. The walk records nothing +// at or below it and says so. The record an earlier scan left there is +// kept although its file is gone. +func TestRunScanExcludeNested(t *testing.T) { + path := testDBPath(t) + t.Setenv(databaseEnv, path) + + dir := realTempDir(t) + excluded := filepath.Join(dir, "a", "b") + kept := writeFile(t, dir, "a/f", pattern(1, 10)) + gone := writeFile(t, excluded, "gone", pattern(2, 10)) + + scanOK(t, dir) + + err := os.Remove(gone) + if err != nil { + t.Fatal(err) + } + + unseen := writeFile(t, excluded, "c/new", pattern(3, 10)) + + stderr := scanOK(t, excludeFlag, excluded, dir) + + line := "scan: skipped " + excluded + " (--exclude)\n" + if !strings.Contains(stderr, line) { + t.Errorf("stderr = %q, want %q", stderr, line) + } + + assertRecorded(t, path, kept, true) + assertRecorded(t, path, gone, true) + assertRecorded(t, path, unseen, false) +} + +// TestRunScanExcludeRepeated gives --exclude twice, once for a +// directory and once for a file, the file through a symlink to it. The +// walk records neither, and records the file beside them. +func TestRunScanExcludeRepeated(t *testing.T) { + path := testDBPath(t) + t.Setenv(databaseEnv, path) + + dir := realTempDir(t) + inDir := writeFile(t, dir, "skip/f", pattern(1, 10)) + file := writeFile(t, dir, "skip-file", pattern(2, 10)) + kept := writeFile(t, dir, "keep", pattern(3, 10)) + + link := filepath.Join(t.TempDir(), "link") + symlink(t, file, link) + + scanOK(t, excludeFlag, filepath.Dir(inDir), excludeFlag, link, dir) + + assertRecorded(t, path, inDir, false) + assertRecorded(t, path, file, false) + assertRecorded(t, path, kept, true) +} + +// TestRunScanExcludeReachedByAnotherPath gives --exclude a directory +// through a symlink and a file through "..", so the walk reaches +// neither by the path given. The walk also reaches the file through a +// hard link. It records none of these, prints a line naming each, and +// records the file beside them. +func TestRunScanExcludeReachedByAnotherPath(t *testing.T) { + path := testDBPath(t) + t.Setenv(databaseEnv, path) + + dir := realTempDir(t) + inDir := writeFile(t, dir, "a/cache/f", pattern(1, 10)) + file := writeFile(t, dir, "a/big", pattern(2, 10)) + kept := writeFile(t, dir, "keep", pattern(3, 10)) + + hardLink := filepath.Join(dir, "big-link") + + err := os.Link(file, hardLink) + if err != nil { + t.Fatal(err) + } + + link := filepath.Join(t.TempDir(), "link") + symlink(t, filepath.Join(dir, "a"), link) + + // Joined as text, since filepath.Join would drop the "..". + dotDot := dir + "/a/cache/../big" + + 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" + if !strings.Contains(stderr, line) { + t.Errorf("stderr = %q, want %q", stderr, line) + } + } + + assertRecorded(t, path, inDir, false) + assertRecorded(t, path, file, false) + assertRecorded(t, path, hardLink, false) + assertRecorded(t, path, kept, true) +} + +// TestRunScanExcludedOperand checks that an operand at or below an +// --exclude path is a fatal error that names both, and so is an +// --exclude path that does not exist. An operand that is a hard link to +// an --exclude file is at it. Neither scan writes a database. +func TestRunScanExcludedOperand(t *testing.T) { + dir := realTempDir(t) + sub := filepath.Join(dir, "sub") + f := writeFile(t, sub, "f", pattern(1, 10)) + + hardLink := filepath.Join(dir, "f-link") + + err := os.Link(f, hardLink) + if err != nil { + t.Fatal(err) + } + + cases := map[string]struct { + args []string + want string + }{ + "at it": { + []string{excludeFlag, sub, sub}, + "scan " + sub + ": operand excluded by --exclude " + sub + "\n", + }, + "below it": { + []string{excludeFlag, dir, sub}, + "scan " + sub + ": operand excluded by --exclude " + dir + "\n", + }, + "after an operand it does not exclude": { + []string{dir, excludeFlag, sub, sub}, + "scan " + sub + ": operand excluded by --exclude " + sub + "\n", + }, + "a hard link to it": { + []string{excludeFlag, f, hardLink}, + "scan " + hardLink + ": operand excluded by --exclude " + f + "\n", + }, + "exclude missing": { + []string{excludeFlag, filepath.Join(dir, "nope"), dir}, + "--exclude: resolve " + filepath.Join(dir, "nope") + ": ", + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + path := testDBPath(t) + t.Setenv(databaseEnv, path) + + stdout := captureStdout(t) + stderr := captureStderr(t) + + code := run(append([]string{cmdScan}, tc.args...), os.Stdout, + os.Stderr) + if code != exitFatal { + t.Errorf("run(scan %q) = %d, want %d", tc.args, code, + exitFatal) + } + + assertFatalOutput(t, stderr(), stdout()) + + if !strings.Contains(stderr(), "sfdupes: "+tc.want) { + t.Errorf("stderr = %q, want %q", stderr(), tc.want) + } + + _, err := os.Lstat(path) + if !errors.Is(err, os.ErrNotExist) { + t.Errorf("lstat %s: %v, want no database", path, err) + } + }) + } +} + +// TestRunScanRejectsEmptyExclude checks that an empty --exclude value, +// given as its own argument, after "=" or after another --exclude, is +// a usage error reported in one line before anything is scanned +// (README "Settings"). +func TestRunScanRejectsEmptyExclude(t *testing.T) { + dir := t.TempDir() + + cases := map[string][]string{ + "separate": {excludeFlag, "", dir}, + "joined by =": {excludeFlag + "=", dir}, + "after a path": {excludeFlag, t.TempDir(), excludeFlag, "", dir}, + } + + for name, args := range cases { + t.Run(name, func(t *testing.T) { + path := testDBPath(t) + t.Setenv(databaseEnv, path) + + var stdout, stderr bytes.Buffer + + code := run(append([]string{cmdScan}, args...), &stdout, + &stderr) + if code != exitUsage { + t.Errorf("run(scan %q) = %d, want %d", args, code, + exitUsage) + } + + want := "Error: --exclude must not be empty\n" + if stdout.String() != "" || stderr.String() != want { + t.Errorf("stdout, stderr = %q, %q, want \"\", %q", + stdout.String(), stderr.String(), want) + } + + _, err := os.Lstat(path) + if !errors.Is(err, os.ErrNotExist) { + t.Errorf("lstat %s: %v, want no database", path, err) + } + }) + } +} + // TestRunScanSkipsNonPersistentOperand scans, through the command, a // directory on a filesystem that is not persistent, with and without // --no-default-excludes: either way it is skipped, with a line saying diff --git a/internal/sfdupes/scan.go b/internal/sfdupes/scan.go index a90d534..fdb0953 100644 --- a/internal/sfdupes/scan.go +++ b/internal/sfdupes/scan.go @@ -102,40 +102,44 @@ func scanPhase(name string) string { // scan command rejects anything less. Before the lock is taken, the // scan lists the filesystems that are not persistent, on which the walk // records nothing; unless noDefaultExcludes is set, the walk does not -// enter the default excludes either. Once the database is open, the -// scan notes its files, which the walk passes over. +// enter the default excludes either. The walk neither enters nor +// records the paths in excludePaths, the --exclude values; an operand +// at or below one is a fatal error. Once the database is open, the scan +// notes its files, which the walk passes over. func runScan(ctx context.Context, roots []string, workers int, - oneFS, noDefaultExcludes bool, + oneFS, noDefaultExcludes bool, excludePaths []string, ) error { dbPath, source, err := databasePath() if err != nil { return err } - // A database file missing now is created by lockScanDatabase below. - what := "database" + printDatabasePath(dbPath, source) - _, err = os.Stat(dbPath) - if errors.Is(err, fs.ErrNotExist) { - what = "new database" - } - - fmt.Fprintf(os.Stderr, "scan: %s %s (%s)\n", what, escapePath(dbPath), - source) - - roots, err = resolveRoots(roots) + roots, err = realPaths(roots) if err != nil { return err } - nonPersistent, err := nonPersistentFilesystems() + var rules walkRules + + rules.excludePaths, err = excludePathInodes(excludePaths) + if err != nil { + return fmt.Errorf("--exclude: %w", err) + } + + err = checkOperandsNotExcluded(roots, rules.excludePaths) + if err != nil { + return err + } + + rules.nonPersistent, err = nonPersistentFilesystems() if err != nil { return fmt.Errorf("list the mounted filesystems: %w", err) } - rules := walkRules{nonPersistent: nonPersistent} if !noDefaultExcludes { - rules.excludes = defaultExcludes(runtime.GOOS) + rules.defaultExcludes = defaultExcludes(runtime.GOOS) } lock, err := lockScanDatabase(dbPath) @@ -181,6 +185,21 @@ func runScan(ctx context.Context, roots []string, workers int, return nil } +// printDatabasePath prints on stderr the line that names the database +// at dbPath, with source, saying whether it is new. A database file +// missing now is new: lockScanDatabase creates it. +func printDatabasePath(dbPath, source string) { + what := "database" + + _, err := os.Stat(dbPath) + if errors.Is(err, fs.ErrNotExist) { + what = "new database" + } + + fmt.Fprintf(os.Stderr, "scan: %s %s (%s)\n", what, escapePath(dbPath), + source) +} + // interruptContext returns a copy of ctx that the first SIGINT or // SIGTERM cancels; the scan command runs the scan under it. stop // releases the signals. @@ -209,24 +228,25 @@ func interrupted(walked int) error { return errInterrupted } -// resolveRoots converts each PATH operand to its real path: absolute, -// with every symlink in it followed. Database records are keyed by -// that path, so scan results depend neither on the working directory -// nor on the symlinks an operand was given through. An operand that -// cannot be resolved, because it or a symlink's target does not exist -// or cannot be read, is a fatal error before any scanning. -func resolveRoots(roots []string) ([]string, error) { - resolved := make([]string, 0, len(roots)) +// realPaths returns the real path of each of paths, the PATH operands +// or the --exclude values: absolute, with every symlink in it followed. +// Database records are keyed by that path, so scan results depend +// neither on the working directory nor on the symlinks an operand was +// given through. A path that cannot be resolved, because it or a +// symlink's target does not exist or cannot be read, is a fatal error +// before any scanning. +func realPaths(paths []string) ([]string, error) { + resolved := make([]string, 0, len(paths)) - for _, root := range roots { - a, err := absolutePath(root) + for _, path := range paths { + a, err := absolutePath(path) if err != nil { - return nil, fmt.Errorf("resolve %s: %w", root, err) + return nil, fmt.Errorf("resolve %s: %w", path, err) } r, err := filepath.EvalSymlinks(a) if err != nil { - return nil, fmt.Errorf("resolve %s: %w", root, err) + return nil, fmt.Errorf("resolve %s: %w", path, err) } resolved = append(resolved, r) @@ -295,7 +315,7 @@ func walkReaches(above, r string, oneFS bool, rules walkRules, } for ; dir != above; dir = filepath.Dir(dir) { - if isDefaultExclude(dir, rules.excludes) { + if isDefaultExclude(dir, rules.defaultExcludes) { return false } @@ -418,7 +438,7 @@ func (s *scanState) runPhases(ctx context.Context, roots []string, // walkableRoots returns the operands the walk can start from: regular // files, and directories not named .zfs, none of them inside a -// directory named .zfs. The operands are real paths from resolveRoots, +// directory named .zfs. The operands are real paths from realPaths, // so these checks apply to what a symlink operand points at. Every // other operand is warned about, counted as skipped, and dropped. A // dropped operand is no longer a root, so the records stored beneath it @@ -493,10 +513,11 @@ func (s *scanState) loadIndex(ctx context.Context, roots []string) error { // as verified. When its disk_size differs from the record's, it is // queued for the update phase, which writes the new disk_size. A record // at or below a path the walk left out, because -x stopped at it, it is -// a default exclude, or it is not on a persistent filesystem, counts as -// outside the roots: it is not deleted. That does not hold below a root -// at or inside that path, since that root's own walk verifies the -// records below it; nor is a line printed for such a root. +// an --exclude path or a default exclude, or it is not on a persistent +// filesystem, counts as outside the roots: it is not deleted. That does +// not hold below a root at or inside that path, since that root's own +// walk verifies the records below it; nor is a line printed for such a +// root. func (s *scanState) walkPhase(events <-chan walkEvent, roots []string) { // The records the last scan left under the roots estimate how many // files the walk will find. A first scan has none, and no estimate. @@ -834,9 +855,9 @@ type dirJob struct { // file's statted record, a warning when fail is set, or a path the walk // left out: when otherFSDir is set, a directory -x did not enter // because it is on another filesystem than its operand, and when -// leftOut is set, a default exclude or a directory or file on a -// filesystem that is not persistent, with warn holding the line that -// says so. +// leftOut is set, an --exclude path, a default exclude or a directory or +// file on a filesystem that is not persistent, with warn holding the +// line that says so. type walkEvent struct { rec fileRec warn string @@ -887,7 +908,7 @@ func sendEvent(ctx context.Context, events chan<- walkEvent, // operandWarning returns the one-line warning for an operand the walk // does not start from, naming the path and what it is, or "" for one it // does: a regular file, or a directory not named .zfs, that is not -// inside a directory named .zfs. resolveRoots has already followed the +// inside a directory named .zfs. realPaths has already followed the // operand's symlinks, so a symlink here is an operand replaced by one // after it was resolved, and is not followed. func operandWarning(root string, fi fs.FileInfo) string { @@ -1118,8 +1139,9 @@ func walkOneDir(ctx context.Context, job dirJob, oneFS bool, // no longer exists, and one that has stopped being a regular file is // skipped. A file on a filesystem in rules that is not persistent, as // through a bind mount of one file, is not recorded: it gets an event -// naming it instead. One of the scan's own database files in rules gets -// no event at all. +// naming it instead, and so does a file given with --exclude, reached +// by any path, a hard link to it included. One of the scan's own +// database files in rules gets no event at all. func emitFile(ctx context.Context, p string, e fs.DirEntry, rules walkRules, events chan<- walkEvent, ) { @@ -1143,6 +1165,10 @@ func emitFile(ctx context.Context, p string, e fs.DirEntry, } line := rules.nonPersistentLine(p, info) + if line == "" { + line = rules.excludePathLine(p, info) + } + if line != "" { sendEvent(ctx, events, walkEvent{leftOut: p, warn: line}) @@ -1167,11 +1193,11 @@ func emitFile(ctx context.Context, p string, e fs.DirEntry, // subdirJob applies the descent rules to directory p: never enter // .zfs (ZFS snapshot pseudo-dirs would list every file once per // snapshot), never enter a directory on a filesystem in rules that is -// not persistent, nor a default exclude in rules, and with -x never -// enter a directory on a different filesystem than its operand. For the -// last three it sends an event that names the directory, so the scan -// keeps the records below it, except those below an operand at or -// inside it. +// not persistent, nor an --exclude path or a default exclude in rules, +// and with -x never enter a directory on a different filesystem than +// its operand. For all but .zfs it sends an event that names the +// directory, so the scan keeps the records below it, except those below +// an operand at or inside it. func subdirJob(ctx context.Context, p string, e fs.DirEntry, parent dirJob, oneFS bool, rules walkRules, events chan<- walkEvent, @@ -1196,13 +1222,17 @@ func subdirJob(ctx context.Context, p string, e fs.DirEntry, } line := rules.nonPersistentLine(p, info) + if line == "" { + line = rules.excludePathLine(p, info) + } + if line != "" { sendEvent(ctx, events, walkEvent{leftOut: p, warn: line}) return dirJob{}, false } - if isDefaultExclude(p, rules.excludes) { + if isDefaultExclude(p, rules.defaultExcludes) { sendEvent(ctx, events, walkEvent{ leftOut: p, warn: fmt.Sprintf("scan: skipped %s (default exclude)", p), diff --git a/internal/sfdupes/scan_test.go b/internal/sfdupes/scan_test.go index e9e78b5..b078b5f 100644 --- a/internal/sfdupes/scan_test.go +++ b/internal/sfdupes/scan_test.go @@ -2289,7 +2289,7 @@ func TestPruneRoots(t *testing.T) { // The walk of /p does not enter /p/q when it is a default exclude, so // /p/q is kept, without -x too; /p/q/r lies under it and is dropped. - got = pruneRoots(nested, false, walkRules{excludes: []string{pqRoot}}, + got = pruneRoots(nested, false, walkRules{defaultExcludes: []string{pqRoot}}, device) if !slices.Equal(got, want) { t.Fatalf("pruneRoots excluding %s = %q, want %q", pqRoot, got, want)