From a2b98c1abd6e6a1469936af5d93e4ace713b3fba Mon Sep 17 00:00:00 2001 From: Sean Hall Date: Mon, 31 Jan 2022 16:43:44 -0600 Subject: Remove some assumptions in dependency planning. * A package might be installed even if it was already present. * A package might be uninstalled even if it was already absent. * The bundle might not actually be uninstalled even if the planned action was uninstall. Fixes #6510 --- src/burn/engine/dependency.cpp | 257 +++++++++------------ src/burn/engine/detect.cpp | 4 +- src/burn/engine/package.h | 7 +- src/burn/engine/plan.cpp | 79 +++---- src/burn/engine/plan.h | 1 - src/burn/test/BurnUnitTest/PlanTest.cpp | 18 +- .../burn/WixToolsetTest.BurnE2E/DependencyTests.cs | 2 +- 7 files changed, 158 insertions(+), 210 deletions(-) diff --git a/src/burn/engine/dependency.cpp b/src/burn/engine/dependency.cpp index 01f27d72..b783d4c4 100644 --- a/src/burn/engine/dependency.cpp +++ b/src/burn/engine/dependency.cpp @@ -41,7 +41,6 @@ static BOOL GetProviderExists( static void CalculateDependencyActionStates( __in const BURN_PACKAGE* pPackage, - __in const BOOTSTRAPPER_ACTION action, __out BURN_DEPENDENCY_ACTION* pDependencyExecuteAction, __out BURN_DEPENDENCY_ACTION* pDependencyRollbackAction ); @@ -497,7 +496,7 @@ extern "C" HRESULT DependencyPlanPackageBegin( } // Calculate the dependency actions before the package itself is planned. - CalculateDependencyActionStates(pPackage, pPlan->action, &dependencyExecuteAction, &dependencyRollbackAction); + CalculateDependencyActionStates(pPackage, &dependencyExecuteAction, &dependencyRollbackAction); // If dependents were found, change the action to not uninstall the package. if (fDependentBlocksUninstall) @@ -510,37 +509,54 @@ extern "C" HRESULT DependencyPlanPackageBegin( } else { - // Use the calculated dependency actions as the provider actions if there - // are any non-imported providers that need to be registered and the package - // is current (not obsolete). + // Only plan providers when the package is current (not obsolete). if (BOOTSTRAPPER_PACKAGE_STATE_OBSOLETE != pPackage->currentState) { - BOOL fAllImportedProviders = TRUE; // assume all providers were imported. for (DWORD i = 0; i < pPackage->cDependencyProviders; ++i) { - const BURN_DEPENDENCY_PROVIDER* pProvider = &pPackage->rgDependencyProviders[i]; - if (!pProvider->fImported) + BURN_DEPENDENCY_PROVIDER* pProvider = &pPackage->rgDependencyProviders[i]; + + // Only need to handle providers that were authored directly in the bundle. + if (pProvider->fImported) { - fAllImportedProviders = FALSE; - break; + continue; } - } - if (!fAllImportedProviders) - { - for (DWORD i = 0; i < pPackage->cDependencyProviders; ++i) + pProvider->providerExecute = dependencyExecuteAction; + pProvider->providerRollback = dependencyRollbackAction; + + // Don't overwrite providers that we don't own. + if (pPackage->compatiblePackage.fDetected) + { + if (BURN_DEPENDENCY_ACTION_REGISTER == pProvider->providerExecute) + { + pProvider->providerExecute = BURN_DEPENDENCY_ACTION_NONE; + pProvider->providerRollback = BURN_DEPENDENCY_ACTION_NONE; + } + + if (BURN_DEPENDENCY_ACTION_REGISTER == pProvider->providerRollback) + { + pProvider->providerRollback = BURN_DEPENDENCY_ACTION_NONE; + } + } + + if (BURN_DEPENDENCY_ACTION_UNREGISTER == pProvider->providerExecute && !pProvider->fExists) + { + pProvider->providerExecute = BURN_DEPENDENCY_ACTION_NONE; + } + + if (BURN_DEPENDENCY_ACTION_UNREGISTER == pProvider->providerRollback && pProvider->fExists || + BURN_DEPENDENCY_ACTION_REGISTER == pProvider->providerRollback && !pProvider->fExists) { - BURN_DEPENDENCY_PROVIDER* pProvider = &pPackage->rgDependencyProviders[i]; - pProvider->providerExecute = dependencyExecuteAction; - pProvider->providerRollback = dependencyRollbackAction; + pProvider->providerRollback = BURN_DEPENDENCY_ACTION_NONE; } - if (BURN_DEPENDENCY_ACTION_NONE != dependencyExecuteAction) + if (BURN_DEPENDENCY_ACTION_NONE != pProvider->providerExecute) { pPackage->fProviderExecute = TRUE; } - if (BURN_DEPENDENCY_ACTION_NONE != dependencyRollbackAction) + if (BURN_DEPENDENCY_ACTION_NONE != pProvider->providerRollback) { pPackage->fProviderRollback = TRUE; } @@ -567,6 +583,23 @@ extern "C" HRESULT DependencyPlanPackageBegin( pProvider->dependentExecute = dependencyExecuteAction; pProvider->dependentRollback = dependencyRollbackAction; + if (BURN_DEPENDENCY_ACTION_REGISTER == pProvider->dependentRollback && + BURN_DEPENDENCY_ACTION_UNREGISTER == pProvider->providerExecute && BURN_DEPENDENCY_ACTION_REGISTER != pProvider->providerRollback) + { + pProvider->dependentRollback = BURN_DEPENDENCY_ACTION_NONE; + } + + if (BURN_DEPENDENCY_ACTION_UNREGISTER == pProvider->dependentExecute && !pProvider->fBundleRegisteredAsDependent) + { + pProvider->dependentExecute = BURN_DEPENDENCY_ACTION_NONE; + } + + if (BURN_DEPENDENCY_ACTION_UNREGISTER == pProvider->dependentRollback && pProvider->fBundleRegisteredAsDependent || + BURN_DEPENDENCY_ACTION_REGISTER == pProvider->dependentRollback && !pProvider->fBundleRegisteredAsDependent) + { + pProvider->dependentRollback = BURN_DEPENDENCY_ACTION_NONE; + } + // The highest aggregate action state found will be returned. if (pPackage->dependencyExecute < pProvider->dependentExecute) { @@ -655,16 +688,8 @@ extern "C" HRESULT DependencyPlanPackageComplete( // installed and all that good stuff. if (BURN_DEPENDENCY_ACTION_REGISTER == pPackage->dependencyExecute) { - // Recalculate the dependency actions in case other operations may have changed - // the package execution state. - CalculateDependencyActionStates(pPackage, pPlan->action, &pPackage->dependencyExecute, &pPackage->dependencyRollback); - - // If the dependency execution action is *still* to register, add the dependency actions to the plan. - if (BURN_DEPENDENCY_ACTION_REGISTER == pPackage->dependencyExecute) - { - hr = AddPackageDependencyActions(NULL, pPackage, pPlan, pPackage->dependencyExecute, pPackage->dependencyRollback); - ExitOnFailure(hr, "Failed to plan the dependency actions for package: %ls", pPackage->sczId); - } + hr = AddPackageDependencyActions(NULL, pPackage, pPlan, pPackage->dependencyExecute, pPackage->dependencyRollback); + ExitOnFailure(hr, "Failed to plan the dependency actions for package: %ls", pPackage->sczId); } LExit: @@ -863,15 +888,6 @@ extern "C" HRESULT DependencyDetectCompatibleEntry( LPCWSTR wzPackageProviderId = GetPackageProviderId(pPackage); HKEY hkHive = pRegistration->fPerMachine ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER; - switch (pPackage->type) - { - case BURN_PACKAGE_TYPE_MSI: - // Only MSI packages can handle compatible entries. - break; - default: - ExitFunction(); - } - for (DWORD i = 0; i < pPackage->cDependencyProviders; ++i) { BURN_DEPENDENCY_PROVIDER* pProvider = &pPackage->rgDependencyProviders[i]; @@ -944,46 +960,30 @@ static HRESULT DetectPackageDependents( BURN_DEPENDENCY_PROVIDER* pProvider = &pPackage->rgDependencyProviders[i]; hr = DepCheckDependents(hkHive, pProvider->sczKey, 0, NULL, &pProvider->rgDependents, &pProvider->cDependents); - if (E_FILENOTFOUND != hr) + if (E_FILENOTFOUND == hr) { - ExitOnFailure(hr, "Failed dependents check on package provider: %ls", pProvider->sczKey); - - if (!pPackage->fPackageProviderExists && (0 < pProvider->cDependents || GetProviderExists(hkHive, pProvider->sczKey))) - { - pPackage->fPackageProviderExists = TRUE; - } - - if (fCanIgnorePresence && !fBundleRegisteredAsDependent) - { - for (DWORD iDependent = 0; iDependent < pProvider->cDependents; ++iDependent) - { - DEPENDENCY* pDependent = pProvider->rgDependents + iDependent; + hr = S_OK; + } + ExitOnFailure(hr, "Failed dependents check on package provider: %ls", pProvider->sczKey); - if (CSTR_EQUAL == ::CompareStringW(LOCALE_NEUTRAL, NORM_IGNORECASE, pRegistration->sczId, -1, pDependent->sczKey, -1)) - { - fBundleRegisteredAsDependent = TRUE; - break; - } - } - } + if (0 < pProvider->cDependents || GetProviderExists(hkHive, pProvider->sczKey)) + { + pProvider->fExists = TRUE; } - else + + for (DWORD iDependent = 0; iDependent < pProvider->cDependents; ++iDependent) { - hr = S_OK; + DEPENDENCY* pDependent = pProvider->rgDependents + iDependent; - if (!pPackage->fPackageProviderExists && GetProviderExists(hkHive, pProvider->sczKey)) + if (CSTR_EQUAL == ::CompareStringW(LOCALE_NEUTRAL, NORM_IGNORECASE, pRegistration->sczId, -1, pDependent->sczKey, -1)) { - pPackage->fPackageProviderExists = TRUE; + pProvider->fBundleRegisteredAsDependent = TRUE; + fBundleRegisteredAsDependent = TRUE; + break; } } } - // Older bundles may not have written the id so try the default. - if (!pPackage->fPackageProviderExists && BURN_PACKAGE_TYPE_MSI == pPackage->type && pPackage->Msi.sczProductCode && GetProviderExists(hkHive, pPackage->Msi.sczProductCode)) - { - pPackage->fPackageProviderExists = TRUE; - } - if (fCanIgnorePresence && !fBundleRegisteredAsDependent) { if (BURN_PACKAGE_REGISTRATION_STATE_PRESENT == pPackage->cacheRegistrationState) @@ -1190,95 +1190,54 @@ static BOOL GetProviderExists( *********************************************************************/ static void CalculateDependencyActionStates( __in const BURN_PACKAGE* pPackage, - __in const BOOTSTRAPPER_ACTION action, __out BURN_DEPENDENCY_ACTION* pDependencyExecuteAction, __out BURN_DEPENDENCY_ACTION* pDependencyRollbackAction ) { - switch (action) + switch (pPackage->execute) { - case BOOTSTRAPPER_ACTION_UNINSTALL: - // Always remove the dependency when uninstalling a bundle even if the package is absent. - *pDependencyExecuteAction = BURN_DEPENDENCY_ACTION_UNREGISTER; - break; - case BOOTSTRAPPER_ACTION_INSTALL: __fallthrough; - case BOOTSTRAPPER_ACTION_CACHE: - // Always remove the dependency during rollback when installing a bundle. - *pDependencyRollbackAction = BURN_DEPENDENCY_ACTION_UNREGISTER; - __fallthrough; - case BOOTSTRAPPER_ACTION_MODIFY: __fallthrough; - case BOOTSTRAPPER_ACTION_REPAIR: - switch (pPackage->execute) + case BOOTSTRAPPER_ACTION_STATE_NONE: + switch (pPackage->requested) { - case BOOTSTRAPPER_ACTION_STATE_NONE: - switch (pPackage->requested) + case BOOTSTRAPPER_REQUEST_STATE_ABSENT: + // Unregister if the package is not requested but already not installed. + *pDependencyExecuteAction = BURN_DEPENDENCY_ACTION_UNREGISTER; + break; + case BOOTSTRAPPER_REQUEST_STATE_NONE: + // Register if a newer, compatible package is already installed. + switch (pPackage->currentState) { - case BOOTSTRAPPER_REQUEST_STATE_NONE: - // Register if a newer, compatible package is already installed. - switch (pPackage->currentState) - { - case BOOTSTRAPPER_PACKAGE_STATE_OBSOLETE: - if (!pPackage->fPackageProviderExists) - { - break; - } - __fallthrough; - case BOOTSTRAPPER_PACKAGE_STATE_SUPERSEDED: - *pDependencyExecuteAction = BURN_DEPENDENCY_ACTION_REGISTER; - break; - } - break; - case BOOTSTRAPPER_REQUEST_STATE_PRESENT: __fallthrough; - case BOOTSTRAPPER_REQUEST_STATE_REPAIR: - // Register if the package is requested but already installed. - switch (pPackage->currentState) - { - case BOOTSTRAPPER_PACKAGE_STATE_OBSOLETE: - if (!pPackage->fPackageProviderExists) - { - break; - } - __fallthrough; - case BOOTSTRAPPER_PACKAGE_STATE_PRESENT: __fallthrough; - case BOOTSTRAPPER_PACKAGE_STATE_SUPERSEDED: - *pDependencyExecuteAction = BURN_DEPENDENCY_ACTION_REGISTER; - break; - } + case BOOTSTRAPPER_PACKAGE_STATE_OBSOLETE: __fallthrough; + case BOOTSTRAPPER_PACKAGE_STATE_SUPERSEDED: + *pDependencyExecuteAction = BURN_DEPENDENCY_ACTION_REGISTER; break; } break; - case BOOTSTRAPPER_ACTION_STATE_UNINSTALL: - *pDependencyExecuteAction = BURN_DEPENDENCY_ACTION_UNREGISTER; - break; - case BOOTSTRAPPER_ACTION_STATE_INSTALL: __fallthrough; - case BOOTSTRAPPER_ACTION_STATE_MODIFY: __fallthrough; - case BOOTSTRAPPER_ACTION_STATE_REPAIR: __fallthrough; - case BOOTSTRAPPER_ACTION_STATE_MINOR_UPGRADE: + case BOOTSTRAPPER_REQUEST_STATE_PRESENT: __fallthrough; + case BOOTSTRAPPER_REQUEST_STATE_REPAIR: + // Register if the package is requested but already installed. *pDependencyExecuteAction = BURN_DEPENDENCY_ACTION_REGISTER; break; } break; + case BOOTSTRAPPER_ACTION_STATE_UNINSTALL: + *pDependencyExecuteAction = BURN_DEPENDENCY_ACTION_UNREGISTER; + break; + case BOOTSTRAPPER_ACTION_STATE_INSTALL: __fallthrough; + case BOOTSTRAPPER_ACTION_STATE_MODIFY: __fallthrough; + case BOOTSTRAPPER_ACTION_STATE_REPAIR: __fallthrough; + case BOOTSTRAPPER_ACTION_STATE_MINOR_UPGRADE: + *pDependencyExecuteAction = BURN_DEPENDENCY_ACTION_REGISTER; + break; } switch (*pDependencyExecuteAction) { case BURN_DEPENDENCY_ACTION_REGISTER: - switch (pPackage->currentState) - { - case BOOTSTRAPPER_PACKAGE_STATE_OBSOLETE: __fallthrough; - case BOOTSTRAPPER_PACKAGE_STATE_ABSENT: - *pDependencyRollbackAction = BURN_DEPENDENCY_ACTION_UNREGISTER; - break; - } + *pDependencyRollbackAction = BURN_DEPENDENCY_ACTION_UNREGISTER; break; case BURN_DEPENDENCY_ACTION_UNREGISTER: - switch (pPackage->currentState) - { - case BOOTSTRAPPER_PACKAGE_STATE_PRESENT: __fallthrough; - case BOOTSTRAPPER_PACKAGE_STATE_SUPERSEDED: - *pDependencyRollbackAction = BURN_DEPENDENCY_ACTION_REGISTER; - break; - } + *pDependencyRollbackAction = BURN_DEPENDENCY_ACTION_REGISTER; break; } } @@ -1376,13 +1335,10 @@ static HRESULT RegisterPackageProvider( { HRESULT hr = S_OK; - if (!pProvider->fImported) - { - LogId(REPORT_VERBOSE, MSG_DEPENDENCY_PACKAGE_REGISTER, pProvider->sczKey, pProvider->sczVersion, wzPackageId); + LogId(REPORT_VERBOSE, MSG_DEPENDENCY_PACKAGE_REGISTER, pProvider->sczKey, pProvider->sczVersion, wzPackageId); - hr = DepRegisterDependency(hkRoot, pProvider->sczKey, pProvider->sczVersion, pProvider->sczDisplayName, wzPackageProviderId, 0); - ExitOnFailure(hr, "Failed to register the package dependency provider: %ls", pProvider->sczKey); - } + hr = DepRegisterDependency(hkRoot, pProvider->sczKey, pProvider->sczVersion, pProvider->sczDisplayName, wzPackageProviderId, 0); + ExitOnFailure(hr, "Failed to register the package dependency provider: %ls", pProvider->sczKey); LExit: if (!fVital) @@ -1406,17 +1362,14 @@ static void UnregisterPackageProvider( { HRESULT hr = S_OK; - if (!pProvider->fImported) + hr = DepUnregisterDependency(hkRoot, pProvider->sczKey); + if (SUCCEEDED(hr)) { - hr = DepUnregisterDependency(hkRoot, pProvider->sczKey); - if (SUCCEEDED(hr)) - { - LogId(REPORT_VERBOSE, MSG_DEPENDENCY_PACKAGE_UNREGISTERED, pProvider->sczKey, wzPackageId); - } - else if (FAILED(hr) && E_FILENOTFOUND != hr) - { - LogId(REPORT_VERBOSE, MSG_DEPENDENCY_PACKAGE_UNREGISTERED_FAILED, pProvider->sczKey, wzPackageId, hr); - } + LogId(REPORT_VERBOSE, MSG_DEPENDENCY_PACKAGE_UNREGISTERED, pProvider->sczKey, wzPackageId); + } + else if (FAILED(hr) && E_FILENOTFOUND != hr) + { + LogId(REPORT_VERBOSE, MSG_DEPENDENCY_PACKAGE_UNREGISTERED_FAILED, pProvider->sczKey, wzPackageId, hr); } } diff --git a/src/burn/engine/detect.cpp b/src/burn/engine/detect.cpp index 617b418b..e251871c 100644 --- a/src/burn/engine/detect.cpp +++ b/src/burn/engine/detect.cpp @@ -57,7 +57,6 @@ extern "C" void DetectReset( BURN_PACKAGE* pPackage = pPackages->rgPackages + iPackage; pPackage->currentState = BOOTSTRAPPER_PACKAGE_STATE_UNKNOWN; - pPackage->fPackageProviderExists = FALSE; pPackage->cacheRegistrationState = BURN_PACKAGE_REGISTRATION_STATE_UNKNOWN; pPackage->installRegistrationState = BURN_PACKAGE_REGISTRATION_STATE_UNKNOWN; @@ -92,6 +91,9 @@ extern "C" void DetectReset( { BURN_DEPENDENCY_PROVIDER* pProvider = pPackage->rgDependencyProviders + iProvider; + pProvider->fExists = FALSE; + pProvider->fBundleRegisteredAsDependent = FALSE; + if (pProvider->rgDependents) { ReleaseDependencyArray(pProvider->rgDependents, pProvider->cDependents); diff --git a/src/burn/engine/package.h b/src/burn/engine/package.h index 38be2098..6d1b5dd9 100644 --- a/src/burn/engine/package.h +++ b/src/burn/engine/package.h @@ -193,8 +193,10 @@ typedef struct _BURN_DEPENDENCY_PROVIDER LPWSTR sczDisplayName; BOOL fImported; - DEPENDENCY* rgDependents; // only valid after Detect. - UINT cDependents; // only valid after Detect. + BOOL fExists; // only valid after Detect. + BOOL fBundleRegisteredAsDependent; // only valid after Detect. + DEPENDENCY* rgDependents; // only valid after Detect. + UINT cDependents; // only valid after Detect. BURN_DEPENDENCY_ACTION dependentExecute; // only valid during Plan. BURN_DEPENDENCY_ACTION dependentRollback; // only valid during Plan. @@ -264,7 +266,6 @@ typedef struct _BURN_PACKAGE BOOTSTRAPPER_PACKAGE_STATE currentState; // only valid after Detect. BOOL fCached; // only valid after Detect. - BOOL fPackageProviderExists; // only valid after Detect. BOOTSTRAPPER_CACHE_TYPE cacheType; // only valid during Plan. BOOTSTRAPPER_REQUEST_STATE defaultRequested;// only valid during Plan. BOOTSTRAPPER_REQUEST_STATE requested; // only valid during Plan. diff --git a/src/burn/engine/plan.cpp b/src/burn/engine/plan.cpp index 812dcd15..f850d49c 100644 --- a/src/burn/engine/plan.cpp +++ b/src/burn/engine/plan.cpp @@ -67,7 +67,6 @@ static HRESULT ProcessPackageRollbackBoundary( ); static HRESULT GetActionDefaultRequestState( __in BOOTSTRAPPER_ACTION action, - __in BOOL fPermanent, __in BOOTSTRAPPER_PACKAGE_STATE currentState, __out BOOTSTRAPPER_REQUEST_STATE* pRequestState ); @@ -318,7 +317,6 @@ LExit: extern "C" HRESULT PlanDefaultPackageRequestState( __in BURN_PACKAGE_TYPE packageType, __in BOOTSTRAPPER_PACKAGE_STATE currentState, - __in BOOL fPermanent, __in BOOTSTRAPPER_ACTION action, __in BOOTSTRAPPER_PACKAGE_CONDITION_RESULT installCondition, __in BOOTSTRAPPER_RELATION_TYPE relationType, @@ -333,6 +331,20 @@ extern "C" HRESULT PlanDefaultPackageRequestState( { *pRequestState = BOOTSTRAPPER_REQUEST_STATE_CACHE; } + else if (BOOTSTRAPPER_ACTION_CACHE == action) + { + switch (currentState) + { + case BOOTSTRAPPER_PACKAGE_STATE_PRESENT: __fallthrough; + case BOOTSTRAPPER_PACKAGE_STATE_SUPERSEDED: + *pRequestState = BOOTSTRAPPER_REQUEST_STATE_PRESENT; + break; + + default: + *pRequestState = BOOTSTRAPPER_REQUEST_STATE_CACHE; + break; + } + } else if (BOOTSTRAPPER_RELATION_PATCH == relationType && BURN_PACKAGE_TYPE_MSP == packageType) { // For patch related bundles, only install a patch if currently absent during install, modify, or repair. @@ -345,33 +357,30 @@ extern "C" HRESULT PlanDefaultPackageRequestState( *pRequestState = BOOTSTRAPPER_REQUEST_STATE_NONE; } } - else if (BOOTSTRAPPER_PACKAGE_STATE_SUPERSEDED == currentState && BOOTSTRAPPER_ACTION_UNINSTALL != action) - { - // Superseded means the package is on the machine but not active, so only uninstall operations are allowed. - // All other operations do nothing. - *pRequestState = BOOTSTRAPPER_REQUEST_STATE_NONE; - } - else if (BOOTSTRAPPER_PACKAGE_STATE_OBSOLETE == currentState && !(BOOTSTRAPPER_ACTION_UNINSTALL == action && BURN_PACKAGE_TYPE_MSP == packageType)) - { - // Obsolete means the package is not on the machine and should not be installed, *except* patches can be obsolete - // and present so allow them to be removed during uninstall. Everyone else, gets nothing. - *pRequestState = BOOTSTRAPPER_REQUEST_STATE_NONE; - } else // pick the best option for the action state and install condition. { - hr = GetActionDefaultRequestState(action, fPermanent, currentState, &defaultRequestState); + hr = GetActionDefaultRequestState(action, currentState, &defaultRequestState); ExitOnFailure(hr, "Failed to get default request state for action."); - // If we're doing an install, use the install condition - // to determine whether to use the default request state or make the package absent. - if (BOOTSTRAPPER_ACTION_UNINSTALL != action && BOOTSTRAPPER_PACKAGE_CONDITION_FALSE == installCondition) - { - *pRequestState = BOOTSTRAPPER_REQUEST_STATE_ABSENT; - } - else // just set the package to the default request state. + if (BOOTSTRAPPER_ACTION_UNINSTALL != action) { - *pRequestState = defaultRequestState; + // If we're not doing an uninstall, use the install condition + // to determine whether to use the default request state or make the package absent. + if (BOOTSTRAPPER_PACKAGE_CONDITION_FALSE == installCondition) + { + defaultRequestState = BOOTSTRAPPER_REQUEST_STATE_ABSENT; + } + // Obsolete means the package is not on the machine and should not be installed, + // *except* patches can be obsolete and present. + // Superseded means the package is on the machine but not active, so only uninstall operations are allowed. + // All other operations do nothing. + else if (BOOTSTRAPPER_PACKAGE_STATE_OBSOLETE == currentState || BOOTSTRAPPER_PACKAGE_STATE_SUPERSEDED == currentState) + { + defaultRequestState = BOOTSTRAPPER_REQUEST_STATE_PRESENT <= defaultRequestState ? BOOTSTRAPPER_REQUEST_STATE_NONE : defaultRequestState; + } } + + *pRequestState = defaultRequestState; } LExit: @@ -873,7 +882,7 @@ static HRESULT InitializePackage( } // Remember the default requested state so the engine doesn't get blamed for planning the wrong thing if the BA changes it. - hr = PlanDefaultPackageRequestState(pPackage->type, pPackage->currentState, pPackage->fPermanent, pPlan->action, installCondition, relationType, &pPackage->defaultRequested); + hr = PlanDefaultPackageRequestState(pPackage->type, pPackage->currentState, pPlan->action, installCondition, relationType, &pPackage->defaultRequested); ExitOnFailure(hr, "Failed to set default package state."); pPackage->requested = pPackage->defaultRequested; @@ -1993,7 +2002,6 @@ static void ResetPlannedRollbackBoundaryState( static HRESULT GetActionDefaultRequestState( __in BOOTSTRAPPER_ACTION action, - __in BOOL fPermanent, __in BOOTSTRAPPER_PACKAGE_STATE currentState, __out BOOTSTRAPPER_REQUEST_STATE* pRequestState ) @@ -2002,22 +2010,7 @@ static HRESULT GetActionDefaultRequestState( switch (action) { - case BOOTSTRAPPER_ACTION_CACHE: - switch (currentState) - { - case BOOTSTRAPPER_PACKAGE_STATE_PRESENT: - *pRequestState = BOOTSTRAPPER_REQUEST_STATE_PRESENT; - break; - - default: - *pRequestState = BOOTSTRAPPER_REQUEST_STATE_CACHE; - break; - } - break; - - case BOOTSTRAPPER_ACTION_INSTALL: __fallthrough; - case BOOTSTRAPPER_ACTION_UPDATE_REPLACE: __fallthrough; - case BOOTSTRAPPER_ACTION_UPDATE_REPLACE_EMBEDDED: + case BOOTSTRAPPER_ACTION_INSTALL: *pRequestState = BOOTSTRAPPER_REQUEST_STATE_PRESENT; break; @@ -2026,7 +2019,7 @@ static HRESULT GetActionDefaultRequestState( break; case BOOTSTRAPPER_ACTION_UNINSTALL: - *pRequestState = fPermanent ? BOOTSTRAPPER_REQUEST_STATE_NONE : BOOTSTRAPPER_REQUEST_STATE_ABSENT; + *pRequestState = BOOTSTRAPPER_REQUEST_STATE_ABSENT; break; case BOOTSTRAPPER_ACTION_MODIFY: @@ -2052,7 +2045,7 @@ static HRESULT GetActionDefaultRequestState( } LExit: - return hr; + return hr; } static HRESULT AddRegistrationAction( diff --git a/src/burn/engine/plan.h b/src/burn/engine/plan.h index 3e5ab159..0734e39f 100644 --- a/src/burn/engine/plan.h +++ b/src/burn/engine/plan.h @@ -317,7 +317,6 @@ HRESULT PlanSetVariables( HRESULT PlanDefaultPackageRequestState( __in BURN_PACKAGE_TYPE packageType, __in BOOTSTRAPPER_PACKAGE_STATE currentState, - __in BOOL fPermanent, __in BOOTSTRAPPER_ACTION action, __in BOOTSTRAPPER_PACKAGE_CONDITION_RESULT installCondition, __in BOOTSTRAPPER_RELATION_TYPE relationType, diff --git a/src/burn/test/BurnUnitTest/PlanTest.cpp b/src/burn/test/BurnUnitTest/PlanTest.cpp index 1c390782..99644115 100644 --- a/src/burn/test/BurnUnitTest/PlanTest.cpp +++ b/src/burn/test/BurnUnitTest/PlanTest.cpp @@ -305,7 +305,7 @@ namespace Bootstrapper InitializeEngineStateForCorePlan(wzSingleMsiManifestFileName, pEngineState); DetectPackagesAsAbsent(pEngineState); - DetectCompatibleMsiPackage(pEngineState->packages.rgPackages, L"{C24F3903-38E7-4D44-8037-D9856B3C5046}", L"2.0.0.0"); + DetectCompatibleMsiPackage(pEngineState, pEngineState->packages.rgPackages, L"{C24F3903-38E7-4D44-8037-D9856B3C5046}", L"2.0.0.0"); hr = CorePlan(pEngineState, BOOTSTRAPPER_ACTION_UNINSTALL); NativeAssert::Succeeded(hr, "CorePlan failed"); @@ -491,7 +491,6 @@ namespace Bootstrapper ValidateExecuteCheckpoint(pPlan, fRollback, dwIndex++, dwExecuteCheckpointId++); ValidateExecuteWaitCachePackage(pPlan, fRollback, dwIndex++, L"PackageA"); ValidateExecuteCheckpoint(pPlan, fRollback, dwIndex++, dwExecuteCheckpointId++); - ValidateExecuteCheckpoint(pPlan, fRollback, dwIndex++, dwExecuteCheckpointId++); ValidateExecuteRollbackBoundaryEnd(pPlan, fRollback, dwIndex++); Assert::Equal(dwIndex, pPlan->cExecuteActions); @@ -501,8 +500,6 @@ namespace Bootstrapper ValidateExecuteRollbackBoundaryStart(pPlan, fRollback, dwIndex++, L"WixDefaultBoundary", TRUE, FALSE); ValidateExecuteUncachePackage(pPlan, fRollback, dwIndex++, L"PackageA"); ValidateExecuteCheckpoint(pPlan, fRollback, dwIndex++, dwExecuteCheckpointId++); - ValidateExecutePackageProvider(pPlan, fRollback, dwIndex++, L"PackageA", unregisterActions1, 1); - ValidateExecuteCheckpoint(pPlan, fRollback, dwIndex++, dwExecuteCheckpointId++); ValidateExecuteCheckpoint(pPlan, fRollback, dwIndex++, dwExecuteCheckpointId++); ValidateExecuteRollbackBoundaryEnd(pPlan, fRollback, dwIndex++); Assert::Equal(dwIndex, pPlan->cRollbackActions); @@ -561,8 +558,6 @@ namespace Bootstrapper dwIndex = 0; DWORD dwExecuteCheckpointId = 1; ValidateExecuteRollbackBoundaryStart(pPlan, fRollback, dwIndex++, L"WixDefaultBoundary", TRUE, FALSE); - ValidateExecutePackageDependency(pPlan, fRollback, dwIndex++, L"PackageA", L"{A6F0CBF7-1578-450C-B9D7-9CF2EEC40002}", unregisterActions1, 1); - ValidateExecutePackageProvider(pPlan, fRollback, dwIndex++, L"PackageA", unregisterActions1, 1); ValidateExecuteMsiPackage(pPlan, fRollback, dwIndex++, L"PackageA", BOOTSTRAPPER_ACTION_STATE_UNINSTALL, BURN_MSI_PROPERTY_UNINSTALL, INSTALLUILEVEL_NONE, FALSE, BOOTSTRAPPER_MSI_FILE_VERSIONING_MISSING_OR_OLDER, 0); ValidateExecuteCheckpoint(pPlan, fRollback, dwIndex++, dwExecuteCheckpointId++); ValidateExecuteCheckpoint(pPlan, fRollback, dwIndex++, dwExecuteCheckpointId++); @@ -613,10 +608,10 @@ namespace Bootstrapper vfUseRelatedBundleRequestState = TRUE; vRelatedBundleRequestState = BOOTSTRAPPER_REQUEST_STATE_FORCE_PRESENT; - hr = CorePlan(pEngineState, BOOTSTRAPPER_ACTION_MODIFY); + hr = CorePlan(pEngineState, BOOTSTRAPPER_ACTION_INSTALL); NativeAssert::Succeeded(hr, "CorePlan failed"); - Assert::Equal(BOOTSTRAPPER_ACTION_MODIFY, pPlan->action); + Assert::Equal(BOOTSTRAPPER_ACTION_INSTALL, pPlan->action); Assert::Equal(TRUE, pPlan->fPerMachine); Assert::Equal(FALSE, pPlan->fDisableRollback); @@ -1258,7 +1253,7 @@ namespace Bootstrapper } } - void DetectCompatibleMsiPackage(BURN_PACKAGE* pPackage, LPCWSTR wzProductCode, LPCWSTR wzVersion) + void DetectCompatibleMsiPackage(BURN_ENGINE_STATE* pEngineState, BURN_PACKAGE* pPackage, LPCWSTR wzProductCode, LPCWSTR wzVersion) { HRESULT hr = S_OK; Assert(BOOTSTRAPPER_PACKAGE_STATE_PRESENT > pPackage->currentState); @@ -1286,6 +1281,8 @@ namespace Bootstrapper hr = StrAllocString(&pCompatiblePackage->compatibleEntry.sczProviderKey, pProvider->sczKey, 0); NativeAssert::Succeeded(hr, "Failed to copy provider key"); + + DetectPackageDependent(pPackage, pEngineState->registration.sczId); } void DetectPackageAsAbsent(BURN_PACKAGE* pPackage) @@ -1319,6 +1316,9 @@ namespace Bootstrapper hr = DepDependencyArrayAlloc(&pProvider->rgDependents, &pProvider->cDependents, wzId, NULL); NativeAssert::Succeeded(hr, "Failed to add package dependent"); + + pProvider->fExists = TRUE; + pProvider->fBundleRegisteredAsDependent = TRUE; } } diff --git a/src/test/burn/WixToolsetTest.BurnE2E/DependencyTests.cs b/src/test/burn/WixToolsetTest.BurnE2E/DependencyTests.cs index ecd12fb3..7c74f348 100644 --- a/src/test/burn/WixToolsetTest.BurnE2E/DependencyTests.cs +++ b/src/test/burn/WixToolsetTest.BurnE2E/DependencyTests.cs @@ -611,7 +611,7 @@ namespace WixToolsetTest.BurnE2E packageGv2.VerifyInstalled(false); } - [Fact(Skip = "https://github.com/wixtoolset/issues/issues/6510")] + [Fact] public void DoesntLoseDependenciesOnFailedMajorUpgradeBundleFromMinorUpdateMsi() { var packageAv1 = this.CreatePackageInstaller("PackageAv1"); -- cgit v1.2.3-55-g6feb