From 328d6df64373cf340628a09e52dd77ea338bc838 Mon Sep 17 00:00:00 2001 From: Sean Hall Date: Mon, 31 Jan 2022 16:48:58 -0600 Subject: Don't uninstall package during rollback if there are dependents. --- src/burn/engine/dependency.cpp | 63 ++++----- .../burn/WixToolsetTest.BurnE2E/DependencyTests.cs | 150 ++++++++++++++++++++- 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"); @@ -685,6 +757,80 @@ namespace WixToolsetTest.BurnE2E packageGv101.VerifyInstalledWithVersion(false); } + [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() { -- cgit v1.2.3-55-g6feb