Non-regular and .zfs operands are silently ignored, and their records are deleted #9

Closed
opened 2026-08-09 03:44:16 +02:00 by clawbot · 2 comments
Collaborator

seedRoot's default: branch (scan.go:581-583) returns nil for an operand that is a symlink, socket, FIFO or device node — no warning, no skip counter. scan.go:562-564 does the same for a directory operand literally named .zfs.

Two problems follow.

First, silence. sfdupes scan /srv/data where /srv/data is a symlink to the real tree prints scan: 0 files seen (0 added, 0 updated, 0 removed, 0 unchanged), 0 skipped and exits 0. README §Rules for the walk requires that any skipped path "print a one-line warning to stderr, skip the path, and continue" — the skip is correct, the silence is not. Symlinked operands are an ordinary mistake to make.

Second, and worse: the operand stays in roots, so loadIndex (scan.go:193-215) treats every existing database record beneath that path as in-scope, and updatePhase deletes all of them because nothing verified them this run. A single mistyped symlink operand therefore wipes the records for that whole subtree.

Definition of done

  1. A non-regular, non-directory operand emits a one-line stderr warning naming the path and its type, and counts as skipped in the summary.
  2. An operand that is itself named .zfs does the same.
  3. An operand that produced no walkable root is removed from the in-scope set, so records beneath it are left untouched rather than deleted.
  4. Exit status stays 0 — this is a per-path skip, not a fatal error. A nonexistent operand remains fatal (exit 1) as README §scan mode specifies.
  5. Tests: a symlink operand asserts the warning text, the 1 skipped summary, exit 0, and that pre-existing records beneath the symlink target survive; a .zfs operand asserts the same.
  6. make check green.
`seedRoot`'s `default:` branch (`scan.go:581-583`) returns `nil` for an operand that is a symlink, socket, FIFO or device node — no warning, no skip counter. `scan.go:562-564` does the same for a directory operand literally named `.zfs`. Two problems follow. First, silence. `sfdupes scan /srv/data` where `/srv/data` is a symlink to the real tree prints `scan: 0 files seen (0 added, 0 updated, 0 removed, 0 unchanged), 0 skipped` and exits 0. README §Rules for the walk requires that any skipped path "print a one-line warning to stderr, skip the path, and continue" — the skip is correct, the silence is not. Symlinked operands are an ordinary mistake to make. Second, and worse: the operand stays in `roots`, so `loadIndex` (`scan.go:193-215`) treats every existing database record beneath that path as in-scope, and `updatePhase` deletes all of them because nothing verified them this run. A single mistyped symlink operand therefore wipes the records for that whole subtree. ## Definition of done 1. A non-regular, non-directory operand emits a one-line stderr warning naming the path and its type, and counts as skipped in the summary. 2. An operand that is itself named `.zfs` does the same. 3. An operand that produced no walkable root is removed from the in-scope set, so records beneath it are left untouched rather than deleted. 4. Exit status stays 0 — this is a per-path skip, not a fatal error. A nonexistent operand remains fatal (exit 1) as README §scan mode specifies. 5. Tests: a symlink operand asserts the warning text, the `1 skipped` summary, exit 0, and that pre-existing records beneath the symlink target survive; a `.zfs` operand asserts the same. 6. `make check` green.
clawbot added this to the 1.0.0 milestone 2026-08-09 03:44:16 +02:00
Author
Collaborator

Plan (implementer's brief).

  • Check each operand's type once, right after resolveRoots, before the database index is loaded: a directory not named .zfs or a regular file goes on; anything else (symlink, socket, FIFO, device, or a directory named .zfs) gets one stderr warning naming the path and what it is, adds one to skipped, and is dropped from the roots. Because the dropped operand is no longer a root, loadIndex does not count records under it as in scope and the update phase leaves them alone, which is item 3.
  • Use the same warning shape as the scan's other per-path warnings.
  • seedRoot keeps a branch for a path that changed type between that check and the walk; it must not silently return nothing there either.
  • A nonexistent operand stays fatal, exit 1. If every operand is dropped, the scan still runs, touches nothing, prints its summary and exits 0.
  • README.md §Rules for the walk and §scan mode say what happens to such an operand.
  • Tests per the definition of done, including that records already stored under a symlink operand's target survive.

scan.go is also being edited for #53 soon; whichever lands second rebases.

Model: opus-5-5

Plan (implementer's brief). - Check each operand's type once, right after `resolveRoots`, before the database index is loaded: a directory not named `.zfs` or a regular file goes on; anything else (symlink, socket, FIFO, device, or a directory named `.zfs`) gets one stderr warning naming the path and what it is, adds one to `skipped`, and is dropped from the roots. Because the dropped operand is no longer a root, `loadIndex` does not count records under it as in scope and the update phase leaves them alone, which is item 3. - Use the same warning shape as the scan's other per-path warnings. - `seedRoot` keeps a branch for a path that changed type between that check and the walk; it must not silently return nothing there either. - A nonexistent operand stays fatal, exit 1. If every operand is dropped, the scan still runs, touches nothing, prints its summary and exits 0. - `README.md` §Rules for the walk and §scan mode say what happens to such an operand. - Tests per the definition of done, including that records already stored under a symlink operand's target survive. `scan.go` is also being edited for https://git.eeqj.de/sneak/sfdupes/issues/53 soon; whichever lands second rebases. Model: opus-5-5
clawbot self-assigned this 2026-10-03 14:09:45 +02:00
Author
Collaborator

Implemented in #71, as the plan above lays out: such an operand is warned about, counted as skipped and dropped before the database index is loaded, so the records beneath it are kept. The PR body lists what the diff does not show.

Model: opus-5-5

Implemented in https://git.eeqj.de/sneak/sfdupes/pulls/71, as the plan above lays out: such an operand is warned about, counted as skipped and dropped before the database index is loaded, so the records beneath it are kept. The PR body lists what the diff does not show. Model: opus-5-5
Sign in to join this conversation.