Keep the module cache out of the build stage's chown (closes #43)

The build stage handed /src and the whole Go module cache to the
unprivileged user with chown -R, in a layer that re-ran on every source
change. On this host that step took from about 80 s to over ten minutes,
depending on load.

The module cache now sits at /go/pkg/mod and stays root's: bootstrap
fills it as root. The sources are copied with --chown. A small chown,
cached with bootstrap, hands builder the /src directory itself, the
module cache's cache/download directory (where make build saves its
lookup of this module's own version), and the telemetry files root's go
commands left in its home. Tests and the build still run as builder.

Model: opus-5-5

Dockerfile

@@ -51,14 +51,21 @@ RUN echo "gate lint, epoch ${CHECK_EPOCH}" && \
 FROM golang@sha256:56961d79ea8129efddcc0b8643fd8a5416b4e6228cfd477e3fd61deb2672c587 AS builder
 
 # We never build or run as root. Create an unprivileged user and point
-# HOME and the Go caches at its home so go build and go test can write
-# their caches when we drop to it below. $GOPATH/bin is deliberately not
-# on PATH: script/bootstrap no longer `go install`s anything (the linter
-# runs from a pinned image, never from a host install), so nothing lands
+# HOME and the build cache at its home so go build and go test can write
+# it when we drop to it below. $GOPATH/bin is deliberately not on PATH:
+# script/bootstrap no longer `go install`s anything (the linter runs
+# from a pinned image, never from a host install), so nothing lands
 # there and adding it would only widen what this image resolves.
+#
+# The module cache is kept outside that home, at the base image's
+# default /go/pkg/mod, and belongs to root: script/bootstrap fills it as
+# root. Do not move it into the home and hand it over with `chown -R`:
+# that walks every file in it, which took from about 80 s to over ten
+# minutes on a shared host, depending on load.
 RUN adduser -D -u 1000 builder
 ENV HOME=/home/builder
 ENV GOPATH=/home/builder/go
+ENV GOMODCACHE=/go/pkg/mod
 ENV GOCACHE=/home/builder/.cache/go-build
 
 WORKDIR /src
@@ -83,11 +90,22 @@ COPY script/ script/
 COPY go.mod go.sum ./
 RUN script/bootstrap
 
-COPY . .
+# Hand builder only what it writes to, without walking the module cache.
+# This layer stays cached with bootstrap.
+# - /src itself: make build writes the binary into it, and git refuses
+#   a repository whose top directory belongs to another user.
+# - the module cache's cache/download directory itself, not what is in
+#   it: Go only reads the downloaded modules, but make build saves its
+#   lookup of this module's own version from git there, in a new
+#   directory named after the module path.
+# - builder's home: the go commands bootstrap ran as root left Go's
+#   telemetry files there, a few small files.
+RUN chown builder:builder /src /go/pkg/mod/cache/download && \
+    chown -R builder:builder /home/builder
 
-# Hand the sources and caches to the unprivileged user, then drop root
-# before running any checks or builds.
-RUN chown -R builder:builder /src /home/builder
+# The sources are handed to builder as they are copied, so no layer has
+# to walk them. Then drop root before running any checks or builds.
+COPY --chown=builder:builder . .
 USER builder
 
 # Fail the build unless the branch is green. Runs as non-root so the

README.md

@@ -113,8 +113,10 @@ Goals, in order:
     `sfdupes`.
 -   Dependencies: standard library, `github.com/spf13/cobra` for the
     CLI, **one progress-bar library**
-    (`github.com/schollz/progressbar/v3`), and **one SQLite driver**
-    (`modernc.org/sqlite`, pure Go, so builds keep cgo disabled).
+    (`github.com/schollz/progressbar/v3`), `golang.org/x/term` to tell
+    whether stderr is a terminal, **one SQLite driver**
+    (`modernc.org/sqlite`, pure Go, so builds keep cgo disabled), and
+    `golang.org/x/sys` for `flock(2)` (the scan lock, see "Database").
     `github.com/spf13/viper` is permitted if configuration-file support
     is ever needed, but is not currently used. No other third-party
     deps.
@@ -152,16 +154,42 @@ All three subcommands operate on a single SQLite database file:
     use. `report` and `trees` require an existing database; a missing
     database file is a fatal error (exit 1) telling the user to run
     `scan` first.
--   The database uses WAL journal mode and a busy timeout, so running a
-    report while a cron `scan` is in progress is safe. The filesystem
-    is authoritative; the database is an eventually-consistent
-    reflection of it. Hashed records are committed in batched
-    transactions while the scan is still running (keeping the WAL
-    small and letting concurrent reports observe progress), so a
-    report may see a scan's changes partially applied, and a scan
-    that dies partway leaves a valid database holding everything
-    hashed so far; the next scan skips those records and converges
-    toward the filesystem.
+-   Only one `scan` runs against a database at a time. For its whole
+    run, `scan` holds an exclusive `flock(2)` lock on a lock file
+    beside the database, named by appending `.lock` to the database
+    path (`/var/lib/sfdupes/db.sqlite.lock` by default), taken before
+    it walks the filesystem or opens the database. A second `scan`
+    against the same database does not wait: it fails at once with a
+    one-line error naming the lock file and exits 1, without walking
+    anything or opening the database, and the running scan carries on.
+    The lock file is created on first use, open to its owner only, and
+    left in place: a leftover file blocks nothing, because the lock
+    ends with the process holding it however it ends, a fatal error or
+    an interrupt included, and deleting the file while a scan runs
+    would let a second scan start. `report` and `trees` never take the
+    lock, so they run during a scan.
+-   While `scan` runs, the database is in WAL journal mode with a busy
+    timeout, so running a report while a cron `scan` is in progress is
+    safe. The filesystem is authoritative; the database is an
+    eventually-consistent reflection of it. Hashed records are
+    committed in batched transactions while the scan is still running
+    (keeping the WAL small and letting concurrent reports observe
+    progress), so a report may see a scan's changes partially applied,
+    and a scan that dies partway leaves a valid database holding
+    everything hashed so far; the next scan skips those records and
+    converges toward the filesystem.
+-   `scan` switches the database back to rollback-journal mode when it
+    closes it, so between scans the database file alone holds the whole
+    database. Each switch needs the database to itself: a `scan` that
+    starts while a report is still reading waits for it up to the
+    10-second busy timeout, then fails; a `scan` that ends while a
+    report has the database open warns and leaves the database in WAL
+    mode until the next scan.
+-   `report` and `trees` open the database read-only and need only read
+    access to the database file, and no write access to its directory.
+    While the database is in WAL mode they also read the `-wal` and
+    `-shm` files beside it, which SQLite creates with the database
+    file's permissions.
 -   Schema (`PRAGMA user_version` is the schema version, currently 1; a
     database with any other version is a fatal error):
 
@@ -251,6 +279,18 @@ duplicates another or lies under another is dropped before walking,
 so every file is reached exactly once and produces one database
 record.
 
+An operand that is a symlink (never followed, not even as an operand),
+socket, FIFO, or device node, or a directory named `.zfs`, is not
+scanned. `scan` prints a one-line warning naming the path and what it
+is, counts it as skipped, and drops it from the scanned operands before
+reading the database. Another operand beneath it is still scanned. The
+records stored beneath it are not deleted: they are treated like any
+other record outside the scanned operands, including the content-phase
+exception below. If it lies under another operand, they are under that
+operand instead, and are deleted like any other record there that this
+scan did not verify. This is not an error: a scan whose every operand
+is dropped walks nothing and exits 0.
+
 `scan` synchronizes the database with the filesystem state under the
 scanned operands:
 
@@ -356,9 +396,12 @@ during the hash phase:
 Rules for the walk:
 
 -   Only regular files. Skip directories, symlinks (do not follow,
-    including symlink operands), sockets, FIFOs, and device nodes.
+    including symlink operands), sockets, FIFOs, and device nodes. An
+    operand that is a symlink, socket, FIFO, or device node is dropped
+    as described in "`scan` mode" above.
 -   Never descend into a directory named `.zfs` (ZFS snapshot pseudo-dirs;
-    walking them would list every file once per snapshot).
+    walking them would list every file once per snapshot), not even
+    when it is an operand; such an operand is dropped the same way.
 -   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
@@ -369,9 +412,11 @@ Rules for the walk:
     path, and continue. Per-file errors never abort the run; the final
     summary reports how many were skipped. As specified above, a
     skipped path that has a database record from an earlier scan loses
