Print progress at once off a terminal, keep warnings out of redraws (closes #13)

When stderr is not a terminal, each phase prints its zero-state line
as it starts instead of after its first item. stderrIsTTY uses
term.IsTerminal from golang.org/x/term, now a direct dependency, so
/dev/null is no longer taken for a terminal.

A spinner keeps the library's background redraw, so its count and
elapsed time stay current while a phase waits for its next item. A
warning printed during a spinner phase goes through the bar
(progressbar.Bprintln), which prints it before its next redraw instead
of racing it. Bars with a total have no background redraw and still
print warnings directly.

Model: opus-5-5

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**
+    (`github.com/schollz/progressbar/v3`), `golang.org/x/term` to tell
-    (`modernc.org/sqlite`, pure Go, so builds keep cgo disabled).
+    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,6 +154,20 @@ 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.
+-   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
@@ -557,19 +573,26 @@ 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
+-   When stderr is not a terminal (a pipe, a file, `/dev/null`), do not
-    one-line progress update no more often than every 5 seconds instead.
+    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
+-   `1`: fatal error (e.g., a `PATH` operand does not exist, another
-    database cannot be created/opened/read/written, a missing database
+    `scan` is already running against the same database, the database
-    for `report`/`trees`, stdout write failure).
+    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).
 
@@ -722,7 +745,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"
@@ -750,9 +773,9 @@ All of the following, run in this directory, must pass:
     ./sfdupes report
     ```
 
-    (The scan database lives inside `$d` here purely for test hygiene;
+    (The database lives in a temp directory of its own: inside `$d`,
-    scanning `$d` therefore also records the SQLite file itself, which
+    the scan would record it, and its empty lock file would join the
-    is harmless.)
+    `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,6 +29,14 @@
 
 # Completed Steps
 
+- 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)
+
+- `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)

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 {
@@ -104,6 +114,43 @@ func openDB(path, params 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) {

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_test.go

@@ -412,6 +412,95 @@ func TestRunReportsNeedOnlyReadAccess(t *testing.T) {
 	}
 }
 
+// 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.

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()
+	return term.IsTerminal(int(os.Stderr.Fd()))
-	if err != nil {
-		return false
-	}
-
-	return fi.Mode()&os.ModeCharDevice != 0
 }
 
 // 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
+// All methods must be called from the main goroutine only. On a TTY
-// *progress is a valid no-display receiver: every method is a no-op,
+// the library also redraws a spinner from its own goroutine, several
-// so batched database flushes during the streaming pass can reuse the
+// times a second, so its count and elapsed time stay current while a
-// update-pass helpers without rendering anything.
+// 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 progressbar.NewOptions64(total, opts...)
-
-	return p
 }
 
 // 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,192 @@
+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
+
+// captureStderr points os.Stderr at a file for the rest of the test and
+// returns a function reading back everything written to it.
+func captureStderr(t *testing.T) func() string {
+	t.Helper()
+
+	path := filepath.Join(t.TempDir(), "stderr")
+
+	f, err := os.Create(path) //nolint:gosec // test-controlled path
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	saved := os.Stderr
+	os.Stderr = f
+
+	t.Cleanup(func() {
+		os.Stderr = saved
+
+		_ = f.Close()
+	})
+
+	return func() string {
+		b, err := os.ReadFile(path) //nolint:gosec // test-controlled path
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		return string(b)
+	}
+}
+
+//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)
+	}
+}

scan.go

@@ -85,11 +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
+// parsing and the at-least-one-operand check are done by cobra. The
-// are returned rather than exiting, so that the deferred close — which
+// scan holds the lock on the database for its whole run, so a second
-// takes the database out of WAL mode — always runs. Cancelling ctx
+// scan fails before it walks the filesystem or opens the database.
-// unwinds the worker pools and aborts the scan with the context's
+// Errors are returned rather than exiting, so that the deferred close —
-// error.
+// 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 {
@@ -104,6 +106,13 @@ 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