diff --git a/README.md b/README.md index 5859aed..4e4ac42 100644 --- a/README.md +++ b/README.md @@ -566,7 +566,9 @@ Rules for the walk: Concurrency: the walk phase (which also stats files), the hash phase, and the content phase each use a worker pool of `--workers` workers (default `runtime.NumCPU()`); the walk parallelizes across -directories, hashing across files. All three phases are seek-bound on +directories, hashing across files. `--workers` must be at least 1: a +smaller value is a usage error, reported in one line on stderr with +exit 2 before anything is scanned. All three phases are seek-bound on spinning disks, so raising `--workers` well past the core count can help on pools with many spindles. The main goroutine owns partitioning, database writes, and progress rendering; progress @@ -761,8 +763,9 @@ Additional requirements: cannot be created/opened/read/written, a missing database for `report`/`trees`, stdout write failure), or a `scan` stopped by `SIGINT` or `SIGTERM` (see below). -- `2`: usage error (including `scan` with no `PATH` operand and - `report`/`trees` with any positional argument). +- `2`: usage error (including `scan` with no `PATH` operand, `scan` + with `--workers` below 1, and `report`/`trees` with any positional + argument). A stdout write failure, such as a full disk, is reported in one line on stderr and exits 1. Two cases never reach sfdupes as a failed write: diff --git a/TODO.md b/TODO.md index d824ca3..df64ee8 100644 --- a/TODO.md +++ b/TODO.md @@ -29,6 +29,10 @@ # Completed Steps +- `scan` rejects `--workers` below 1 as a usage error instead of + running single-threaded (2026-10-04, + https://git.eeqj.de/sneak/sfdupes/issues/10) + - test that `scan` refuses a database with another schema version (2026-10-04, https://git.eeqj.de/sneak/sfdupes/issues/64) diff --git a/main.go b/main.go index 9379ede..6133b50 100644 --- a/main.go +++ b/main.go @@ -51,6 +51,10 @@ const ( // cobra prints for it is the whole message. var errNoSubcommand = errors.New("no subcommand") +// errWorkersBelowOne is the usage error for a scan --workers value +// below 1. +var errWorkersBelowOne = errors.New("--workers must be at least 1") + // Version is the build version, injected at link time via -ldflags // (see the Makefile); "dev" for a plain go build. // @@ -98,8 +102,7 @@ func run(args []string, stdout, stderr io.Writer) int { return exitFatal default: - // A usage error: cobra has already printed the message and - // the usage text. + // A usage error, which cobra has already reported on stderr. return exitUsage } } @@ -154,6 +157,9 @@ func newRootCommand(stdout, stderr io.Writer) *cobra.Command { Use: cmdScan + " [--workers N] [-x] PATH...", Short: "Walk trees and synchronize the scan database", Args: cobra.MinimumNArgs(1), + PreRunE: func(cmd *cobra.Command, _ []string) error { + return checkScanWorkers(cmd, scanWorkers) + }, RunE: runE(func(ctx context.Context, args []string) error { ctx, stop := interruptContext(ctx) defer stop() @@ -189,6 +195,19 @@ func newRootCommand(stdout, stderr io.Writer) *cobra.Command { return root } +// checkScanWorkers rejects a scan --workers value below 1. That is a +// usage error reported in one line: cobra prints the returned message +// without the usage text, and run exits 2. +func checkScanWorkers(cmd *cobra.Command, workers int) error { + if workers >= 1 { + return nil + } + + cmd.SilenceUsage = true + + return fmt.Errorf("%w, got %d", errWorkersBelowOne, workers) +} + // 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/main_test.go b/main_test.go index bddb2a9..8a09878 100644 --- a/main_test.go +++ b/main_test.go @@ -295,6 +295,42 @@ func TestRunUsageErrors(t *testing.T) { } } +func TestRunScanRejectsWorkersBelowOne(t *testing.T) { + // README §scan mode: --workers below 1 is a usage error reported in + // one line on stderr, before the scan opens the database. + for _, workers := range []string{"0", "-1"} { + t.Run(workers, func(t *testing.T) { + dbPath := testDBPath(t) + t.Setenv(databaseEnv, dbPath) + + var stdout, stderr bytes.Buffer + + args := []string{cmdScan, "--workers", workers, t.TempDir()} + + code := run(args, &stdout, &stderr) + if code != exitUsage { + t.Errorf("run(%v) = %d, want %d", args, code, exitUsage) + } + + want := "Error: --workers must be at least 1, got " + workers + + "\n" + if got := stderr.String(); got != want { + t.Errorf("stderr = %q, want %q", got, want) + } + + if got := stdout.String(); got != "" { + t.Errorf("stdout = %q, want nothing (data only)", got) + } + + _, err := os.Stat(dbPath) + if !errors.Is(err, fs.ErrNotExist) { + t.Errorf("stat %s: %v, want the database never created", + dbPath, err) + } + }) + } +} + func TestRunHelp(t *testing.T) { t.Parallel() diff --git a/scan.go b/scan.go index c61a9d8..f181f0a 100644 --- a/scan.go +++ b/scan.go @@ -98,14 +98,11 @@ type fileMeta struct { // is released after it. When ctx is cancelled, as by the SIGINT or // SIGTERM that interruptContext catches, the scan keeps what it has // hashed (see syncScan), prints how many files its walk reached, and -// returns errInterrupted. +// returns errInterrupted. workers must be at least 1; the scan command +// rejects anything less. func runScan(ctx context.Context, roots []string, workers int, oneFS bool, ) error { - if workers < 1 { - workers = 1 - } - roots, err := resolveRoots(roots) if err != nil { return err