-    that record, unless it failed only in the content phase; an
-    unreadable directory subtree likewise loses its records (accepted:
-    the database mirrors what the latest scan could actually verify).
+    that record, unless it failed only in the content phase, or is an
+    operand dropped before the database was read that lies under no
+    other operand; an unreadable directory subtree likewise loses its
+    records (accepted: the database mirrors what the latest scan could
+    actually verify).
 
 Concurrency: the walk phase (which also stats files), the hash phase,
 and the content phase each use a worker pool of `--workers` workers
@@ -545,22 +590,41 @@ hash: [12345/98765] 12% |████      | 92 files/s elapsed 2:32 eta 17:54
 
 Additional requirements:
 
--   When stderr is not a TTY, do not emit ANSI redraws: print a plain
-    one-line progress update no more often than every 5 seconds instead.
+-   When stderr is not a terminal (a pipe, a file, `/dev/null`), do not
+    emit ANSI redraws: print a plain one-line progress update the moment
+    each phase starts, then no more often than every 5 seconds.
 -   Progress updates are driven from the main goroutine and must be
-    non-blocking with respect to the worker pool.
+    non-blocking with respect to the worker pool. On a terminal the
+    spinner-style displays also redraw on their own several times a
+    second, so their count and elapsed time stay current while a phase
+    waits for its next item.
+-   A warning printed during a phase always lands on a line of its own,
+    never inside the progress display.
 -   `report` and `trees` modes need no progress display, only their
     stderr summaries.
 
 ### Error handling and exit codes
 
 -   `0`: success, even if individual files were skipped with warnings.
