Improve Directory.Move and DirectoryInfo.MoveTo #63675
Conversation
…rectoryInfo_MoveTo class and allows for testing DirectoryInfo.MoveTo as well
…mon part to FileSystem.MoveDirectory
ghost
assigned 
|
Tagging subscribers to this area: @dotnet/area-system-io Issue DetailsFixes #63287 Explanation:
|
adamsitnik
changed the title
so we need to handle it before rename is called
| ref MemoryMarshal.GetReference(converterOldPath.ConvertAndTerminateString(oldPath)), | ||
| ref MemoryMarshal.GetReference(converterNewPath.ConvertAndTerminateString(newPath))); | ||
| converterNewPath.Dispose(); | ||
| converterOldPath.Dispose(); |
There was a problem hiding this comment.
Why do you use explicit Dispose calls (instead of using-Statements)?
|
@tmds could you please take a look at the Unix part? |
| } | ||
|
|
||
| if (FileExists(destFullPath)) | ||
| if (!sameDirectoryDifferentCase) // This check is to allow renaming of directories |
There was a problem hiding this comment.
Because file systems are case sensitive I don't understand why we need a specific check for sameDirectoryDifferentCase?
There was a problem hiding this comment.
this is something that already existed and to be honest I have not questioned that before you have asked the question. It seems that @wfurt agrees with you (#65059 (comment)):
The MoveDirectory_FailToMoveLowerCaseDirectoryWhenUpperCaseDirectoryExists is technically wrong on Unix IMHO.
The case sensitivity is determined by given filesystem not by the OS itself.
There was a problem hiding this comment.
My guess is that this may be there to surface better exceptions. We could defer to actual IO but that may give different behaviors on different platforms. And Some OSes and file systems do care about case and some don't.
@adamsitnik apologies, I forgot about your request. I have one question. |
Fixes #63287
Explanation:
Directory.Movealready contained the bug fix (dotnet/corefx#40570), so I've just moved this logic toFileSystem.MoveDirectoryand now bothDirectory.MoveandDirectoryInfo.MoveToare using the same code.Update: I've found few places for improvements and reduced the number of sys-calls and got allocations to zero.
Explanation:
renamesys-call overwrites destination if it exists. To keep old .NET Framework behavior that is inherited from Windows, we perform a check for file/dictionary existence before callingrename. But we don't need to do that for source, asrenameis just going to fail if it does not exist. Moreover, there is no need to perform two sys-calls to check whether destination exists (one to check if directory exists, another to check if file exists). We can perform a single sys-call (stat) which just fails if given path does not exist.Linux before:
Linux after:
Windows before:
Windows after:
Source code: dotnet/performance#2208