Handle a source path changing type against a non-empty destination - #70
Merged
Conversation
A source directory replaced by a symlink (or file, or special) between passes left the destination stuck forever: remove_dst's plain unlinkat/rmdir fails with ENOTEMPTY against the still-populated old directory, then the create call that follows (symlinkat, mkdirat, mknodat, or open-for-copy) fails too (EEXIST) since the old object is still there. Reported live for the directory->symlink case, but remove_dst is the single shared helper every d_type transition in handle_entry's switch already routes through, so the bug applied to any type change against a non-empty former directory, not just symlinks. remove_dst now renames a non-empty directory aside under a dedicated .drsync.stale.<job>-<pass>.<shard>.<seq> prefix (distinct from temp_prefix, so the orphan sweep's temp-reclaim logic never mistakes it for live copy residue) and journals the new name as an ordinary JR_ORPHAN, instead of just failing. The caller then creates the new object at the now-vacated original name in the same pass. The stale subtree is picked up and removed by the next explicit DELETE pass through the same fan-out machinery any other orphan already uses - safe at arbitrary depth/size, no new coordinator-side code needed. Adds type_change_e2e.sh: a directory with nested content, synced normally, then replaced on the source with a symlink - verifies the symlink lands with the right target, the old content is renamed aside and journaled as an orphan, and a subsequent delete pass fully reclaims it while leaving the new symlink untouched. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PsdNZLfmAFrMX2VUtkLtmm
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
1 participant
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.
Summary
remove_dst(agent/src/walker.c) is the single shared helper every type transition (dir<->file<->symlink<->special) already routes through - not a symlink-specific bug, any type change against a non-empty former directory hit this..drsync.stale.*prefix and journal it as an ordinaryJR_ORPHAN, instead of just failing. The new object is then created at the now-vacated name in the same pass. The stale subtree is reclaimed by the next DELETE pass through the existing fan-out machinery - no new coordinator-side code.type_change_e2e.shcovering the exact reported scenario end-to-end (sync a directory with nested content, replace it with a symlink on the source, verify the symlink lands correctly, the old content is orphaned and journaled, and a delete pass fully reclaims it).Test plan
make -C agent test- all greengo test -count=1 ./...- all green (no Go changes needed for this fix)type_change_e2e.sh- verified it fails against the pre-fix code (symlink never created) and passes with the fix🤖 Generated with Claude Code
https://claude.ai/code/session_01PsdNZLfmAFrMX2VUtkLtmm