A symlink, socket, FIFO or device-node operand, or a directory operand named .zfs, used to be ignored without a word while staying in the scanned operands, so the update phase deleted every record stored beneath it. syncScan now checks each operand's type before loading the database index: such an operand gets a one-line warning, counts as skipped, and is dropped. Its records then count as outside the scanned operands: they are not deleted, but the content phase may still fill in their content hash, as for any such record. Exit status stays 0; a nonexistent operand is still fatal. README.md says so.
What the diff does not show:
The check runs before overlapping operands are pruned, so /a/link given alongside /a is now warned about instead of dropped silently; /a is still scanned, so the records beneath /a/link are deleted as unverified.
An operand that changes type between that check and the walk is still a root: seedRoot warns and counts it skipped, and its records are deleted like any path that fails in the walk.
Records exist beneath a symlink or .zfs path only if something was scanned through it, so the tests store theirs that way first. Records under the symlink target's own path were never at risk: paths are compared as text.
Judgement call: the warning reads walk <path>: skipping <kind> operand, kind being symlink, socket, FIFO, device node or .zfs directory.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/sfdupes/issues/9.
A symlink, socket, FIFO or device-node operand, or a directory operand named `.zfs`, used to be ignored without a word while staying in the scanned operands, so the update phase deleted every record stored beneath it. `syncScan` now checks each operand's type before loading the database index: such an operand gets a one-line warning, counts as skipped, and is dropped. Its records then count as outside the scanned operands: they are not deleted, but the content phase may still fill in their `content` hash, as for any such record. Exit status stays 0; a nonexistent operand is still fatal. `README.md` says so.
What the diff does not show:
- The check runs before overlapping operands are pruned, so `/a/link` given alongside `/a` is now warned about instead of dropped silently; `/a` is still scanned, so the records beneath `/a/link` are deleted as unverified.
- An operand that changes type between that check and the walk is still a root: `seedRoot` warns and counts it skipped, and its records are deleted like any path that fails in the walk.
- Records exist beneath a symlink or `.zfs` path only if something was scanned through it, so the tests store theirs that way first. Records under the symlink target's own path were never at risk: paths are compared as text.
Judgement call: the warning reads `walk <path>: skipping <kind> operand`, kind being `symlink`, `socket`, `FIFO`, `device node` or `.zfs directory`.
Model: opus-5-5
README.md, the new paragraph in "scan mode" ("so the records stored beneath it are left as they are") and the new exception in "Rules for the walk" ("or is an operand dropped before the database was read"): both are false when the dropped operand lies under another operand. sfdupes scan /a /a/link, with /a/link a symlink, warns about and skips /a/link but still deletes the records stored beneath /a/link, because /a is scanned and its walk does not verify them. This matters because the README is the specification and promises a keep-records guarantee the code does not give in this case. Acceptable: both places say the records are kept only when no other operand covers the dropped one; otherwise they are deleted like any other unverified record under that operand.
Model: opus-5-5
- `README.md`, the new paragraph in "`scan` mode" ("so the records stored beneath it are left as they are") and the new exception in "Rules for the walk" ("or is an operand dropped before the database was read"): both are false when the dropped operand lies under another operand. `sfdupes scan /a /a/link`, with `/a/link` a symlink, warns about and skips `/a/link` but still deletes the records stored beneath `/a/link`, because `/a` is scanned and its walk does not verify them. This matters because the README is the specification and promises a keep-records guarantee the code does not give in this case. Acceptable: both places say the records are kept only when no other operand covers the dropped one; otherwise they are deleted like any other unverified record under that operand.
Model: opus-5-5
Records beneath a dropped operand: both places in README.md, and the matching comments in scan.go, now say they are kept only when the dropped operand lies under no other operand, and otherwise deleted like any other unverified record under that operand; the code is unchanged. The PR body says the same.
From the rebase: #7 made README.md promise that every scan warning is escaped, so the operand warning now goes through escapePath too; it has no test of its own.
Model: opus-5-5
Rework, rebased onto `next`:
- Records beneath a dropped operand: both places in `README.md`, and the matching comments in `scan.go`, now say they are kept only when the dropped operand lies under no other operand, and otherwise deleted like any other unverified record under that operand; the code is unchanged. The PR body says the same.
- From the rebase: https://git.eeqj.de/sneak/sfdupes/issues/7 made `README.md` promise that every `scan` warning is escaped, so the operand warning now goes through `escapePath` too; it has no test of its own.
Model: opus-5-5
main_test.go, assertOperandSkipped: the call openDB(dbPath) no longer compiles on current next, where #8 gave openDB a second argument, the connection parameters. Once rebased, the test package fails to build. The TODO.md Completed Steps entry also conflicts with the entry for #8 at the top. Acceptable: rebase onto current next, pass connection parameters (the read-only reportParams fits a test that only reads), and keep the new entry above that one.
README.md, the new paragraph in "scan mode", says "The records stored beneath it are left as they are", and the walkableRoots comment in scan.go says the same. This is not true: those records now count as outside the scanned operands, so the content phase still fills in the content hash of any such record of 10 MiB or more whose size, head and tail match another record's. Every other place the README describes records outside the scanned operands states this exception. Acceptable: say the records are not deleted and are treated like any other record outside the scanned operands, the content-phase exception included.
Model: opus-5-5
- `main_test.go`, `assertOperandSkipped`: the call `openDB(dbPath)` no longer compiles on current `next`, where https://git.eeqj.de/sneak/sfdupes/issues/8 gave `openDB` a second argument, the connection parameters. Once rebased, the test package fails to build. The `TODO.md` Completed Steps entry also conflicts with the entry for https://git.eeqj.de/sneak/sfdupes/issues/8 at the top. Acceptable: rebase onto current `next`, pass connection parameters (the read-only `reportParams` fits a test that only reads), and keep the new entry above that one.
- `README.md`, the new paragraph in "`scan` mode", says "The records stored beneath it are left as they are", and the `walkableRoots` comment in `scan.go` says the same. This is not true: those records now count as outside the scanned operands, so the content phase still fills in the `content` hash of any such record of 10 MiB or more whose size, `head` and `tail` match another record's. Every other place the README describes records outside the scanned operands states this exception. Acceptable: say the records are not deleted and are treated like any other record outside the scanned operands, the content-phase exception included.
Model: opus-5-5
openDB in assertOperandSkipped: now passes reportParams; the TODO.md entry sits above the one for #8.
Records beneath a dropped operand: the "scan mode" paragraph in README.md and the walkableRoots comment now say they are not deleted and are treated like any other record outside the scanned operands, the content-phase exception included. The PR body and commit message say the same.
Model: opus-5-5
Rework, rebased onto `next`:
- `openDB` in `assertOperandSkipped`: now passes `reportParams`; the `TODO.md` entry sits above the one for https://git.eeqj.de/sneak/sfdupes/issues/8.
- Records beneath a dropped operand: the "`scan` mode" paragraph in `README.md` and the `walkableRoots` comment now say they are not deleted and are treated like any other record outside the scanned operands, the content-phase exception included. The PR body and commit message say the same.
Model: opus-5-5
scan.go, syncScan: the operand type check has to run before overlapping operands are pruned. Otherwise sfdupes scan /a /a/b, with /a a symlink to a directory, prunes /a/b as lying under /a, then drops /a, and scans nothing without a word, which is the defect #9 fixes. The code has the order right, but nothing keeps it that way. Swapping the two calls in pruneRoots(s.walkableRoots(roots)) breaks no test, no comment says the order matters, and README.md "scan mode" says an operand lying under another is dropped without excepting the case where the other operand is itself dropped. Acceptable: a test that scans a symlink operand together with an operand beneath it and checks that the file beneath it is scanned and recorded; a short comment at that call saying why the type check comes first; and a sentence in the new "scan mode" paragraph saying that an operand beneath a dropped operand is still scanned.
PR body: about 280 words, over the limit of about 250. Acceptable: about 250 words or fewer. The bullets on captureStream and on escaping describe what the diff already shows, so they can go.
Model: opus-5-5
- `scan.go`, `syncScan`: the operand type check has to run before overlapping operands are pruned. Otherwise `sfdupes scan /a /a/b`, with `/a` a symlink to a directory, prunes `/a/b` as lying under `/a`, then drops `/a`, and scans nothing without a word, which is the defect https://git.eeqj.de/sneak/sfdupes/issues/9 fixes. The code has the order right, but nothing keeps it that way. Swapping the two calls in `pruneRoots(s.walkableRoots(roots))` breaks no test, no comment says the order matters, and `README.md` "`scan` mode" says an operand lying under another is dropped without excepting the case where the other operand is itself dropped. Acceptable: a test that scans a symlink operand together with an operand beneath it and checks that the file beneath it is scanned and recorded; a short comment at that call saying why the type check comes first; and a sentence in the new "`scan` mode" paragraph saying that an operand beneath a dropped operand is still scanned.
- PR body: about 280 words, over the limit of about 250. Acceptable: about 250 words or fewer. The bullets on `captureStream` and on escaping describe what the diff already shows, so they can go.
Model: opus-5-5
Type check before pruning: added TestRunScanWalksOperandUnderSymlinkOperand, a comment at the call in syncScan, and the sentence "Another operand beneath it is still scanned." in the new "scan mode" paragraph.
PR body: cut to about 240 words, without the captureStream and escaping bullets.
Model: opus-5-5
Rework, on current `next`:
- Type check before pruning: added `TestRunScanWalksOperandUnderSymlinkOperand`, a comment at the call in `syncScan`, and the sentence "Another operand beneath it is still scanned." in the new "`scan` mode" paragraph.
- PR body: cut to about 240 words, without the `captureStream` and escaping bullets.
Model: opus-5-5
main_test.go and TODO.md do not rebase onto current next. #30 changed run to take a stdout writer as well as a stderr writer, and removed captureStdout. So captureStream, captureStdout and scanOK conflict, and scanOK still calls the old run(args, os.Stderr). The new TODO.md entry also conflicts with the one for #30 at the top of Completed Steps. Acceptable: rebase onto current next, and rewrite scanOK and the new tests against run(args, stdout, stderr). They still need to capture os.Stderr for the operand warning and the summary, because scan writes those straight to os.Stderr, not to the writer run is given. Keep the new TODO.md entry above the one for #30.
Model: opus-5-5
- `main_test.go` and `TODO.md` do not rebase onto current `next`. https://git.eeqj.de/sneak/sfdupes/issues/30 changed `run` to take a stdout writer as well as a stderr writer, and removed `captureStdout`. So `captureStream`, `captureStdout` and `scanOK` conflict, and `scanOK` still calls the old `run(args, os.Stderr)`. The new `TODO.md` entry also conflicts with the one for https://git.eeqj.de/sneak/sfdupes/issues/30 at the top of Completed Steps. Acceptable: rebase onto current `next`, and rewrite `scanOK` and the new tests against `run(args, stdout, stderr)`. They still need to capture `os.Stderr` for the operand warning and the summary, because `scan` writes those straight to `os.Stderr`, not to the writer `run` is given. Keep the new `TODO.md` entry above the one for https://git.eeqj.de/sneak/sfdupes/issues/30.
Model: opus-5-5
#30 changed run to take a stdout writer and removed captureStdout. scanOK now passes a buffer as stdout and os.Stderr as stderr, after redirecting os.Stderr to a file, because scan writes the operand warning and its summary there directly. The PR's captureStream became captureStderr, since only os.Stderr is captured now. The new tests go through scanOK and are otherwise unchanged.
Rebased onto `next`:
- https://git.eeqj.de/sneak/sfdupes/issues/30 changed `run` to take a stdout writer and removed `captureStdout`. `scanOK` now passes a buffer as stdout and `os.Stderr` as stderr, after redirecting `os.Stderr` to a file, because `scan` writes the operand warning and its summary there directly. The PR's `captureStream` became `captureStderr`, since only `os.Stderr` is captured now. The new tests go through `scanOK` and are otherwise unchanged.
- The `TODO.md` entry sits above the one for https://git.eeqj.de/sneak/sfdupes/issues/30.
Nothing else changed.
Model: opus-5-5
A symlink, socket, FIFO or device-node operand, or a directory operand
named .zfs, was silently ignored yet stayed in the scanned operands, so
the update phase deleted every record stored beneath it. Such an
operand now gets a one-line warning, counts as skipped, and is dropped
before overlapping operands are pruned and the database index is
loaded: another operand beneath it is still scanned, and the records
beneath it count as outside the scanned operands and are not deleted,
unless it lies under another operand. The exit status stays 0. An
operand that turns into one of these after that check is warned about
and skipped by the walk instead. README "scan mode" and "Rules for the
walk" say so.
Model: opus-5-5
TODO.md conflicted with the entry for #53 at the top of Completed Steps. Both entries are kept, this one first.
README.md, main_test.go and scan.go merged without conflicts; the operand check runs after the scan lock is taken, so a second scan still fails before warning about any operand. Nothing else changed.
Model: opus-5-5
Rebased onto `next`:
- `TODO.md` conflicted with the entry for https://git.eeqj.de/sneak/sfdupes/issues/53 at the top of Completed Steps. Both entries are kept, this one first.
`README.md`, `main_test.go` and `scan.go` merged without conflicts; the operand check runs after the scan lock is taken, so a second scan still fails before warning about any operand. Nothing else changed.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Fixes #9.
A symlink, socket, FIFO or device-node operand, or a directory operand named
.zfs, used to be ignored without a word while staying in the scanned operands, so the update phase deleted every record stored beneath it.syncScannow checks each operand's type before loading the database index: such an operand gets a one-line warning, counts as skipped, and is dropped. Its records then count as outside the scanned operands: they are not deleted, but the content phase may still fill in theircontenthash, as for any such record. Exit status stays 0; a nonexistent operand is still fatal.README.mdsays so.What the diff does not show:
/a/linkgiven alongside/ais now warned about instead of dropped silently;/ais still scanned, so the records beneath/a/linkare deleted as unverified.seedRootwarns and counts it skipped, and its records are deleted like any path that fails in the walk..zfspath only if something was scanned through it, so the tests store theirs that way first. Records under the symlink target's own path were never at risk: paths are compared as text.Judgement call: the warning reads
walk <path>: skipping <kind> operand, kind beingsymlink,socket,FIFO,device nodeor.zfs directory.Model: opus-5-5
README.md, the new paragraph in "scanmode" ("so the records stored beneath it are left as they are") and the new exception in "Rules for the walk" ("or is an operand dropped before the database was read"): both are false when the dropped operand lies under another operand.sfdupes scan /a /a/link, with/a/linka symlink, warns about and skips/a/linkbut still deletes the records stored beneath/a/link, because/ais scanned and its walk does not verify them. This matters because the README is the specification and promises a keep-records guarantee the code does not give in this case. Acceptable: both places say the records are kept only when no other operand covers the dropped one; otherwise they are deleted like any other unverified record under that operand.Model: opus-5-5
3fcf4993e2tof829542b3bRework, rebased onto
next:README.md, and the matching comments inscan.go, now say they are kept only when the dropped operand lies under no other operand, and otherwise deleted like any other unverified record under that operand; the code is unchanged. The PR body says the same.README.mdpromise that everyscanwarning is escaped, so the operand warning now goes throughescapePathtoo; it has no test of its own.Model: opus-5-5
main_test.go,assertOperandSkipped: the callopenDB(dbPath)no longer compiles on currentnext, where #8 gaveopenDBa second argument, the connection parameters. Once rebased, the test package fails to build. TheTODO.mdCompleted Steps entry also conflicts with the entry for #8 at the top. Acceptable: rebase onto currentnext, pass connection parameters (the read-onlyreportParamsfits a test that only reads), and keep the new entry above that one.README.md, the new paragraph in "scanmode", says "The records stored beneath it are left as they are", and thewalkableRootscomment inscan.gosays the same. This is not true: those records now count as outside the scanned operands, so the content phase still fills in thecontenthash of any such record of 10 MiB or more whose size,headandtailmatch another record's. Every other place the README describes records outside the scanned operands states this exception. Acceptable: say the records are not deleted and are treated like any other record outside the scanned operands, the content-phase exception included.Model: opus-5-5
f829542b3btocf34b59632Rework, rebased onto
next:openDBinassertOperandSkipped: now passesreportParams; theTODO.mdentry sits above the one for #8.scanmode" paragraph inREADME.mdand thewalkableRootscomment now say they are not deleted and are treated like any other record outside the scanned operands, the content-phase exception included. The PR body and commit message say the same.Model: opus-5-5
scan.go,syncScan: the operand type check has to run before overlapping operands are pruned. Otherwisesfdupes scan /a /a/b, with/aa symlink to a directory, prunes/a/bas lying under/a, then drops/a, and scans nothing without a word, which is the defect #9 fixes. The code has the order right, but nothing keeps it that way. Swapping the two calls inpruneRoots(s.walkableRoots(roots))breaks no test, no comment says the order matters, andREADME.md"scanmode" says an operand lying under another is dropped without excepting the case where the other operand is itself dropped. Acceptable: a test that scans a symlink operand together with an operand beneath it and checks that the file beneath it is scanned and recorded; a short comment at that call saying why the type check comes first; and a sentence in the new "scanmode" paragraph saying that an operand beneath a dropped operand is still scanned.captureStreamand on escaping describe what the diff already shows, so they can go.Model: opus-5-5
cf34b59632tod8ca0c1f6cRework, on current
next:TestRunScanWalksOperandUnderSymlinkOperand, a comment at the call insyncScan, and the sentence "Another operand beneath it is still scanned." in the new "scanmode" paragraph.captureStreamand escaping bullets.Model: opus-5-5
main_test.goandTODO.mddo not rebase onto currentnext. #30 changedrunto take a stdout writer as well as a stderr writer, and removedcaptureStdout. SocaptureStream,captureStdoutandscanOKconflict, andscanOKstill calls the oldrun(args, os.Stderr). The newTODO.mdentry also conflicts with the one for #30 at the top of Completed Steps. Acceptable: rebase onto currentnext, and rewritescanOKand the new tests againstrun(args, stdout, stderr). They still need to captureos.Stderrfor the operand warning and the summary, becausescanwrites those straight toos.Stderr, not to the writerrunis given. Keep the newTODO.mdentry above the one for #30.Model: opus-5-5
d8ca0c1f6ctoa3471e01fdRebased onto
next:runto take a stdout writer and removedcaptureStdout.scanOKnow passes a buffer as stdout andos.Stderras stderr, after redirectingos.Stderrto a file, becausescanwrites the operand warning and its summary there directly. The PR'scaptureStreambecamecaptureStderr, since onlyos.Stderris captured now. The new tests go throughscanOKand are otherwise unchanged.TODO.mdentry sits above the one for #30.Nothing else changed.
Model: opus-5-5
Review passed; needs a rebase onto
nextonly.Model: opus-5-5
a3471e01fdto1fe65bc4b0Rebased onto
next:TODO.mdconflicted with the entry for #53 at the top of Completed Steps. Both entries are kept, this one first.README.md,main_test.goandscan.gomerged without conflicts; the operand check runs after the scan lock is taken, so a second scan still fails before warning about any operand. Nothing else changed.Model: opus-5-5