From 941c47e5a3f57ce9626b447a95740b1444e69343 Mon Sep 17 00:00:00 2001 From: Sean Hall Date: Fri, 16 Apr 2021 09:40:18 -0500 Subject: Detect a package as cached if any of its payloads exist. Detect is supposed to be fast, so it can't fully verify every payload for every package. The engine was wasting its time by trying to verify file sizes without the hash. Even worse, it was making decisions during planning based on that insufficient verification. Contributes to #3640 --- src/engine/apply.cpp | 2 + src/engine/cache.cpp | 25 +++++++++---- src/engine/core.cpp | 44 ++++++++-------------- src/engine/detect.cpp | 10 ++--- src/engine/engine.mc | 8 ++-- src/engine/exeengine.cpp | 5 +-- src/engine/exeengine.h | 3 +- src/engine/externalengine.cpp | 2 +- src/engine/logging.cpp | 17 --------- src/engine/logging.h | 4 -- src/engine/msiengine.cpp | 5 +-- src/engine/msiengine.h | 3 +- src/engine/mspengine.cpp | 5 +-- src/engine/mspengine.h | 3 +- src/engine/msuengine.cpp | 5 +-- src/engine/msuengine.h | 3 +- src/engine/package.h | 13 ++----- src/engine/plan.cpp | 76 ++++++++++++++++---------------------- src/engine/plan.h | 13 ++----- src/engine/pseudobundle.cpp | 8 ++-- src/engine/pseudobundle.h | 2 +- src/engine/relatedbundle.cpp | 19 ++++------ src/test/BurnUnitTest/PlanTest.cpp | 7 ++-- 23 files changed, 111 insertions(+), 171 deletions(-) diff --git a/src/engine/apply.cpp b/src/engine/apply.cpp index 2815f7da..b79bf934 100644 --- a/src/engine/apply.cpp +++ b/src/engine/apply.cpp @@ -1910,6 +1910,8 @@ static HRESULT DoRollbackActions( ExitFunction1(hr = S_OK); case BURN_EXECUTE_ACTION_TYPE_UNCACHE_PACKAGE: + // TODO: This used to be skipped if the package was already cached. + // Need to figure out new logic for when (if?) to skip it. hr = CleanPackage(pEngineState->companionConnection.hPipe, pRollbackAction->uncachePackage.pPackage); IgnoreRollbackError(hr, "Failed to uncache package for rollback."); break; diff --git a/src/engine/cache.cpp b/src/engine/cache.cpp index 2349a357..667ca9e0 100644 --- a/src/engine/cache.cpp +++ b/src/engine/cache.cpp @@ -91,6 +91,7 @@ static HRESULT RemoveBundleOrPackage( static HRESULT VerifyHash( __in BYTE* pbHash, __in DWORD cbHash, + __in DWORD64 qwFileSize, __in_z LPCWSTR wzUnverifiedPayloadPath, __in HANDLE hFile ); @@ -1288,7 +1289,7 @@ static HRESULT VerifyThenTransferContainer( // Container should have a hash we can use to verify with. if (pContainer->pbHash) { - hr = VerifyHash(pContainer->pbHash, pContainer->cbHash, wzUnverifiedContainerPath, hFile); + hr = VerifyHash(pContainer->pbHash, pContainer->cbHash, pContainer->qwFileSize, wzUnverifiedContainerPath, hFile); ExitOnFailure(hr, "Failed to verify container hash: %ls", wzCachedPath); } @@ -1330,7 +1331,7 @@ static HRESULT VerifyThenTransferPayload( if (pPayload->pbHash) // the payload should have a hash we can use to verify it. { - hr = VerifyHash(pPayload->pbHash, pPayload->cbHash, wzUnverifiedPayloadPath, hFile); + hr = VerifyHash(pPayload->pbHash, pPayload->cbHash, pPayload->qwFileSize, wzUnverifiedPayloadPath, hFile); ExitOnFailure(hr, "Failed to verify payload hash: %ls", wzCachedPath); } @@ -1398,7 +1399,7 @@ static HRESULT VerifyFileAgainstPayload( if (pPayload->pbHash) // the payload should have a hash we can use to verify it. { - hr = VerifyHash(pPayload->pbHash, pPayload->cbHash, wzVerifyPath, hFile); + hr = VerifyHash(pPayload->pbHash, pPayload->cbHash, pPayload->qwFileSize, wzVerifyPath, hFile); ExitOnFailure(hr, "Failed to verify hash of payload: %ls", pPayload->sczKey); } @@ -1712,7 +1713,7 @@ static HRESULT RemoveBundleOrPackage( } } - if (FAILED(hr)) + if (E_PATHNOTFOUND != hr && FAILED(hr)) { LogId(REPORT_STANDARD, fBundle ? MSG_UNABLE_UNCACHE_BUNDLE : MSG_UNABLE_UNCACHE_PACKAGE, wzBundleOrPackageId, sczDirectory, hr); hr = S_OK; @@ -1743,6 +1744,7 @@ LExit: static HRESULT VerifyHash( __in BYTE* pbHash, __in DWORD cbHash, + __in DWORD64 qwFileSize, __in_z LPCWSTR wzUnverifiedPayloadPath, __in HANDLE hFile ) @@ -1751,22 +1753,31 @@ static HRESULT VerifyHash( HRESULT hr = S_OK; BYTE rgbActualHash[SHA512_HASH_LEN] = { }; - DWORD64 qwHashedBytes; + DWORD64 qwHashedBytes = 0; + LONGLONG llSize = 0; LPWSTR pszExpected = NULL; LPWSTR pszActual = NULL; + hr = FileSizeByHandle(hFile, &llSize); + ExitOnFailure(hr, "Failed to get file size for path: %ls", wzUnverifiedPayloadPath); + + if (static_cast(llSize) != qwFileSize) + { + ExitOnFailure(hr = ERROR_FILE_CORRUPT, "File size mismatch for path: %ls, expected: %llu, actual: %lld", wzUnverifiedPayloadPath, qwFileSize, llSize); + } + // TODO: create a cryp hash file that sends progress. hr = CrypHashFileHandle(hFile, PROV_RSA_AES, CALG_SHA_512, rgbActualHash, sizeof(rgbActualHash), &qwHashedBytes); ExitOnFailure(hr, "Failed to calculate hash for path: %ls", wzUnverifiedPayloadPath); // Compare hashes. - if (cbHash != sizeof(rgbActualHash) || 0 != memcmp(pbHash, rgbActualHash, SHA512_HASH_LEN)) + if (cbHash != sizeof(rgbActualHash) || 0 != memcmp(pbHash, rgbActualHash, sizeof(rgbActualHash))) { hr = CRYPT_E_HASH_VALUE; // Best effort to log the expected and actual hash value strings. if (SUCCEEDED(StrAllocHexEncode(pbHash, cbHash, &pszExpected)) && - SUCCEEDED(StrAllocHexEncode(rgbActualHash, (SIZE_T)qwHashedBytes, &pszActual))) + SUCCEEDED(StrAllocHexEncode(rgbActualHash, sizeof(rgbActualHash), &pszActual))) { ExitOnFailure(hr, "Hash mismatch for path: %ls, expected: %ls, actual: %ls", wzUnverifiedPayloadPath, pszExpected, pszActual); } diff --git a/src/engine/core.cpp b/src/engine/core.cpp index 50ed6ea0..7341e4b8 100644 --- a/src/engine/core.cpp +++ b/src/engine/core.cpp @@ -388,7 +388,7 @@ extern "C" HRESULT CoreDetect( pEngineState->registration.fEligibleForCleanup = FALSE; } - LogId(REPORT_STANDARD, MSG_DETECTED_PACKAGE, pPackage->sczId, LoggingPackageStateToString(pPackage->currentState), LoggingCacheStateToString(pPackage->cache), LoggingPackageRegistrationStateToString(pPackage->fCanAffectRegistration, pPackage->installRegistrationState), LoggingPackageRegistrationStateToString(pPackage->fCanAffectRegistration, pPackage->cacheRegistrationState)); + LogId(REPORT_STANDARD, MSG_DETECTED_PACKAGE, pPackage->sczId, LoggingPackageStateToString(pPackage->currentState), LoggingBoolToString(pPackage->fCached), LoggingPackageRegistrationStateToString(pPackage->fCanAffectRegistration, pPackage->installRegistrationState), LoggingPackageRegistrationStateToString(pPackage->fCanAffectRegistration, pPackage->cacheRegistrationState)); if (BURN_PACKAGE_TYPE_MSI == pPackage->type) { @@ -439,7 +439,6 @@ extern "C" HRESULT CorePlan( HRESULT hr = S_OK; BOOL fPlanBegan = FALSE; LPWSTR sczLayoutDirectory = NULL; - HANDLE hSyncpointEvent = NULL; BURN_PACKAGE* pUpgradeBundlePackage = NULL; BURN_PACKAGE* pForwardCompatibleBundlePackage = NULL; BOOL fContinuePlanning = TRUE; // assume we won't skip planning due to dependencies. @@ -489,7 +488,7 @@ extern "C" HRESULT CorePlan( ExitOnFailure(hr, "Failed to plan the layout of the bundle."); // Plan the packages' layout. - hr = PlanPackages(&pEngineState->userExperience, &pEngineState->packages, &pEngineState->plan, &pEngineState->log, &pEngineState->variables, pEngineState->command.display, pEngineState->command.relationType, sczLayoutDirectory, &hSyncpointEvent); + hr = PlanPackages(&pEngineState->userExperience, &pEngineState->packages, &pEngineState->plan, &pEngineState->log, &pEngineState->variables, pEngineState->command.display, pEngineState->command.relationType, sczLayoutDirectory); ExitOnFailure(hr, "Failed to plan packages."); } else if (BOOTSTRAPPER_ACTION_UPDATE_REPLACE == action || BOOTSTRAPPER_ACTION_UPDATE_REPLACE_EMBEDDED == action) @@ -498,7 +497,7 @@ extern "C" HRESULT CorePlan( pUpgradeBundlePackage = &pEngineState->update.package; - hr = PlanUpdateBundle(&pEngineState->userExperience, pUpgradeBundlePackage, &pEngineState->plan, &pEngineState->log, &pEngineState->variables, pEngineState->command.display, pEngineState->command.relationType, &hSyncpointEvent); + hr = PlanUpdateBundle(&pEngineState->userExperience, pUpgradeBundlePackage, &pEngineState->plan, &pEngineState->log, &pEngineState->variables, pEngineState->command.display, pEngineState->command.relationType); ExitOnFailure(hr, "Failed to plan update."); } else @@ -512,7 +511,7 @@ extern "C" HRESULT CorePlan( pForwardCompatibleBundlePackage = &pEngineState->plan.forwardCompatibleBundle; - hr = PlanPassThroughBundle(&pEngineState->userExperience, pForwardCompatibleBundlePackage, &pEngineState->plan, &pEngineState->log, &pEngineState->variables, pEngineState->command.display, pEngineState->command.relationType, &hSyncpointEvent); + hr = PlanPassThroughBundle(&pEngineState->userExperience, pForwardCompatibleBundlePackage, &pEngineState->plan, &pEngineState->log, &pEngineState->variables, pEngineState->command.display, pEngineState->command.relationType); ExitOnFailure(hr, "Failed to plan passthrough."); } else // doing an action that modifies the machine state. @@ -533,11 +532,11 @@ extern "C" HRESULT CorePlan( hr = PlanRelatedBundlesBegin(&pEngineState->userExperience, &pEngineState->registration, pEngineState->command.relationType, &pEngineState->plan); ExitOnFailure(hr, "Failed to plan related bundles."); - hr = PlanPackages(&pEngineState->userExperience, &pEngineState->packages, &pEngineState->plan, &pEngineState->log, &pEngineState->variables, pEngineState->command.display, pEngineState->command.relationType, NULL, &hSyncpointEvent); + hr = PlanPackages(&pEngineState->userExperience, &pEngineState->packages, &pEngineState->plan, &pEngineState->log, &pEngineState->variables, pEngineState->command.display, pEngineState->command.relationType, NULL); ExitOnFailure(hr, "Failed to plan packages."); // Schedule the update of related bundles last. - hr = PlanRelatedBundlesComplete(&pEngineState->registration, &pEngineState->plan, &pEngineState->log, &pEngineState->variables, &hSyncpointEvent, dwExecuteActionEarlyIndex); + hr = PlanRelatedBundlesComplete(&pEngineState->registration, &pEngineState->plan, &pEngineState->log, &pEngineState->variables, dwExecuteActionEarlyIndex); ExitOnFailure(hr, "Failed to schedule related bundles."); } } @@ -1669,9 +1668,8 @@ static HRESULT DetectPackagePayloadsCached( { HRESULT hr = S_OK; LPWSTR sczCachePath = NULL; - BURN_CACHE_STATE cache = BURN_CACHE_STATE_NONE; // assume the package will not be cached. + BOOL fCached = FALSE; // assume the package is not cached. LPWSTR sczPayloadCachePath = NULL; - LONGLONG llSize = 0; if (pPackage->sczCacheId && *pPackage->sczCacheId) { @@ -1681,9 +1679,7 @@ static HRESULT DetectPackagePayloadsCached( // If the cached directory exists, we have something. if (DirExists(sczCachePath, NULL)) { - cache = BURN_CACHE_STATE_COMPLETE; // assume all payloads are cached. - - // Check all payloads to see if any are missing or not the right size. + // Check all payloads to see if they exist. for (DWORD i = 0; i < pPackage->cPayloads; ++i) { BURN_PACKAGE_PAYLOAD* pPackagePayload = pPackage->rgPayloads + i; @@ -1691,35 +1687,25 @@ static HRESULT DetectPackagePayloadsCached( hr = PathConcat(sczCachePath, pPackagePayload->pPayload->sczFilePath, &sczPayloadCachePath); ExitOnFailure(hr, "Failed to concat payload cache path."); - hr = FileSize(sczPayloadCachePath, &llSize); - if (SUCCEEDED(hr) && static_cast(llSize) != pPackagePayload->pPayload->qwFileSize) - { - hr = HRESULT_FROM_WIN32(ERROR_FILE_CORRUPT); // size did not match expectations, so cache must have the wrong file. - } - - if (SUCCEEDED(hr)) + if (FileExistsEx(sczPayloadCachePath, NULL)) { - // TODO: should we do a full on hash verification on the file to ensure - // the exact right file is cached? - + // TODO: We shouldn't track whether the payload was cached since all we did was check whether the file exists. pPackagePayload->fCached = TRUE; + fCached = TRUE; } else { - LogId(REPORT_STANDARD, MSG_DETECT_PACKAGE_NOT_FULLY_CACHED, pPackage->sczId, pPackagePayload->pPayload->sczKey, hr); - - cache = BURN_CACHE_STATE_PARTIAL; // found a payload that was not cached so we are partial. - hr = S_OK; + LogId(REPORT_STANDARD, MSG_DETECT_PACKAGE_NOT_FULLY_CACHED, pPackage->sczId, pPackagePayload->pPayload->sczKey); } } } } - pPackage->cache = cache; + pPackage->fCached = fCached; if (pPackage->fCanAffectRegistration) { - pPackage->cacheRegistrationState = BURN_CACHE_STATE_NONE < pPackage->cache ? BURN_PACKAGE_REGISTRATION_STATE_PRESENT : BURN_PACKAGE_REGISTRATION_STATE_ABSENT; + pPackage->cacheRegistrationState = pPackage->fCached ? BURN_PACKAGE_REGISTRATION_STATE_PRESENT : BURN_PACKAGE_REGISTRATION_STATE_ABSENT; } LExit: @@ -1808,7 +1794,7 @@ static void LogPackages( const DWORD iPackage = (BOOTSTRAPPER_ACTION_UNINSTALL == action) ? pPackages->cPackages - 1 - i : i; const BURN_PACKAGE* pPackage = &pPackages->rgPackages[iPackage]; - LogId(REPORT_STANDARD, MSG_PLANNED_PACKAGE, pPackage->sczId, LoggingPackageStateToString(pPackage->currentState), LoggingRequestStateToString(pPackage->defaultRequested), LoggingRequestStateToString(pPackage->requested), LoggingActionStateToString(pPackage->execute), LoggingActionStateToString(pPackage->rollback), LoggingBoolToString(pPackage->fAcquire), LoggingBoolToString(pPackage->fUncache), LoggingDependencyActionToString(pPackage->dependencyExecute), LoggingPackageRegistrationStateToString(pPackage->fCanAffectRegistration, pPackage->expectedInstallRegistrationState), LoggingPackageRegistrationStateToString(pPackage->fCanAffectRegistration, pPackage->expectedCacheRegistrationState)); + LogId(REPORT_STANDARD, MSG_PLANNED_PACKAGE, pPackage->sczId, LoggingPackageStateToString(pPackage->currentState), LoggingRequestStateToString(pPackage->defaultRequested), LoggingRequestStateToString(pPackage->requested), LoggingActionStateToString(pPackage->execute), LoggingActionStateToString(pPackage->rollback), LoggingBoolToString(pPackage->fPlannedCache), LoggingBoolToString(pPackage->fPlannedUncache), LoggingDependencyActionToString(pPackage->dependencyExecute), LoggingPackageRegistrationStateToString(pPackage->fCanAffectRegistration, pPackage->expectedInstallRegistrationState), LoggingPackageRegistrationStateToString(pPackage->fCanAffectRegistration, pPackage->expectedCacheRegistrationState)); if (BURN_PACKAGE_TYPE_MSI == pPackage->type) { diff --git a/src/engine/detect.cpp b/src/engine/detect.cpp index 3a58f3dc..355b49f5 100644 --- a/src/engine/detect.cpp +++ b/src/engine/detect.cpp @@ -67,7 +67,7 @@ extern "C" void DetectReset( pPackage->cacheRegistrationState = BURN_PACKAGE_REGISTRATION_STATE_UNKNOWN; pPackage->installRegistrationState = BURN_PACKAGE_REGISTRATION_STATE_UNKNOWN; - pPackage->cache = BURN_CACHE_STATE_NONE; + pPackage->fCached = FALSE; for (DWORD iPayload = 0; iPayload < pPackage->cPayloads; ++iPayload) { BURN_PACKAGE_PAYLOAD* pPayload = pPackage->rgPayloads + iPayload; @@ -149,10 +149,10 @@ extern "C" HRESULT DetectForwardCompatibleBundles( pRegistration->fForwardCompatibleBundleExists = TRUE; } - hr = UserExperienceOnDetectForwardCompatibleBundle(pUX, pRelatedBundle->package.sczId, pRelatedBundle->relationType, pRelatedBundle->sczTag, pRelatedBundle->package.fPerMachine, pRelatedBundle->pVersion, BURN_CACHE_STATE_COMPLETE != pRelatedBundle->package.cache); + hr = UserExperienceOnDetectForwardCompatibleBundle(pUX, pRelatedBundle->package.sczId, pRelatedBundle->relationType, pRelatedBundle->sczTag, pRelatedBundle->package.fPerMachine, pRelatedBundle->pVersion, !pRelatedBundle->package.fCached); ExitOnRootFailure(hr, "BA aborted detect forward compatible bundle."); - LogId(REPORT_STANDARD, MSG_DETECTED_FORWARD_COMPATIBLE_BUNDLE, pRelatedBundle->package.sczId, LoggingRelationTypeToString(pRelatedBundle->relationType), LoggingPerMachineToString(pRelatedBundle->package.fPerMachine), pRelatedBundle->pVersion->sczVersion, LoggingCacheStateToString(pRelatedBundle->package.cache)); + LogId(REPORT_STANDARD, MSG_DETECTED_FORWARD_COMPATIBLE_BUNDLE, pRelatedBundle->package.sczId, LoggingRelationTypeToString(pRelatedBundle->relationType), LoggingPerMachineToString(pRelatedBundle->package.fPerMachine), pRelatedBundle->pVersion->sczVersion, LoggingBoolToString(pRelatedBundle->package.fCached)); } } } @@ -225,9 +225,9 @@ extern "C" HRESULT DetectReportRelatedBundles( break; } - LogId(REPORT_STANDARD, MSG_DETECTED_RELATED_BUNDLE, pRelatedBundle->package.sczId, LoggingRelationTypeToString(pRelatedBundle->relationType), LoggingPerMachineToString(pRelatedBundle->package.fPerMachine), pRelatedBundle->pVersion->sczVersion, LoggingRelatedOperationToString(operation), LoggingCacheStateToString(pRelatedBundle->package.cache)); + LogId(REPORT_STANDARD, MSG_DETECTED_RELATED_BUNDLE, pRelatedBundle->package.sczId, LoggingRelationTypeToString(pRelatedBundle->relationType), LoggingPerMachineToString(pRelatedBundle->package.fPerMachine), pRelatedBundle->pVersion->sczVersion, LoggingRelatedOperationToString(operation), LoggingBoolToString(pRelatedBundle->package.fCached)); - hr = UserExperienceOnDetectRelatedBundle(pUX, pRelatedBundle->package.sczId, pRelatedBundle->relationType, pRelatedBundle->sczTag, pRelatedBundle->package.fPerMachine, pRelatedBundle->pVersion, operation, BURN_CACHE_STATE_COMPLETE != pRelatedBundle->package.cache); + hr = UserExperienceOnDetectRelatedBundle(pUX, pRelatedBundle->package.sczId, pRelatedBundle->relationType, pRelatedBundle->sczTag, pRelatedBundle->package.fPerMachine, pRelatedBundle->pVersion, operation, !pRelatedBundle->package.fCached); ExitOnRootFailure(hr, "BA aborted detect related bundle."); // For now, if any related bundles will be executed during uninstall by default then never automatically clean up the bundle. diff --git a/src/engine/engine.mc b/src/engine/engine.mc index d0897a95..365da1d9 100644 --- a/src/engine/engine.mc +++ b/src/engine/engine.mc @@ -242,16 +242,16 @@ Detected forward compatible bundle: %1!ls!, type: %2!hs!, scope: %3!hs!, version MessageId=108 Severity=Warning -SymbolicName=MSG_DETECT_RELATED_BUNDLE_NOT_FULLY_CACHED +SymbolicName=MSG_DETECT_RELATED_BUNDLE_NOT_CACHED Language=English -Detected partially cached related bundle: %1!ls!, cache path: %2!ls!, reason: 0x%3!x! +Detected related bundle missing from cache: %1!ls!, cache path: %2!ls! . MessageId=120 Severity=Warning SymbolicName=MSG_DETECT_PACKAGE_NOT_FULLY_CACHED Language=English -Detected partially cached package: %1!ls!, invalid payload: %2!ls!, reason: 0x%3!x! +Detected partially cached package: %1!ls!, missing payload: %2!ls! . MessageId=121 @@ -363,7 +363,7 @@ MessageId=208 Severity=Warning SymbolicName=MSG_PLAN_DISABLING_ROLLBACK_NO_CACHE Language=English -Plan disabled rollback for package: %1!ls!, due to incomplete cache: %2!hs!, original rollback action: %3!hs! +Plan disabled rollback due to incomplete cache for package: %1!ls!, original rollback action: %2!hs! . MessageId=209 diff --git a/src/engine/exeengine.cpp b/src/engine/exeengine.cpp index cee755e3..46905fc3 100644 --- a/src/engine/exeengine.cpp +++ b/src/engine/exeengine.cpp @@ -269,8 +269,7 @@ extern "C" HRESULT ExeEnginePlanAddPackage( __in BURN_PLAN* pPlan, __in BURN_LOGGING* pLog, __in BURN_VARIABLES* pVariables, - __in_opt HANDLE hCacheEvent, - __in BOOL fPlanPackageCacheRollback + __in_opt HANDLE hCacheEvent ) { HRESULT hr = S_OK; @@ -279,7 +278,7 @@ extern "C" HRESULT ExeEnginePlanAddPackage( // add wait for cache if (hCacheEvent) { - hr = PlanExecuteCacheSyncAndRollback(pPlan, pPackage, hCacheEvent, fPlanPackageCacheRollback); + hr = PlanExecuteCacheSyncAndRollback(pPlan, pPackage, hCacheEvent); ExitOnFailure(hr, "Failed to plan package cache syncpoint"); } diff --git a/src/engine/exeengine.h b/src/engine/exeengine.h index 88a884eb..e032ea01 100644 --- a/src/engine/exeengine.h +++ b/src/engine/exeengine.h @@ -29,8 +29,7 @@ HRESULT ExeEnginePlanAddPackage( __in BURN_PLAN* pPlan, __in BURN_LOGGING* pLog, __in BURN_VARIABLES* pVariables, - __in_opt HANDLE hCacheEvent, - __in BOOL fPlanPackageCacheRollback + __in_opt HANDLE hCacheEvent ); HRESULT ExeEngineExecutePackage( __in BURN_EXECUTE_ACTION* pExecuteAction, diff --git a/src/engine/externalengine.cpp b/src/engine/externalengine.cpp index fffd96bf..7e9bb25c 100644 --- a/src/engine/externalengine.cpp +++ b/src/engine/externalengine.cpp @@ -327,7 +327,7 @@ HRESULT ExternalEngineSetUpdate( sczId = pEngineState->registration.sczId; } - hr = PseudoBundleInitialize(FILEMAKEVERSION(rmj, rmm, rup, rpr), &pEngineState->update.package, FALSE, sczId, BOOTSTRAPPER_RELATION_UPDATE, BOOTSTRAPPER_PACKAGE_STATE_ABSENT, BURN_CACHE_STATE_NONE, pEngineState->registration.sczExecutableName, sczLocalSource ? sczLocalSource : wzLocalSource, wzDownloadSource, qwSize, TRUE, sczCommandline, NULL, NULL, NULL, rgbHash, cbHash); + hr = PseudoBundleInitialize(FILEMAKEVERSION(rmj, rmm, rup, rpr), &pEngineState->update.package, FALSE, sczId, BOOTSTRAPPER_RELATION_UPDATE, BOOTSTRAPPER_PACKAGE_STATE_ABSENT, FALSE, pEngineState->registration.sczExecutableName, sczLocalSource ? sczLocalSource : wzLocalSource, wzDownloadSource, qwSize, TRUE, sczCommandline, NULL, NULL, NULL, rgbHash, cbHash); ExitOnFailure(hr, "Failed to set update bundle."); pEngineState->update.fUpdateAvailable = TRUE; diff --git a/src/engine/logging.cpp b/src/engine/logging.cpp index 67f39c85..fd2343fa 100644 --- a/src/engine/logging.cpp +++ b/src/engine/logging.cpp @@ -426,23 +426,6 @@ extern "C" LPCSTR LoggingPackageRegistrationStateToString( } } -extern "C" LPCSTR LoggingCacheStateToString( - __in BURN_CACHE_STATE cacheState - ) -{ - switch (cacheState) - { - case BURN_CACHE_STATE_NONE: - return "None"; - case BURN_CACHE_STATE_PARTIAL: - return "Partial"; - case BURN_CACHE_STATE_COMPLETE: - return "Complete"; - default: - return "Invalid"; - } -} - extern "C" LPCSTR LoggingMsiFeatureStateToString( __in BOOTSTRAPPER_FEATURE_STATE featureState ) diff --git a/src/engine/logging.h b/src/engine/logging.h index a69e34c5..601039f9 100644 --- a/src/engine/logging.h +++ b/src/engine/logging.h @@ -94,10 +94,6 @@ LPCSTR LoggingPackageRegistrationStateToString( __in BURN_PACKAGE_REGISTRATION_STATE registrationState ); -LPCSTR LoggingCacheStateToString( - __in BURN_CACHE_STATE cacheState - ); - LPCSTR LoggingMsiFeatureStateToString( __in BOOTSTRAPPER_FEATURE_STATE featureState ); diff --git a/src/engine/msiengine.cpp b/src/engine/msiengine.cpp index 269fc831..35ca5c62 100644 --- a/src/engine/msiengine.cpp +++ b/src/engine/msiengine.cpp @@ -932,8 +932,7 @@ extern "C" HRESULT MsiEnginePlanAddPackage( __in BURN_PLAN* pPlan, __in BURN_LOGGING* pLog, __in BURN_VARIABLES* pVariables, - __in_opt HANDLE hCacheEvent, - __in BOOL fPlanPackageCacheRollback + __in_opt HANDLE hCacheEvent ) { HRESULT hr = S_OK; @@ -963,7 +962,7 @@ extern "C" HRESULT MsiEnginePlanAddPackage( // add wait for cache if (hCacheEvent) { - hr = PlanExecuteCacheSyncAndRollback(pPlan, pPackage, hCacheEvent, fPlanPackageCacheRollback); + hr = PlanExecuteCacheSyncAndRollback(pPlan, pPackage, hCacheEvent); ExitOnFailure(hr, "Failed to plan package cache syncpoint"); } diff --git a/src/engine/msiengine.h b/src/engine/msiengine.h index 99f97413..8b5bcdd0 100644 --- a/src/engine/msiengine.h +++ b/src/engine/msiengine.h @@ -50,8 +50,7 @@ HRESULT MsiEnginePlanAddPackage( __in BURN_PLAN* pPlan, __in BURN_LOGGING* pLog, __in BURN_VARIABLES* pVariables, - __in_opt HANDLE hCacheEvent, - __in BOOL fPlanPackageCacheRollback + __in_opt HANDLE hCacheEvent ); HRESULT MsiEngineBeginTransaction( __in BURN_ROLLBACK_BOUNDARY* pRollbackBoundary diff --git a/src/engine/mspengine.cpp b/src/engine/mspengine.cpp index ed105d24..5ccb6718 100644 --- a/src/engine/mspengine.cpp +++ b/src/engine/mspengine.cpp @@ -506,8 +506,7 @@ extern "C" HRESULT MspEnginePlanAddPackage( __in BURN_PLAN* pPlan, __in BURN_LOGGING* pLog, __in BURN_VARIABLES* pVariables, - __in_opt HANDLE hCacheEvent, - __in BOOL fPlanPackageCacheRollback + __in_opt HANDLE hCacheEvent ) { HRESULT hr = S_OK; @@ -517,7 +516,7 @@ extern "C" HRESULT MspEnginePlanAddPackage( // add wait for cache if (hCacheEvent) { - hr = PlanExecuteCacheSyncAndRollback(pPlan, pPackage, hCacheEvent, fPlanPackageCacheRollback); + hr = PlanExecuteCacheSyncAndRollback(pPlan, pPackage, hCacheEvent); ExitOnFailure(hr, "Failed to plan package cache syncpoint"); } diff --git a/src/engine/mspengine.h b/src/engine/mspengine.h index 1530954b..79998030 100644 --- a/src/engine/mspengine.h +++ b/src/engine/mspengine.h @@ -58,8 +58,7 @@ HRESULT MspEnginePlanAddPackage( __in BURN_PLAN* pPlan, __in BURN_LOGGING* pLog, __in BURN_VARIABLES* pVariables, - __in_opt HANDLE hCacheEvent, - __in BOOL fPlanPackageCacheRollback + __in_opt HANDLE hCacheEvent ); HRESULT MspEngineExecutePackage( __in_opt HWND hwndParent, diff --git a/src/engine/msuengine.cpp b/src/engine/msuengine.cpp index 91bbf361..59f9a656 100644 --- a/src/engine/msuengine.cpp +++ b/src/engine/msuengine.cpp @@ -193,8 +193,7 @@ extern "C" HRESULT MsuEnginePlanAddPackage( __in BURN_PLAN* pPlan, __in BURN_LOGGING* pLog, __in BURN_VARIABLES* pVariables, - __in HANDLE hCacheEvent, - __in BOOL fPlanPackageCacheRollback + __in HANDLE hCacheEvent ) { HRESULT hr = S_OK; @@ -203,7 +202,7 @@ extern "C" HRESULT MsuEnginePlanAddPackage( // add wait for cache if (hCacheEvent) { - hr = PlanExecuteCacheSyncAndRollback(pPlan, pPackage, hCacheEvent, fPlanPackageCacheRollback); + hr = PlanExecuteCacheSyncAndRollback(pPlan, pPackage, hCacheEvent); ExitOnFailure(hr, "Failed to plan package cache syncpoint"); } diff --git a/src/engine/msuengine.h b/src/engine/msuengine.h index cd966b0a..fda7a5ab 100644 --- a/src/engine/msuengine.h +++ b/src/engine/msuengine.h @@ -28,8 +28,7 @@ HRESULT MsuEnginePlanAddPackage( __in BURN_PLAN* pPlan, __in BURN_LOGGING* pLog, __in BURN_VARIABLES* pVariables, - __in HANDLE hCacheEvent, - __in BOOL fPlanPackageCacheRollback + __in HANDLE hCacheEvent ); HRESULT MsuEngineExecutePackage( __in BURN_EXECUTE_ACTION* pExecuteAction, diff --git a/src/engine/package.h b/src/engine/package.h index 42f1febe..262262ab 100644 --- a/src/engine/package.h +++ b/src/engine/package.h @@ -41,13 +41,6 @@ enum BURN_PACKAGE_TYPE BURN_PACKAGE_TYPE_MSU, }; -enum BURN_CACHE_STATE -{ - BURN_CACHE_STATE_NONE, - BURN_CACHE_STATE_PARTIAL, - BURN_CACHE_STATE_COMPLETE, -}; - enum BURN_CACHE_TYPE { BURN_CACHE_TYPE_NO, @@ -246,12 +239,12 @@ typedef struct _BURN_PACKAGE BURN_ROLLBACK_BOUNDARY* pRollbackBoundaryBackward; // used during uninstall. BOOTSTRAPPER_PACKAGE_STATE currentState; // only valid after Detect. - BURN_CACHE_STATE cache; // only valid after Detect. + BOOL fCached; // only valid after Detect. BOOL fPackageProviderExists; // only valid after Detect. BOOTSTRAPPER_REQUEST_STATE defaultRequested;// only valid during Plan. BOOTSTRAPPER_REQUEST_STATE requested; // only valid during Plan. - BOOL fAcquire; // only valid during Plan. - BOOL fUncache; // only valid during Plan. + BOOL fPlannedCache; // only valid during Plan. + BOOL fPlannedUncache; // only valid during Plan. BOOTSTRAPPER_ACTION_STATE execute; // only valid during Plan. BOOTSTRAPPER_ACTION_STATE rollback; // only valid during Plan. BURN_DEPENDENCY_ACTION providerExecute; // only valid during Plan. diff --git a/src/engine/plan.cpp b/src/engine/plan.cpp index c75c1753..187b1f15 100644 --- a/src/engine/plan.cpp +++ b/src/engine/plan.cpp @@ -31,8 +31,7 @@ static HRESULT PlanPackagesHelper( __in BURN_VARIABLES* pVariables, __in BOOTSTRAPPER_DISPLAY display, __in BOOTSTRAPPER_RELATION_TYPE relationType, - __in_z_opt LPCWSTR wzLayoutDirectory, - __inout HANDLE* phSyncpointEvent + __in_z_opt LPCWSTR wzLayoutDirectory ); static HRESULT InitializePackage( __in BURN_PLAN* pPlan, @@ -564,13 +563,12 @@ extern "C" HRESULT PlanPackages( __in BURN_VARIABLES* pVariables, __in BOOTSTRAPPER_DISPLAY display, __in BOOTSTRAPPER_RELATION_TYPE relationType, - __in_z_opt LPCWSTR wzLayoutDirectory, - __inout HANDLE* phSyncpointEvent + __in_z_opt LPCWSTR wzLayoutDirectory ) { HRESULT hr = S_OK; - hr = PlanPackagesHelper(pPackages->rgPackages, pPackages->cPackages, TRUE, pUX, pPlan, pLog, pVariables, display, relationType, wzLayoutDirectory, phSyncpointEvent); + hr = PlanPackagesHelper(pPackages->rgPackages, pPackages->cPackages, TRUE, pUX, pPlan, pLog, pVariables, display, relationType, wzLayoutDirectory); return hr; } @@ -776,8 +774,7 @@ extern "C" HRESULT PlanPassThroughBundle( __in BURN_LOGGING* pLog, __in BURN_VARIABLES* pVariables, __in BOOTSTRAPPER_DISPLAY display, - __in BOOTSTRAPPER_RELATION_TYPE relationType, - __inout HANDLE* phSyncpointEvent + __in BOOTSTRAPPER_RELATION_TYPE relationType ) { HRESULT hr = S_OK; @@ -785,7 +782,7 @@ extern "C" HRESULT PlanPassThroughBundle( // Plan passthrough package. // Passthrough packages are never cleaned up by the calling bundle (they delete themselves when appropriate) // so we don't need to plan clean up. - hr = PlanPackagesHelper(pPackage, 1, FALSE, pUX, pPlan, pLog, pVariables, display, relationType, NULL, phSyncpointEvent); + hr = PlanPackagesHelper(pPackage, 1, FALSE, pUX, pPlan, pLog, pVariables, display, relationType, NULL); ExitOnFailure(hr, "Failed to process passthrough package."); LExit: @@ -799,14 +796,13 @@ extern "C" HRESULT PlanUpdateBundle( __in BURN_LOGGING* pLog, __in BURN_VARIABLES* pVariables, __in BOOTSTRAPPER_DISPLAY display, - __in BOOTSTRAPPER_RELATION_TYPE relationType, - __inout HANDLE* phSyncpointEvent + __in BOOTSTRAPPER_RELATION_TYPE relationType ) { HRESULT hr = S_OK; // Plan update package. - hr = PlanPackagesHelper(pPackage, 1, TRUE, pUX, pPlan, pLog, pVariables, display, relationType, NULL, phSyncpointEvent); + hr = PlanPackagesHelper(pPackage, 1, TRUE, pUX, pPlan, pLog, pVariables, display, relationType, NULL); ExitOnFailure(hr, "Failed to process update package."); LExit: @@ -823,13 +819,13 @@ static HRESULT PlanPackagesHelper( __in BURN_VARIABLES* pVariables, __in BOOTSTRAPPER_DISPLAY display, __in BOOTSTRAPPER_RELATION_TYPE relationType, - __in_z_opt LPCWSTR wzLayoutDirectory, - __inout HANDLE* phSyncpointEvent + __in_z_opt LPCWSTR wzLayoutDirectory ) { HRESULT hr = S_OK; BOOL fBundlePerMachine = pPlan->fPerMachine; // bundle is per-machine if plan starts per-machine. BURN_ROLLBACK_BOUNDARY* pRollbackBoundary = NULL; + HANDLE hSyncpointEvent = NULL; // Initialize the packages. for (DWORD i = 0; i < cPackages; ++i) @@ -860,7 +856,7 @@ static HRESULT PlanPackagesHelper( DWORD iPackage = (BOOTSTRAPPER_ACTION_UNINSTALL == pPlan->action) ? cPackages - 1 - i : i; BURN_PACKAGE* pPackage = rgPackages + iPackage; - hr = ProcessPackage(fBundlePerMachine, pUX, pPlan, pPackage, pLog, pVariables, display, wzLayoutDirectory, phSyncpointEvent, &pRollbackBoundary); + hr = ProcessPackage(fBundlePerMachine, pUX, pPlan, pPackage, pLog, pVariables, display, wzLayoutDirectory, &hSyncpointEvent, &pRollbackBoundary); ExitOnFailure(hr, "Failed to process package."); } @@ -1126,10 +1122,11 @@ extern "C" HRESULT PlanExecutePackage( hr = AddCachePackage(pPlan, pPackage, phSyncpointEvent); ExitOnFailure(hr, "Failed to plan cache package."); } - else if (BURN_CACHE_STATE_COMPLETE != pPackage->cache && NeedsCache(pPackage, FALSE)) + else if (!pPackage->fCached && NeedsCache(pPackage, FALSE)) { + // TODO: this decision should be made during apply instead of plan based on whether the package is actually cached. // If the package is not in the cache, disable any rollback that would require the package from the cache. - LogId(REPORT_STANDARD, MSG_PLAN_DISABLING_ROLLBACK_NO_CACHE, pPackage->sczId, LoggingCacheStateToString(pPackage->cache), LoggingActionStateToString(pPackage->rollback)); + LogId(REPORT_STANDARD, MSG_PLAN_DISABLING_ROLLBACK_NO_CACHE, pPackage->sczId, LoggingActionStateToString(pPackage->rollback)); pPackage->rollback = BOOTSTRAPPER_ACTION_STATE_NONE; } @@ -1162,19 +1159,19 @@ extern "C" HRESULT PlanExecutePackage( switch (pPackage->type) { case BURN_PACKAGE_TYPE_EXE: - hr = ExeEnginePlanAddPackage(NULL, pPackage, pPlan, pLog, pVariables, *phSyncpointEvent, pPackage->fAcquire); + hr = ExeEnginePlanAddPackage(NULL, pPackage, pPlan, pLog, pVariables, *phSyncpointEvent); break; case BURN_PACKAGE_TYPE_MSI: - hr = MsiEnginePlanAddPackage(display, pUserExperience, pPackage, pPlan, pLog, pVariables, *phSyncpointEvent, pPackage->fAcquire); + hr = MsiEnginePlanAddPackage(display, pUserExperience, pPackage, pPlan, pLog, pVariables, *phSyncpointEvent); break; case BURN_PACKAGE_TYPE_MSP: - hr = MspEnginePlanAddPackage(display, pUserExperience, pPackage, pPlan, pLog, pVariables, *phSyncpointEvent, pPackage->fAcquire); + hr = MspEnginePlanAddPackage(display, pUserExperience, pPackage, pPlan, pLog, pVariables, *phSyncpointEvent); break; case BURN_PACKAGE_TYPE_MSU: - hr = MsuEnginePlanAddPackage(pPackage, pPlan, pLog, pVariables, *phSyncpointEvent, pPackage->fAcquire); + hr = MsuEnginePlanAddPackage(pPackage, pPlan, pLog, pVariables, *phSyncpointEvent); break; default: @@ -1368,7 +1365,6 @@ extern "C" HRESULT PlanRelatedBundlesComplete( __in BURN_PLAN* pPlan, __in BURN_LOGGING* pLog, __in BURN_VARIABLES* pVariables, - __inout HANDLE* phSyncpointEvent, __in DWORD dwExecuteActionEarlyIndex ) { @@ -1488,7 +1484,7 @@ extern "C" HRESULT PlanRelatedBundlesComplete( } } - hr = ExeEnginePlanAddPackage(pdwInsertIndex, &pRelatedBundle->package, pPlan, pLog, pVariables, *phSyncpointEvent, FALSE); + hr = ExeEnginePlanAddPackage(pdwInsertIndex, &pRelatedBundle->package, pPlan, pLog, pVariables, NULL); ExitOnFailure(hr, "Failed to add to plan related bundle: %ls", pRelatedBundle->package.sczId); // Calculate package states based on reference count for addon and patch related bundles. @@ -1560,11 +1556,8 @@ extern "C" HRESULT PlanCleanPackage( BOOL fPlanCleanPackage = FALSE; BURN_CLEAN_ACTION* pCleanAction = NULL; - // The following is a complex set of logic that determines when a package should be cleaned - // from the cache. Start by noting that we only clean if the package is being acquired or - // already cached and the package is not supposed to always be cached. - if ((pPackage->fAcquire || BURN_CACHE_STATE_PARTIAL == pPackage->cache || BURN_CACHE_STATE_COMPLETE == pPackage->cache) && - (BURN_CACHE_TYPE_ALWAYS > pPackage->cacheType || BOOTSTRAPPER_ACTION_CACHE > pPlan->action)) + // The following is a complex set of logic that determines when a package should be cleaned from the cache. + if (BURN_CACHE_TYPE_ALWAYS > pPackage->cacheType || BOOTSTRAPPER_ACTION_CACHE > pPlan->action) { // The following are all different reasons why the package should be cleaned from the cache. // The else-ifs are used to make the conditions easier to see (rather than have them combined @@ -1607,7 +1600,7 @@ extern "C" HRESULT PlanCleanPackage( pCleanAction->pPackage = pPackage; - pPackage->fUncache = TRUE; + pPackage->fPlannedUncache = TRUE; if (pPackage->fCanAffectRegistration) { @@ -1622,8 +1615,7 @@ LExit: extern "C" HRESULT PlanExecuteCacheSyncAndRollback( __in BURN_PLAN* pPlan, __in BURN_PACKAGE* pPackage, - __in HANDLE hCacheEvent, - __in BOOL fPlanPackageCacheRollback + __in HANDLE hCacheEvent ) { HRESULT hr = S_OK; @@ -1635,17 +1627,14 @@ extern "C" HRESULT PlanExecuteCacheSyncAndRollback( pAction->type = BURN_EXECUTE_ACTION_TYPE_WAIT_SYNCPOINT; pAction->syncpoint.hEvent = hCacheEvent; - if (fPlanPackageCacheRollback) - { - hr = PlanAppendRollbackAction(pPlan, &pAction); - ExitOnFailure(hr, "Failed to append rollback action."); + hr = PlanAppendRollbackAction(pPlan, &pAction); + ExitOnFailure(hr, "Failed to append rollback action."); - pAction->type = BURN_EXECUTE_ACTION_TYPE_UNCACHE_PACKAGE; - pAction->uncachePackage.pPackage = pPackage; + pAction->type = BURN_EXECUTE_ACTION_TYPE_UNCACHE_PACKAGE; + pAction->uncachePackage.pPackage = pPackage; - hr = PlanExecuteCheckpoint(pPlan); - ExitOnFailure(hr, "Failed to append execute checkpoint for cache rollback."); - } + hr = PlanExecuteCheckpoint(pPlan); + ExitOnFailure(hr, "Failed to append execute checkpoint for cache rollback."); LExit: return hr; @@ -1895,8 +1884,8 @@ static void ResetPlannedPackageState( // Reset package state that is a result of planning. pPackage->defaultRequested = BOOTSTRAPPER_REQUEST_STATE_NONE; pPackage->requested = BOOTSTRAPPER_REQUEST_STATE_NONE; - pPackage->fAcquire = FALSE; - pPackage->fUncache = FALSE; + pPackage->fPlannedCache = FALSE; + pPackage->fPlannedUncache = FALSE; pPackage->execute = BOOTSTRAPPER_ACTION_STATE_NONE; pPackage->rollback = BOOTSTRAPPER_ACTION_STATE_NONE; pPackage->providerExecute = BURN_DEPENDENCY_ACTION_NONE; @@ -2191,10 +2180,7 @@ static HRESULT AddCachePackageHelper( ++pPlan->cOverallProgressTicksTotal; - // If the package was not already fully cached then note that we planned the cache here. Otherwise, we only - // did cache operations to verify the cache is valid so we did not plan the acquisition of the package. - pPackage->fAcquire = (BURN_CACHE_STATE_COMPLETE != pPackage->cache); - + pPackage->fPlannedCache = TRUE; if (pPackage->fCanAffectRegistration) { pPackage->expectedCacheRegistrationState = BURN_PACKAGE_REGISTRATION_STATE_PRESENT; diff --git a/src/engine/plan.h b/src/engine/plan.h index 77f82e1f..0024b0aa 100644 --- a/src/engine/plan.h +++ b/src/engine/plan.h @@ -411,8 +411,7 @@ HRESULT PlanPackages( __in BURN_VARIABLES* pVariables, __in BOOTSTRAPPER_DISPLAY display, __in BOOTSTRAPPER_RELATION_TYPE relationType, - __in_z_opt LPCWSTR wzLayoutDirectory, - __inout HANDLE* phSyncpointEvent + __in_z_opt LPCWSTR wzLayoutDirectory ); HRESULT PlanRegistration( __in BURN_PLAN* pPlan, @@ -428,8 +427,7 @@ HRESULT PlanPassThroughBundle( __in BURN_LOGGING* pLog, __in BURN_VARIABLES* pVariables, __in BOOTSTRAPPER_DISPLAY display, - __in BOOTSTRAPPER_RELATION_TYPE relationType, - __inout HANDLE* phSyncpointEvent + __in BOOTSTRAPPER_RELATION_TYPE relationType ); HRESULT PlanUpdateBundle( __in BURN_USER_EXPERIENCE* pUX, @@ -438,8 +436,7 @@ HRESULT PlanUpdateBundle( __in BURN_LOGGING* pLog, __in BURN_VARIABLES* pVariables, __in BOOTSTRAPPER_DISPLAY display, - __in BOOTSTRAPPER_RELATION_TYPE relationType, - __inout HANDLE* phSyncpointEvent + __in BOOTSTRAPPER_RELATION_TYPE relationType ); HRESULT PlanLayoutPackage( __in BURN_PLAN* pPlan, @@ -475,7 +472,6 @@ HRESULT PlanRelatedBundlesComplete( __in BURN_PLAN* pPlan, __in BURN_LOGGING* pLog, __in BURN_VARIABLES* pVariables, - __inout HANDLE* phSyncpointEvent, __in DWORD dwExecuteActionEarlyIndex ); HRESULT PlanFinalizeActions( @@ -488,8 +484,7 @@ HRESULT PlanCleanPackage( HRESULT PlanExecuteCacheSyncAndRollback( __in BURN_PLAN* pPlan, __in BURN_PACKAGE* pPackage, - __in HANDLE hCacheEvent, - __in BOOL fPlanPackageCacheRollback + __in HANDLE hCacheEvent ); HRESULT PlanExecuteCheckpoint( __in BURN_PLAN* pPlan diff --git a/src/engine/pseudobundle.cpp b/src/engine/pseudobundle.cpp index 63300065..73fbb019 100644 --- a/src/engine/pseudobundle.cpp +++ b/src/engine/pseudobundle.cpp @@ -10,7 +10,7 @@ extern "C" HRESULT PseudoBundleInitialize( __in_z LPCWSTR wzId, __in BOOTSTRAPPER_RELATION_TYPE relationType, __in BOOTSTRAPPER_PACKAGE_STATE state, - __in BURN_CACHE_STATE cacheState, + __in BOOL fCached, __in_z LPCWSTR wzFilePath, __in_z LPCWSTR wzLocalSource, __in_z_opt LPCWSTR wzDownloadSource, @@ -67,14 +67,14 @@ extern "C" HRESULT PseudoBundleInitialize( memcpy_s(pPackage->rgPayloads->pPayload->pbHash, pPackage->rgPayloads->pPayload->cbHash, pbHash, cbHash); } - pPackage->rgPayloads->fCached = BURN_CACHE_STATE_NONE < cacheState; + pPackage->rgPayloads->fCached = fCached; pPackage->Exe.fPseudoBundle = TRUE; pPackage->type = BURN_PACKAGE_TYPE_EXE; pPackage->fPerMachine = fPerMachine; pPackage->currentState = state; - pPackage->cache = cacheState; + pPackage->fCached = fCached; pPackage->qwInstallSize = qwSize; pPackage->qwSize = qwSize; pPackage->fVital = fVital; @@ -216,7 +216,7 @@ extern "C" HRESULT PseudoBundleInitializePassthrough( pPassthroughPackage->fPerMachine = FALSE; // passthrough bundles are always launched per-user. pPassthroughPackage->type = pPackage->type; pPassthroughPackage->currentState = pPackage->currentState; - pPassthroughPackage->cache = pPackage->cache; + pPassthroughPackage->fCached = pPackage->fCached; pPassthroughPackage->qwInstallSize = pPackage->qwInstallSize; pPassthroughPackage->qwSize = pPackage->qwSize; pPassthroughPackage->fVital = pPackage->fVital; diff --git a/src/engine/pseudobundle.h b/src/engine/pseudobundle.h index c7940976..9fb530aa 100644 --- a/src/engine/pseudobundle.h +++ b/src/engine/pseudobundle.h @@ -13,7 +13,7 @@ HRESULT PseudoBundleInitialize( __in_z LPCWSTR wzId, __in BOOTSTRAPPER_RELATION_TYPE relationType, __in BOOTSTRAPPER_PACKAGE_STATE state, - __in BURN_CACHE_STATE cacheState, + __in BOOL fCached, __in_z LPCWSTR wzFilePath, __in_z LPCWSTR wzLocalSource, __in_z_opt LPCWSTR wzDownloadSource, diff --git a/src/engine/relatedbundle.cpp b/src/engine/relatedbundle.cpp index a4948a88..6953c678 100644 --- a/src/engine/relatedbundle.cpp +++ b/src/engine/relatedbundle.cpp @@ -391,14 +391,14 @@ static HRESULT LoadRelatedBundleFromKey( __in HKEY hkBundleId, __in BOOL fPerMachine, __in BOOTSTRAPPER_RELATION_TYPE relationType, - __inout BURN_RELATED_BUNDLE *pRelatedBundle + __inout BURN_RELATED_BUNDLE* pRelatedBundle ) { HRESULT hr = S_OK; DWORD64 qwEngineVersion = 0; LPWSTR sczBundleVersion = NULL; LPWSTR sczCachePath = NULL; - BURN_CACHE_STATE cacheState = BURN_CACHE_STATE_NONE; + BOOL fCached = FALSE; DWORD64 qwFileSize = 0; BURN_DEPENDENCY_PROVIDER dependencyProvider = { }; @@ -423,19 +423,16 @@ static HRESULT LoadRelatedBundleFromKey( hr = RegReadString(hkBundleId, BURN_REGISTRATION_REGISTRY_BUNDLE_CACHE_PATH, &sczCachePath); ExitOnFailure(hr, "Failed to read cache path from registry for bundle: %ls", wzRelatedBundleId); - hr = FileSize(sczCachePath, reinterpret_cast(&qwFileSize)); - if (SUCCEEDED(hr)) + if (FileExistsEx(sczCachePath, NULL)) { - cacheState = BURN_CACHE_STATE_COMPLETE; + fCached = TRUE; } - else if (E_FILENOTFOUND != hr) + else { - cacheState = BURN_CACHE_STATE_PARTIAL; - LogId(REPORT_STANDARD, MSG_DETECT_RELATED_BUNDLE_NOT_FULLY_CACHED, wzRelatedBundleId, sczCachePath, hr); + LogId(REPORT_STANDARD, MSG_DETECT_RELATED_BUNDLE_NOT_CACHED, wzRelatedBundleId, sczCachePath); } - hr = S_OK; - pRelatedBundle->fPlannable = BURN_CACHE_STATE_COMPLETE == cacheState; + pRelatedBundle->fPlannable = fCached; hr = RegReadString(hkBundleId, BURN_REGISTRATION_REGISTRY_BUNDLE_PROVIDER_KEY, &dependencyProvider.sczKey); if (E_FILENOTFOUND != hr) @@ -464,7 +461,7 @@ static HRESULT LoadRelatedBundleFromKey( pRelatedBundle->relationType = relationType; hr = PseudoBundleInitialize(qwEngineVersion, &pRelatedBundle->package, fPerMachine, wzRelatedBundleId, pRelatedBundle->relationType, - BOOTSTRAPPER_PACKAGE_STATE_PRESENT, cacheState, sczCachePath, sczCachePath, NULL, qwFileSize, FALSE, + BOOTSTRAPPER_PACKAGE_STATE_PRESENT, fCached, sczCachePath, sczCachePath, NULL, qwFileSize, FALSE, L"-quiet", L"-repair -quiet", L"-uninstall -quiet", (dependencyProvider.sczKey && *dependencyProvider.sczKey) ? &dependencyProvider : NULL, NULL, 0); diff --git a/src/test/BurnUnitTest/PlanTest.cpp b/src/test/BurnUnitTest/PlanTest.cpp index 2a34cb16..2ebbca74 100644 --- a/src/test/BurnUnitTest/PlanTest.cpp +++ b/src/test/BurnUnitTest/PlanTest.cpp @@ -127,7 +127,6 @@ namespace Bootstrapper ValidateExecuteCheckpoint(pPlan, fRollback, dwIndex++, dwExecuteCheckpointId++); ValidateExecuteCommitMsiTransaction(pPlan, fRollback, dwIndex++, L"rbaOCA08D8ky7uBOK71_6FWz1K3TuQ"); ValidateExecuteCheckpoint(pPlan, fRollback, dwIndex++, dwExecuteCheckpointId++); - ValidateExecuteWaitSyncpoint(pPlan, fRollback, dwIndex++, pPlan->rgCacheActions[23].syncpoint.hEvent); ValidateExecuteExePackage(pPlan, fRollback, dwIndex++, L"{FD9920AD-DBCA-4C6C-8CD5-B47431CE8D21}", BOOTSTRAPPER_ACTION_STATE_UNINSTALL, NULL); Assert::Equal(dwIndex, pPlan->cExecuteActions); @@ -509,7 +508,6 @@ namespace Bootstrapper ValidateExecutePackageDependency(pPlan, fRollback, dwIndex++, L"PackageA", L"{A6F0CBF7-1578-450C-B9D7-9CF2EEC40002}", BURN_DEPENDENCY_ACTION_REGISTER); ValidateExecuteCheckpoint(pPlan, fRollback, dwIndex++, dwExecuteCheckpointId++); ValidateExecuteCheckpoint(pPlan, fRollback, dwIndex++, dwExecuteCheckpointId++); - ValidateExecuteWaitSyncpoint(pPlan, fRollback, dwIndex++, pPlan->rgCacheActions[6].syncpoint.hEvent); ValidateExecuteExePackage(pPlan, fRollback, dwIndex++, L"{FD9920AD-DBCA-4C6C-8CD5-B47431CE8D21}", BOOTSTRAPPER_ACTION_STATE_UNINSTALL, NULL); Assert::Equal(dwIndex, pPlan->cExecuteActions); @@ -593,6 +591,7 @@ namespace Bootstrapper Assert::Equal(0ul, pPlan->cOverallProgressTicksTotal); dwIndex = 0; + ValidateCleanAction(pPlan, dwIndex++, L"PackageA"); Assert::Equal(dwIndex, pPlan->cCleanActions); UINT uIndex = 0; @@ -1042,7 +1041,7 @@ namespace Bootstrapper void DetectPackageAsPresentAndCached(BURN_PACKAGE* pPackage) { pPackage->currentState = BOOTSTRAPPER_PACKAGE_STATE_PRESENT; - pPackage->cache = BURN_CACHE_STATE_COMPLETE; + pPackage->fCached = TRUE; if (pPackage->fCanAffectRegistration) { pPackage->cacheRegistrationState = BURN_PACKAGE_REGISTRATION_STATE_PRESENT; @@ -1158,7 +1157,7 @@ namespace Bootstrapper pRelatedBundle->fPlannable = TRUE; pRelatedBundle->relationType = BOOTSTRAPPER_RELATION_UPGRADE; - hr = PseudoBundleInitialize(0, &pRelatedBundle->package, TRUE, wzId, pRelatedBundle->relationType, BOOTSTRAPPER_PACKAGE_STATE_PRESENT, BURN_CACHE_STATE_COMPLETE, NULL, NULL, NULL, 0, FALSE, L"-quiet", L"-repair -quiet", L"-uninstall -quiet", &dependencyProvider, NULL, 0); + hr = PseudoBundleInitialize(0, &pRelatedBundle->package, TRUE, wzId, pRelatedBundle->relationType, BOOTSTRAPPER_PACKAGE_STATE_PRESENT, TRUE, NULL, NULL, NULL, 0, FALSE, L"-quiet", L"-repair -quiet", L"-uninstall -quiet", &dependencyProvider, NULL, 0); NativeAssert::Succeeded(hr, "Failed to initialize related bundle to represent bundle: %ls", wzId); ++pRelatedBundles->cRelatedBundles; -- cgit v1.2.3-55-g6feb