Delete workload-related things if the rest of the operation was unsuccessful (#47110)

Using a file-based installer, we assume that certain components will update in tandem. If a workload set is successfully installed, for instance, that means all the manifests were successfully installed. To effect this, we have a CliTransaction. If some part of the CLI transaction fails, the whole transaction executes rollback logic.

The rollback logic for installing some kinds of workload-related things—I'm trying not to say "packages" because that sounds like workload packs, which actually are not affected by this issue—involves overwriting the current version with what was there before. If there was nothing there before, however, we just leave it. That means, as an example, that if:

No workload set had been installed previously
We install a workload set as part of this transaction
That transaction as a whole ultimately fails but not when installing the workload set
Using a file-based install
Then the workload set is not properly uninstalled, and users end up in a broken state where the only solution I've found at this point is to delete a specific folder in your SDK installation, which is not a workaround I'd feel particularly comfortable with giving users.

After this PR, I intend to look at previous versions of the SDK to see where this was introduced to help decide if this is worth backporting. I have not proven this, but I have reason to suspect that this is only an issue when using workload sets—not because workload sets themselves are broken but because they go through the same installation procedure as workload manifests except workload manifests don't rely on each other as much, and the packs are rolled back properly. For that reason, I don't think this should be needed in previous versions of the SDK, or rather it shouldn't be needed at high priority, but I do want to validate that.

Author: Forgind

src/Cli/dotnet/commands/dotnet-workload/install/FileBasedInstaller.cs

@@ -2,6 +2,8 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Collections.Concurrent;
+using System.IO;
+using System.Linq;
 using System.Text.Json;
 using Microsoft.DotNet.Cli;
 using Microsoft.DotNet.Cli.NuGetPackageDownloader;
@@ -300,6 +302,7 @@ void InstallPackage(PackageId packageId, string packageVersion, string targetFol
         {
             string packagePath = null;
             string tempBackupDir = null;
+            bool directoryExists = Directory.Exists(targetFolder) && Directory.GetFileSystemEntries(targetFolder).Any();
 
             transactionContext.Run(
                 action: () =>
@@ -319,7 +322,7 @@ void InstallPackage(PackageId packageId, string packageVersion, string targetFol
                     }
 
                     //  If target directory already exists, back it up in case we roll back
-                    if (Directory.Exists(targetFolder) && Directory.GetFileSystemEntries(targetFolder).Any())
+                    if (directoryExists)
                     {
                         tempBackupDir = Path.Combine(_tempPackagesDir.Value, $"{packageId} - {packageVersion}-backup");
                         if (Directory.Exists(tempBackupDir))
@@ -336,8 +339,18 @@ void InstallPackage(PackageId packageId, string packageVersion, string targetFol
                 {
                     if (!string.IsNullOrEmpty(tempBackupDir) && Directory.Exists(tempBackupDir))
                     {
+                        // Delete the folder first to account for new files added
+                        Directory.Delete(targetFolder, recursive: true);
                         FileAccessRetrier.RetryOnMoveAccessFailure(() => DirectoryPath.MoveDirectory(tempBackupDir, targetFolder));
                     }
+                    else if (!directoryExists && Directory.Exists(targetFolder))
+                    {
+                        // If files are copied to the targetFolder, then another operation in this transaction fails, we want to
+                        // ensure that we roll back to the prior state. In this case, the directory did not exist (or at least
+                        // didn't have files in it), so roll back to that state. A folder without files should not exist for
+                        // our purposes, so we can ignore the possibility of there having been an empty folder before.
+                        Directory.Delete(targetFolder, recursive: true);
+                    }
                 },
                 cleanup: () =>
                 {