Reject scan --workers below 1 as a usage error (closes #10)
check / check (push) Failing after 1s
check / check (push) Failing after 1s
A --workers value of 0 or less used to be quietly raised to 1, so a typo ran the whole scan on one worker with nothing on stderr to say why. scan now refuses it before anything is scanned: one line on stderr and exit 2, like the other usage errors. The clamp in runScan is gone, and README states the rule and the default. Model: opus-5-5
This commit is contained in:
@@ -566,7 +566,9 @@ Rules for the walk:
|
|||||||
Concurrency: the walk phase (which also stats files), the hash phase,
|
Concurrency: the walk phase (which also stats files), the hash phase,
|
||||||
and the content phase each use a worker pool of `--workers` workers
|
and the content phase each use a worker pool of `--workers` workers
|
||||||
(default `runtime.NumCPU()`); the walk parallelizes across
|
(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
|
spinning disks, so raising `--workers` well past the core count can
|
||||||
help on pools with many spindles. The main goroutine owns
|
help on pools with many spindles. The main goroutine owns
|
||||||
partitioning, database writes, and progress rendering; progress
|
partitioning, database writes, and progress rendering; progress
|
||||||
@@ -761,8 +763,9 @@ Additional requirements:
|
|||||||
cannot be created/opened/read/written, a missing database for
|
cannot be created/opened/read/written, a missing database for
|
||||||
`report`/`trees`, stdout write failure), or a `scan` stopped by
|
`report`/`trees`, stdout write failure), or a `scan` stopped by
|
||||||
`SIGINT` or `SIGTERM` (see below).
|
`SIGINT` or `SIGTERM` (see below).
|
||||||
- `2`: usage error (including `scan` with no `PATH` operand and
|
- `2`: usage error (including `scan` with no `PATH` operand, `scan`
|
||||||
`report`/`trees` with any positional argument).
|
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
|
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:
|
stderr and exits 1. Two cases never reach sfdupes as a failed write:
|
||||||
|
|||||||
@@ -29,6 +29,10 @@
|
|||||||
|
|
||||||
# Completed Steps
|
# 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
|
- test that `scan` refuses a database with another schema version
|
||||||
(2026-10-04, https://git.eeqj.de/sneak/sfdupes/issues/64)
|
(2026-10-04, https://git.eeqj.de/sneak/sfdupes/issues/64)
|
||||||
|
|
||||||
|
|||||||
@@ -51,6 +51,10 @@ const (
|
|||||||
// cobra prints for it is the whole message.
|
// cobra prints for it is the whole message.
|
||||||
var errNoSubcommand = errors.New("no subcommand")
|
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
|
// Version is the build version, injected at link time via -ldflags
|
||||||
// (see the Makefile); "dev" for a plain go build.
|
// (see the Makefile); "dev" for a plain go build.
|
||||||
//
|
//
|
||||||
@@ -98,8 +102,7 @@ func run(args []string, stdout, stderr io.Writer) int {
|
|||||||
|
|
||||||
return exitFatal
|
return exitFatal
|
||||||
default:
|
default:
|
||||||
// A usage error: cobra has already printed the message and
|
// A usage error, which cobra has already reported on stderr.
|
||||||
// the usage text.
|
|
||||||
return exitUsage
|
return exitUsage
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -154,6 +157,9 @@ func newRootCommand(stdout, stderr io.Writer) *cobra.Command {
|
|||||||
Use: cmdScan + " [--workers N] [-x] PATH...",
|
Use: cmdScan + " [--workers N] [-x] PATH...",
|
||||||
Short: "Walk trees and synchronize the scan database",
|
Short: "Walk trees and synchronize the scan database",
|
||||||
Args: cobra.MinimumNArgs(1),
|
Args: cobra.MinimumNArgs(1),
|
||||||
|
PreRunE: func(cmd *cobra.Command, _ []string) error {
|
||||||
|
return checkScanWorkers(cmd, scanWorkers)
|
||||||
|
},
|
||||||
RunE: runE(func(ctx context.Context, args []string) error {
|
RunE: runE(func(ctx context.Context, args []string) error {
|
||||||
ctx, stop := interruptContext(ctx)
|
ctx, stop := interruptContext(ctx)
|
||||||
defer stop()
|
defer stop()
|
||||||
@@ -189,6 +195,19 @@ func newRootCommand(stdout, stderr io.Writer) *cobra.Command {
|
|||||||
return root
|
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
|
// runE adapts a subcommand implementation, or the version print, to
|
||||||
// cobra's RunE. Cobra prints the error and the command's usage text for
|
// 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
|
// every error RunE returns, but a subcommand that ran and failed has no
|
||||||
|
|||||||
@@ -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) {
|
func TestRunHelp(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
|
|||||||
@@ -98,14 +98,11 @@ type fileMeta struct {
|
|||||||
// is released after it. When ctx is cancelled, as by the SIGINT or
|
// is released after it. When ctx is cancelled, as by the SIGINT or
|
||||||
// SIGTERM that interruptContext catches, the scan keeps what it has
|
// SIGTERM that interruptContext catches, the scan keeps what it has
|
||||||
// hashed (see syncScan), prints how many files its walk reached, and
|
// 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,
|
func runScan(ctx context.Context, roots []string, workers int,
|
||||||
oneFS bool,
|
oneFS bool,
|
||||||
) error {
|
) error {
|
||||||
if workers < 1 {
|
|
||||||
workers = 1
|
|
||||||
}
|
|
||||||
|
|
||||||
roots, err := resolveRoots(roots)
|
roots, err := resolveRoots(roots)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
|
|||||||
Reference in New Issue
Block a user