Fix UnixFileSystem.MoveFile on macOS when just changing file casing#18619
Conversation
| // for case-insensitive file systems, so that we support changing the casing in the naming | ||
| // of the file. If this fails in any way (e.g. source file doesn't exist, dest file doesn't | ||
| // exist, rename fails, etc.), we just fall back to trying the link/unlink approach and | ||
| // generating any exceptional messages from there as necessary. |
There was a problem hiding this comment.
A concrete example (like from #18521) would help make understanding this a bit easier I think.
There was a problem hiding this comment.
Ok, I can add a clarification to the comment that this is particularly impactful for changing the casing in a file name.
| // generating any exceptional messages from there as necessary. | ||
| Interop.Sys.FileStatus sourceStat, destStat; | ||
| if (Interop.Sys.LStat(sourceFullPath, out sourceStat) == 0 && // source file exists | ||
| Interop.Sys.Stat(destFullPath, out destStat) == 0 && // dest file exists |
There was a problem hiding this comment.
What happens if the destination is a link?
There was a problem hiding this comment.
We'll follow it (due to using stat). Does your question mean you think this should be LStat instead?
There was a problem hiding this comment.
you think this should be LStat instead?
I'm unsure. If A links to B and you try to rename A to a it won't give you the same file descriptor, right? But we should still be renaming, no?
There was a problem hiding this comment.
You're probably right. It's probably safest to just not follow symlinks and purely look at the target being passed.
Neither the link/unlink nor copy-all-data approaches work well on a case-insensitive file system when just changing the casing of the file name. This commit adds an upfront check that detects when the files are the same, and in such cases, just does a rename.
stephentoub
force-pushed
the
file_move_fix
branch
from
9129b4f to
f29050c
Compare
5 participants
Neither the link/unlink nor copy-all-data approaches work well on a case-insensitive file system when just changing the casing of the file name. This PR adds an upfront check that detects when the files are the same, and in such cases, just does a rename.
(Note that this depends on System.Private.CoreLib having the same understanding of the size of Interop.Sys.FileStatus, and thus the Unix legs of CI will fail until dotnet/coreclr#11070 is merged and consumed into CoreFx.)
https://github.com/dotnet/corefx/issues/18521
cc: @ianhays, @JeremyKuhne, @bording