fix: fall back to copy+unlink when rename fails with EXDEV - #1
dyk1454683243-sudo wants to merge 1 commit into
Conversation
When fs.rename fails with EXDEV (Windows AppX virtualized paths, or a true cross-device move), write the temp file with copy+unlink so writeFile / writeFileSync can still succeed. Rename remains the default atomic path. Other rename errors are unchanged. Fixes npm#71 Co-authored-by: David <dyk1454683243-sudo@users.noreply.github.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 70f6ca9316
ℹ️ 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".
| if (!isExdevError(err)) { | ||
| throw err | ||
| } | ||
| await promisify(fs.copyFile)(tmpfile, dest) |
There was a problem hiding this comment.
When rename returns EXDEV and fsync is enabled, only the temporary source has been synced; copyFile creates dirty data in the destination, after which the source is unlinked and the operation reports success. A crash immediately after this fallback can therefore lose or truncate a write that the default fsync path would have persisted. The synchronous fallback has the same issue and should also sync the destination after copying.
Useful? React with 👍 / 👎.
| if (!isExdevError(err)) { | ||
| throw err | ||
| } | ||
| await promisify(fs.copyFile)(tmpfile, dest) |
There was a problem hiding this comment.
On filesystems where copyFile overwrites an existing destination in place, the EXDEV fallback changes normal replacement semantics: if the destination has another hard link, that other pathname is also rewritten, whereas the original rename path replaces only the requested directory entry and leaves the other link's contents intact. The synchronous copyFileSync path behaves similarly, so this fallback should copy to a destination-side temporary file and replace the destination rather than copying directly over it.
Useful? React with 👍 / 👎.
2 participants
This fork-only PR was opened by the agent workflow. It is not the review target.
The upstream pull request is npm#241