Repository navigation
fix(core): rename instead of deleting when a patch only changes a file's case - #63655
Conversation
![opencodejr[bot]](https://avatars.githubusercontent.com/in/5497873?s=60&v=4){kind=small}
There was a problem hiding this comment.
On a case-insensitive filesystem the patch no longer deletes the file, but the new check matches too broadly and breaks two moves on Linux that work on dev today. Changes requested.
How I checked it: I ran the new test on a vfat (case-insensitive) mount. On the base it fails and the directory ends up empty, which is the #63126 data loss. With this PR the file survives, but it keeps the name same.txt. As far as I know, Linux's vfat driver ignores case-only renames, so this probably says nothing about macOS or Windows. I couldn't check those. I also compared the two moves below against the base on a normal Linux tmp dir. tool-patch.test.ts passes (43 tests) and typecheck:tsc is clean.
Public API: unchanged.
Concern (inline): recased matches any two paths that are equal ignoring case. That includes moves between directories that differ only in case, and moves of symlinks. On Linux both now behave differently from dev: one fails partway through, the other writes through the link into its target.
| const target = moveTarget ?? change.target | ||
| // A move that only changes case is the same file on macOS and Windows, where writing the new name | ||
| // and removing the old one deletes it, so it is written in place and renamed. | ||
| const recased = moveTarget?.absolute.toLowerCase() === change.target.absolute.toLowerCase() |
There was a problem hiding this comment.
This matches any two paths that are equal ignoring case, not only ones that are the same file. On a case-sensitive filesystem (Linux), this breaks two moves that work on dev. I compared both against the base:
Foo/a.txt→foo/a.txtwhenfoo/doesn't exist:devcreatesfoo/a.txt. Here the content is written intoFoo/a.txtin place, thenmovefails withENOENT(it doesn't create parent directories). The tool errors with the old file already changed.- Symlink
link.txt→Link.txt:devleaves the link's target alone and writes a regularLink.txt. Here the new content is written through the link into its target, and the link is renamed. That's what the "moves a symlink without deleting its target" test guards against for normal moves.
One option: take the rename path only when the two paths really point to the same file. For example, require that the move target already exists, which only happens for a case-only rename on a case-insensitive filesystem, and that the source isn't a symlink. Everything else would keep the write-and-remove path.
Issue for this PR
Closes #63126
Type of change
What does this PR do?
A patch that moves a file writes the new name, then removes the old one. On macOS and Windows file names ignore case by default, so a move that only changes case (
same.txt→Same.txt) wrote the file in place and then deleted it, and the tool failed with "file does not exist" on an empty directory.A move now takes the rename path only when the two paths match ignoring case, the source is a regular file, and the new name already exists, which on a case-insensitive filesystem means it's the same file. Everything else keeps the write-and-remove path, so on Linux a move into a directory that differs only in case still creates it, and a symlink's target is left alone. If a different file with the new name exists on Linux, the rename ends the same way write-and-remove does: the new name holds the new contents and the old name is gone.
How did you verify your code works?
same.txtmoved toSame.txtleaves onlySame.txtwith the new contents. It fails on dev on macOS (the directory ends up empty).Foo/a.txtmoved tofoo/a.txtsucceeds. On Linux, wherefoo/doesn't exist, it failed with the first version of this PR.bun run script/test.ts test/tool-patch.test.tsin packages/core (44 pass),bun run check, andtypecheck:tscin packages/core.Checklist