FileSystem.Unix.File.Move uses "rename" in more cases (dotnet/corefx#40611)
authorSylvain <sf@ellisys.com>
Mon, 7 Oct 2019 20:50:47 +0000 (22:50 +0200)
committerCarlos Sanchez Lopez <1175054+carlossanlop@users.noreply.github.com>
Mon, 7 Oct 2019 20:50:47 +0000 (13:50 -0700)
* FileSystem.Unix.File.Move use rename in more cases, avoid link/copy when possible, improve performance on file systems that do not support hard links, such as FAT

* Adapt Unit Tests for accounting FileSystemWatcher events fired by FileSystem.Unix.File.Move implementation that use rename in more cases, avoiding link/copy when possible

Commit migrated from https://github.com/dotnet/corefx/commit/89b087ba5548338bb604db4e1cca0e42f8273e2b

src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.File.Move.cs
src/libraries/System.IO.FileSystem/src/System/IO/FileSystem.Unix.cs

index 123827c..b9d5d97 100644 (file)
@@ -20,7 +20,7 @@ namespace System.IO.Tests
         [PlatformSpecific(TestPlatforms.AnyUnix)]  // Expected WatcherChangeTypes are different based on OS
         public void Unix_File_Move_To_Same_Directory()
         {
-            FileMove_SameDirectory(WatcherChangeTypes.Created | WatcherChangeTypes.Deleted);
+            FileMove_SameDirectory(WatcherChangeTypes.Renamed);
         }
 
         [Fact]
@@ -40,7 +40,7 @@ namespace System.IO.Tests
         [PlatformSpecific(TestPlatforms.OSX)]  // Expected WatcherChangeTypes are different based on OS
         public void OSX_File_Move_To_Different_Watched_Directory()
         {
-            FileMove_DifferentWatchedDirectory(WatcherChangeTypes.Changed);
+            FileMove_DifferentWatchedDirectory(0);
         }
 
         [Fact]
@@ -75,7 +75,7 @@ namespace System.IO.Tests
         [PlatformSpecific(TestPlatforms.AnyUnix)]  // Expected WatcherChangeTypes are different based on OS
         public void Unix_File_Move_In_Nested_Directory(bool includeSubdirectories)
         {
-            FileMove_NestedDirectory(includeSubdirectories ? WatcherChangeTypes.Created | WatcherChangeTypes.Deleted : 0, includeSubdirectories);
+            FileMove_NestedDirectory(includeSubdirectories ? WatcherChangeTypes.Renamed : 0, includeSubdirectories);
         }
 
         [Fact]
@@ -89,7 +89,7 @@ namespace System.IO.Tests
         [PlatformSpecific(TestPlatforms.AnyUnix)]  // Expected WatcherChangeTypes are different based on OS
         public void Unix_File_Move_With_Set_NotifyFilter()
         {
-            FileMove_WithNotifyFilter(WatcherChangeTypes.Deleted);
+            FileMove_WithNotifyFilter(WatcherChangeTypes.Renamed);
         }
 
         #region Test Helpers
index cae2d0a..4b65e25 100644 (file)
@@ -142,20 +142,18 @@ namespace System.IO
         {
             // 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. 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.
-            //
-            // Rename is also appropriate for overwrite=true with same source/dest file
+            // link/unlink are used instead. Rename is more efficient than link/unlink on file systems
+            // where hard links are not supported (such as FAT). Therefore, given that source file exists,
+            // rename is used in 2 cases: when dest file does not exist or when source path and dest
+            // path refer to the same file (on the same device). 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.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.LStat(destFullPath, out destStat) != 0 || // dest file does not exist
+                    sourceStat.Ino == destStat.Ino) && // source and dest are the same file on that device
                 Interop.Sys.Rename(sourceFullPath, destFullPath) == 0) // try the rename
             {
                 // Renamed successfully.