@joebigelow / wix-1 / commits / 328d6df6

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

Sean Hall committed Jan 31, 2022 at 16:48 UTC 328d6df64373cf340628a09e52dd77ea338bc838
2 files changed +181 -32
src/burn/engine/dependency.cpp
+33 -30
@@ -445,47 +445,44 @@ extern "C" HRESULT DependencyPlanPackageBegin(
445 ExitFunction1(hr = S_OK);
446 }
447
448 - // If we're uninstalling the package, check if any dependents are registered.
449 - if (fAttemptingUninstall)
448 + // Check if any dependents are registered which would prevent the package from being uninstalled.
449 + // Build up a list of dependents to ignore, including the current bundle.
450 + hr = GetIgnoredDependents(pPackage, pPlan, &sdIgnoredDependents);
451 + ExitOnFailure(hr, "Failed to build the list of ignored dependents.");
452 +
453 + // Skip the dependency check if "ALL" was authored for IGNOREDEPENDENCIES.
454 + hr = DictKeyExists(sdIgnoredDependents, L"ALL");
455 + if (E_NOTFOUND != hr)
456 + {
457 + ExitOnFailure(hr, "Failed to check if \"ALL\" was set in IGNOREDEPENDENCIES.");
458 + }
459 + else
460 {
451 - // Build up a list of dependents to ignore, including the current bundle.
452 - hr = GetIgnoredDependents(pPackage, pPlan, &sdIgnoredDependents);
453 - ExitOnFailure(hr, "Failed to build the list of ignored dependents.");
461 + hr = S_OK;
462
455 - // Skip the dependency check if "ALL" was authored for IGNOREDEPENDENCIES.
456 - hr = DictKeyExists(sdIgnoredDependents, L"ALL");
457 - if (E_NOTFOUND != hr)
458 - {
459 - ExitOnFailure(hr, "Failed to check if \"ALL\" was set in IGNOREDEPENDENCIES.");
460 - }
461 - else
463 + for (DWORD i = 0; i < pPackage->cDependencyProviders; ++i)
464 {
463 - hr = S_OK;
465 + const BURN_DEPENDENCY_PROVIDER* pProvider = pPackage->rgDependencyProviders + i;
466
465 - for (DWORD i = 0; i < pPackage->cDependencyProviders; ++i)
467 + for (DWORD j = 0; j < pProvider->cDependents; ++j)
468 {
467 - const BURN_DEPENDENCY_PROVIDER* pProvider = pPackage->rgDependencyProviders + i;
469 + const DEPENDENCY* pDependency = pProvider->rgDependents + j;
470
469 - for (DWORD j = 0; j < pProvider->cDependents; ++j)
471 + hr = DictKeyExists(sdIgnoredDependents, pDependency->sczKey);
472 + if (E_NOTFOUND == hr)
473 {
471 - const DEPENDENCY* pDependency = pProvider->rgDependents + j;
474 + hr = S_OK;
475
473 - hr = DictKeyExists(sdIgnoredDependents, pDependency->sczKey);
474 - if (E_NOTFOUND == hr)
476 + if (!fDependentBlocksUninstall)
477 {
476 - hr = S_OK;
477 -
478 - if (!fDependentBlocksUninstall)
479 - {
480 - fDependentBlocksUninstall = TRUE;
481 -
482 - LogId(REPORT_STANDARD, MSG_DEPENDENCY_PACKAGE_HASDEPENDENTS, pPackage->sczId);
483 - }
478 + fDependentBlocksUninstall = TRUE;
479
485 - LogId(REPORT_VERBOSE, MSG_DEPENDENCY_PACKAGE_DEPENDENT, pDependency->sczKey, LoggingStringOrUnknownIfNull(pDependency->sczName));
480 + LogId(REPORT_STANDARD, MSG_DEPENDENCY_PACKAGE_HASDEPENDENTS, pPackage->sczId);
481 }
487 - ExitOnFailure(hr, "Failed to check the dictionary of ignored dependents.");
482 +
483 + LogId(REPORT_VERBOSE, MSG_DEPENDENCY_PACKAGE_DEPENDENT, pDependency->sczKey, LoggingStringOrUnknownIfNull(pDependency->sczName));
484 }
485 + ExitOnFailure(hr, "Failed to check the dictionary of ignored dependents.");
486 }
487 }
488 }
@@ -499,7 +496,7 @@ extern "C" HRESULT DependencyPlanPackageBegin(
496 CalculateDependencyActionStates(pPackage, &dependencyExecuteAction, &dependencyRollbackAction);
497
498 // If dependents were found, change the action to not uninstall the package.
502 - if (fDependentBlocksUninstall)
499 + if (fAttemptingUninstall && fDependentBlocksUninstall)
500 {
501 pPackage->execute = BOOTSTRAPPER_ACTION_STATE_NONE;
502 pPackage->rollback = BOOTSTRAPPER_ACTION_STATE_NONE;
@@ -509,6 +506,12 @@ extern "C" HRESULT DependencyPlanPackageBegin(
506 }
507 else
508 {
509 + // Trust the forward compatible nature of providers - don't uninstall the package during rollback if there were dependents.
510 + if (fDependentBlocksUninstall && BOOTSTRAPPER_ACTION_STATE_UNINSTALL == pPackage->rollback)
511 + {
512 + pPackage->rollback = BOOTSTRAPPER_ACTION_STATE_NONE;
513 + }
514 +
515 // Only plan providers when the package is current (not obsolete).
516 if (BOOTSTRAPPER_PACKAGE_STATE_OBSOLETE != pPackage->currentState)
517 {
src/test/burn/WixToolsetTest.BurnE2E/DependencyTests.cs
+148 -2
@@ -540,7 +540,7 @@ namespace WixToolsetTest.BurnE2E
540 }
541
542 [Fact(Skip = "https://github.com/wixtoolset/issues/issues/3421")]
543 - public void DoesntLoseDependenciesOnFailedMajorUpgradeBundleFromMajorUpdateMsi()
543 + public void DoesntLoseDependenciesOnFailedMajorUpgradeBundleFromMajorUpdateMsiFifo()
544 {
545 var packageAv1 = this.CreatePackageInstaller("PackageAv1");
546 var packageC = this.CreatePackageInstaller("PackageC");
@@ -611,8 +611,80 @@ namespace WixToolsetTest.BurnE2E
611 packageGv2.VerifyInstalled(false);
612 }
613
614 + [Fact(Skip = "https://github.com/wixtoolset/issues/issues/3421")]
615 + public void DoesntLoseDependenciesOnFailedMajorUpgradeBundleFromMajorUpdateMsiLifo()
616 + {
617 + var packageAv1 = this.CreatePackageInstaller("PackageAv1");
618 + var packageC = this.CreatePackageInstaller("PackageC");
619 + var packageFv1 = this.CreatePackageInstaller("PackageFv1");
620 + var packageFv2 = this.CreatePackageInstaller("PackageFv2");
621 + var packageGv1 = this.CreatePackageInstaller("PackageGv1");
622 + var packageGv2 = this.CreatePackageInstaller("PackageGv2");
623 + var bundleM = this.CreateBundleInstaller("BundleM");
624 + var bundleNv1 = this.CreateBundleInstaller("BundleNv1");
625 + var bundleNv2 = this.CreateBundleInstaller("BundleNv2");
626 + var testBAController = this.CreateTestBAController();
627 +
628 + packageAv1.VerifyInstalled(false);
629 + packageC.VerifyInstalled(false);
630 + packageFv1.VerifyInstalled(false);
631 + packageFv2.VerifyInstalled(false);
632 + packageGv1.VerifyInstalled(false);
633 + packageGv2.VerifyInstalled(false);
634 +
635 + bundleM.Install();
636 + bundleM.VerifyRegisteredAndInPackageCache();
637 +
638 + packageAv1.VerifyInstalled(true);
639 + packageFv1.VerifyInstalled(true);
640 + packageFv2.VerifyInstalled(false);
641 + packageGv1.VerifyInstalled(false);
642 + packageGv2.VerifyInstalled(false);
643 +
644 + bundleNv1.Install();
645 + bundleNv1.VerifyRegisteredAndInPackageCache();
646 +
647 + packageAv1.VerifyInstalled(true);
648 + packageFv1.VerifyInstalled(true);
649 + packageFv2.VerifyInstalled(false);
650 + packageGv1.VerifyInstalled(true);
651 + packageGv2.VerifyInstalled(false);
652 +
653 + // Make PackageC fail.
654 + testBAController.SetPackageCancelExecuteAtProgress("PackageC", 10);
655 +
656 + bundleNv2.Install((int)MSIExec.MSIExecReturnCode.ERROR_INSTALL_USEREXIT);
657 + bundleNv2.VerifyUnregisteredAndRemovedFromPackageCache();
658 + bundleNv1.VerifyRegisteredAndInPackageCache();
659 +
660 + packageAv1.VerifyInstalled(true);
661 + packageC.VerifyInstalled(false);
662 + packageFv1.VerifyInstalled(true);
663 + packageFv2.VerifyInstalled(false);
664 + packageGv1.VerifyInstalled(true);
665 + packageGv2.VerifyInstalled(false);
666 +
667 + bundleNv1.Uninstall();
668 + bundleNv1.VerifyUnregisteredAndRemovedFromPackageCache();
669 +
670 + packageAv1.VerifyInstalled(true);
671 + packageFv1.VerifyInstalled(true);
672 + packageFv2.VerifyInstalled(false);
673 + packageGv1.VerifyInstalled(false);
674 + packageGv2.VerifyInstalled(false);
675 +
676 + bundleM.Uninstall();
677 + bundleM.VerifyUnregisteredAndRemovedFromPackageCache();
678 +
679 + packageAv1.VerifyInstalled(false);
680 + packageFv1.VerifyInstalled(false);
681 + packageFv2.VerifyInstalled(false);
682 + packageGv1.VerifyInstalled(false);
683 + packageGv2.VerifyInstalled(false);
684 + }
685 +
686 [Fact]
615 - public void DoesntLoseDependenciesOnFailedMajorUpgradeBundleFromMinorUpdateMsi()
687 + public void DoesntLoseDependenciesOnFailedMajorUpgradeBundleFromMinorUpdateMsiFifo()
688 {
689 var packageAv1 = this.CreatePackageInstaller("PackageAv1");
690 var packageC = this.CreatePackageInstaller("PackageC");
@@ -685,6 +757,80 @@ namespace WixToolsetTest.BurnE2E
757 packageGv101.VerifyInstalledWithVersion(false);
758 }
759
760 + [Fact]
761 + public void DoesntLoseDependenciesOnFailedMajorUpgradeBundleFromMinorUpdateMsiLifo()
762 + {
763 + var packageAv1 = this.CreatePackageInstaller("PackageAv1");
764 + var packageC = this.CreatePackageInstaller("PackageC");
765 + var packageFv1 = this.CreatePackageInstaller("PackageFv1");
766 + var packageFv101 = this.CreatePackageInstaller("PackageFv1_0_1");
767 + var packageGv1 = this.CreatePackageInstaller("PackageGv1");
768 + var packageGv101 = this.CreatePackageInstaller("PackageGv1_0_1");
769 + var bundleM = this.CreateBundleInstaller("BundleM");
770 + var bundleNv1 = this.CreateBundleInstaller("BundleNv1");
771 + var bundleNv101 = this.CreateBundleInstaller("BundleNv1_0_1");
772 + var testBAController = this.CreateTestBAController();
773 +
774 + packageAv1.VerifyInstalled(false);
775 + packageC.VerifyInstalled(false);
776 + packageFv1.VerifyInstalledWithVersion(false);
777 + packageFv101.VerifyInstalledWithVersion(false);
778 + packageGv1.VerifyInstalledWithVersion(false);
779 + packageGv101.VerifyInstalledWithVersion(false);
780 +
781 + bundleM.Install();
782 + bundleM.VerifyRegisteredAndInPackageCache();
783 +
784 + packageAv1.VerifyInstalled(true);
785 + packageFv1.VerifyInstalledWithVersion(true);
786 + packageFv101.VerifyInstalledWithVersion(false);
787 + packageGv1.VerifyInstalledWithVersion(false);
788 + packageGv101.VerifyInstalledWithVersion(false);
789 +
790 + bundleNv1.Install();
791 + bundleNv1.VerifyRegisteredAndInPackageCache();
792 +
793 + packageAv1.VerifyInstalled(true);
794 + packageFv1.VerifyInstalledWithVersion(true);
795 + packageFv101.VerifyInstalledWithVersion(false);
796 + packageGv1.VerifyInstalledWithVersion(true);
797 + packageGv101.VerifyInstalledWithVersion(false);
798 +
799 + // Make PackageC fail.
800 + testBAController.SetPackageCancelExecuteAtProgress("PackageC", 10);
801 +
802 + // Verify https://github.com/wixtoolset/issues/issues/6510 - Dependency provider removed on rollback even though package is not rolled back
803 + bundleNv101.Install((int)MSIExec.MSIExecReturnCode.ERROR_INSTALL_USEREXIT);
804 + bundleNv101.VerifyUnregisteredAndRemovedFromPackageCache();
805 + bundleNv1.VerifyRegisteredAndInPackageCache();
806 +
807 + // The expected values will change after implementing https://github.com/wixtoolset/issues/issues/6535 and https://github.com/wixtoolset/issues/issues/3421
808 + packageAv1.VerifyInstalled(true);
809 + packageC.VerifyInstalled(false);
810 + packageFv1.VerifyInstalledWithVersion(false);
811 + packageFv101.VerifyInstalledWithVersion(true);
812 + packageGv1.VerifyInstalledWithVersion(false);
813 + packageGv101.VerifyInstalledWithVersion(true);
814 +
815 + bundleNv1.Uninstall();
816 + bundleNv1.VerifyUnregisteredAndRemovedFromPackageCache();
817 +
818 + packageAv1.VerifyInstalled(true);
819 + packageFv1.VerifyInstalledWithVersion(false);
820 + packageFv101.VerifyInstalledWithVersion(true);
821 + packageGv1.VerifyInstalledWithVersion(false);
822 + packageGv101.VerifyInstalledWithVersion(false);
823 +
824 + bundleM.Uninstall();
825 + bundleM.VerifyUnregisteredAndRemovedFromPackageCache();
826 +
827 + packageAv1.VerifyInstalled(false);
828 + packageFv1.VerifyInstalledWithVersion(false);
829 + packageFv101.VerifyInstalledWithVersion(false);
830 + packageGv1.VerifyInstalledWithVersion(false);
831 + packageGv101.VerifyInstalledWithVersion(false);
832 + }
833 +
834 [Fact]
835 public void DoesntRegisterDependencyOnPackageNotSelectedForInstall()
836 {