Don't uninstall package during rollback if there are dependents.

2 files changed, 181 insertions, 32 deletions

diff --git a/src/burn/engine/dependency.cpp b/src/burn/engine/dependency.cpp
index b783d4c4..221c7bbf 100644
--- a/src/burn/engine/dependency.cpp
+++ b/src/burn/engine/dependency.cpp
@@ -445,47 +445,44 @@ extern "C" HRESULT DependencyPlanPackageBegin(
         ExitFunction1(hr = S_OK);
     }
 
-    // If we're uninstalling the package, check if any dependents are registered.
-    if (fAttemptingUninstall)
+    // Check if any dependents are registered which would prevent the package from being uninstalled.
+    // Build up a list of dependents to ignore, including the current bundle.
+    hr = GetIgnoredDependents(pPackage, pPlan, &sdIgnoredDependents);
+    ExitOnFailure(hr, "Failed to build the list of ignored dependents.");
+
+    // Skip the dependency check if "ALL" was authored for IGNOREDEPENDENCIES.
+    hr = DictKeyExists(sdIgnoredDependents, L"ALL");
+    if (E_NOTFOUND != hr)
+    {
+        ExitOnFailure(hr, "Failed to check if \"ALL\" was set in IGNOREDEPENDENCIES.");
+    }
+    else
     {
-        // Build up a list of dependents to ignore, including the current bundle.
-        hr = GetIgnoredDependents(pPackage, pPlan, &sdIgnoredDependents);
-        ExitOnFailure(hr, "Failed to build the list of ignored dependents.");
+        hr = S_OK;
 
-        // Skip the dependency check if "ALL" was authored for IGNOREDEPENDENCIES.
-        hr = DictKeyExists(sdIgnoredDependents, L"ALL");
-        if (E_NOTFOUND != hr)
-        {
-            ExitOnFailure(hr, "Failed to check if \"ALL\" was set in IGNOREDEPENDENCIES.");
-        }
-        else
+        for (DWORD i = 0; i < pPackage->cDependencyProviders; ++i)
         {
-            hr = S_OK;
+            const BURN_DEPENDENCY_PROVIDER* pProvider = pPackage->rgDependencyProviders + i;
 
-            for (DWORD i = 0; i < pPackage->cDependencyProviders; ++i)
+            for (DWORD j = 0; j < pProvider->cDependents; ++j)
             {
-                const BURN_DEPENDENCY_PROVIDER* pProvider = pPackage->rgDependencyProviders + i;
+                const DEPENDENCY* pDependency = pProvider->rgDependents + j;
 
-                for (DWORD j = 0; j < pProvider->cDependents; ++j)
+                hr = DictKeyExists(sdIgnoredDependents, pDependency->sczKey);
+                if (E_NOTFOUND == hr)
                 {
-                    const DEPENDENCY* pDependency = pProvider->rgDependents + j;
+                    hr = S_OK;
 
-                    hr = DictKeyExists(sdIgnoredDependents, pDependency->sczKey);
-                    if (E_NOTFOUND == hr)
+                    if (!fDependentBlocksUninstall)
                     {
-                        hr = S_OK;
-
-                        if (!fDependentBlocksUninstall)
-                        {
-                            fDependentBlocksUninstall = TRUE;
-
-                            LogId(REPORT_STANDARD, MSG_DEPENDENCY_PACKAGE_HASDEPENDENTS, pPackage->sczId);
-                        }
+                        fDependentBlocksUninstall = TRUE;
 
-                        LogId(REPORT_VERBOSE, MSG_DEPENDENCY_PACKAGE_DEPENDENT, pDependency->sczKey, LoggingStringOrUnknownIfNull(pDependency->sczName));
+                        LogId(REPORT_STANDARD, MSG_DEPENDENCY_PACKAGE_HASDEPENDENTS, pPackage->sczId);
                     }
-                    ExitOnFailure(hr, "Failed to check the dictionary of ignored dependents.");
+
+                    LogId(REPORT_VERBOSE, MSG_DEPENDENCY_PACKAGE_DEPENDENT, pDependency->sczKey, LoggingStringOrUnknownIfNull(pDependency->sczName));
                 }
+                ExitOnFailure(hr, "Failed to check the dictionary of ignored dependents.");
             }
         }
     }
