@joebigelow / wix-1 / commits / dc992c49

Change the implementation of Cache="always" to request the CACHE state.

This makes it possible for the prereq BA to not cache those packages while installing the prereqs, which allows the engine to automatically cleanup if necessary. #6297 for this commit and the previous 6.

Sean Hall committed Feb 3, 2021 at 17:16 UTC dc992c49f30e0d2b912a6449a33b4448ef862f31
3 files changed +30 -77
src/engine/plan.cpp
+28 -68
@@ -153,7 +153,6 @@ static HRESULT CalculateExecuteActions(
153 __out_opt BOOL* pfBARequestedCache
154 );
155 static BOOL NeedsCache(
156 - __in BURN_PLAN* pPlan,
156 __in BURN_PACKAGE* pPackage
157 );
158 static HRESULT CreateContainerProgress(
@@ -323,6 +322,7 @@ extern "C" HRESULT PlanDefaultPackageRequestState(
322 __in BURN_PACKAGE_TYPE packageType,
323 __in BOOTSTRAPPER_PACKAGE_STATE currentState,
324 __in BOOL fPermanent,
325 + __in BURN_CACHE_TYPE cacheType,
326 __in BOOTSTRAPPER_ACTION action,
327 __in BURN_VARIABLES* pVariables,
328 __in_z_opt LPCWSTR wzInstallCondition,
@@ -333,12 +333,19 @@ extern "C" HRESULT PlanDefaultPackageRequestState(
333 HRESULT hr = S_OK;
334 BOOTSTRAPPER_REQUEST_STATE defaultRequestState = BOOTSTRAPPER_REQUEST_STATE_NONE;
335 BOOL fCondition = FALSE;
336 + BOOL fFallbackToCache = BURN_CACHE_TYPE_ALWAYS == cacheType && BOOTSTRAPPER_ACTION_UNINSTALL != action && BOOTSTRAPPER_PACKAGE_STATE_CACHED > currentState;
337
338 // If doing layout, then always default to requesting the file be cached.
339 if (BOOTSTRAPPER_ACTION_LAYOUT == action)
340 {
341 *pRequestState = BOOTSTRAPPER_REQUEST_STATE_CACHE;
342 }
343 + else if (BOOTSTRAPPER_PACKAGE_STATE_SUPERSEDED == currentState && BOOTSTRAPPER_ACTION_UNINSTALL != action)
344 + {
345 + // Superseded means the package is on the machine but not active, so only uninstall operations are allowed.
346 + // Requesting present makes sure always-cached packages are cached.
347 + *pRequestState = BOOTSTRAPPER_REQUEST_STATE_PRESENT;
348 + }
349 else if (BOOTSTRAPPER_RELATION_PATCH == relationType && BURN_PACKAGE_TYPE_MSP == packageType)
350 {
351 // For patch related bundles, only install a patch if currently absent during install, modify, or repair.
@@ -346,22 +353,20 @@ extern "C" HRESULT PlanDefaultPackageRequestState(
353 {
354 *pRequestState = BOOTSTRAPPER_REQUEST_STATE_PRESENT;
355 }
356 + else if (fFallbackToCache)
357 + {
358 + *pRequestState = BOOTSTRAPPER_REQUEST_STATE_CACHE;
359 + }
360 else
361 {
362 *pRequestState = BOOTSTRAPPER_REQUEST_STATE_NONE;
363 }
364 }
354 - else if (BOOTSTRAPPER_PACKAGE_STATE_SUPERSEDED == currentState && BOOTSTRAPPER_ACTION_UNINSTALL != action)
355 - {
356 - // Superseded means the package is on the machine but not active, so only uninstall operations are allowed.
357 - // All other operations do nothing.
358 - *pRequestState = BOOTSTRAPPER_REQUEST_STATE_NONE;
359 - }
365 else if (BOOTSTRAPPER_PACKAGE_STATE_OBSOLETE == currentState && !(BOOTSTRAPPER_ACTION_UNINSTALL == action && BURN_PACKAGE_TYPE_MSP == packageType))
366 {
367 // Obsolete means the package is not on the machine and should not be installed, *except* patches can be obsolete
368 // and present so allow them to be removed during uninstall. Everyone else, gets nothing.
364 - *pRequestState = BOOTSTRAPPER_REQUEST_STATE_NONE;
369 + *pRequestState = fFallbackToCache ? BOOTSTRAPPER_REQUEST_STATE_CACHE : BOOTSTRAPPER_REQUEST_STATE_NONE;
370 }
371 else // pick the best option for the action state and install condition.
372 {
@@ -381,6 +386,11 @@ extern "C" HRESULT PlanDefaultPackageRequestState(
386 {
387 *pRequestState = defaultRequestState;
388 }
389 +
390 + if (fFallbackToCache && BOOTSTRAPPER_REQUEST_STATE_CACHE > *pRequestState)
391 + {
392 + *pRequestState = BOOTSTRAPPER_REQUEST_STATE_CACHE;
393 + }
394 }
395
396 LExit:
@@ -816,7 +826,7 @@ static HRESULT ProcessPackage(
826 BOOL fPlanPackageBegan = FALSE;
827
828 // Remember the default requested state so the engine doesn't get blamed for planning the wrong thing if the BA changes it.
819 - hr = PlanDefaultPackageRequestState(pPackage->type, pPackage->currentState, !pPackage->fUninstallable, pPlan->action, pVariables, pPackage->sczInstallCondition, relationType, &pPackage->defaultRequested);
829 + hr = PlanDefaultPackageRequestState(pPackage->type, pPackage->currentState, !pPackage->fUninstallable, pPackage->cacheType, pPlan->action, pVariables, pPackage->sczInstallCondition, relationType, &pPackage->defaultRequested);
830 ExitOnFailure(hr, "Failed to set default package state.");
831
832 pPackage->requested = pPackage->defaultRequested;
@@ -863,18 +873,9 @@ static HRESULT ProcessPackage(
873 }
874 else if (BOOTSTRAPPER_ACTION_LAYOUT != pPlan->action)
875 {
866 - // All packages that have cacheType set to always should be cached if the bundle is going to be present.
867 - if (BURN_CACHE_TYPE_ALWAYS == pPackage->cacheType && BOOTSTRAPPER_ACTION_INSTALL <= pPlan->action)
868 - {
869 - hr = PlanCachePackage(fBundlePerMachine, pUX, pPlan, pPackage, pVariables, phSyncpointEvent);
870 - ExitOnFailure(hr, "Failed to plan cache package.");
871 - }
872 - else
873 - {
874 - // Make sure the package is properly ref-counted even if no plan is requested.
875 - hr = PlanDependencyActions(fBundlePerMachine, pPlan, pPackage);
876 - ExitOnFailure(hr, "Failed to plan dependency actions for package: %ls", pPackage->sczId);
877 - }
876 + // Make sure the package is properly ref-counted even if no plan is requested.
877 + hr = PlanDependencyActions(fBundlePerMachine, pPlan, pPackage);
878 + ExitOnFailure(hr, "Failed to plan dependency actions for package: %ls", pPackage->sczId);
879 }
880
881 if (pPackage->fCanAffectRegistration)
@@ -1008,41 +1009,6 @@ LExit:
1009 return hr;
1010 }
1011
1011 -extern "C" HRESULT PlanCachePackage(
1012 - __in BOOL fPerMachine,
1013 - __in BURN_USER_EXPERIENCE* pUserExperience,
1014 - __in BURN_PLAN* pPlan,
1015 - __in BURN_PACKAGE* pPackage,
1016 - __in BURN_VARIABLES* pVariables,
1017 - __out HANDLE* phSyncpointEvent
1018 - )
1019 -{
1020 - HRESULT hr = S_OK;
1021 - BOOL fBARequestedCache = FALSE;
1022 -
1023 - // Calculate the execute actions because we need them to decide whether the package should be cached.
1024 - hr = CalculateExecuteActions(pUserExperience, pPackage, pVariables, pPlan->pActiveRollbackBoundary, &fBARequestedCache);
1025 - ExitOnFailure(hr, "Failed to calculate execute actions for package: %ls", pPackage->sczId);
1026 -
1027 - if (fBARequestedCache || NeedsCache(pPlan, pPackage))
1028 - {
1029 - hr = AddCachePackage(pPlan, pPackage, phSyncpointEvent);
1030 - ExitOnFailure(hr, "Failed to plan cache package.");
1031 -
1032 - if (pPackage->fPerMachine)
1033 - {
1034 - pPlan->fPerMachine = TRUE;
1035 - }
1036 - }
1037 -
1038 - // Make sure the package is properly ref-counted.
1039 - hr = PlanDependencyActions(fPerMachine, pPlan, pPackage);
1040 - ExitOnFailure(hr, "Failed to plan dependency actions for package: %ls", pPackage->sczId);
1041 -
1042 -LExit:
1043 - return hr;
1044 -}
1045 -
1012 extern "C" HRESULT PlanExecutePackage(
1013 __in BOOL fPerMachine,
1014 __in BOOTSTRAPPER_DISPLAY display,
@@ -1064,7 +1030,7 @@ extern "C" HRESULT PlanExecutePackage(
1030 hr = DependencyPlanPackageBegin(fPerMachine, pPackage, pPlan);
1031 ExitOnFailure(hr, "Failed to begin plan dependency actions for package: %ls", pPackage->sczId);
1032
1067 - if (fBARequestedCache || NeedsCache(pPlan, pPackage))
1033 + if (fBARequestedCache || NeedsCache(pPackage))
1034 {
1035 hr = AddCachePackage(pPlan, pPackage, phSyncpointEvent);
1036 ExitOnFailure(hr, "Failed to plan cache package.");
@@ -1079,12 +1045,12 @@ extern "C" HRESULT PlanExecutePackage(
1045
1046 // Add the cache and install size to estimated size if it will be on the machine at the end of the install
1047 if (BOOTSTRAPPER_REQUEST_STATE_PRESENT == pPackage->requested ||
1082 - (BOOTSTRAPPER_PACKAGE_STATE_PRESENT == pPackage->currentState && BOOTSTRAPPER_REQUEST_STATE_ABSENT < pPackage->requested) ||
1083 - BURN_CACHE_TYPE_ALWAYS == pPackage->cacheType
1048 + BOOTSTRAPPER_REQUEST_STATE_CACHE == pPackage->requested ||
1049 + (BOOTSTRAPPER_PACKAGE_STATE_PRESENT == pPackage->currentState && BOOTSTRAPPER_REQUEST_STATE_ABSENT < pPackage->requested)
1050 )
1051 {
1052 // If the package will remain in the cache, add the package size to the estimated size
1087 - if (BURN_CACHE_TYPE_YES <= pPackage->cacheType)
1053 + if (BURN_CACHE_TYPE_NO < pPackage->cacheType)
1054 {
1055 pPlan->qwEstimatedSize += pPackage->qwSize;
1056 }
@@ -1511,7 +1477,7 @@ extern "C" HRESULT PlanCleanPackage(
1477 // from the cache. Start by noting that we only clean if the package is being acquired or
1478 // already cached and the package is not supposed to always be cached.
1479 if ((pPackage->fAcquire || BURN_CACHE_STATE_PARTIAL == pPackage->cache || BURN_CACHE_STATE_COMPLETE == pPackage->cache) &&
1514 - (BURN_CACHE_TYPE_ALWAYS > pPackage->cacheType || BOOTSTRAPPER_ACTION_INSTALL > pPlan->action))
1480 + (BURN_CACHE_TYPE_ALWAYS > pPackage->cacheType || BOOTSTRAPPER_ACTION_CACHE > pPlan->action))
1481 {
1482 // The following are all different reasons why the package should be cleaned from the cache.
1483 // The else-ifs are used to make the conditions easier to see (rather than have them combined
@@ -2836,16 +2802,10 @@ LExit:
2802 }
2803
2804 static BOOL NeedsCache(
2839 - __in BURN_PLAN* pPlan,
2805 __in BURN_PACKAGE* pPackage
2806 )
2807 {
2843 - // All packages that have cacheType set to always should be cached if the bundle is going to be present.
2844 - if (BURN_CACHE_TYPE_ALWAYS == pPackage->cacheType && BOOTSTRAPPER_ACTION_INSTALL <= pPlan->action)
2845 - {
2846 - return TRUE;
2847 - }
2848 - else if (BURN_PACKAGE_TYPE_EXE == pPackage->type) // Exe packages require the package for all operations (even uninstall).
2808 + if (BURN_PACKAGE_TYPE_EXE == pPackage->type) // Exe packages require the package for all operations (even uninstall).
2809 {
2810 return BOOTSTRAPPER_ACTION_STATE_NONE != pPackage->execute;
2811 }
src/engine/plan.h
+1 -8
@@ -380,6 +380,7 @@ HRESULT PlanDefaultPackageRequestState(
380 __in BURN_PACKAGE_TYPE packageType,
381 __in BOOTSTRAPPER_PACKAGE_STATE currentState,
382 __in BOOL fPermanent,
383 + __in BURN_CACHE_TYPE cacheType,
384 __in BOOTSTRAPPER_ACTION action,
385 __in BURN_VARIABLES* pVariables,
386 __in_z_opt LPCWSTR wzInstallCondition,
@@ -439,14 +440,6 @@ HRESULT PlanLayoutPackage(
440 __in BURN_PACKAGE* pPackage,
441 __in_z_opt LPCWSTR wzLayoutDirectory
442 );
442 -HRESULT PlanCachePackage(
443 - __in BOOL fPerMachine,
444 - __in BURN_USER_EXPERIENCE* pUserExperience,
445 - __in BURN_PLAN* pPlan,
446 - __in BURN_PACKAGE* pPackage,
447 - __in BURN_VARIABLES* pVariables,
448 - __out HANDLE* phSyncpointEvent
449 - );
443 HRESULT PlanExecutePackage(
444 __in BOOL fPerMachine,
445 __in BOOTSTRAPPER_DISPLAY display,
src/test/BurnUnitTest/PlanTest.cpp
+1 -1
@@ -317,7 +317,7 @@ namespace Bootstrapper
317 dwIndex = 0;
318 Assert::Equal(dwIndex, pPlan->cRollbackCacheActions);
319
320 - Assert::Equal(0ull, pPlan->qwEstimatedSize);
320 + Assert::Equal(33743ull, pPlan->qwEstimatedSize);
321 Assert::Equal(33743ull, pPlan->qwCacheSizeTotal);
322
323 fRollback = FALSE;