--   `1`: fatal error (e.g., a `PATH` operand does not exist, the
-    database cannot be created/opened/read/written, a missing database
-    for `report`/`trees`, stdout write failure).
+-   `1`: fatal error (e.g., a `PATH` operand does not exist, another
+    `scan` is already running against the same database, the database
+    cannot be created/opened/read/written, a missing database for
+    `report`/`trees`, stdout write failure).
 -   `2`: usage error (including `scan` with no `PATH` operand 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:
+
+-   When the reader of a stdout pipe exits early, as in
+    `sfdupes report | head`, the next write ends sfdupes with `SIGPIPE`,
+    quietly and without a summary, the way `cat` or `sort` end. The
+    shell reports the signal (status 141 in most shells), not exit 1.
+-   When stdout is closed outright (`sfdupes report >&-`), the Go
+    runtime opens `/dev/null` in its place before sfdupes starts, so
+    the output is discarded and the run succeeds, as with
+    `> /dev/null`.
+
 ## Entrypoints
 
 This repository adheres to the
@@ -698,7 +762,7 @@ All of the following, run in this directory, must pass:
 
     ```sh
     d=$(mktemp -d)
-    export SFDUPES_DATABASE="$d/db.sqlite"
+    export SFDUPES_DATABASE="$(mktemp -d)/db.sqlite"
     mkdir -p "$d/a" "$d/b"
     head -c 2000 /dev/urandom > "$d/a/one.bin"
     cp "$d/a/one.bin" "$d/b/copy.bin"
@@ -726,9 +790,9 @@ All of the following, run in this directory, must pass:
     ./sfdupes report
     ```
 
-    (The scan database lives inside `$d` here purely for test hygiene;
-    scanning `$d` therefore also records the SQLite file itself, which
-    is harmless.)
+    (The database lives in a temp directory of its own: inside `$d`,
+    the scan would record it, and its empty lock file would join the
+    `empty1`/`empty2` group.)
 
     Expected from the first `report`: `one.bin`/`copy.bin`/`copy2.bin`
     form one group (two dupe rows, `first` is the lexicographically

TODO.md

@@ -29,9 +29,29 @@
 
 # Completed Steps
 
-- refuse an unversioned database that already has a `files` table with
-  a clear schema-version error (2026-10-03,
-  https://git.eeqj.de/sneak/sfdupes/issues/11)
+- the `Dockerfile` build stage keeps the Go module cache out of `builder`'s
+  home and copies the sources with `--chown`, so no `chown -R` walks them
+  (2026-10-04, https://git.eeqj.de/sneak/sfdupes/issues/43)
+
+- progress prints at once on a non-terminal, uses a real terminal test,
+  and prints warnings through a spinner instead of racing its redraw
+  (2026-10-03, https://git.eeqj.de/sneak/sfdupes/issues/13)
+
+- warn about and skip symlink, socket, FIFO, device and `.zfs`
+  operands, keeping the records beneath them (2026-10-03,
+  https://git.eeqj.de/sneak/sfdupes/issues/9)
+
+- `scan` holds a lock on a lock file beside the database for its whole run,
+  so a second `scan` fails at once with exit 1 (2026-10-03,
+  https://git.eeqj.de/sneak/sfdupes/issues/53)
+
+- test stdout write failures in `report` and `trees`; README states that
+  `| head` ends sfdupes by `SIGPIPE` and `>&-` writes to `/dev/null`
+  (2026-10-03, https://git.eeqj.de/sneak/sfdupes/issues/30)
+
+- `report` and `trees` open the database read-only, and `scan` leaves it
+  out of WAL mode, so reading needs only read access (2026-10-03, closes
+  https://git.eeqj.de/sneak/sfdupes/issues/8)
 
 - escape tabs, newlines, carriage returns and backslashes in report,
   trees and warning paths; the root directory's path is `/`

db.go

@@ -11,6 +11,7 @@ import (
 	"slices"
 	"strconv"
 
+	"golang.org/x/sys/unix"
 	// The pure-Go SQLite driver, registered as "sqlite"; keeps cgo
 	// disabled.
 	_ "modernc.org/sqlite"
@@ -32,6 +33,11 @@ const schemaVersion = 1
 // scan.
 const dbDirPerm = 0o755
 
+// lockFilePerm is the mode for the scan lock file. Anyone who can open
+// the file can hold the lock and keep every scan from running, so it
+// is open to its owner only.
+const lockFilePerm = 0o600
+
 // createTableSQL is the schema applied to a fresh database. Paths are
 // BLOBs because Unix paths are raw bytes, not guaranteed UTF-8.
 const createTableSQL = `
@@ -64,6 +70,10 @@ var errNoDatabase = errors.New(
 // does not understand.
 var errSchemaVersion = errors.New("unsupported database schema version")
 
+// errScanRunning reports that another scan holds the lock on the
+// database.
+var errScanRunning = errors.New("another scan is running")
+
 // databasePath resolves the database location: SFDUPES_DATABASE when
 // set and non-empty, the compiled-in default otherwise.
 func databasePath() string {
@@ -74,16 +84,24 @@ func databasePath() string {
 	return defaultDatabasePath
 }
 
-// openDB opens the SQLite database at path with WAL journaling and a
-// busy timeout, so a report can run while a cron scan is in progress.
-// It does not create or verify the schema.
-func openDB(path string) (*sql.DB, error) {
-	dsn := "file:" + path +
-		"?_pragma=busy_timeout(10000)" +
-		"&_pragma=journal_mode(WAL)" +
-		"&_pragma=synchronous(NORMAL)"
+// scanParams are the connection parameters for scan: read-write, with
+// WAL journaling and a busy timeout, so a report can run while a cron
+// scan is in progress. closeScanDatabase leaves WAL mode again.
+const scanParams = "_pragma=busy_timeout(10000)" +
+	"&_pragma=journal_mode(WAL)" +
+	"&_pragma=synchronous(NORMAL)"
 
-	db, err := sql.Open("sqlite", dsn)
+// reportParams are the connection parameters for report and trees:
+// read-only, with the same busy timeout. They set no journal mode,
+// because setting one is a write.
+const reportParams = "mode=ro" +
+	"&_pragma=busy_timeout(10000)" +
+	"&_pragma=query_only(1)"
+
+// openDB opens the SQLite database at path with the connection
+// parameters params. It does not create or verify the schema.
+func openDB(path, params string) (*sql.DB, error) {
+	db, err := sql.Open("sqlite", "file:"+path+"?"+params)
 	if err != nil {
 		return nil, fmt.Errorf("open database %s: %w", path, err)
 	}
@@ -96,6 +114,43 @@ func openDB(path string) (*sql.DB, error) {
 	return db, nil
 }
 
+// lockScanDatabase takes the lock that keeps a second scan off the
+// database at path: an exclusive flock(2) on the file beside it named
+// path with ".lock" appended, created along with the database's parent
+// directory if missing. A lock held by another scan fails at once
+// instead of waiting. The lock lasts until the returned file is closed
+// or the process ends. The file is never deleted: a scan that deleted
+// it would let the next scan lock a new file while another still holds
+// the old one.
+func lockScanDatabase(path string) (*os.File, error) {
+	err := os.MkdirAll(filepath.Dir(path), dbDirPerm)
+	if err != nil {
+		return nil, fmt.Errorf("create database directory: %w", err)
+	}
+
+	lockPath := path + ".lock"
+
+	//nolint:gosec // the operator chooses the database path
+	f, err := os.OpenFile(lockPath, os.O_RDWR|os.O_CREATE, lockFilePerm)
+	if err != nil {
+		return nil, err
+	}
+
+	err = unix.Flock(int(f.Fd()), unix.LOCK_EX|unix.LOCK_NB)
+	if err != nil {
+		_ = f.Close()
+
+		if errors.Is(err, unix.EWOULDBLOCK) {
+			return nil, fmt.Errorf("%w (lock held on %s)",
+				errScanRunning, lockPath)
+		}
+
+		return nil, fmt.Errorf("lock %s: %w", lockPath, err)
+	}
+
+	return f, nil
+}
+
 // openScanDatabase opens the database for the scan subcommand, creating
 // the file, its parent directory, and the schema as needed.
 func openScanDatabase(ctx context.Context, path string) (*sql.DB, error) {
@@ -104,7 +159,7 @@ func openScanDatabase(ctx context.Context, path string) (*sql.DB, error) {
 		return nil, fmt.Errorf("create database directory: %w", err)
 	}
 
-	db, err := openDB(path)
+	db, err := openDB(path, scanParams)
 	if err != nil {
 		return nil, err
 	}
@@ -119,6 +174,24 @@ func openScanDatabase(ctx context.Context, path string) (*sql.DB, error) {
 	return db, nil
 }
 
+// closeScanDatabase switches the database at path from WAL back to
+// rollback-journal mode and closes it. Out of WAL mode the database
+// file alone holds the whole database, so a reader needs no -wal or
+// -shm file beside it, nor write access to create them. The switch
+// fails while a report has the database open; the database then stays
+// in WAL mode, still readable, until a later scan closes it.
+func closeScanDatabase(ctx context.Context, db *sql.DB, path string) {
+	// Runs on the way out of a cancelled scan too.
+	_, err := db.ExecContext(context.WithoutCancel(ctx),
+		"PRAGMA journal_mode = DELETE")
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "scan: database %s left in WAL mode: %v\n",
+			path, err)
+	}
+
+	_ = db.Close()
+}
+
 // openReportDatabase opens an existing database for the report and
 // trees subcommands. A missing database file is an error directing the
 // user to run scan first; the schema version must match exactly.
@@ -134,7 +207,7 @@ func openReportDatabase(ctx context.Context,
 		return nil, fmt.Errorf("database: %w", err)
 	}
 
-	db, err := openDB(path)
+	db, err := openDB(path, reportParams)
 	if err != nil {
 		return nil, err
 	}
@@ -176,30 +249,9 @@ func initSchema(ctx context.Context, db *sql.DB) error {
 }
 
 // createSchema applies the schema to a fresh database and stamps the
-// schema version. A database with user_version 0 that already has a
-// files table was not created by this build — a foreign or partially
-// initialized file. Adopting it silently could corrupt unrelated data,
-// so that is a fatal schema-version error telling the operator to
-// remove the file and rescan.
+// schema version.
 func createSchema(ctx context.Context, db *sql.DB) error {
-	var name string
-
-	err := db.QueryRowContext(ctx,
-		"SELECT name FROM sqlite_master "+
-			"WHERE type = 'table' AND name = 'files'").Scan(&name)
-
-	switch {
-	case err == nil:
-		return fmt.Errorf(
-			"has a files table but no schema version; "+
-				"remove the file and rescan: %w", errSchemaVersion)
-	case errors.Is(err, sql.ErrNoRows):
-		// Genuinely empty: create the schema below.
-	default:
-		return fmt.Errorf("check for files table: %w", err)
-	}
-
-	_, err = db.ExecContext(ctx, createTableSQL)
+	_, err := db.ExecContext(ctx, createTableSQL)
 	if err != nil {
 		return fmt.Errorf("create schema: %w", err)
 	}

db_test.go

@@ -5,6 +5,7 @@ import (
 	"database/sql"
 	"errors"
 	"fmt"
+	"os"
 	"path/filepath"
 	"slices"
 	"strings"
@@ -78,38 +79,6 @@ func TestOpenScanDatabaseCreates(t *testing.T) {
 	}
 }
 
-func TestOpenScanDatabaseUnversionedForeign(t *testing.T) {
-	t.Parallel()
-
-	path := testDBPath(t)
-
-	// A database that has a files table but user_version 0 — a foreign
-	// or partially initialized file. scan must refuse it with a clear
-	// schema-version error, not adopt it and not emit a raw SQLite
-	// "table files already exists".
-	db, err := openDB(path)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	_, err = db.ExecContext(t.Context(), "CREATE TABLE files (x INTEGER)")
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	_ = db.Close()
-
-	_, err = openScanDatabase(t.Context(), path)
-	if !errors.Is(err, errSchemaVersion) {
-		t.Fatalf("err = %v, want errSchemaVersion", err)
-	}
-
-	if !strings.Contains(err.Error(), "remove the file and rescan") {
-		t.Fatalf("err = %v, want it to tell the operator to remove and rescan",
-			err)
-	}
-}
-
 func TestOpenReportDatabaseMissing(t *testing.T) {
 	t.Parallel()
 
@@ -162,6 +131,49 @@ func TestOpenReportDatabaseOK(t *testing.T) {
 	_ = db.Close()
 }
 
+func TestCloseScanDatabaseWhileReportOpen(t *testing.T) {
+	t.Parallel()
+
+	// A report holding the database open stops scan from taking it out
+	// of WAL mode. The -wal and -shm files must then stay beside it, so
+	// that a later report still needs only read access.
+	path := testDBPath(t)
+
+	scanDB, err := openScanDatabase(t.Context(), path)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	reportDB, err := openReportDatabase(t.Context(), path)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	closeScanDatabase(t.Context(), scanDB, path)
+
+	_ = reportDB.Close()
+
+	_, err = os.Stat(path + "-wal")
+	if err != nil {
+		t.Fatalf("no -wal left: the switch out of WAL mode was not "+
+			"stopped: %v", err)
+	}
+
+	makeReadOnly(t, path)
+
+	reportDB, err = openReportDatabase(t.Context(), path)
+	if err != nil {
+		t.Fatalf("openReportDatabase: %v", err)
+	}
+
+	defer func() { _ = reportDB.Close() }()
+
+	_, err = loadFileRows(t.Context(), reportDB)
+	if err != nil {
+		t.Fatalf("loadFileRows: %v", err)
+	}
+}
+
 func TestApplyChangesRoundTrip(t *testing.T) {
 	t.Parallel()
 

go.mod

@@ -5,6 +5,8 @@ go 1.25.7
 require (
 	github.com/schollz/progressbar/v3 v3.19.1
 	github.com/spf13/cobra v1.10.2
+	golang.org/x/sys v0.46.0
+	golang.org/x/term v0.44.0
 	modernc.org/sqlite v1.54.0
 )
 
@@ -18,8 +20,6 @@ require (
 	github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec // indirect
 	github.com/rivo/uniseg v0.4.7 // indirect
 	github.com/spf13/pflag v1.0.9 // indirect
-	golang.org/x/sys v0.46.0 // indirect
-	golang.org/x/term v0.44.0 // indirect
 	modernc.org/libc v1.74.1 // indirect
 	modernc.org/mathutil v1.7.1 // indirect
 	modernc.org/memory v1.11.0 // indirect

main.go

@@ -57,22 +57,27 @@ var errNoSubcommand = errors.New("no subcommand")
 var Version = "dev"
 
 func main() {
-	os.Exit(run(os.Args[1:], os.Stderr))
+	// Once the reader of a stdout pipe has gone, as in "sfdupes report |
+	// head", the Go runtime ends the process with SIGPIPE on the next
+	// write instead of returning an error (README "Error handling").
+	// Registering for SIGPIPE with os/signal would change that.
+	os.Exit(run(os.Args[1:], os.Stdout, os.Stderr))
 }
 
 // run executes args against the command tree and returns the process
 // exit code. It is the program's single exit point: the subcommands
 // return their errors instead of exiting, so every deferred cleanup —
 // above all closing the database, which checkpoints the SQLite WAL —
-// runs before the process ends.
-func run(args []string, stderr io.Writer) int {
+// runs before the process ends. The report and trees subcommands write
+// their data to stdout.
+func run(args []string, stdout, stderr io.Writer) int {
 	// A nil slice makes cobra fall back to os.Args, which would let a
 	// test binary's own flags reach the command tree.
 	if args == nil {
 		args = []string{}
 	}
 
-	root := newRootCommand(stderr)
+	root := newRootCommand(stdout, stderr)
 	root.SetArgs(args)
 
 	err := root.Execute()
@@ -98,7 +103,7 @@ func run(args []string, stderr io.Writer) int {
 // newRootCommand builds the command tree. Everything on stdout is
 // machine-readable data; all human-facing output (help, usage, errors)
 // goes to stderr.
-func newRootCommand(stderr io.Writer) *cobra.Command {
+func newRootCommand(stdout, stderr io.Writer) *cobra.Command {
 	root := &cobra.Command{
 		Use:     "sfdupes",
 		Short:   "Find candidate duplicate files by size and head/tail/content SHA-256",
@@ -140,7 +145,7 @@ func newRootCommand(stderr io.Writer) *cobra.Command {
 		Short: "Read the scan database and print the file-level duplicates report",
 		Args:  cobra.NoArgs,
 		RunE: runE(func(ctx context.Context, _ []string) error {
-			return runReport(ctx)
+			return runReport(ctx, stdout)
 		}),
 	}
 
@@ -149,7 +154,7 @@ func newRootCommand(stderr io.Writer) *cobra.Command {
 		Short: "Read the scan database and print the duplicate-tree report",
 		Args:  cobra.NoArgs,
 		RunE: runE(func(ctx context.Context, _ []string) error {
-			return runTrees(ctx)
+			return runTrees(ctx, stdout)
 		}),
 	}
 

main_test.go

@@ -44,23 +44,61 @@ func assertNoSidecars(t *testing.T, path string) {
 	}
 }
 
-// captureStdout redirects os.Stdout to a file for the rest of the test
-// and returns a function reading back everything written to it. Only
-// machine-readable data belongs on stdout (README design goal 4), so
-// the tests assert on it directly.
-func captureStdout(t *testing.T) func() string {
+// makeReadOnly takes write permission away from the database at path,
+// from any WAL sidecar beside it, and from their directory, as for a
+// user reading a database that a root cron scan keeps. Root ignores
+// file permissions, so it skips the test when run as root.
+func makeReadOnly(t *testing.T, path string) {
 	t.Helper()
 
-	f, err := os.Create(filepath.Join(t.TempDir(), "stdout"))
+	if os.Geteuid() == 0 {
+		t.Skip("root ignores file permissions")
+	}
+
+	err := os.Chmod(path, 0o400)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	saved := os.Stdout
-	os.Stdout = f
+	for _, suffix := range walSuffixes {
+		err = os.Chmod(path+suffix, 0o400)
+		if err != nil && !errors.Is(err, fs.ErrNotExist) {
+			t.Fatal(err)
+		}
+	}
+
+	dir := filepath.Dir(path)
+
+	//nolint:gosec // reaching the database needs the search bit
+	err = os.Chmod(dir, 0o500)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Runs before t.TempDir's own cleanup, which must delete the files.
+	t.Cleanup(func() {
+		//nolint:gosec // removing the directory needs its search bit back
+		_ = os.Chmod(dir, 0o700)
+	})
+}
+
+// captureStderr redirects os.Stderr to a file for the rest of the test
+// and returns a function reading back everything written to it. scan
+// writes its warnings and summary straight to os.Stderr, not to the
+// stderr writer run is given.
+func captureStderr(t *testing.T) func() string {
+	t.Helper()
+
+	f, err := os.Create(filepath.Join(t.TempDir(), "stderr"))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	saved := os.Stderr
+	os.Stderr = f
 
 	t.Cleanup(func() {
-		os.Stdout = saved
+		os.Stderr = saved
 
 		_ = f.Close()
 	})
@@ -91,13 +129,13 @@ func captureStdout(t *testing.T) func() string {
 // brokenDatabase writes a database that opens cleanly and passes the
 // schema-version check but has no files table, so the first query
 // fails with the database already open: a fatal error on a path that
-// owns an open database.
+// owns an open database. It closes the database the way scan does.
 func brokenDatabase(t *testing.T) string {
 	t.Helper()
 
 	path := testDBPath(t)
 
-	db, err := openDB(path)
+	db, err := openDB(path, scanParams)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -108,10 +146,7 @@ func brokenDatabase(t *testing.T) string {
 		t.Fatal(err)
 	}
 
-	err = db.Close()
-	if err != nil {
-		t.Fatal(err)
-	}
+	closeScanDatabase(t.Context(), db, path)
 
 	return path
 }
@@ -144,7 +179,10 @@ func TestOpenDatabaseKeepsWALWhileOpen(t *testing.T) {
 
 func TestRunFatalAfterOpenClosesDatabase(t *testing.T) {
 	// Every subcommand that owns an open database must close it when
-	// it fails: no os.Exit between the open and the return.
+	// it fails: no os.Exit between the open and the return. The
+	// sidecar check is evidence of the close only for scan: report and
+	// trees only read a database that is out of WAL mode, which leaves
+	// nothing on disk whether they close it or not.
 	cases := map[string][]string{
 		cmdScan:   {cmdScan},
 		cmdReport: {cmdReport},
@@ -160,17 +198,15 @@ func TestRunFatalAfterOpenClosesDatabase(t *testing.T) {
 				args = append(args, t.TempDir())
 			}
 
-			var stderr bytes.Buffer
+			var stdout, stderr bytes.Buffer
 
-			stdout := captureStdout(t)
-
-			code := run(args, &stderr)
+			code := run(args, &stdout, &stderr)
 			if code != exitFatal {
 				t.Errorf("run(%v) = %d, want %d", args, code, exitFatal)
 			}
 
 			assertNoSidecars(t, path)
-			assertFatalOutput(t, stderr.String(), stdout())
+			assertFatalOutput(t, stderr.String(), stdout.String())
 
 			// Proof that the failure happened after the open: only a
 			// query against the opened database can report this.
@@ -188,18 +224,16 @@ func TestRunMissingOperandIsFatalNotUsage(t *testing.T) {
 	// must not dump the usage text.
 	t.Setenv(databaseEnv, testDBPath(t))
 
-	var stderr bytes.Buffer
-
-	stdout := captureStdout(t)
+	var stdout, stderr bytes.Buffer
 
 	missing := filepath.Join(t.TempDir(), "nope")
 
-	code := run([]string{cmdScan, missing}, &stderr)
+	code := run([]string{cmdScan, missing}, &stdout, &stderr)
 	if code != exitFatal {
 		t.Errorf("run(scan %s) = %d, want %d", missing, code, exitFatal)
 	}
 
-	assertFatalOutput(t, stderr.String(), stdout())
+	assertFatalOutput(t, stderr.String(), stdout.String())
 }
 
 // assertFatalOutput checks that a fatal error was reported the way
@@ -243,11 +277,9 @@ func TestRunUsageErrors(t *testing.T) {
 			// path that does not exist.
 			t.Setenv(databaseEnv, testDBPath(t))
 
-			var stderr bytes.Buffer
+			var stdout, stderr bytes.Buffer
 
-			stdout := captureStdout(t)
-
-			code := run(tc.args, &stderr)
+			code := run(tc.args, &stdout, &stderr)
 			if code != exitUsage {
 				t.Errorf("run(%v) = %d, want %d", tc.args, code, exitUsage)
 			}
@@ -256,7 +288,7 @@ func TestRunUsageErrors(t *testing.T) {
 				t.Errorf("stderr = %q, want %q", stderr.String(), tc.want)
 			}
 
-			if got := stdout(); got != "" {
+			if got := stdout.String(); got != "" {
 				t.Errorf("stdout = %q, want nothing (data only)", got)
 			}
 		})
@@ -265,9 +297,9 @@ func TestRunUsageErrors(t *testing.T) {
 
 // TestRunHelpAndVersionSucceed checks that the two informational flags
 // exit 0 and keep their human-facing output on stderr.
-//
-//nolint:paralleltest // captureStdout replaces the process-wide os.Stdout
 func TestRunHelpAndVersionSucceed(t *testing.T) {
+	t.Parallel()
+
 	assertHumanOutput(t, "--help")
 	assertHumanOutput(t, "--version")
 }
@@ -278,11 +310,9 @@ func TestRunHelpAndVersionSucceed(t *testing.T) {
 func assertHumanOutput(t *testing.T, arg string) {
 	t.Helper()
 
-	var stderr bytes.Buffer
+	var stdout, stderr bytes.Buffer
 
-	stdout := captureStdout(t)
-
-	code := run([]string{arg}, &stderr)
+	code := run([]string{arg}, &stdout, &stderr)
 	if code != exitOK {
 		t.Errorf("run(%s) = %d, want %d", arg, code, exitOK)
 	}
@@ -291,7 +321,7 @@ func assertHumanOutput(t *testing.T, arg string) {
 		t.Errorf("run(%s) wrote nothing to stderr", arg)
 	}
 
-	if got := stdout(); got != "" {
+	if got := stdout.String(); got != "" {
 		t.Errorf("stdout = %q, want nothing (data only)", got)
 	}
 }
@@ -320,21 +350,31 @@ func scanFixture(t *testing.T) []string {
 		t.Fatal(err)
 	}
 
-	var stderr bytes.Buffer
+	scanOK(t, dir)
 
-	stdout := captureStdout(t)
+	return dupes
+}
 
-	code := run([]string{cmdScan, dir}, &stderr)
+// scanOK runs scan over operands, fails the test unless it exits 0 with
+// nothing on stdout, and returns everything it printed to stderr.
+func scanOK(t *testing.T, operands ...string) string {
+	t.Helper()
+
+	var stdout bytes.Buffer
+
+	stderr := captureStderr(t)
+
+	code := run(append([]string{cmdScan}, operands...), &stdout, os.Stderr)
 	if code != exitOK {
-		t.Fatalf("run(scan) = %d, want %d; stderr: %s",
-			code, exitOK, stderr.String())
+		t.Fatalf("run(scan %q) = %d, want %d; stderr: %s",
+			operands, code, exitOK, stderr())
 	}
 
-	if got := stdout(); got != "" {
+	if got := stdout.String(); got != "" {
 		t.Errorf("scan stdout = %q, want nothing (data only)", got)
 	}
 
-	return dupes
+	return stderr()
 }
 
 func TestRunScanSucceedsDespiteWarnings(t *testing.T) {
@@ -345,24 +385,119 @@ func TestRunScanSucceedsDespiteWarnings(t *testing.T) {
 	assertNoSidecars(t, path)
 }
 
+func TestRunScanSkipsSymlinkOperand(t *testing.T) {
+	path := testDBPath(t)
+	t.Setenv(databaseEnv, path)
+
+	dir := t.TempDir()
+	writeFile(t, dir, "target/sub/f", pattern(1, 10))
+
+	link := filepath.Join(dir, "link")
+
+	err := os.Symlink(filepath.Join(dir, "target"), link)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Scanning a directory through the symlink stores a record beneath
+	// the symlink's own path for a file beneath its target.
+	scanOK(t, filepath.Join(link, "sub"))
+
+	assertOperandSkipped(t, path, link, "symlink",
+		filepath.Join(link, "sub", "f"))
+}
+
+func TestRunScanWalksOperandUnderSymlinkOperand(t *testing.T) {
+	path := testDBPath(t)
+	t.Setenv(databaseEnv, path)
+
+	dir := t.TempDir()
+	writeFile(t, dir, "target/sub/f", pattern(1, 10))
+
+	link := filepath.Join(dir, "link")
+
+	err := os.Symlink(filepath.Join(dir, "target"), link)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// link is dropped as a symlink, but link/sub must still be scanned,
+	// not dropped as lying under link.
+	scanOK(t, link, filepath.Join(link, "sub"))
+
+	db, err := openDB(path, reportParams)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	t.Cleanup(func() { _ = db.Close() })
+
+	recordByPath(t, dbRecords(t, db), filepath.Join(link, "sub", "f"))
+}
+
+func TestRunScanSkipsZFSOperand(t *testing.T) {
+	path := testDBPath(t)
+	t.Setenv(databaseEnv, path)
+
+	zfs := filepath.Join(t.TempDir(), ".zfs")
+	snapshot := filepath.Join(zfs, "snapshot", "hourly")
+	f := writeFile(t, snapshot, "f", pattern(1, 10))
+
+	// An operand beneath a .zfs directory is walked, because it is not
+	// itself named .zfs.
+	scanOK(t, snapshot)
+
+	assertOperandSkipped(t, path, zfs, ".zfs directory", f)
+}
+
+// assertOperandSkipped scans operand alone and checks that it is skipped
+// as kind: a warning naming it, one skip in the summary, exit 0, and the
+// record for kept, which an earlier scan stored beneath operand, still
+// in the database at dbPath.
+func assertOperandSkipped(t *testing.T, dbPath, operand, kind,
+	kept string,
+) {
+	t.Helper()
+
+	stderr := scanOK(t, operand)
+
+	warning := "walk " + operand + ": skipping " + kind + " operand\n"
+	if !strings.Contains(stderr, warning) {
+		t.Errorf("stderr = %q, want %q", stderr, warning)
+	}
+
+	summary := "scan: 0 files seen (0 added, 0 updated, 0 removed, " +
+		"0 unchanged), 1 skipped\n"
+	if !strings.Contains(stderr, summary) {
+		t.Errorf("stderr = %q, want %q", stderr, summary)
+	}
+
+	db, err := openDB(dbPath, reportParams)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	t.Cleanup(func() { _ = db.Close() })
+
+	recordByPath(t, dbRecords(t, db), kept)
+}
+
 func TestRunReportSucceeds(t *testing.T) {
 	path := testDBPath(t)
 	t.Setenv(databaseEnv, path)
 
 	dupes := scanFixture(t)
 
-	var stderr bytes.Buffer
+	var stdout, stderr bytes.Buffer
 
-	stdout := captureStdout(t)
-
-	code := run([]string{cmdReport}, &stderr)
+	code := run([]string{cmdReport}, &stdout, &stderr)
 	if code != exitOK {
 		t.Fatalf("run(report) = %d, want %d; stderr: %s",
 			code, exitOK, stderr.String())
 	}
 
 	want := "first\tdupe\tsize\n" + dupes[0] + "\t" + dupes[1] + "\t300\n"
-	if got := stdout(); got != want {
+	if got := stdout.String(); got != want {
 		t.Errorf("stdout = %q, want %q", got, want)
 	}
 
@@ -375,11 +510,9 @@ func TestRunTreesSucceeds(t *testing.T) {
 
 	dupes := scanFixture(t)
 
-	var stderr bytes.Buffer
+	var stdout, stderr bytes.Buffer
 
-	stdout := captureStdout(t)
-
-	code := run([]string{cmdTrees}, &stderr)
+	code := run([]string{cmdTrees}, &stdout, &stderr)
 	if code != exitOK {
 		t.Fatalf("run(trees) = %d, want %d; stderr: %s",
 			code, exitOK, stderr.String())
@@ -389,9 +522,197 @@ func TestRunTreesSucceeds(t *testing.T) {
 	// trees of each other.
 	want := "first\tdupe\tfiles\tsize\n" +
 		filepath.Dir(dupes[0]) + "\t" + filepath.Dir(dupes[1]) + "\t1\t300\n"
-	if got := stdout(); got != want {
+	if got := stdout.String(); got != want {
 		t.Errorf("stdout = %q, want %q", got, want)
 	}
 
 	assertNoSidecars(t, path)
 }
+
+func TestRunReportsNeedOnlyReadAccess(t *testing.T) {
+	// README §Database: report and trees need only read access to the
+	// database file. With its directory read-only as well, SQLite
+	// cannot create any file beside it.
+	path := testDBPath(t)
+	t.Setenv(databaseEnv, path)
+
+	dupes := scanFixture(t)
+	assertNoSidecars(t, path)
+	makeReadOnly(t, path)
+
+	cases := map[string]string{
+		cmdReport: "first\tdupe\tsize\n" +
+			dupes[0] + "\t" + dupes[1] + "\t300\n",
+		cmdTrees: "first\tdupe\tfiles\tsize\n" +
+			filepath.Dir(dupes[0]) + "\t" + filepath.Dir(dupes[1]) +
+			"\t1\t300\n",
+	}
+
+	for name, want := range cases {
+		var stdout, stderr bytes.Buffer
+
+		code := run([]string{name}, &stdout, &stderr)
+		if code != exitOK {
+			t.Errorf("run(%s) = %d, want %d; stderr: %s",
+				name, code, exitOK, stderr.String())
+
+			continue
+		}
+
+		if got := stdout.String(); got != want {
+			t.Errorf("%s stdout = %q, want %q", name, got, want)
+		}
+	}
+}
+
+// holdScanLock takes the lock on the database at path, as a running
+// scan does, and holds it until the test ends. It fails the test when
+// the lock is already held.
+func holdScanLock(t *testing.T, path string) {
+	t.Helper()
+
+	lock, err := lockScanDatabase(path)
+	if err != nil {
+		t.Fatalf("lock %s: %v", path, err)
+	}
+
+	t.Cleanup(func() { _ = lock.Close() })
+}
+
+func TestRunSecondScanFails(t *testing.T) {
+	// README §Database: while one scan holds the lock, a second scan
+	// fails at once, naming the lock file, without creating the
+	// database.
+	path := testDBPath(t)
+	t.Setenv(databaseEnv, path)
+
+	holdScanLock(t, path)
+
+	var stdout, stderr bytes.Buffer
+
+	code := run([]string{cmdScan, t.TempDir()}, &stdout, &stderr)
+	if code != exitFatal {
+		t.Errorf("run(scan) = %d, want %d", code, exitFatal)
+	}
+
+	want := "sfdupes: another scan is running (lock held on " +
+		path + ".lock)\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(path)
+	if !errors.Is(err, fs.ErrNotExist) {
+		t.Errorf("stat %s = %v, want the database not created", path, err)
+	}
+}
+
+func TestRunScanReleasesLock(t *testing.T) {
+	// README §Database: a scan releases the lock however it ends.
+	t.Run("success", func(t *testing.T) {
+		path := testDBPath(t)
+		t.Setenv(databaseEnv, path)
+
+		scanFixture(t)
+		holdScanLock(t, path)
+	})
+
+	t.Run("fatal error", func(t *testing.T) {
+		path := brokenDatabase(t)
+		t.Setenv(databaseEnv, path)
+
+		code := run([]string{cmdScan, t.TempDir()}, io.Discard, io.Discard)
+		if code != exitFatal {
+			t.Fatalf("run(scan) = %d, want %d", code, exitFatal)
+		}
+
+		holdScanLock(t, path)
+	})
+}
+
+func TestRunReportsDuringScan(t *testing.T) {
+	// README §Database: report and trees never take the lock, so they
+	// run while a scan holds it.
+	path := testDBPath(t)
+	t.Setenv(databaseEnv, path)
+
+	scanFixture(t)
+	holdScanLock(t, path)
+
+	for _, name := range []string{cmdReport, cmdTrees} {
+		var stderr bytes.Buffer
+
+		code := run([]string{name}, io.Discard, &stderr)
+		if code != exitOK {
+			t.Errorf("run(%s) = %d, want %d; stderr: %s",
+				name, code, exitOK, stderr.String())
+		}
+	}
+}
+
+func TestRunStdoutClosedIsFatal(t *testing.T) {
+	// README §Error handling: a stdout write failure exits 1, reported
+	// in one line on stderr.
+	for _, name := range []string{cmdReport, cmdTrees} {
+		t.Run(name, func(t *testing.T) {
+			t.Setenv(databaseEnv, testDBPath(t))
+
+			scanFixture(t)
+
+			stdout, err := os.Create(filepath.Join(t.TempDir(), "stdout"))
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			err = stdout.Close()
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			var stderr bytes.Buffer
+
+			code := run([]string{name}, stdout, &stderr)
+			if code != exitFatal {
+				t.Errorf("run(%s) = %d, want %d", name, code, exitFatal)
+			}
+
+			got := stderr.String()
+			if !strings.HasPrefix(got, "sfdupes: write stdout: ") ||
+				!strings.Contains(got, os.ErrClosed.Error()) ||
+				strings.Count(got, "\n") != 1 {
+				t.Errorf("stderr = %q, want one line reporting the "+
+					"failed stdout write", got)
+			}
+		})
+	}
+}
+
+// errWriteFailed is the error failingWriter returns.
+var errWriteFailed = errors.New("write failed")
+
+// failingWriter is a stdout that fails every write.
+type failingWriter struct{}
+
+func (failingWriter) Write([]byte) (int, error) { return 0, errWriteFailed }
+
+func TestStdoutWriteErrorPropagates(t *testing.T) {
+	t.Setenv(databaseEnv, testDBPath(t))
+
+	scanFixture(t)
+
+	cases := map[string]func(context.Context, io.Writer) error{
+		cmdReport: runReport,
+		cmdTrees:  runTrees,
+	}
+
+	for name, fn := range cases {
+		err := fn(t.Context(), failingWriter{})
+		if !errors.Is(err, errWriteFailed) {
+			t.Errorf("%s: error = %v, want %v", name, err, errWriteFailed)
+		}
+	}
+}

progress.go

@@ -6,6 +6,7 @@ import (
 	"time"
 
 	"github.com/schollz/progressbar/v3"
+	"golang.org/x/term"
 )
 
 // plainInterval is the minimum time between progress lines when stderr
@@ -24,24 +25,23 @@ const percentScale = 100
 
 // stderrIsTTY reports whether stderr is attached to a terminal.
 func stderrIsTTY() bool {
-	fi, err := os.Stderr.Stat()
-	if err != nil {
-		return false
-	}
-
-	return fi.Mode()&os.ModeCharDevice != 0
+	return term.IsTerminal(int(os.Stderr.Fd()))
 }
 
 // progress renders one scan pass's progress on stderr. On a TTY it
 // delegates to the progressbar library (spinner style when the total is
 // unknown, full bar with count/percent/rate/elapsed/ETA otherwise). When
 // stderr is not a TTY it emits no ANSI redraws: it prints a plain
-// one-line update no more often than every plainInterval.
+// one-line update as the pass starts, then no more often than every
+// plainInterval.
 //
-// All methods must be called from the main goroutine only. A nil
-// *progress is a valid no-display receiver: every method is a no-op,
-// so batched database flushes during the streaming pass can reuse the
-// update-pass helpers without rendering anything.
+// All methods must be called from the main goroutine only. On a TTY
+// the library also redraws a spinner from its own goroutine, several
+// times a second, so its count and elapsed time stay current while a
+// pass waits for its next item. A nil *progress is a valid
+// no-display receiver: every method is a no-op, so batched database
+// flushes during the streaming pass can reuse the update-pass helpers
+// without rendering anything.
 type progress struct {
 	label string
 	total int64 // -1 when unknown (walk pass)
@@ -53,10 +53,22 @@ type progress struct {
 
 func newProgress(label string, total int64) *progress {
 	p := &progress{label: label, total: total, start: time.Now()}
-	if !stderrIsTTY() {
+	if stderrIsTTY() {
+		p.bar = newBar(label, total)
+
 		return p
 	}
 
+	// Print the zero state at once: the first item may take minutes,
+	// and a pass must never look hung.
+	p.last = p.start
+	fmt.Fprintln(os.Stderr, p.plainLine())
+
+	return p
+}
+
+// newBar builds the TTY display for newProgress.
+func newBar(label string, total int64) *progressbar.ProgressBar {
 	opts := []progressbar.Option{
 		progressbar.OptionSetWriter(os.Stderr),
 		progressbar.OptionSetDescription(label),
@@ -81,9 +93,7 @@ func newProgress(label string, total int64) *progress {
 		)
 	}
 
-	p.bar = progressbar.NewOptions64(total, opts...)
-
-	return p
+	return progressbar.NewOptions64(total, opts...)
 }
 
 // increment records one completed item and refreshes the display.
@@ -113,11 +123,22 @@ func (p *progress) warnf(format string, args ...any) {
 		return
 	}
 
+	msg := escapePath(fmt.Sprintf(format, args...))
+
+	if p.bar != nil && p.total < 0 {
+		// The library also redraws a spinner from its own goroutine, so
+		// a direct write could land inside a redraw. The bar prints the
+		// warning itself, just before its next redraw.
+		_, _ = progressbar.Bprintln(p.bar, msg)
+
+		return
+	}
+
 	if p.bar != nil {
 		_ = p.bar.Clear()
 	}
 
-	fmt.Fprintln(os.Stderr, escapePath(fmt.Sprintf(format, args...)))
+	fmt.Fprintln(os.Stderr, msg)
 }
 
 // finish terminates the pass's display.

progress_test.go

@@ -0,0 +1,161 @@
+package main
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
+)
+
+// spinnerIdle comfortably outlasts the 100ms interval at which the
+// progressbar library redraws a spinner from its own goroutine.
+const spinnerIdle = 500 * time.Millisecond
+
+//nolint:paralleltest // replaces the process-wide os.Stderr
+func TestStderrIsTTYFalseForNonTerminals(t *testing.T) {
+	r, pipe, err := os.Pipe()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	regular, err := os.Create(filepath.Join(t.TempDir(), "stderr"))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	devNull, err := os.OpenFile(os.DevNull, os.O_WRONLY, 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	saved := os.Stderr
+
+	t.Cleanup(func() {
+		os.Stderr = saved
+
+		for _, f := range []*os.File{r, pipe, regular, devNull} {
+			_ = f.Close()
+		}
+	})
+
+	cases := map[string]*os.File{
+		"a pipe":         pipe,
+		"a regular file": regular,
+		os.DevNull:       devNull,
+	}
+
+	for name, f := range cases {
+		os.Stderr = f
+
+		if stderrIsTTY() {
+			t.Errorf("stderrIsTTY() = true with stderr on %s", name)
+		}
+	}
+}
+
+// TestNewProgressPrintsBeforeFirstItem checks that each pass shows its
+// zero state the moment it starts when stderr is not a terminal, and
+// that the next line still waits for plainInterval.
+//
+//nolint:paralleltest // captureStderr replaces the process-wide os.Stderr
+func TestNewProgressPrintsBeforeFirstItem(t *testing.T) {
+	stderr := captureStderr(t)
+
+	newProgress("walk", -1).increment()
+	newProgress("hash", 10).increment()
+
+	want := "walk: 0 files, elapsed 0s\n" +
+		"hash: [0/10] 0% 0 files/s elapsed 0s eta ?\n"
+	if got := stderr(); got != want {
+		t.Errorf("stderr = %q, want %q", got, want)
+	}
+}
+
+// newWalkSpinner returns the walk pass's terminal display, writing to
+// os.Stderr whether or not it is a terminal, and stops the library's
+// redraws when the test ends.
+func newWalkSpinner(t *testing.T) *progress {
+	t.Helper()
+
+	p := &progress{
+		label: "walk", total: -1, start: time.Now(),
+		bar: newBar("walk", -1),
+	}
+	t.Cleanup(p.finish)
+
+	return p
+}
+
+// TestProgressWarningsOnOwnLines drives the terminal display of the walk
+// pass through a run of warnings with no items between them, as when the
+// walk meets many unreadable paths, for several of the spinner's
+// redraws: every warning must land on a line of its own, never inside a
+// redraw.
+//
+//nolint:paralleltest // captureStderr replaces the process-wide os.Stderr
+func TestProgressWarningsOnOwnLines(t *testing.T) {
+	stderr := captureStderr(t)
+	p := newWalkSpinner(t)
+
+	// No pause between warnings: one written straight to stderr is
+	// garbled only if a redraw lands while it is being written.
+	issued := 0
+	for start := time.Now(); time.Since(start) < spinnerIdle; issued++ {
+		p.warnf("warning")
+	}
+
+	// The spinner prints the warnings at its next redraw.
+	time.Sleep(spinnerIdle)
+
+	// A terminal shows each line as the text after its last carriage
+	// return.
+	shown := 0
+
+	for line := range strings.SplitSeq(stderr(), "\n") {
+		if !strings.Contains(line, "warning") {
+			continue
+		}
+
+		shown++
+
+		if text := line[strings.LastIndex(line, "\r")+1:]; text != "warning" {
+			t.Errorf("terminal shows %q, want %q", text, "warning")
+		}
+	}
+
+	if shown != issued {
+		t.Errorf("%d warning lines, want %d", shown, issued)
+	}
+}
+
+// TestSpinnerShowsCountAfterBurst checks that once a burst of items
+// faster than the redraw limit is over, the walk display shows every
+// item completed while it waits for the next one.
+//
+//nolint:paralleltest // captureStderr replaces the process-wide os.Stderr
+func TestSpinnerShowsCountAfterBurst(t *testing.T) {
+	stderr := captureStderr(t)
+	p := newWalkSpinner(t)
+
+	for range 50 {
+		p.increment()
+	}
+
+	time.Sleep(spinnerIdle)
+
+	// A terminal shows the last frame drawn. The library starts each
+	// frame with a carriage return and erases the previous one with
+	// spaces first.
+	var shown string
+
+	for frame := range strings.SplitSeq(stderr(), "\r") {
+		if strings.TrimSpace(frame) != "" {
+			shown = frame
+		}
+	}
+
+	if !strings.Contains(shown, "(50/-,") {
+		t.Errorf("terminal shows %q, want a count of 50", shown)
+	}
+}

report.go

@@ -4,6 +4,7 @@ import (
 	"bufio"
 	"context"
 	"fmt"
+	"io"
 	"os"
 	"slices"
 	"strings"
@@ -31,9 +32,9 @@ type scanRec struct {
 // loadRecords opens the database and reads every file record for the
 // report and trees subcommands. Any database problem — including a
 // missing database — is fatal. The error is returned rather than
-// exiting, so that the deferred close — which checkpoints the SQLite
-// WAL — always runs; the database is closed before the caller formats
-// its output, so it stays closed even if that output fails.
+// exiting, so that the deferred close always runs; the database is
+// closed before the caller formats its output, so it stays closed even
+// if that output fails.
 func loadRecords(ctx context.Context) ([]scanRec, error) {
 	dbPath := databasePath()
 
@@ -65,7 +66,7 @@ type dupeGroup struct {
 // from the database and prints the file-level duplicates report as TSV
 // on stdout. It never touches the scanned filesystem; its only I/O is
 // the database, stdout, and stderr.
-func runReport(ctx context.Context) error {
+func runReport(ctx context.Context, stdout io.Writer) error {
 	recs, err := loadRecords(ctx)
 	if err != nil {
 		return err
@@ -73,7 +74,7 @@ func runReport(ctx context.Context) error {
 
 	dupes := collectDupeGroups(recs)
 
-	out := bufio.NewWriterSize(os.Stdout, ioBufSize)
+	out := bufio.NewWriterSize(stdout, ioBufSize)
 
 	_, err = fmt.Fprintln(out, "first\tdupe\tsize")
 	if err != nil {

report_test.go

@@ -50,11 +50,9 @@ func seedDatabase(t *testing.T, recs []scanRec) string {
 func TestRunReportEscapesPaths(t *testing.T) {
 	t.Setenv(databaseEnv, seedDatabase(t, awkwardPairRecs()))
 
-	var stderr bytes.Buffer
+	var stdout, stderr bytes.Buffer
 
-	stdout := captureStdout(t)
-
-	code := run([]string{cmdReport}, &stderr)
+	code := run([]string{cmdReport}, &stdout, &stderr)
 	if code != exitOK {
 		t.Fatalf("run(report) = %d, want %d; stderr: %s",
 			code, exitOK, stderr.String())
@@ -62,7 +60,7 @@ func TestRunReportEscapesPaths(t *testing.T) {
 
 	want := "first\tdupe\tsize\n" +
 		`/d/\tone\ntwo\rthree\\four/f` + "\t/d/A/f\t5\n"
-	if got := stdout(); got != want {
+	if got := stdout.String(); got != want {
 		t.Errorf("stdout = %q, want %q", got, want)
 	}
 }

scan.go

@@ -85,10 +85,13 @@ type fileMeta struct {
 // least one other file shares are ever hashed: a size-unique file
 // cannot be a duplicate. A file of headTailMin or more gets its content
 // hash only when its size, head, and tail match another file's. Flag
-// parsing and the at-least-one-operand check are done by cobra. Errors
-// are returned rather than exiting, so that the deferred close — which
-// checkpoints the SQLite WAL — always runs. Cancelling ctx unwinds the
-// worker pools and aborts the scan with the context's error.
+// parsing and the at-least-one-operand check are done by cobra. The
+// scan holds the lock on the database for its whole run, so a second
+// scan fails before it walks the filesystem or opens the database.
+// Errors are returned rather than exiting, so that the deferred close —
+// which takes the database out of WAL mode — always runs, and the lock
+// is released after it. Cancelling ctx unwinds the worker pools and
+// aborts the scan with the context's error.
 func runScan(ctx context.Context, roots []string, workers int,
 	oneFS bool,
 ) error {
@@ -103,12 +106,19 @@ func runScan(ctx context.Context, roots []string, workers int,
 
 	dbPath := databasePath()
 
+	lock, err := lockScanDatabase(dbPath)
+	if err != nil {
+		return err
+	}
+
+	defer func() { _ = lock.Close() }()
+
 	db, err := openScanDatabase(ctx, dbPath)
 	if err != nil {
 		return err
 	}
 
-	defer func() { _ = db.Close() }()
+	defer closeScanDatabase(ctx, db, dbPath)
 
 	st, err := syncScan(ctx, db, roots, workers, oneFS)
 	if err != nil {
@@ -210,14 +220,18 @@ type scanState struct {
 // in the content hash of every record of headTailMin or more whose
 // size, head, and tail match another record's). Records outside the
 // roots are never touched, except that the content phase fills in
-// their content hash.
+// their content hash. Operands the walk cannot start from are dropped
+// first, so the records beneath them count as outside the roots unless
+// they lie under another root.
 func syncScan(ctx context.Context, db *sql.DB, roots []string,
 	workers int, oneFS bool,
 ) (scanStats, error) {
-	roots = pruneRoots(roots)
-
 	s := &scanState{db: db}
 
+	// Types are checked before pruning so that an operand under a
+	// dropped one is still scanned, not dropped as lying under it.
+	roots = pruneRoots(s.walkableRoots(roots))
+
 	err := s.loadIndex(ctx, roots)
 	if err != nil {
 		return s.st, err
@@ -253,6 +267,35 @@ func syncScan(ctx context.Context, db *sql.DB, roots []string,
 	return s.st, s.contentPhase(ctx, workers)
 }
 
+// walkableRoots returns the operands the walk can start from: regular
+// files, and directories not named .zfs. 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 count as outside the
+// roots and are not deleted as unverified, unless it lies under another
+// root. An operand that fails lstat here is kept, and the walk warns
+// about it.
+func (s *scanState) walkableRoots(roots []string) []string {
+	kept := make([]string, 0, len(roots))
+
+	for _, root := range roots {
+		fi, err := os.Lstat(root)
+		if err == nil {
+			warn := operandWarning(root, fi)
+			if warn != "" {
+				s.st.skipped++
+
+				fmt.Fprintln(os.Stderr, escapePath(warn))
+
+				continue
+			}
+		}
+
+		kept = append(kept, root)
+	}
+
+	return kept
+}
+
 // loadIndex indexes the database records under the scan roots for
 // change detection and collects the sizes of every record outside
 // them: out-of-scope records join the size census so a scanned file
@@ -789,11 +832,43 @@ 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. Symlinks are
+// never followed, including as operands.
+func operandWarning(root string, fi fs.FileInfo) string {
+	var kind string
+
+	switch mode := fi.Mode(); {
+	case mode.IsRegular():
+		return ""
+	case mode.IsDir():
+		if filepath.Base(root) != ".zfs" {
+			return ""
+		}
+
+		kind = ".zfs directory"
+	case mode&fs.ModeSymlink != 0:
+		kind = "symlink"
+	case mode&fs.ModeSocket != 0:
+		kind = "socket"
+	case mode&fs.ModeNamedPipe != 0:
+		kind = "FIFO"
+	case mode&fs.ModeDevice != 0:
+		kind = "device node"
+	default:
+		kind = "non-regular file"
+	}
+
+	return fmt.Sprintf("walk %s: skipping %s operand", root, kind)
+}
+
 // seedRoot turns one PATH operand into the walk's starting state: a
-// regular-file operand is statted and emitted directly, a directory
-// operand becomes an initial job, and a symlink or other non-regular
-// operand yields nothing (symlinks are never followed, including as
-// operands).
+// regular-file operand is statted and emitted directly, and a directory
+// operand becomes an initial job. walkableRoots has already dropped
+// every other operand. One that has changed into something else since
+// is warned about and skipped here; it is still a root, so the records
+// stored beneath it are deleted as unverified.
 func seedRoot(ctx context.Context, root string,
 	events chan<- walkEvent,
 ) []dirJob {
@@ -807,30 +882,30 @@ func seedRoot(ctx context.Context, root string,
 		return nil
 	}
 
-	switch {
-	case fi.IsDir():
-		if filepath.Base(root) == ".zfs" {
-			return nil
-		}
+	warn := operandWarning(root, fi)
+	if warn != "" {
+		sendEvent(ctx, events, walkEvent{warn: warn, fail: true})
 
+		return nil
+	}
+
+	if fi.IsDir() {
 		dev, ok := deviceOfInfo(fi)
 
 		return []dirJob{{path: root, rootDev: dev, rootDevOK: ok}}
-	case fi.Mode().IsRegular():
-		dev, ino := inodeOfInfo(fi)
-
-		sendEvent(ctx, events, walkEvent{rec: fileRec{
-			path:  root,
-			size:  fi.Size(),
-			mtime: fi.ModTime().Unix(),
-			dev:   dev,
-			ino:   ino,
-		}})
-
-		return nil
-	default:
-		return nil
 	}
+
+	dev, ino := inodeOfInfo(fi)
+
+	sendEvent(ctx, events, walkEvent{rec: fileRec{
+		path:  root,
+		size:  fi.Size(),
+		mtime: fi.ModTime().Unix(),
+		dev:   dev,
+		ino:   ino,
+	}})
+
+	return nil
 }
 
 // startWalkWorkers starts the walk worker pool. Each worker processes

scan_test.go

@@ -7,6 +7,7 @@ import (
 	"database/sql"
 	"encoding/hex"
 	"fmt"
+	"io"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -856,9 +857,11 @@ func TestWalkFileAndSymlinkOperands(t *testing.T) {
 		t.Fatalf("file operand: recs = %+v, errs = %d", recs, errs)
 	}
 
-	// A symlink operand is not followed and yields nothing.
+	// A symlink operand that reaches the walk (it became one after
+	// walkableRoots checked it) is not followed: it yields a warning and
+	// no records.
 	recs, errs = collectWalk(t, []string{link}, false, 2)
-	if errs != 0 || len(recs) != 0 {
+	if errs != 1 || len(recs) != 0 {
 		t.Fatalf("symlink operand: recs = %+v, errs = %d", recs, errs)
 	}
 }
@@ -1548,7 +1551,7 @@ func TestScanHashWriteFailureUnwindsPool(t *testing.T) {
 
 	code := run([]string{
 		cmdScan, "--workers", strconv.Itoa(hashLeakWorkers), dir,
-	}, &stderr)
+	}, io.Discard, &stderr)
 	if code != exitFatal {
 		t.Fatalf("run(scan) = %d, want %d; stderr: %s",
 			code, exitFatal, stderr.String())

trees.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"crypto/sha256"
 	"fmt"
+	"io"
 	"os"
 	"slices"
 	"strconv"
@@ -36,7 +37,7 @@ type treeNode struct {
 // maximal duplicate-tree groups as TSV on stdout. It never touches the
 // scanned filesystem; its only I/O is the database, stdout, and
 // stderr.
-func runTrees(ctx context.Context) error {
+func runTrees(ctx context.Context, stdout io.Writer) error {
 	recs, err := loadRecords(ctx)
 	if err != nil {
 		return err
@@ -47,7 +48,7 @@ func runTrees(ctx context.Context) error {
 
 	dupes := collectTreeGroups(allDirs, super)
 
-	out := bufio.NewWriterSize(os.Stdout, ioBufSize)
+	out := bufio.NewWriterSize(stdout, ioBufSize)
 
 	_, err = fmt.Fprintln(out, "first\tdupe\tfiles\tsize")
 	if err != nil {

trees_test.go

@@ -108,11 +108,9 @@ func TestBuildHierarchyRootPath(t *testing.T) {
 func TestRunTreesEscapesPaths(t *testing.T) {
 	t.Setenv(databaseEnv, seedDatabase(t, awkwardPairRecs()))
 
-	var stderr bytes.Buffer
+	var stdout, stderr bytes.Buffer
 
-	stdout := captureStdout(t)
-
-	code := run([]string{cmdTrees}, &stderr)
+	code := run([]string{cmdTrees}, &stdout, &stderr)
 	if code != exitOK {
 		t.Fatalf("run(trees) = %d, want %d; stderr: %s",
 			code, exitOK, stderr.String())
@@ -120,7 +118,7 @@ func TestRunTreesEscapesPaths(t *testing.T) {
 
 	want := "first\tdupe\tfiles\tsize\n" +
 		`/d/\tone\ntwo\rthree\\four` + "\t/d/A\t1\t5\n"
-	if got := stdout(); got != want {
+	if got := stdout.String(); got != want {
 		t.Errorf("stdout = %q, want %q", got, want)
 	}
 }