Keep records on other filesystems under an operand with -x (closes #99)

With -x the walk does not enter a directory on another filesystem than
its operand, but the update phase still deleted every record below it.
A separate scan of a filesystem mounted under the operand therefore lost
its records on every -x scan and re-hashed its whole tree.

The walk now sends an event naming each directory it declines this way,
and the walk phase treats the records below those directories like
records outside the operands: they are not deleted, and their sizes join
the size census. README "scan mode" and "Rules for the walk" state the
rule.

Model: opus-5-5

README.md

@@ -271,16 +271,21 @@ All three subcommands operate on a single SQLite database file:
   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.
+  by appending `.lock` to the database's path with symlinks resolved
+  (`/var/lib/sfdupes/db.sqlite.lock` by default), taken before it walks the
+  filesystem or opens the database. Every path to one database, through a
+  symlink to the file or to a directory above it, names the same lock file. Just
+  before it takes the lock, `scan` creates the database file, empty, if it is
+  missing (through a symlink at the database path, it creates the symlink's
+  target), so the lock file is named the same way before the first scan sets up
+  the database as after. 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.
@@ -442,6 +447,11 @@ operands:
   a `content` hash is read for one, wherever it lies, once its size, `head`, and
   `tail` match another record's. If that file is gone or has changed since its
   record was written, the record is left as it is.
+- With `-x`, a record below a directory the walk did not enter because that
+  directory is on another filesystem than its operand counts as outside the
+  scanned operands: it is not deleted, and its size joins the size census. A
+  filesystem mounted under an operand can therefore be scanned on its own
+  schedule into the same database.
 
 `scan` runs **four sequential phases over the whole scan**. Parallelism lives
 inside each phase; batched database writes begin during the hash phase:
@@ -508,7 +518,9 @@ Rules for the walk:
 - 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 filesystem than its `PATH` operand; each
-  operand is bounded by its own filesystem.
+  operand is bounded by its own filesystem. The records below such a directory
+  count as outside the scanned operands, as described in "`scan` mode" above:
+  they are not deleted, and their sizes join the size census.
 - On any per-path error (permission denied, file vanished between passes,
   unreadable): print a one-line warning to stderr, skip the path, and continue.
   Per-file errors never abort the run; the final summary reports how many were

TODO.md

@@ -29,6 +29,20 @@
 
 # Completed Steps
 
+- with `-x`, `scan` keeps the records below a directory on another filesystem
+  under its operand, and counts their sizes in the size census, instead of
+  deleting them (2026-10-14, https://git.eeqj.de/sneak/sfdupes/issues/99)
+
+- the hash and content phases open each file without blocking and check that it
+  is still a regular file, so one replaced by a FIFO or device node after it was
+  statted is warned about and skipped instead of hanging the scan (2026-10-14,
+  https://git.eeqj.de/sneak/sfdupes/issues/100)
+
+- the scan lock file is named from the database's path with symlinks resolved,
+  after `scan` creates the database file if it is missing, so a scan through a
+  symlink to the database fails while another scan holds the lock (2026-10-14,
+  https://git.eeqj.de/sneak/sfdupes/issues/101)
+
 - on a terminal, a warning for the last file of a phase no longer erases the
   phase's finished bar: the bar is drawn again below the warning (2026-10-14,
   https://git.eeqj.de/sneak/sfdupes/issues/102)

db.go

@@ -35,6 +35,10 @@ const schemaVersion = 1
 // scan.
 const dbDirPerm = 0o755
 
+// dbFilePerm is the mode for a database file created by scan, the mode
+// SQLite gives a database file it creates.
+const dbFilePerm = 0o644
+
 // 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.
@@ -140,19 +144,37 @@ func openDB(path, params string) (*sql.DB, error) {
 
 // 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.
+// by the database's path with symlinks resolved and ".lock" appended,
+// so every path to one database names the same lock file. If missing,
+// the database's parent directory and an empty database file are
+// created first, so the lock file is named the same way before the
+// database is set up as after. 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 lock 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"
+	// Like SQLite, this follows a symlink at path and creates its
+	// target. SQLite opens an empty file as a new database.
+	//nolint:gosec // the operator chooses the database path
+	dbFile, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE, dbFilePerm)
+	if err != nil {
+		return nil, fmt.Errorf("create database: %w", err)
+	}
+
+	_ = dbFile.Close()
+
+	resolved, err := filepath.EvalSymlinks(path)
+	if err != nil {
+		return nil, fmt.Errorf("resolve database path: %w", err)
+	}
+
+	lockPath := resolved + ".lock"
 
 	//nolint:gosec // the operator chooses the database path
 	f, err := os.OpenFile(lockPath, os.O_RDWR|os.O_CREATE, lockFilePerm)

db_test.go

@@ -13,11 +13,19 @@ import (
 	"time"
 )
 
-// testDBPath returns a database path inside a fresh temp dir.
+// testDBPath returns a database path inside a fresh temp dir, with
+// symlinks resolved as the scan lock resolves them, so the lock file
+// is this path plus ".lock" even where the temp dir is reached through
+// a symlink.
 func testDBPath(t *testing.T) string {
 	t.Helper()
 
-	return filepath.Join(t.TempDir(), "db.sqlite")
+	dir, err := filepath.EvalSymlinks(t.TempDir())
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	return filepath.Join(dir, "db.sqlite")
 }
 
 // openTestDB creates a fresh scan database in a temp dir.

main_test.go

@@ -822,7 +822,7 @@ func holdScanLock(t *testing.T, path string) {
 
 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
+	// fails at once, naming the lock file, without opening the
 	// database.
 	path := testDBPath(t)
 	t.Setenv(databaseEnv, path)
@@ -848,10 +848,99 @@ func TestRunSecondScanFails(t *testing.T) {
 		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)
+	// Taking the lock created the database file, empty; opening it as
+	// a database would have written to it.
+	info, err := os.Stat(path)
+	if err != nil {
+		t.Fatal(err)
 	}
+
+	if info.Size() != 0 {
+		t.Errorf("database size = %d, want 0 (not opened)", info.Size())
+	}
+}
+
+// assertSecondScanFails runs a scan against the database
+// SFDUPES_DATABASE names while the test holds the lock on it. It fails
+// the test unless that scan fails at once, naming the lock file beside
+// path, the database's real path.
+func assertSecondScanFails(t *testing.T, path string) {
+	t.Helper()
+
+	var stderr bytes.Buffer
+
+	code := run([]string{cmdScan, t.TempDir()}, io.Discard, &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)
+	}
+}
+
+func TestRunSecondScanThroughSymlinkFails(t *testing.T) {
+	// README §Database: every path to one database names the same lock
+	// file, so while a scan holds it through one path, a scan through
+	// another fails, naming the lock file beside the database's real
+	// path.
+	t.Run("symlink to the database", func(t *testing.T) {
+		path := testDBPath(t)
+		t.Setenv(databaseEnv, path)
+
+		scanFixture(t)
+
+		link := filepath.Join(t.TempDir(), "link.sqlite")
+
+		err := os.Symlink(path, link)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		holdScanLock(t, path)
+		t.Setenv(databaseEnv, link)
+		assertSecondScanFails(t, path)
+	})
+
+	t.Run("symlink to its directory, no database yet", func(t *testing.T) {
+		path := testDBPath(t)
+		link := filepath.Join(t.TempDir(), "link")
+
+		err := os.Symlink(filepath.Dir(path), link)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		holdScanLock(t, path)
+		t.Setenv(databaseEnv, filepath.Join(link, filepath.Base(path)))
+		assertSecondScanFails(t, path)
+	})
+
+	t.Run("symlink whose target does not exist yet", func(t *testing.T) {
+		path := testDBPath(t)
+		link := filepath.Join(t.TempDir(), "link.sqlite")
+
+		err := os.Symlink(path, link)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		// The first scan takes the lock through the link before the
+		// database exists, then creates the database through it.
+		holdScanLock(t, link)
+
+		db, err := openScanDatabase(t.Context(), link)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		closeScanDatabase(t.Context(), db, link)
+
+		t.Setenv(databaseEnv, link)
+		assertSecondScanFails(t, path)
+	})
 }
 
 func TestRunScanReleasesLock(t *testing.T) {

scan.go

@@ -391,15 +391,26 @@ func (s *scanState) loadIndex(ctx context.Context, roots []string) error {
 // (the content phase may still fill in its content hash). It returns
 // the new-or-changed files and the unchanged files whose records lack
 // hashes; both remain candidates until the census decides whether
-// their sizes are shared.
+// their sizes are shared. A record below a directory -x did not enter
+// counts as outside the roots: it is not deleted, and its size joins
+// the census.
 func (s *scanState) walkPhase(
 	events <-chan walkEvent,
 ) ([]fileRec, []fileRec) {
 	prog := newProgress("walk", -1)
 
-	var changed, unhashed []fileRec
+	var (
+		changed, unhashed []fileRec
+		otherFSDirs       []string
+	)
 
 	for ev := range events {
+		if ev.otherFSDir != "" {
+			otherFSDirs = append(otherFSDirs, ev.otherFSDir)
+
+			continue
+		}
+
 		if ev.fail {
 			s.st.skipped++
 
@@ -429,6 +440,14 @@ func (s *scanState) walkPhase(
 
 	prog.finish()
 
+	for path, old := range s.existing {
+		if underAnyRoot(path, otherFSDirs) {
+			delete(s.existing, path)
+
+			s.sizes = append(s.sizes, old.size)
+		}
+	}
+
 	return changed, unhashed
 }
 
@@ -866,16 +885,20 @@ type dirJob struct {
 }
 
 // walkEvent is one walk result delivered to the walk phase: a regular
-// file's statted record, or a warning when fail is set.
+// file's statted record, a warning when fail is set, or, when
+// otherFSDir is set, a directory -x did not enter because it is on
+// another filesystem than its operand.
 type walkEvent struct {
-	rec  fileRec
-	warn string
-	fail bool
+	rec        fileRec
+	warn       string
+	fail       bool
+	otherFSDir string
 }
 
 // startWalk seeds every root into the shared walk worker pool and
 // returns the event stream: one record per regular file, one warning
-// event per per-path error. The channel is closed when the walk
+// event per per-path error, and one event per directory -x did not
+// enter. The channel is closed when the walk
 // completes, and also when ctx is cancelled — every goroutine in the
 // pool abandons its blocking send in that case, so the consumer sees a
 // truncated but properly terminated stream instead of a stalled one.
@@ -1144,7 +1167,8 @@ func emitFile(ctx context.Context, p string, e fs.DirEntry,
 // subdirJob applies the descent rules to directory p: never enter
 // .zfs (ZFS snapshot pseudo-dirs would list every file once per
 // snapshot), and with -x never enter a directory on a different
-// filesystem than its operand.
+// filesystem than its operand, sending an event that names it so the
+// scan keeps the records below it.
 func subdirJob(ctx context.Context, p string, e fs.DirEntry,
 	parent dirJob, oneFS bool, events chan<- walkEvent,
 ) (dirJob, bool) {
@@ -1168,6 +1192,8 @@ func subdirJob(ctx context.Context, p string, e fs.DirEntry,
 	}
 
 	if dev, ok := deviceOfInfo(info); ok && dev != parent.rootDev {
+		sendEvent(ctx, events, walkEvent{otherFSDir: p})
+
 		return dirJob{}, false
 	}
 
@@ -1309,6 +1335,38 @@ func hashWorker(ctx context.Context, jobs <-chan []fileRec,
 	}
 }
 
+// errNotRegular reports a path that is no longer a regular file when it
+// is opened to be read.
+var errNotRegular = errors.New("not a regular file")
+
+// openRegularFile opens path for reading and returns it only if it is
+// still a regular file, since the path may have been replaced after it
+// was statted. O_NONBLOCK keeps the open of a FIFO or a device node
+// from waiting forever, and fstat then rejects it. Reads of a regular
+// file ignore O_NONBLOCK.
+func openRegularFile(path string) (*os.File, error) {
+	//nolint:gosec // hashing operator-supplied paths is the tool's purpose
+	f, err := os.OpenFile(path, os.O_RDONLY|syscall.O_NONBLOCK, 0)
+	if err != nil {
+		return nil, err
+	}
+
+	fi, err := f.Stat()
+	if err != nil {
+		_ = f.Close()
+
+		return nil, err
+	}
+
+	if !fi.Mode().IsRegular() {
+		_ = f.Close()
+
+		return nil, errNotRegular
+	}
+
+	return f, nil
+}
+
 // emptyHash is the lowercase-hex SHA-256 of the empty input: the head,
 // tail, and content hash of every zero-length file.
 const emptyHash = "e3b0c44298fc1c149afbf4c8996fb924" +
@@ -1331,8 +1389,7 @@ func hashSignature(path string, size int64) (string, string, string, error) {
 		return emptyHash, emptyHash, emptyHash, nil
 	}
 
-	//nolint:gosec // hashing operator-supplied paths is the tool's purpose
-	f, err := os.Open(path)
+	f, err := openRegularFile(path)
 	if err != nil {
 		return "", "", "", err
 	}
@@ -1363,8 +1420,7 @@ func hashSignature(path string, size int64) (string, string, string, error) {
 // tail are returned empty, because the content phase keeps the ones its
 // records already hold.
 func hashContentOnly(path string, size int64) (string, string, string, error) {
-	//nolint:gosec // hashing operator-supplied paths is the tool's purpose
-	f, err := os.Open(path)
+	f, err := openRegularFile(path)
 	if err != nil {
 		return "", "", "", err
 	}

scan_test.go

@@ -997,7 +997,7 @@ func dirEntryFor(t *testing.T, dir, name string) fs.DirEntry {
 }
 
 // callSubdirJob runs subdirJob against e under parent, collecting any
-// warning events it emits (subdirJob emits at most one).
+// events it emits (subdirJob emits at most one).
 func callSubdirJob(t *testing.T, p string, e fs.DirEntry,
 	parent dirJob, oneFS bool,
 ) (dirJob, bool, []walkEvent) {
@@ -1075,8 +1075,15 @@ func TestSubdirJobOneFilesystem(t *testing.T) {
 				t.Fatalf("accepted = %v, want %v", ok, tc.wantOK)
 			}
 
-			if len(evs) != 0 {
-				t.Fatalf("unexpected events: %+v", evs)
+			// A refused descent names the directory, so the scan keeps
+			// the records below it.
+			var wantEvs []walkEvent
+			if !ok {
+				wantEvs = []walkEvent{{otherFSDir: sub}}
+			}
+
+			if !slices.Equal(evs, wantEvs) {
+				t.Fatalf("events = %+v, want %+v", evs, wantEvs)
 			}
 
 			// The accepted job must carry the operand's device down, or -x
@@ -1128,6 +1135,84 @@ func TestSubdirJobStatError(t *testing.T) {
 	}
 }
 
+// TestScanOneFilesystemKeepsOtherFilesystemRecords checks that with -x a
+// record below a directory the walk does not enter, because it is on
+// another filesystem than its operand, counts as outside the operands:
+// it is not deleted, and its size joins the size census. The record of
+// a gone file that is not below that directory is still deleted, even
+// when its path begins with the directory's name. The walk starts from
+// an operand device that no directory here is on, so no second real
+// filesystem is needed; the phases after it run as runPhases runs them.
+func TestScanOneFilesystemKeepsOtherFilesystemRecords(t *testing.T) {
+	t.Parallel()
+
+	dir := t.TempDir()
+	db := openTestDB(t)
+
+	// A scan without -x records the file below mnt, and mnt2, which is
+	// beside mnt, not below it.
+	mounted := writeFile(t, dir, "mnt/f", pattern(1, 100))
+	gone := writeFile(t, dir, "mnt2", pattern(3, 50))
+	syncTree(t, db, dir)
+
+	err := os.Remove(gone)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// The same size as mnt/f, so it is hashed only if mnt/f's size is
+	// in the census.
+	added := writeFile(t, dir, "added", pattern(2, 100))
+
+	info, err := os.Lstat(dir)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	dev, ok := deviceOfInfo(info)
+	if !ok {
+		t.Skip("platform exposes no device id")
+	}
+
+	ctx := t.Context()
+	s := &scanState{db: db}
+
+	err = s.loadIndex(ctx, []string{dir})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	jobs, subdirs, events := startWalkWorkers(ctx, 2, true)
+	root := dirJob{path: dir, rootDev: dev + 1, rootDevOK: true}
+	dispatchDirs(ctx, []dirJob{root}, jobs, subdirs)
+
+	changed, unhashed := s.walkPhase(events)
+	s.partition(changed, unhashed)
+
+	err = s.hashPhase(ctx, 2)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	err = s.updatePhase(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	recs := dbRecords(t, db)
+	recordByPath(t, recs, mounted)
+
+	if recordByPath(t, recs, added).head == "" {
+		t.Errorf("%s not hashed: the census lacks the size of %s",
+			added, mounted)
+	}
+
+	if slices.Contains(recordPaths(recs), gone) {
+		t.Errorf("record for %s kept: the file is gone and not below mnt",
+			gone)
+	}
+}
+
 // buildSmokeTree recreates the README smoke-test filesystem layout
 // with deterministic content and returns the tree root.
 func buildSmokeTree(t *testing.T) string {
@@ -1797,6 +1882,54 @@ func TestScanHardlinkRunFailsTogether(t *testing.T) {
 	}
 }
 
+// TestHashPhaseSkipsFileReplacedByFIFO replaces a walked regular file
+// with a FIFO before the hash phase reads it. Opening a FIFO that has
+// no writer waits for one, so a blocking open would hang the phase. The
+// path must be warned about once and skipped, with no record written.
+//
+//nolint:paralleltest // captureStderr replaces the process-wide os.Stderr
+func TestHashPhaseSkipsFileReplacedByFIFO(t *testing.T) {
+	stderr := captureStderr(t)
+	dir := t.TempDir()
+	p := writeFile(t, dir, "a.bin", pattern(1, 300))
+
+	walked, _ := collectWalk(t, []string{dir}, false, 1)
+
+	err := os.Remove(p)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	err = unix.Mkfifo(p, 0o600)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	s := &scanState{db: openTestDB(t), toHash: walked}
+
+	err = s.hashPhase(t.Context(), 1)
+	if err != nil {
+		t.Fatalf("hashPhase = %v, want nil", err)
+	}
+
+	if s.st.skipped != 1 || len(s.batch) != 0 {
+		t.Errorf("skipped = %d, records = %d, want 1 skipped and none",
+			s.st.skipped, len(s.batch))
+	}
+
+	warning := "hash " + p + ": not a regular file\n"
+	if got := stderr(); strings.Count(got, warning) != 1 {
+		t.Errorf("stderr = %q, want %q once", got, warning)
+	}
+
+	// The content phase opens files the same way.
+	_, _, content, err := hashContentOnly(p, headTailMin)
+	if content != "" || !errors.Is(err, errNotRegular) {
+		t.Errorf("hashContentOnly on a FIFO = %q, %v; want no hash and %v",
+			content, err, errNotRegular)
+	}
+}
+
 // injectedWriteFailure is the message the injected database trigger
 // aborts with, so the test can recognize its own failure in the error
 // the scan reports.