(GH-1241) More Robust File.Replace

Using File.Replace can have some access violation concerns. It demands full access to a file, which can cause issues if the file is locked. If a file is open in another process, it has issues renaming the file, which causes the rest to not work appropriately. Implement file replace methods in a move step that will work with files that are open and then copy the original file to the destination and delete the original file. If the file locked is the backup file, it should log that it was not able to produce the backup and continue without failure. If it has issues removing the source file, it should log that it was not able to delete the file and continue without failure.
chocolatey · May 29, 2017 · f9d4298 · f9d4298
1 parent db91b2a
commit f9d4298
Showing 1 changed file with 40 additions and 1 deletion.
diff --git a/src/chocolatey/infrastructure/filesystem/DotNetFileSystem.cs b/src/chocolatey/infrastructure/filesystem/DotNetFileSystem.cs
@@ -400,7 +400,46 @@ public void replace_file(string sourceFilePath, string destinationFilePath, stri
                 {
                     try
                     {
-                        File.Replace(sourceFilePath, destinationFilePath, backupFilePath);
+                        // File.Replace is very sensitive to issues with file access
+                        // the docs mention that using a backup fixes this, but this
+                        // has not been the case with choco - the backup file has been
+                        // the most sensitive to issues with file locking
+                        //File.Replace(sourceFilePath, destinationFilePath, backupFilePath);
+
+                        // move existing file to backup location
+                        if (!string.IsNullOrEmpty(backupFilePath) && file_exists(destinationFilePath))
+                        {
+                            try
+                            {
+                                this.Log().Debug(ChocolateyLoggers.Trace, "Backing up '{0}' to '{1}'.".format_with(destinationFilePath, backupFilePath));
+
+                                if (file_exists(backupFilePath))
+                                {
+                                    this.Log().Debug(ChocolateyLoggers.Trace, "Deleting existing backup file at '{0}'.".format_with(backupFilePath));
+                                    delete_file(backupFilePath);
+                                }
+                                this.Log().Debug(ChocolateyLoggers.Trace, "Moving '{0}' to '{1}'.".format_with(destinationFilePath, backupFilePath));
+                                move_file(destinationFilePath, backupFilePath);
+                            }
+                            catch (Exception ex)
+                            {
+                                this.Log().Debug("Error capturing backup of '{0}':{1} {2}".format_with(destinationFilePath, Environment.NewLine, ex.Message));
+                            }
+                        }
+                        // copy source file to destination
+                        this.Log().Debug(ChocolateyLoggers.Trace, "Copying '{0}' to '{1}'.".format_with(sourceFilePath, destinationFilePath));
+                        copy_file(sourceFilePath, destinationFilePath, overwriteExisting: true);
+
+                        // delete source file
+                        try
+                        {
+                            this.Log().Debug(ChocolateyLoggers.Trace, "Removing '{0}'".format_with(sourceFilePath));
+                            delete_file(sourceFilePath);
+                        }
+                        catch (Exception ex)
+                        {
+                            this.Log().Debug("Error removing '{0}':{1} {2}".format_with(sourceFilePath, Environment.NewLine, ex.Message));
+                        }
                     }
                     catch (IOException)
                     {