From bfec456aae2ad7abcec0f8c4c8b2f44308e48961 Mon Sep 17 00:00:00 2001 From: Rob Mensching Date: Tue, 4 Oct 2022 10:36:39 -0700 Subject: Add retries when trying to update bundle resources Fixes 6882 and 6902 --- .../Services/IFileSystem.cs | 19 +++++++++++ .../Bundles/CreateBundleExeCommand.cs | 12 ++----- src/wix/WixToolset.Core.Burn/BurnBackendErrors.cs | 12 +++---- .../WixToolset.Core/Bind/TransferFilesCommand.cs | 2 +- .../ExtensibilityServices/FileSystem.cs | 37 ++++++++++++---------- .../BundleFixture.cs | 2 +- 6 files changed, 51 insertions(+), 33 deletions(-) diff --git a/src/api/wix/WixToolset.Extensibility/Services/IFileSystem.cs b/src/api/wix/WixToolset.Extensibility/Services/IFileSystem.cs index d633c823..26184462 100644 --- a/src/api/wix/WixToolset.Extensibility/Services/IFileSystem.cs +++ b/src/api/wix/WixToolset.Extensibility/Services/IFileSystem.cs @@ -2,6 +2,8 @@ namespace WixToolset.Extensibility.Services { + using System; + /// /// Abstracts basic file system operations. /// @@ -15,11 +17,28 @@ namespace WixToolset.Extensibility.Services /// Allow hardlinks. void CopyFile(string source, string destination, bool allowHardlink); + /// + /// Deletes a file. + /// + /// The file to delete. + /// Indicates the file must be deleted. Default is a best effort delete. + /// Maximum retry attempts. Default is 4. + void DeleteFile(string source, bool throwOnError = false, int maxRetries = 4); + /// /// Moves a file. /// /// The file to move. /// The destination file. void MoveFile(string source, string destination); + + /// + /// Executes an action and retries on any exception a few times with short pause + /// between each attempt. Primarily intended for use with file system operations + /// that might get interrupted by external systems (usually anti-virus). + /// + /// Action to execute. + /// Maximum retry attempts. Default is 4. + void ExecuteWithRetries(Action action, int maxRetries = 4); } } diff --git a/src/wix/WixToolset.Core.Burn/Bundles/CreateBundleExeCommand.cs b/src/wix/WixToolset.Core.Burn/Bundles/CreateBundleExeCommand.cs index aa4730fb..fdc3dfe3 100644 --- a/src/wix/WixToolset.Core.Burn/Bundles/CreateBundleExeCommand.cs +++ b/src/wix/WixToolset.Core.Burn/Bundles/CreateBundleExeCommand.cs @@ -349,17 +349,11 @@ namespace WixToolset.Core.Burn.Bundles try { - resources.Save(bundleTempPath); + this.FileSystem.ExecuteWithRetries(() => resources.Save(bundleTempPath)); } - catch (IOException) + catch (IOException e) { - // If there was no icon or splash screen then this is unexpected so rethrow. - if (bundleInfo.IconSourceFile == null && bundleInfo.SplashScreenSourceFile == null) - { - throw; - } - - this.Messaging.Write(BurnBackendErrors.FailedToAddIconOrSplashScreenToBundle(bundleInfo.SourceLineNumbers, bundleInfo.IconSourceFile?.Path, bundleInfo.SplashScreenSourceFile?.Path)); + this.Messaging.Write(BurnBackendErrors.FailedToUpdateBundleResources(bundleInfo.SourceLineNumbers, bundleInfo.IconSourceFile?.Path, bundleInfo.SplashScreenSourceFile?.Path, e.Message)); } } diff --git a/src/wix/WixToolset.Core.Burn/BurnBackendErrors.cs b/src/wix/WixToolset.Core.Burn/BurnBackendErrors.cs index 04290667..ee6fc0f8 100644 --- a/src/wix/WixToolset.Core.Burn/BurnBackendErrors.cs +++ b/src/wix/WixToolset.Core.Burn/BurnBackendErrors.cs @@ -42,7 +42,7 @@ namespace WixToolset.Core.Burn return Message(sourceLineNumbers, Ids.ExternalPayloadCollision2, "The location of the symbol related to the previous error."); } - public static Message FailedToAddIconOrSplashScreenToBundle(SourceLineNumber sourceLineNumbers, string iconPath, string splashScreenPath) + public static Message FailedToUpdateBundleResources(SourceLineNumber sourceLineNumbers, string iconPath, string splashScreenPath, string detail) { var additionalDetail = String.Empty; @@ -51,18 +51,18 @@ namespace WixToolset.Core.Burn } else if (String.IsNullOrEmpty(iconPath)) { - additionalDetail = $" Ensure the splash screen file is a bitmap file at '{splashScreenPath}'"; + additionalDetail = $" Ensure the splash screen file is a bitmap file at '{splashScreenPath}'."; } else if (String.IsNullOrEmpty(splashScreenPath)) { - additionalDetail = $" Ensure the bundle icon file is an icon file at '{iconPath}'"; + additionalDetail = $" Ensure the bundle icon file is an icon file at '{iconPath}'."; } else { - additionalDetail = $" Ensure the bundle icon file is an icon file at '{iconPath}' and the splash screen file is a bitmap file at '{splashScreenPath}'"; + additionalDetail = $" Ensure the bundle icon file is an icon file at '{iconPath}' and the splash screen file is a bitmap file at '{splashScreenPath}'."; } - return Message(sourceLineNumbers, Ids.FailedToAddIconOrSplashScreenToBundle, "Failed to add resources to the bundle.{0}", additionalDetail); + return Message(sourceLineNumbers, Ids.FailedToUpdateBundleResources, "Failed to update resources in the bundle.{0} Detail: {1}", additionalDetail, detail); } public static Message PackageCachePayloadCollision(SourceLineNumber sourceLineNumbers, string payloadId, string payloadName, string packageId) @@ -113,7 +113,7 @@ namespace WixToolset.Core.Burn TooManyAttachedContainers = 8008, IncompatibleWixBurnSection = 8009, UnsupportedRemotePackagePayload = 8010, - FailedToAddIconOrSplashScreenToBundle = 8011, + FailedToUpdateBundleResources = 8011, InvalidBundleManifest = 8012, BundleMultipleProviders = 8013, } // last available is 8499. 8500 is BurnBackendWarnings. diff --git a/src/wix/WixToolset.Core/Bind/TransferFilesCommand.cs b/src/wix/WixToolset.Core/Bind/TransferFilesCommand.cs index 87c2590f..4a0329bc 100644 --- a/src/wix/WixToolset.Core/Bind/TransferFilesCommand.cs +++ b/src/wix/WixToolset.Core/Bind/TransferFilesCommand.cs @@ -204,7 +204,7 @@ namespace WixToolset.Core.Bind foreach (var file in files) { var fileInfo = new FileInfo(file); - ExtensibilityServices.FileSystem.ActionWithRetries(() => fileInfo.SetAccessControl(aclReset)); + this.FileSystem.ExecuteWithRetries(() => fileInfo.SetAccessControl(aclReset)); } } } diff --git a/src/wix/WixToolset.Core/ExtensibilityServices/FileSystem.cs b/src/wix/WixToolset.Core/ExtensibilityServices/FileSystem.cs index 8bda4966..4269cabe 100644 --- a/src/wix/WixToolset.Core/ExtensibilityServices/FileSystem.cs +++ b/src/wix/WixToolset.Core/ExtensibilityServices/FileSystem.cs @@ -12,13 +12,13 @@ namespace WixToolset.Core.ExtensibilityServices { public void CopyFile(string source, string destination, bool allowHardlink) { - EnsureDirectoryWithoutFile(destination); + this.EnsureDirectoryWithoutFile(destination); var hardlinked = false; if (allowHardlink) { - ActionWithRetries(() => hardlinked = CreateHardLink(destination, source, IntPtr.Zero)); + this.ExecuteWithRetries(() => hardlinked = CreateHardLink(destination, source, IntPtr.Zero)); } if (!hardlinked) @@ -27,25 +27,30 @@ namespace WixToolset.Core.ExtensibilityServices var er = Marshal.GetLastWin32Error(); #endif - ActionWithRetries(() => File.Copy(source, destination, overwrite: true)); + this.ExecuteWithRetries(() => File.Copy(source, destination, overwrite: true)); + } + } + + public void DeleteFile(string source, bool throwOnError = false, int maxRetries = 4) + { + try + { + this.ExecuteWithRetries(() => File.Delete(source), maxRetries); + } + catch when (!throwOnError) + { + // Do nothing on best-effort deletes. } } public void MoveFile(string source, string destination) { - EnsureDirectoryWithoutFile(destination); + this.EnsureDirectoryWithoutFile(destination); - ActionWithRetries(() => File.Move(source, destination)); + this.ExecuteWithRetries(() => File.Move(source, destination)); } - /// - /// Executes an action and retries on any exception up to a few times. Primarily - /// intended for use with file system operations that might get interrupted by - /// external systems (usually anti-virus). - /// - /// Action to execute. - /// Maximum retry attempts. - internal static void ActionWithRetries(Action action, int maxRetries = 3) + public void ExecuteWithRetries(Action action, int maxRetries = 4) { for (var attempt = 1; attempt <= maxRetries; ++attempt) { @@ -61,16 +66,16 @@ namespace WixToolset.Core.ExtensibilityServices } } - private static void EnsureDirectoryWithoutFile(string path) + private void EnsureDirectoryWithoutFile(string path) { var directory = Path.GetDirectoryName(path); if (!String.IsNullOrEmpty(directory)) { - ActionWithRetries(() => Directory.CreateDirectory(directory)); + this.ExecuteWithRetries(() => Directory.CreateDirectory(directory)); } - ActionWithRetries(() => File.Delete(path)); + this.ExecuteWithRetries(() => File.Delete(path)); } [DllImport("Kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)] diff --git a/src/wix/test/WixToolsetTest.CoreIntegration/BundleFixture.cs b/src/wix/test/WixToolsetTest.CoreIntegration/BundleFixture.cs index ad31fc6d..cbfe6f85 100644 --- a/src/wix/test/WixToolsetTest.CoreIntegration/BundleFixture.cs +++ b/src/wix/test/WixToolsetTest.CoreIntegration/BundleFixture.cs @@ -428,7 +428,7 @@ namespace WixToolsetTest.CoreIntegration var message = result.Messages.Where(m => m.Level == MessageLevel.Error).Select(m => m.ToString().Replace(folder, "")).ToArray(); WixAssert.CompareLineByLine(new[] { - @"Failed to add resources to the bundle. Ensure the bundle icon file is an icon file at '\.Data\burn.exe'" + @"Failed to update resources in the bundle. Ensure the bundle icon file is an icon file at '\.Data\burn.exe'. Detail: Failed to save resource. Error: 87" }, message); } } -- cgit v1.2.3-55-g6feb