Repository navigation
Never treat one file reached at two paths as a duplicate of itself [patch] - #181
Merged
Merged
Conversation
…atch] A bind mount, a share mounted twice or a hardlink gives one file several paths. They hashed the same and were grouped as duplicates, and Deduplicate then deleted the "other copy". Through a bind mount that copy is the keeper's own directory entry, so the only copy of the data was deleted and reported as reclaimed space. The keeper re-hash could not catch it, because the keeper was the file being deleted. FileIdentity reads a file's (device, index) identity: (st_dev, st_ino) through the runtime's SystemNative_Stat shim on Unix, and the volume serial number and file index from GetFileInformationByHandle on Windows. FindDuplicates keeps one path per identity, the one SelectFileToKeep would prefer, so Scan, DryRun, Stats and Deduplicate no longer report such a file as a duplicate group. As a last guard, the deletion loop skips any candidate whose identity equals the keeper's and reports it as a skipped file. Fixes #139 Fixes #132 Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01EaKE6BddCbJMXwfuLwNnGh
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
…loop Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01EaKE6BddCbJMXwfuLwNnGh
…ty under the limit SonarCloud S3776 counted 18 against the allowed 15 once the identity guard was added. The try/catch around File.Delete moves into TryDelete unchanged, and the Unix-only identity test uses [OSCondition] instead of an early Inconclusive. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01EaKE6BddCbJMXwfuLwNnGh
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #139
Fixes #132
What was wrong
A bind mount, a share mounted twice, or a hardlink gives one file several paths. None of these is a reparse point, so the scan listed the file once per path. The paths hashed the same, landed in one duplicate group, and
Deduplicatedeleted the "other copy". Through a bind mount, that copy is the keeper's own directory entry, so the only copy of the data was deleted and reported as reclaimed space (#139). Through a hardlink, the deleted path was a name in another snapshot, and the reported reclaimed bytes were wrong (#132).What changed
FileIdentity(new) reads a file's(device, index)identity:(st_dev, st_ino)through the runtime'sSystemNative_Statshim.FileTypeuses the same shim for the same reason: its structure has fixed-width fields on every Unix.(VolumeSerialNumber, FileIndexHigh/Low)fromGetFileInformationByHandle.Deduplicator.FindDuplicateskeeps one path per identity within each hash group, the pathSelectFileToKeepwould prefer. As a result,Scan,DryRun,StatsandDeduplicateno longer report one file seen at two paths as a duplicate group. Identity is read only for files already in a group of two or more, so the scan itself is not slowed.Deduplicator.DeleteDuplicatesgets the last guard the triage suggested: a candidate whose identity equals the keeper's is never deleted. It is reported as aSkippedFileinstead, whatever grouping the loop is handed.For hardlinks, this takes #132's first option: an extra hardlink is treated as the same file, matching the existing symlink policy.
Tests
New
SameFileTests:ls -ireports, which pins the struct offsets.DeleteDuplicates, given a hand-built group of a keeper and its hardlink, deletes nothing and reports the link as skipped.mount --bindofreal/ontoview/, thenDeduplicateconfirmed withy. The file survives with its content, nothing is deleted, andDryRunreports no duplicates. The test is inconclusive where bind mounts are unavailable (non-Linux, or not root).With the
Deduplicator.cschange reverted, 4 tests fail: both grouping tests, the deletion-guard test and the bind-mount repro. The full suite passes locally on Linux: 97 passed, 3 inconclusive. Those 3 were already inconclusive because they need an unprivileged user. The Windows hardlink path (mklink /H) and theGetFileInformationByHandlepath run only in CI.Note
PR #180 changes
TryGetSize's visibility and the verbs. This PR changes neither, so the two should merge cleanly in either order.🤖 Generated with Claude Code
https://claude.ai/code/session_01EaKE6BddCbJMXwfuLwNnGh
Generated by Claude Code