feat: two-phase rename for path collision safety#78
Conversation
Defer renames until after the walk completes and apply them in a collision-safe order. Overlapping source/target names use a two-phase quarantine via temp files; simple renames stay direct. Fixes dolph#35
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2dedcc9a42
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| for _, r := range phase2 { | ||
| log.Printf("Renaming %v to %v (phase 2)", filepath.Base(r.temp), filepath.Base(r.to)) | ||
| if err := os.Rename(r.temp, r.to); err != nil { |
There was a problem hiding this comment.
When any source/target chain triggers the two-phase path, phase 2 uses raw os.Rename, which overwrites an existing destination that was not one of the queued sources. For example, with files a, aa, and aaaa, running find-replace a aa queues a -> aa and aa -> aaaa; because aa is a source this takes the two-phase branch, and the phase-2 rename to aaaa silently clobbers the untouched aaaa file instead of reporting the same collision that renamePath would in the direct path.
Useful? React with 👍 / 👎.
| seq := 0 | ||
| for i := range plans { | ||
| r := &plans[i] | ||
| r.temp = filepath.Join(filepath.Dir(r.from), fmt.Sprintf(".find-replace.tmp.%d.%d", os.Getpid(), seq)) |
There was a problem hiding this comment.
The phase-1 temp is created inside the source's parent directory, so a descendant rename can be invalidated when an ancestor directory is also renamed later in the same two-phase batch. For example, find-replace a aa with a directory a/ containing file a and a sibling aa/ queues both a/a -> a/aa and a -> aa; phase 1 moves a/a to a/.find-replace.tmp..., then moves a itself, so phase 2 later looks for the child temp at the old a/... path and fails, leaving data stranded in the directory temp.
Useful? React with 👍 / 👎.
1 participant
Summary
Fixes #35. Renames are queued during the walk and applied after content rewrites finish. When source and target names overlap (chains or swaps), a two-phase quarantine rename avoids clobbering files.
Test plan
go test ./...TestApplyRenamesSwapcovers foo↔bar swapMade with Cursor