@@ -499,7 +496,7 @@ extern "C" HRESULT DependencyPlanPackageBegin(
     CalculateDependencyActionStates(pPackage, &dependencyExecuteAction, &dependencyRollbackAction);
 
     // If dependents were found, change the action to not uninstall the package.
-    if (fDependentBlocksUninstall)
+    if (fAttemptingUninstall && fDependentBlocksUninstall)
     {
         pPackage->execute = BOOTSTRAPPER_ACTION_STATE_NONE;
         pPackage->rollback = BOOTSTRAPPER_ACTION_STATE_NONE;
@@ -509,6 +506,12 @@ extern "C" HRESULT DependencyPlanPackageBegin(
     }
     else
     {
+        // Trust the forward compatible nature of providers - don't uninstall the package during rollback if there were dependents.
+        if (fDependentBlocksUninstall && BOOTSTRAPPER_ACTION_STATE_UNINSTALL == pPackage->rollback)
+        {
+            pPackage->rollback = BOOTSTRAPPER_ACTION_STATE_NONE;
+        }
+
         // Only plan providers when the package is current (not obsolete).
         if (BOOTSTRAPPER_PACKAGE_STATE_OBSOLETE != pPackage->currentState)
         {
diff --git a/src/test/burn/WixToolsetTest.BurnE2E/DependencyTests.cs b/src/test/burn/WixToolsetTest.BurnE2E/DependencyTests.cs
index 7c74f348..7e3e28c1 100644
--- a/src/test/burn/WixToolsetTest.BurnE2E/DependencyTests.cs
+++ b/src/test/burn/WixToolsetTest.BurnE2E/DependencyTests.cs
@@ -540,7 +540,7 @@ namespace WixToolsetTest.BurnE2E
         }
 
         [Fact(Skip = "https://github.com/wixtoolset/issues/issues/3421")]
-        public void DoesntLoseDependenciesOnFailedMajorUpgradeBundleFromMajorUpdateMsi()
+        public void DoesntLoseDependenciesOnFailedMajorUpgradeBundleFromMajorUpdateMsiFifo()
         {
             var packageAv1 = this.CreatePackageInstaller("PackageAv1");
             var packageC = this.CreatePackageInstaller("PackageC");
@@ -611,8 +611,80 @@ namespace WixToolsetTest.BurnE2E
             packageGv2.VerifyInstalled(false);
         }
 
+        [Fact(Skip = "https://github.com/wixtoolset/issues/issues/3421")]
+        public void DoesntLoseDependenciesOnFailedMajorUpgradeBundleFromMajorUpdateMsiLifo()
+        {
+            var packageAv1 = this.CreatePackageInstaller("PackageAv1");
+            var packageC = this.CreatePackageInstaller("PackageC");
+            var packageFv1 = this.CreatePackageInstaller("PackageFv1");
+            var packageFv2 = this.CreatePackageInstaller("PackageFv2");
+            var packageGv1 = this.CreatePackageInstaller("PackageGv1");
+            var packageGv2 = this.CreatePackageInstaller("PackageGv2");
+            var bundleM = this.CreateBundleInstaller("BundleM");
+            var bundleNv1 = this.CreateBundleInstaller("BundleNv1");
+            var bundleNv2 = this.CreateBundleInstaller("BundleNv2");
+            var testBAController = this.CreateTestBAController();
+
+            packageAv1.VerifyInstalled(false);
+            packageC.VerifyInstalled(false);
+            packageFv1.VerifyInstalled(false);
+            packageFv2.VerifyInstalled(false);
+            packageGv1.VerifyInstalled(false);
+            packageGv2.VerifyInstalled(false);
+
+            bundleM.Install();
+            bundleM.VerifyRegisteredAndInPackageCache();
+
+            packageAv1.VerifyInstalled(true);
+            packageFv1.VerifyInstalled(true);
+            packageFv2.VerifyInstalled(false);
+            packageGv1.VerifyInstalled(false);
+            packageGv2.VerifyInstalled(false);
+
+            bundleNv1.Install();
+            bundleNv1.VerifyRegisteredAndInPackageCache();
+
+            packageAv1.VerifyInstalled(true);
+            packageFv1.VerifyInstalled(true);
+            packageFv2.VerifyInstalled(false);
+            packageGv1.VerifyInstalled(true);
+            packageGv2.VerifyInstalled(false);
+
+            // Make PackageC fail.
+            testBAController.SetPackageCancelExecuteAtProgress("PackageC", 10);
+
+            bundleNv2.Install((int)MSIExec.MSIExecReturnCode.ERROR_INSTALL_USEREXIT);
+            bundleNv2.VerifyUnregisteredAndRemovedFromPackageCache();
+            bundleNv1.VerifyRegisteredAndInPackageCache();
+
+            packageAv1.VerifyInstalled(true);
+            packageC.VerifyInstalled(false);
+            packageFv1.VerifyInstalled(true);
+            packageFv2.VerifyInstalled(false);
+            packageGv1.VerifyInstalled(true);
+            packageGv2.VerifyInstalled(false);
+
+            bundleNv1.Uninstall();
+            bundleNv1.VerifyUnregisteredAndRemovedFromPackageCache();
+
+            packageAv1.VerifyInstalled(true);
+            packageFv1.VerifyInstalled(true);
+            packageFv2.VerifyInstalled(false);
+            packageGv1.VerifyInstalled(false);
+            packageGv2.VerifyInstalled(false);
+
+            bundleM.Uninstall();
+            bundleM.VerifyUnregisteredAndRemovedFromPackageCache();
+
+            packageAv1.VerifyInstalled(false);
+            packageFv1.VerifyInstalled(false);
+            packageFv2.VerifyInstalled(false);
+            packageGv1.VerifyInstalled(false);
+            packageGv2.VerifyInstalled(false);
+        }
+
         [Fact]
-        public void DoesntLoseDependenciesOnFailedMajorUpgradeBundleFromMinorUpdateMsi()
+        public void DoesntLoseDependenciesOnFailedMajorUpgradeBundleFromMinorUpdateMsiFifo()
         {
             var packageAv1 = this.CreatePackageInstaller("PackageAv1");
             var packageC = this.CreatePackageInstaller("PackageC");
@@ -686,6 +758,80 @@ namespace WixToolsetTest.BurnE2E
         }
 
         [Fact]
+        public void DoesntLoseDependenciesOnFailedMajorUpgradeBundleFromMinorUpdateMsiLifo()
+        {
+            var packageAv1 = this.CreatePackageInstaller("PackageAv1");
+            var packageC = this.CreatePackageInstaller("PackageC");
+            var packageFv1 = this.CreatePackageInstaller("PackageFv1");
+            var packageFv101 = this.CreatePackageInstaller("PackageFv1_0_1");
+            var packageGv1 = this.CreatePackageInstaller("PackageGv1");
+            var packageGv101 = this.CreatePackageInstaller("PackageGv1_0_1");
+            var bundleM = this.CreateBundleInstaller("BundleM");
+            var bundleNv1 = this.CreateBundleInstaller("BundleNv1");
+            var bundleNv101 = this.CreateBundleInstaller("BundleNv1_0_1");
+            var testBAController = this.CreateTestBAController();
+
+            packageAv1.VerifyInstalled(false);
+            packageC.VerifyInstalled(false);
+            packageFv1.VerifyInstalledWithVersion(false);
+            packageFv101.VerifyInstalledWithVersion(false);
+            packageGv1.VerifyInstalledWithVersion(false);
+            packageGv101.VerifyInstalledWithVersion(false);
+
+            bundleM.Install();
+            bundleM.VerifyRegisteredAndInPackageCache();
+
+            packageAv1.VerifyInstalled(true);
+            packageFv1.VerifyInstalledWithVersion(true);
+            packageFv101.VerifyInstalledWithVersion(false);
+            packageGv1.VerifyInstalledWithVersion(false);
+            packageGv101.VerifyInstalledWithVersion(false);
+
+            bundleNv1.Install();
+            bundleNv1.VerifyRegisteredAndInPackageCache();
+
+            packageAv1.VerifyInstalled(true);
+            packageFv1.VerifyInstalledWithVersion(true);
+            packageFv101.VerifyInstalledWithVersion(false);
+            packageGv1.VerifyInstalledWithVersion(true);
+            packageGv101.VerifyInstalledWithVersion(false);
+
+            // Make PackageC fail.
+            testBAController.SetPackageCancelExecuteAtProgress("PackageC", 10);
+
+            // Verify https://github.com/wixtoolset/issues/issues/6510 - Dependency provider removed on rollback even though package is not rolled back
+            bundleNv101.Install((int)MSIExec.MSIExecReturnCode.ERROR_INSTALL_USEREXIT);
+            bundleNv101.VerifyUnregisteredAndRemovedFromPackageCache();
+            bundleNv1.VerifyRegisteredAndInPackageCache();
+
+            // The expected values will change after implementing https://github.com/wixtoolset/issues/issues/6535 and https://github.com/wixtoolset/issues/issues/3421
+            packageAv1.VerifyInstalled(true);
+            packageC.VerifyInstalled(false);
+            packageFv1.VerifyInstalledWithVersion(false);
+            packageFv101.VerifyInstalledWithVersion(true);
+            packageGv1.VerifyInstalledWithVersion(false);
+            packageGv101.VerifyInstalledWithVersion(true);
+
+            bundleNv1.Uninstall();
+            bundleNv1.VerifyUnregisteredAndRemovedFromPackageCache();
+
+            packageAv1.VerifyInstalled(true);
+            packageFv1.VerifyInstalledWithVersion(false);
+            packageFv101.VerifyInstalledWithVersion(true);
+            packageGv1.VerifyInstalledWithVersion(false);
+            packageGv101.VerifyInstalledWithVersion(false);
+
+            bundleM.Uninstall();
+            bundleM.VerifyUnregisteredAndRemovedFromPackageCache();
+
+            packageAv1.VerifyInstalled(false);
+            packageFv1.VerifyInstalledWithVersion(false);
+            packageFv101.VerifyInstalledWithVersion(false);
+            packageGv1.VerifyInstalledWithVersion(false);
+            packageGv101.VerifyInstalledWithVersion(false);
+        }
+
+        [Fact]
         public void DoesntRegisterDependencyOnPackageNotSelectedForInstall()
         {
             var testRegistryValueExe = "ExeA";