From 631389b6937df7c9940873704cb6cd1e4d7b665c Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Wed, 19 Apr 2017 06:57:08 -0400 Subject: [PATCH 1/3] Add tests for moving to the same path w/ and w/o different casing --- src/System.IO.FileSystem/tests/File/Move.cs | 28 +++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/System.IO.FileSystem/tests/File/Move.cs b/src/System.IO.FileSystem/tests/File/Move.cs index eeb4c914b772..995754ba5ea2 100644 --- a/src/System.IO.FileSystem/tests/File/Move.cs +++ b/src/System.IO.FileSystem/tests/File/Move.cs @@ -120,6 +120,34 @@ public void MoveIntoParentDirectory() Assert.True(File.Exists(testFileDest.FullName)); } + [Fact] + public void MoveToSameName() + { + string testDir = GetTestFilePath(); + Directory.CreateDirectory(testDir); + + FileInfo testFileSource = new FileInfo(Path.Combine(testDir, GetTestFileName())); + testFileSource.Create().Dispose(); + + Move(testFileSource.FullName, testFileSource.FullName); + Assert.True(File.Exists(testFileSource.FullName)); + } + + [Fact] + public void MoveToSameNameDifferentCasing() + { + string testDir = GetTestFilePath(); + Directory.CreateDirectory(testDir); + + FileInfo testFileSource = new FileInfo(Path.Combine(testDir, Path.GetRandomFileName().ToLowerInvariant())); + testFileSource.Create().Dispose(); + + FileInfo testFileDest = new FileInfo(Path.Combine(testFileSource.DirectoryName, testFileSource.Name.ToUpperInvariant())); + + Move(testFileSource.FullName, testFileDest.FullName); + Assert.True(File.Exists(testFileDest.FullName)); + } + [Fact] public void MultipleMoves() { From f29050cf9cad6e62afc3cb4917e1e9056a1abe3c Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Wed, 19 Apr 2017 07:23:52 -0400 Subject: [PATCH 2/3] Fix UnixFileSystem.MoveFile on macOS when just changing file casing 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. --- .../Unix/System.Native/Interop.Stat.cs | 2 ++ src/Native/Unix/System.Native/pal_io.cpp | 2 ++ src/Native/Unix/System.Native/pal_io.h | 2 ++ .../src/System/IO/UnixFileSystem.cs | 20 +++++++++++++++++-- 4 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/Common/src/Interop/Unix/System.Native/Interop.Stat.cs b/src/Common/src/Interop/Unix/System.Native/Interop.Stat.cs index a8bc2ec7d135..cda00ac6c336 100644 --- a/src/Common/src/Interop/Unix/System.Native/Interop.Stat.cs +++ b/src/Common/src/Interop/Unix/System.Native/Interop.Stat.cs @@ -27,6 +27,8 @@ internal struct FileStatus internal long MTime; internal long CTime; internal long BirthTime; + internal long Dev; + internal long Ino; } internal static class FileTypes diff --git a/src/Native/Unix/System.Native/pal_io.cpp b/src/Native/Unix/System.Native/pal_io.cpp index a672c1e91ed5..a1c41e37d666 100644 --- a/src/Native/Unix/System.Native/pal_io.cpp +++ b/src/Native/Unix/System.Native/pal_io.cpp @@ -140,6 +140,8 @@ static_assert(PAL_IN_ISDIR == IN_ISDIR, ""); static void ConvertFileStatus(const struct stat_& src, FileStatus* dst) { + dst->Dev = static_cast(src.st_dev); + dst->Ino = static_cast(src.st_ino); dst->Flags = FILESTATUS_FLAGS_NONE; dst->Mode = static_cast(src.st_mode); dst->Uid = src.st_uid; diff --git a/src/Native/Unix/System.Native/pal_io.h b/src/Native/Unix/System.Native/pal_io.h index 083efa04ee42..deace9ca302f 100644 --- a/src/Native/Unix/System.Native/pal_io.h +++ b/src/Native/Unix/System.Native/pal_io.h @@ -24,6 +24,8 @@ struct FileStatus int64_t MTime; // time of last modification int64_t CTime; // time of last status change int64_t BirthTime; // time the file was created + int64_t Dev; // ID of the device containing the file + int64_t Ino; // inode number of the file }; /************ diff --git a/src/System.IO.FileSystem/src/System/IO/UnixFileSystem.cs b/src/System.IO.FileSystem/src/System/IO/UnixFileSystem.cs index a74578005f7a..3c821a00ad36 100644 --- a/src/System.IO.FileSystem/src/System/IO/UnixFileSystem.cs +++ b/src/System.IO.FileSystem/src/System/IO/UnixFileSystem.cs @@ -82,8 +82,24 @@ public override void MoveFile(string sourceFullPath, string destFullPath) { // The desired behavior for Move(source, dest) is to not overwrite the destination file // if it exists. Since rename(source, dest) will replace the file at 'dest' if it exists, - // link/unlink are used instead. Note that the Unix FileSystemWatcher will treat a Move - // as a Creation and Deletion instead of a Rename and thus differ from Windows. + // link/unlink are used instead. However, if the source path and the dest path refer to + // the same file, then do a rename rather than a link and an unlink. This is important + // for case-insensitive file systems (e.g. renaming a file in a way that just changes casing), + // 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. + 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 + sourceStat.Dev == destStat.Dev && // source and dest are on the same device + sourceStat.Ino == destStat.Ino && // and source and dest are the same file on that device + Interop.Sys.Rename(sourceFullPath, destFullPath) == 0) // try the rename + { + // Renamed successfully. + return; + } + if (Interop.Sys.Link(sourceFullPath, destFullPath) < 0) { // If link fails, we can fall back to doing a full copy, but we'll only do so for From b6bbe8a43984f291e01113161efe1bc6e322815f Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Wed, 19 Apr 2017 17:58:29 -0400 Subject: [PATCH 3/3] Address PR feedback --- src/System.IO.FileSystem/src/System/IO/UnixFileSystem.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.IO.FileSystem/src/System/IO/UnixFileSystem.cs b/src/System.IO.FileSystem/src/System/IO/UnixFileSystem.cs index 3c821a00ad36..59319fe26e31 100644 --- a/src/System.IO.FileSystem/src/System/IO/UnixFileSystem.cs +++ b/src/System.IO.FileSystem/src/System/IO/UnixFileSystem.cs @@ -91,7 +91,7 @@ public override void MoveFile(string sourceFullPath, string destFullPath) // 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 + Interop.Sys.LStat(destFullPath, out destStat) == 0 && // dest file exists sourceStat.Dev == destStat.Dev && // source and dest are on the same device sourceStat.Ino == destStat.Ino && // and source and dest are the same file on that device Interop.Sys.Rename(sourceFullPath, destFullPath) == 0) // try the rename