@joebigelow / wix / commits / a2b98c1a

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

Sean Hall committed Jan 31, 2022 at 16:43 UTC a2b98c1abd6e6a1469936af5d93e4ace713b3fba
7 files changed +158 -210
src/burn/engine/dependency.cpp
+105 -152
@@ -41,7 +41,6 @@ static BOOL GetProviderExists(
41
42 static void CalculateDependencyActionStates(
43 __in const BURN_PACKAGE* pPackage,
44 - __in const BOOTSTRAPPER_ACTION action,
44 __out BURN_DEPENDENCY_ACTION* pDependencyExecuteAction,
45 __out BURN_DEPENDENCY_ACTION* pDependencyRollbackAction
46 );
@@ -497,7 +496,7 @@ extern "C" HRESULT DependencyPlanPackageBegin(
496 }
497
498 // Calculate the dependency actions before the package itself is planned.
500 - CalculateDependencyActionStates(pPackage, pPlan->action, &dependencyExecuteAction, &dependencyRollbackAction);
499 + CalculateDependencyActionStates(pPackage, &dependencyExecuteAction, &dependencyRollbackAction);
500
501 // If dependents were found, change the action to not uninstall the package.
502 if (fDependentBlocksUninstall)
@@ -510,37 +509,54 @@ extern "C" HRESULT DependencyPlanPackageBegin(
509 }
510 else
511 {
513 - // Use the calculated dependency actions as the provider actions if there
514 - // are any non-imported providers that need to be registered and the package
515 - // is current (not obsolete).
512 + // Only plan providers when the package is current (not obsolete).
513 if (BOOTSTRAPPER_PACKAGE_STATE_OBSOLETE != pPackage->currentState)
514 {
518 - BOOL fAllImportedProviders = TRUE; // assume all providers were imported.
515 for (DWORD i = 0; i < pPackage->cDependencyProviders; ++i)
516 {
521 - const BURN_DEPENDENCY_PROVIDER* pProvider = &pPackage->rgDependencyProviders[i];
522 - if (!pProvider->fImported)
517 + BURN_DEPENDENCY_PROVIDER* pProvider = &pPackage->rgDependencyProviders[i];
518 +
519 + // Only need to handle providers that were authored directly in the bundle.
520 + if (pProvider->fImported)
521 {
524 - fAllImportedProviders = FALSE;
525 - break;
522 + continue;
523 }
527 - }
524
529 - if (!fAllImportedProviders)
530 - {
531 - for (DWORD i = 0; i < pPackage->cDependencyProviders; ++i)
525 + pProvider->providerExecute = dependencyExecuteAction;
526 + pProvider->providerRollback = dependencyRollbackAction;
527 +
528 + // Don't overwrite providers that we don't own.
529 + if (pPackage->compatiblePackage.fDetected)
530 + {
531 + if (BURN_DEPENDENCY_ACTION_REGISTER == pProvider->providerExecute)
532 + {
533 + pProvider->providerExecute = BURN_DEPENDENCY_ACTION_NONE;
534 + pProvider->providerRollback = BURN_DEPENDENCY_ACTION_NONE;
535 + }
536 +
537 + if (BURN_DEPENDENCY_ACTION_REGISTER == pProvider->providerRollback)
538 + {
539 + pProvider->providerRollback = BURN_DEPENDENCY_ACTION_NONE;
540 + }
541 + }
542 +
543 + if (BURN_DEPENDENCY_ACTION_UNREGISTER == pProvider->providerExecute && !pProvider->fExists)
544 + {
545 + pProvider->providerExecute = BURN_DEPENDENCY_ACTION_NONE;
546 + }
547 +
548 + if (BURN_DEPENDENCY_ACTION_UNREGISTER == pProvider->providerRollback && pProvider->fExists ||
549 + BURN_DEPENDENCY_ACTION_REGISTER == pProvider->providerRollback && !pProvider->fExists)
550 {
533 - BURN_DEPENDENCY_PROVIDER* pProvider = &pPackage->rgDependencyProviders[i];
534 - pProvider->providerExecute = dependencyExecuteAction;
535 - pProvider->providerRollback = dependencyRollbackAction;
551 + pProvider->providerRollback = BURN_DEPENDENCY_ACTION_NONE;
552 }
553
538 - if (BURN_DEPENDENCY_ACTION_NONE != dependencyExecuteAction)
554 + if (BURN_DEPENDENCY_ACTION_NONE != pProvider->providerExecute)
555 {
556 pPackage->fProviderExecute = TRUE;
557 }
558
543 - if (BURN_DEPENDENCY_ACTION_NONE != dependencyRollbackAction)
559 + if (BURN_DEPENDENCY_ACTION_NONE != pProvider->providerRollback)
560 {
561 pPackage->fProviderRollback = TRUE;
562 }
@@ -567,6 +583,23 @@ extern "C" HRESULT DependencyPlanPackageBegin(
583 pProvider->dependentExecute = dependencyExecuteAction;
584 pProvider->dependentRollback = dependencyRollbackAction;
585
586 + if (BURN_DEPENDENCY_ACTION_REGISTER == pProvider->dependentRollback &&
587 + BURN_DEPENDENCY_ACTION_UNREGISTER == pProvider->providerExecute && BURN_DEPENDENCY_ACTION_REGISTER != pProvider->providerRollback)
588 + {
589 + pProvider->dependentRollback = BURN_DEPENDENCY_ACTION_NONE;
590 + }
591 +
592 + if (BURN_DEPENDENCY_ACTION_UNREGISTER == pProvider->dependentExecute && !pProvider->fBundleRegisteredAsDependent)
593 + {
594 + pProvider->dependentExecute = BURN_DEPENDENCY_ACTION_NONE;
595 + }
596 +
597 + if (BURN_DEPENDENCY_ACTION_UNREGISTER == pProvider->dependentRollback && pProvider->fBundleRegisteredAsDependent ||
598 + BURN_DEPENDENCY_ACTION_REGISTER == pProvider->dependentRollback && !pProvider->fBundleRegisteredAsDependent)
599 + {
600 + pProvider->dependentRollback = BURN_DEPENDENCY_ACTION_NONE;
601 + }
602 +
603 // The highest aggregate action state found will be returned.
604 if (pPackage->dependencyExecute < pProvider->dependentExecute)
605 {
@@ -655,16 +688,8 @@ extern "C" HRESULT DependencyPlanPackageComplete(
688 // installed and all that good stuff.
689 if (BURN_DEPENDENCY_ACTION_REGISTER == pPackage->dependencyExecute)
690 {
658 - // Recalculate the dependency actions in case other operations may have changed
659 - // the package execution state.
660 - CalculateDependencyActionStates(pPackage, pPlan->action, &pPackage->dependencyExecute, &pPackage->dependencyRollback);
661 -
662 - // If the dependency execution action is *still* to register, add the dependency actions to the plan.
663 - if (BURN_DEPENDENCY_ACTION_REGISTER == pPackage->dependencyExecute)
664 - {
665 - hr = AddPackageDependencyActions(NULL, pPackage, pPlan, pPackage->dependencyExecute, pPackage->dependencyRollback);
666 - ExitOnFailure(hr, "Failed to plan the dependency actions for package: %ls", pPackage->sczId);
667 - }
691 + hr = AddPackageDependencyActions(NULL, pPackage, pPlan, pPackage->dependencyExecute, pPackage->dependencyRollback);
692 + ExitOnFailure(hr, "Failed to plan the dependency actions for package: %ls", pPackage->sczId);
693 }
694
695 LExit:
@@ -863,15 +888,6 @@ extern "C" HRESULT DependencyDetectCompatibleEntry(
888 LPCWSTR wzPackageProviderId = GetPackageProviderId(pPackage);
889 HKEY hkHive = pRegistration->fPerMachine ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;
890
866 - switch (pPackage->type)
867 - {
868 - case BURN_PACKAGE_TYPE_MSI:
869 - // Only MSI packages can handle compatible entries.
870 - break;
871 - default:
872 - ExitFunction();
873 - }
874 -
891 for (DWORD i = 0; i < pPackage->cDependencyProviders; ++i)
892 {
893 BURN_DEPENDENCY_PROVIDER* pProvider = &pPackage->rgDependencyProviders[i];
@@ -944,46 +960,30 @@ static HRESULT DetectPackageDependents(
960 BURN_DEPENDENCY_PROVIDER* pProvider = &pPackage->rgDependencyProviders[i];
961
962 hr = DepCheckDependents(hkHive, pProvider->sczKey, 0, NULL, &pProvider->rgDependents, &pProvider->cDependents);
947 - if (E_FILENOTFOUND != hr)
963 + if (E_FILENOTFOUND == hr)
964 {
949 - ExitOnFailure(hr, "Failed dependents check on package provider: %ls", pProvider->sczKey);
950 -
951 - if (!pPackage->fPackageProviderExists && (0 < pProvider->cDependents || GetProviderExists(hkHive, pProvider->sczKey)))
952 - {
953 - pPackage->fPackageProviderExists = TRUE;
954 - }
955 -
956 - if (fCanIgnorePresence && !fBundleRegisteredAsDependent)
957 - {
958 - for (DWORD iDependent = 0; iDependent < pProvider->cDependents; ++iDependent)
959 - {
960 - DEPENDENCY* pDependent = pProvider->rgDependents + iDependent;
965 + hr = S_OK;
966 + }
967 + ExitOnFailure(hr, "Failed dependents check on package provider: %ls", pProvider->sczKey);
968
962 - if (CSTR_EQUAL == ::CompareStringW(LOCALE_NEUTRAL, NORM_IGNORECASE, pRegistration->sczId, -1, pDependent->sczKey, -1))
963 - {
964 - fBundleRegisteredAsDependent = TRUE;
965 - break;
966 - }
967 - }
968 - }
969 + if (0 < pProvider->cDependents || GetProviderExists(hkHive, pProvider->sczKey))
970 + {
971 + pProvider->fExists = TRUE;
972 }
970 - else
973 +
974 + for (DWORD iDependent = 0; iDependent < pProvider->cDependents; ++iDependent)
975 {
972 - hr = S_OK;
976 + DEPENDENCY* pDependent = pProvider->rgDependents + iDependent;
977
974 - if (!pPackage->fPackageProviderExists && GetProviderExists(hkHive, pProvider->sczKey))
978 + if (CSTR_EQUAL == ::CompareStringW(LOCALE_NEUTRAL, NORM_IGNORECASE, pRegistration->sczId, -1, pDependent->sczKey, -1))
979 {
976 - pPackage->fPackageProviderExists = TRUE;
980 + pProvider->fBundleRegisteredAsDependent = TRUE;
981 + fBundleRegisteredAsDependent = TRUE;
982 + break;
983 }
984 }
985 }
986
981 - // Older bundles may not have written the id so try the default.
982 - if (!pPackage->fPackageProviderExists && BURN_PACKAGE_TYPE_MSI == pPackage->type && pPackage->Msi.sczProductCode && GetProviderExists(hkHive, pPackage->Msi.sczProductCode))
983 - {
984 - pPackage->fPackageProviderExists = TRUE;
985 - }
986 -
987 if (fCanIgnorePresence && !fBundleRegisteredAsDependent)
988 {
989 if (BURN_PACKAGE_REGISTRATION_STATE_PRESENT == pPackage->cacheRegistrationState)
@@ -1190,95 +1190,54 @@ static BOOL GetProviderExists(
1190 *********************************************************************/
1191 static void CalculateDependencyActionStates(
1192 __in const BURN_PACKAGE* pPackage,
1193 - __in const BOOTSTRAPPER_ACTION action,
1193 __out BURN_DEPENDENCY_ACTION* pDependencyExecuteAction,
1194 __out BURN_DEPENDENCY_ACTION* pDependencyRollbackAction
1195 )
1196 {
1198 - switch (action)
1197 + switch (pPackage->execute)
1198 {
1200 - case BOOTSTRAPPER_ACTION_UNINSTALL:
1201 - // Always remove the dependency when uninstalling a bundle even if the package is absent.
1202 - *pDependencyExecuteAction = BURN_DEPENDENCY_ACTION_UNREGISTER;
1203 - break;
1204 - case BOOTSTRAPPER_ACTION_INSTALL: __fallthrough;
1205 - case BOOTSTRAPPER_ACTION_CACHE:
1206 - // Always remove the dependency during rollback when installing a bundle.
1207 - *pDependencyRollbackAction = BURN_DEPENDENCY_ACTION_UNREGISTER;
1208 - __fallthrough;
1209 - case BOOTSTRAPPER_ACTION_MODIFY: __fallthrough;
1210 - case BOOTSTRAPPER_ACTION_REPAIR:
1211 - switch (pPackage->execute)
1199 + case BOOTSTRAPPER_ACTION_STATE_NONE:
1200 + switch (pPackage->requested)
1201 {
1213 - case BOOTSTRAPPER_ACTION_STATE_NONE:
1214 - switch (pPackage->requested)
1202 + case BOOTSTRAPPER_REQUEST_STATE_ABSENT:
1203 + // Unregister if the package is not requested but already not installed.
1204 + *pDependencyExecuteAction = BURN_DEPENDENCY_ACTION_UNREGISTER;
1205 + break;
1206 + case BOOTSTRAPPER_REQUEST_STATE_NONE:
1207 + // Register if a newer, compatible package is already installed.
1208 + switch (pPackage->currentState)
1209 {
1216 - case BOOTSTRAPPER_REQUEST_STATE_NONE:
1217 - // Register if a newer, compatible package is already installed.
1218 - switch (pPackage->currentState)
1219 - {
1220 - case BOOTSTRAPPER_PACKAGE_STATE_OBSOLETE:
1221 - if (!pPackage->fPackageProviderExists)
1222 - {
1223 - break;
1224 - }
1225 - __fallthrough;
1226 - case BOOTSTRAPPER_PACKAGE_STATE_SUPERSEDED:
1227 - *pDependencyExecuteAction = BURN_DEPENDENCY_ACTION_REGISTER;
1228 - break;
1229 - }
1230 - break;
1231 - case BOOTSTRAPPER_REQUEST_STATE_PRESENT: __fallthrough;
1232 - case BOOTSTRAPPER_REQUEST_STATE_REPAIR:
1233 - // Register if the package is requested but already installed.
1234 - switch (pPackage->currentState)
1235 - {
1236 - case BOOTSTRAPPER_PACKAGE_STATE_OBSOLETE:
1237 - if (!pPackage->fPackageProviderExists)
1238 - {
1239 - break;
1240 - }
1241 - __fallthrough;
1242 - case BOOTSTRAPPER_PACKAGE_STATE_PRESENT: __fallthrough;
1243 - case BOOTSTRAPPER_PACKAGE_STATE_SUPERSEDED:
1244 - *pDependencyExecuteAction = BURN_DEPENDENCY_ACTION_REGISTER;
1245 - break;
1246 - }
1210 + case BOOTSTRAPPER_PACKAGE_STATE_OBSOLETE: __fallthrough;
1211 + case BOOTSTRAPPER_PACKAGE_STATE_SUPERSEDED:
1212 + *pDependencyExecuteAction = BURN_DEPENDENCY_ACTION_REGISTER;
1213 break;
1214 }
1215 break;
1250 - case BOOTSTRAPPER_ACTION_STATE_UNINSTALL:
1251 - *pDependencyExecuteAction = BURN_DEPENDENCY_ACTION_UNREGISTER;
1252 - break;
1253 - case BOOTSTRAPPER_ACTION_STATE_INSTALL: __fallthrough;
1254 - case BOOTSTRAPPER_ACTION_STATE_MODIFY: __fallthrough;
1255 - case BOOTSTRAPPER_ACTION_STATE_REPAIR: __fallthrough;
1256 - case BOOTSTRAPPER_ACTION_STATE_MINOR_UPGRADE:
1216 + case BOOTSTRAPPER_REQUEST_STATE_PRESENT: __fallthrough;
1217 + case BOOTSTRAPPER_REQUEST_STATE_REPAIR:
1218 + // Register if the package is requested but already installed.
1219 *pDependencyExecuteAction = BURN_DEPENDENCY_ACTION_REGISTER;
1220 break;
1221 }
1222 break;
1223 + case BOOTSTRAPPER_ACTION_STATE_UNINSTALL:
1224 + *pDependencyExecuteAction = BURN_DEPENDENCY_ACTION_UNREGISTER;
1225 + break;
1226 + case BOOTSTRAPPER_ACTION_STATE_INSTALL: __fallthrough;
1227 + case BOOTSTRAPPER_ACTION_STATE_MODIFY: __fallthrough;
1228 + case BOOTSTRAPPER_ACTION_STATE_REPAIR: __fallthrough;
1229 + case BOOTSTRAPPER_ACTION_STATE_MINOR_UPGRADE:
1230 + *pDependencyExecuteAction = BURN_DEPENDENCY_ACTION_REGISTER;
1231 + break;
1232 }
1233
1234 switch (*pDependencyExecuteAction)
1235 {
1236 case BURN_DEPENDENCY_ACTION_REGISTER:
1266 - switch (pPackage->currentState)
1267 - {
1268 - case BOOTSTRAPPER_PACKAGE_STATE_OBSOLETE: __fallthrough;
1269 - case BOOTSTRAPPER_PACKAGE_STATE_ABSENT:
1270 - *pDependencyRollbackAction = BURN_DEPENDENCY_ACTION_UNREGISTER;
1271 - break;
1272 - }
1237 + *pDependencyRollbackAction = BURN_DEPENDENCY_ACTION_UNREGISTER;
1238 break;
1239 case BURN_DEPENDENCY_ACTION_UNREGISTER:
1275 - switch (pPackage->currentState)
1276 - {
1277 - case BOOTSTRAPPER_PACKAGE_STATE_PRESENT: __fallthrough;
1278 - case BOOTSTRAPPER_PACKAGE_STATE_SUPERSEDED:
1279 - *pDependencyRollbackAction = BURN_DEPENDENCY_ACTION_REGISTER;
1280 - break;
1281 - }
1240 + *pDependencyRollbackAction = BURN_DEPENDENCY_ACTION_REGISTER;
1241 break;
1242 }
1243 }
@@ -1376,13 +1335,10 @@ static HRESULT RegisterPackageProvider(
1335 {
1336 HRESULT hr = S_OK;
1337
1379 - if (!pProvider->fImported)
1380 - {
1381 - LogId(REPORT_VERBOSE, MSG_DEPENDENCY_PACKAGE_REGISTER, pProvider->sczKey, pProvider->sczVersion, wzPackageId);
1338 + LogId(REPORT_VERBOSE, MSG_DEPENDENCY_PACKAGE_REGISTER, pProvider->sczKey, pProvider->sczVersion, wzPackageId);
1339
1383 - hr = DepRegisterDependency(hkRoot, pProvider->sczKey, pProvider->sczVersion, pProvider->sczDisplayName, wzPackageProviderId, 0);
1384 - ExitOnFailure(hr, "Failed to register the package dependency provider: %ls", pProvider->sczKey);
1385 - }
1340 + hr = DepRegisterDependency(hkRoot, pProvider->sczKey, pProvider->sczVersion, pProvider->sczDisplayName, wzPackageProviderId, 0);
1341 + ExitOnFailure(hr, "Failed to register the package dependency provider: %ls", pProvider->sczKey);
1342
1343 LExit:
1344 if (!fVital)
@@ -1406,17 +1362,14 @@ static void UnregisterPackageProvider(
1362 {
1363 HRESULT hr = S_OK;
1364
1409 - if (!pProvider->fImported)
1365 + hr = DepUnregisterDependency(hkRoot, pProvider->sczKey);
1366 + if (SUCCEEDED(hr))
1367 {
1411 - hr = DepUnregisterDependency(hkRoot, pProvider->sczKey);
1412 - if (SUCCEEDED(hr))
1413 - {
1414 - LogId(REPORT_VERBOSE, MSG_DEPENDENCY_PACKAGE_UNREGISTERED, pProvider->sczKey, wzPackageId);
1415 - }
1416 - else if (FAILED(hr) && E_FILENOTFOUND != hr)
1417 - {
1418 - LogId(REPORT_VERBOSE, MSG_DEPENDENCY_PACKAGE_UNREGISTERED_FAILED, pProvider->sczKey, wzPackageId, hr);
1419 - }
1368 + LogId(REPORT_VERBOSE, MSG_DEPENDENCY_PACKAGE_UNREGISTERED, pProvider->sczKey, wzPackageId);
1369 + }
1370 + else if (FAILED(hr) && E_FILENOTFOUND != hr)
1371 + {
1372 + LogId(REPORT_VERBOSE, MSG_DEPENDENCY_PACKAGE_UNREGISTERED_FAILED, pProvider->sczKey, wzPackageId, hr);
1373 }
1374 }
1375
src/burn/engine/detect.cpp
+3 -1
@@ -57,7 +57,6 @@ extern "C" void DetectReset(
57 BURN_PACKAGE* pPackage = pPackages->rgPackages + iPackage;
58
59 pPackage->currentState = BOOTSTRAPPER_PACKAGE_STATE_UNKNOWN;
60 - pPackage->fPackageProviderExists = FALSE;
60 pPackage->cacheRegistrationState = BURN_PACKAGE_REGISTRATION_STATE_UNKNOWN;
61 pPackage->installRegistrationState = BURN_PACKAGE_REGISTRATION_STATE_UNKNOWN;
62
@@ -92,6 +91,9 @@ extern "C" void DetectReset(
91 {
92 BURN_DEPENDENCY_PROVIDER* pProvider = pPackage->rgDependencyProviders + iProvider;
93
94 + pProvider->fExists = FALSE;
95 + pProvider->fBundleRegisteredAsDependent = FALSE;
96 +
97 if (pProvider->rgDependents)
98 {
99 ReleaseDependencyArray(pProvider->rgDependents, pProvider->cDependents);
src/burn/engine/package.h
+4 -3
@@ -193,8 +193,10 @@ typedef struct _BURN_DEPENDENCY_PROVIDER
193 LPWSTR sczDisplayName;
194 BOOL fImported;
195
196 - DEPENDENCY* rgDependents; // only valid after Detect.
197 - UINT cDependents; // only valid after Detect.
196 + BOOL fExists; // only valid after Detect.
197 + BOOL fBundleRegisteredAsDependent; // only valid after Detect.
198 + DEPENDENCY* rgDependents; // only valid after Detect.
199 + UINT cDependents; // only valid after Detect.
200
201 BURN_DEPENDENCY_ACTION dependentExecute; // only valid during Plan.
202 BURN_DEPENDENCY_ACTION dependentRollback; // only valid during Plan.
@@ -264,7 +266,6 @@ typedef struct _BURN_PACKAGE
266
267 BOOTSTRAPPER_PACKAGE_STATE currentState; // only valid after Detect.
268 BOOL fCached; // only valid after Detect.
267 - BOOL fPackageProviderExists; // only valid after Detect.
269 BOOTSTRAPPER_CACHE_TYPE cacheType; // only valid during Plan.
270 BOOTSTRAPPER_REQUEST_STATE defaultRequested;// only valid during Plan.
271 BOOTSTRAPPER_REQUEST_STATE requested; // only valid during Plan.
src/burn/engine/plan.cpp
+36 -43
@@ -67,7 +67,6 @@ static HRESULT ProcessPackageRollbackBoundary(
67 );
68 static HRESULT GetActionDefaultRequestState(
69 __in BOOTSTRAPPER_ACTION action,
70 - __in BOOL fPermanent,
70 __in BOOTSTRAPPER_PACKAGE_STATE currentState,
71 __out BOOTSTRAPPER_REQUEST_STATE* pRequestState
72 );
@@ -318,7 +317,6 @@ LExit:
317 extern "C" HRESULT PlanDefaultPackageRequestState(
318 __in BURN_PACKAGE_TYPE packageType,
319 __in BOOTSTRAPPER_PACKAGE_STATE currentState,
321 - __in BOOL fPermanent,
320 __in BOOTSTRAPPER_ACTION action,
321 __in BOOTSTRAPPER_PACKAGE_CONDITION_RESULT installCondition,
322 __in BOOTSTRAPPER_RELATION_TYPE relationType,
@@ -333,6 +331,20 @@ extern "C" HRESULT PlanDefaultPackageRequestState(
331 {
332 *pRequestState = BOOTSTRAPPER_REQUEST_STATE_CACHE;
333 }
334 + else if (BOOTSTRAPPER_ACTION_CACHE == action)
335 + {
336 + switch (currentState)
337 + {
338 + case BOOTSTRAPPER_PACKAGE_STATE_PRESENT: __fallthrough;
339 + case BOOTSTRAPPER_PACKAGE_STATE_SUPERSEDED:
340 + *pRequestState = BOOTSTRAPPER_REQUEST_STATE_PRESENT;
341 + break;
342 +
343 + default:
344 + *pRequestState = BOOTSTRAPPER_REQUEST_STATE_CACHE;
345 + break;
346 + }
347 + }
348 else if (BOOTSTRAPPER_RELATION_PATCH == relationType && BURN_PACKAGE_TYPE_MSP == packageType)
349 {
350 // For patch related bundles, only install a patch if currently absent during install, modify, or repair.
@@ -345,33 +357,30 @@ extern "C" HRESULT PlanDefaultPackageRequestState(
357 *pRequestState = BOOTSTRAPPER_REQUEST_STATE_NONE;
358 }
359 }
348 - else if (BOOTSTRAPPER_PACKAGE_STATE_SUPERSEDED == currentState && BOOTSTRAPPER_ACTION_UNINSTALL != action)
349 - {
350 - // Superseded means the package is on the machine but not active, so only uninstall operations are allowed.
351 - // All other operations do nothing.
352 - *pRequestState = BOOTSTRAPPER_REQUEST_STATE_NONE;
353 - }
354 - else if (BOOTSTRAPPER_PACKAGE_STATE_OBSOLETE == currentState && !(BOOTSTRAPPER_ACTION_UNINSTALL == action && BURN_PACKAGE_TYPE_MSP == packageType))
355 - {
356 - // Obsolete means the package is not on the machine and should not be installed, *except* patches can be obsolete
357 - // and present so allow them to be removed during uninstall. Everyone else, gets nothing.
358 - *pRequestState = BOOTSTRAPPER_REQUEST_STATE_NONE;
359 - }
360 else // pick the best option for the action state and install condition.
361 {
362 - hr = GetActionDefaultRequestState(action, fPermanent, currentState, &defaultRequestState);
362 + hr = GetActionDefaultRequestState(action, currentState, &defaultRequestState);
363 ExitOnFailure(hr, "Failed to get default request state for action.");
364
365 - // If we're doing an install, use the install condition
366 - // to determine whether to use the default request state or make the package absent.
367 - if (BOOTSTRAPPER_ACTION_UNINSTALL != action && BOOTSTRAPPER_PACKAGE_CONDITION_FALSE == installCondition)
368 - {
369 - *pRequestState = BOOTSTRAPPER_REQUEST_STATE_ABSENT;
370 - }
371 - else // just set the package to the default request state.
365 + if (BOOTSTRAPPER_ACTION_UNINSTALL != action)
366 {
373 - *pRequestState = defaultRequestState;
367 + // If we're not doing an uninstall, use the install condition
368 + // to determine whether to use the default request state or make the package absent.
369 + if (BOOTSTRAPPER_PACKAGE_CONDITION_FALSE == installCondition)
370 + {
371 + defaultRequestState = BOOTSTRAPPER_REQUEST_STATE_ABSENT;
372 + }
373 + // Obsolete means the package is not on the machine and should not be installed,
374 + // *except* patches can be obsolete and present.
375 + // Superseded means the package is on the machine but not active, so only uninstall operations are allowed.
376 + // All other operations do nothing.
377 + else if (BOOTSTRAPPER_PACKAGE_STATE_OBSOLETE == currentState || BOOTSTRAPPER_PACKAGE_STATE_SUPERSEDED == currentState)
378 + {
379 + defaultRequestState = BOOTSTRAPPER_REQUEST_STATE_PRESENT <= defaultRequestState ? BOOTSTRAPPER_REQUEST_STATE_NONE : defaultRequestState;
380 + }
381 }
382 +
383 + *pRequestState = defaultRequestState;
384 }
385
386 LExit:
@@ -873,7 +882,7 @@ static HRESULT InitializePackage(
882 }
883
884 // Remember the default requested state so the engine doesn't get blamed for planning the wrong thing if the BA changes it.
876 - hr = PlanDefaultPackageRequestState(pPackage->type, pPackage->currentState, pPackage->fPermanent, pPlan->action, installCondition, relationType, &pPackage->defaultRequested);
885 + hr = PlanDefaultPackageRequestState(pPackage->type, pPackage->currentState, pPlan->action, installCondition, relationType, &pPackage->defaultRequested);
886 ExitOnFailure(hr, "Failed to set default package state.");
887
888 pPackage->requested = pPackage->defaultRequested;
@@ -1993,7 +2002,6 @@ static void ResetPlannedRollbackBoundaryState(
2002
2003 static HRESULT GetActionDefaultRequestState(
2004 __in BOOTSTRAPPER_ACTION action,
1996 - __in BOOL fPermanent,
2005 __in BOOTSTRAPPER_PACKAGE_STATE currentState,
2006 __out BOOTSTRAPPER_REQUEST_STATE* pRequestState
2007 )
@@ -2002,22 +2010,7 @@ static HRESULT GetActionDefaultRequestState(
2010
2011 switch (action)
2012 {
2005 - case BOOTSTRAPPER_ACTION_CACHE:
2006 - switch (currentState)
2007 - {
2008 - case BOOTSTRAPPER_PACKAGE_STATE_PRESENT:
2009 - *pRequestState = BOOTSTRAPPER_REQUEST_STATE_PRESENT;
2010 - break;
2011 -
2012 - default:
2013 - *pRequestState = BOOTSTRAPPER_REQUEST_STATE_CACHE;
2014 - break;
2015 - }
2016 - break;
2017 -
2018 - case BOOTSTRAPPER_ACTION_INSTALL: __fallthrough;
2019 - case BOOTSTRAPPER_ACTION_UPDATE_REPLACE: __fallthrough;
2020 - case BOOTSTRAPPER_ACTION_UPDATE_REPLACE_EMBEDDED:
2013 + case BOOTSTRAPPER_ACTION_INSTALL:
2014 *pRequestState = BOOTSTRAPPER_REQUEST_STATE_PRESENT;
2015 break;
2016
@@ -2026,7 +2019,7 @@ static HRESULT GetActionDefaultRequestState(
2019 break;
2020
2021 case BOOTSTRAPPER_ACTION_UNINSTALL:
2029 - *pRequestState = fPermanent ? BOOTSTRAPPER_REQUEST_STATE_NONE : BOOTSTRAPPER_REQUEST_STATE_ABSENT;
2022 + *pRequestState = BOOTSTRAPPER_REQUEST_STATE_ABSENT;
2023 break;
2024
2025 case BOOTSTRAPPER_ACTION_MODIFY:
@@ -2052,7 +2045,7 @@ static HRESULT GetActionDefaultRequestState(
2045 }
2046
2047 LExit:
2055 - return hr;
2048 + return hr;
2049 }
2050
2051 static HRESULT AddRegistrationAction(
src/burn/engine/plan.h
-1
@@ -317,7 +317,6 @@ HRESULT PlanSetVariables(
317 HRESULT PlanDefaultPackageRequestState(
318 __in BURN_PACKAGE_TYPE packageType,
319 __in BOOTSTRAPPER_PACKAGE_STATE currentState,
320 - __in BOOL fPermanent,
320 __in BOOTSTRAPPER_ACTION action,
321 __in BOOTSTRAPPER_PACKAGE_CONDITION_RESULT installCondition,
322 __in BOOTSTRAPPER_RELATION_TYPE relationType,
src/burn/test/BurnUnitTest/PlanTest.cpp
+9 -9
@@ -305,7 +305,7 @@ namespace Bootstrapper
305
306 InitializeEngineStateForCorePlan(wzSingleMsiManifestFileName, pEngineState);
307 DetectPackagesAsAbsent(pEngineState);
308 - DetectCompatibleMsiPackage(pEngineState->packages.rgPackages, L"{C24F3903-38E7-4D44-8037-D9856B3C5046}", L"2.0.0.0");
308 + DetectCompatibleMsiPackage(pEngineState, pEngineState->packages.rgPackages, L"{C24F3903-38E7-4D44-8037-D9856B3C5046}", L"2.0.0.0");
309
310 hr = CorePlan(pEngineState, BOOTSTRAPPER_ACTION_UNINSTALL);
311 NativeAssert::Succeeded(hr, "CorePlan failed");
@@ -491,7 +491,6 @@ namespace Bootstrapper
491 ValidateExecuteCheckpoint(pPlan, fRollback, dwIndex++, dwExecuteCheckpointId++);
492 ValidateExecuteWaitCachePackage(pPlan, fRollback, dwIndex++, L"PackageA");
493 ValidateExecuteCheckpoint(pPlan, fRollback, dwIndex++, dwExecuteCheckpointId++);
494 - ValidateExecuteCheckpoint(pPlan, fRollback, dwIndex++, dwExecuteCheckpointId++);
494 ValidateExecuteRollbackBoundaryEnd(pPlan, fRollback, dwIndex++);
495 Assert::Equal(dwIndex, pPlan->cExecuteActions);
496
@@ -501,8 +500,6 @@ namespace Bootstrapper
500 ValidateExecuteRollbackBoundaryStart(pPlan, fRollback, dwIndex++, L"WixDefaultBoundary", TRUE, FALSE);
501 ValidateExecuteUncachePackage(pPlan, fRollback, dwIndex++, L"PackageA");
502 ValidateExecuteCheckpoint(pPlan, fRollback, dwIndex++, dwExecuteCheckpointId++);
504 - ValidateExecutePackageProvider(pPlan, fRollback, dwIndex++, L"PackageA", unregisterActions1, 1);
505 - ValidateExecuteCheckpoint(pPlan, fRollback, dwIndex++, dwExecuteCheckpointId++);
503 ValidateExecuteCheckpoint(pPlan, fRollback, dwIndex++, dwExecuteCheckpointId++);
504 ValidateExecuteRollbackBoundaryEnd(pPlan, fRollback, dwIndex++);
505 Assert::Equal(dwIndex, pPlan->cRollbackActions);
@@ -561,8 +558,6 @@ namespace Bootstrapper
558 dwIndex = 0;
559 DWORD dwExecuteCheckpointId = 1;
560 ValidateExecuteRollbackBoundaryStart(pPlan, fRollback, dwIndex++, L"WixDefaultBoundary", TRUE, FALSE);
564 - ValidateExecutePackageDependency(pPlan, fRollback, dwIndex++, L"PackageA", L"{A6F0CBF7-1578-450C-B9D7-9CF2EEC40002}", unregisterActions1, 1);
565 - ValidateExecutePackageProvider(pPlan, fRollback, dwIndex++, L"PackageA", unregisterActions1, 1);
561 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);
562 ValidateExecuteCheckpoint(pPlan, fRollback, dwIndex++, dwExecuteCheckpointId++);
563 ValidateExecuteCheckpoint(pPlan, fRollback, dwIndex++, dwExecuteCheckpointId++);
@@ -613,10 +608,10 @@ namespace Bootstrapper
608 vfUseRelatedBundleRequestState = TRUE;
609 vRelatedBundleRequestState = BOOTSTRAPPER_REQUEST_STATE_FORCE_PRESENT;
610
616 - hr = CorePlan(pEngineState, BOOTSTRAPPER_ACTION_MODIFY);
611 + hr = CorePlan(pEngineState, BOOTSTRAPPER_ACTION_INSTALL);
612 NativeAssert::Succeeded(hr, "CorePlan failed");
613
619 - Assert::Equal<DWORD>(BOOTSTRAPPER_ACTION_MODIFY, pPlan->action);
614 + Assert::Equal<DWORD>(BOOTSTRAPPER_ACTION_INSTALL, pPlan->action);
615 Assert::Equal<BOOL>(TRUE, pPlan->fPerMachine);
616 Assert::Equal<BOOL>(FALSE, pPlan->fDisableRollback);
617
@@ -1258,7 +1253,7 @@ namespace Bootstrapper
1253 }
1254 }
1255
1261 - void DetectCompatibleMsiPackage(BURN_PACKAGE* pPackage, LPCWSTR wzProductCode, LPCWSTR wzVersion)
1256 + void DetectCompatibleMsiPackage(BURN_ENGINE_STATE* pEngineState, BURN_PACKAGE* pPackage, LPCWSTR wzProductCode, LPCWSTR wzVersion)
1257 {
1258 HRESULT hr = S_OK;
1259 Assert(BOOTSTRAPPER_PACKAGE_STATE_PRESENT > pPackage->currentState);
@@ -1286,6 +1281,8 @@ namespace Bootstrapper
1281
1282 hr = StrAllocString(&pCompatiblePackage->compatibleEntry.sczProviderKey, pProvider->sczKey, 0);
1283 NativeAssert::Succeeded(hr, "Failed to copy provider key");
1284 +
1285 + DetectPackageDependent(pPackage, pEngineState->registration.sczId);
1286 }
1287
1288 void DetectPackageAsAbsent(BURN_PACKAGE* pPackage)
@@ -1319,6 +1316,9 @@ namespace Bootstrapper
1316
1317 hr = DepDependencyArrayAlloc(&pProvider->rgDependents, &pProvider->cDependents, wzId, NULL);
1318 NativeAssert::Succeeded(hr, "Failed to add package dependent");
1319 +
1320 + pProvider->fExists = TRUE;
1321 + pProvider->fBundleRegisteredAsDependent = TRUE;
1322 }
1323 }
1324
src/test/burn/WixToolsetTest.BurnE2E/DependencyTests.cs
+1 -1
@@ -611,7 +611,7 @@ namespace WixToolsetTest.BurnE2E
611 packageGv2.VerifyInstalled(false);
612 }
613
614 - [Fact(Skip = "https://github.com/wixtoolset/issues/issues/6510")]
614 + [Fact]
615 public void DoesntLoseDependenciesOnFailedMajorUpgradeBundleFromMinorUpdateMsi()
616 {
617 var packageAv1 = this.CreatePackageInstaller("PackageAv1");