From 345763f9346a0ef6647f5147e4ab059154f953c6 Mon Sep 17 00:00:00 2001 From: Bob Arnson Date: Mon, 9 Mar 2026 16:21:15 -0400 Subject: [PATCH] Set more detect-phase values when falling back. When configurable-scope bundle is already installed, find it during detect by falling back from machine to user. Set the scope as early as possible so detect-phase checks (e.g., cache path, state file) work as expected. Fixes https://github.com/wixtoolset/issues/issues/9257 --- src/burn/engine/apply.cpp | 4 +- src/burn/engine/core.cpp | 5 +++ src/burn/engine/engine.mc | 2 +- src/burn/engine/registration.cpp | 38 ++++++++++++------- src/burn/engine/registration.h | 3 +- .../ConfigurableScopeTests.cs | 11 ++++++ 6 files changed, 45 insertions(+), 18 deletions(-) diff --git a/src/burn/engine/apply.cpp b/src/burn/engine/apply.cpp index b7a54246d..818427ac1 100644 --- a/src/burn/engine/apply.cpp +++ b/src/burn/engine/apply.cpp @@ -536,8 +536,6 @@ extern "C" HRESULT ApplyUnregister( IgnoreRollbackError(hrRegistrationRollback, "Dependent registration actions failed"); } - LogId(REPORT_STANDARD, MSG_SESSION_END, pEngineState->registration.sczRegistrationKey, LoggingInstallScopeToString(pEngineState->registration.fPerMachine), LoggingResumeModeToString(resumeMode), LoggingRestartToString(restart), LoggingBoolToString(pEngineState->registration.fDisableResume), LoggingRegistrationTypeToString(defaultRegistrationType), LoggingRegistrationTypeToString(registrationType)); - if (BOOTSTRAPPER_ACTION_UNSAFE_UNINSTALL == pEngineState->plan.action) { registrationType = BOOTSTRAPPER_REGISTRATION_TYPE_NONE; @@ -546,6 +544,8 @@ extern "C" HRESULT ApplyUnregister( LogId(REPORT_STANDARD, MSG_UNSAFE_SESSION_END); } + LogId(REPORT_STANDARD, MSG_SESSION_END, pEngineState->registration.sczRegistrationKey, LoggingInstallScopeToString(pEngineState->registration.fPerMachine), LoggingResumeModeToString(resumeMode), LoggingRestartToString(restart), LoggingBoolToString(pEngineState->registration.fDisableResume), LoggingRegistrationTypeToString(defaultRegistrationType), LoggingRegistrationTypeToString(registrationType)); + if (pEngineState->registration.fPerMachine) { hr = ElevationSessionEnd(pEngineState->companionConnection.hPipe, resumeMode, restart, pEngineState->registration.fDetectedForeignProviderKeyBundleCode, qwEstimatedSize, registrationType); diff --git a/src/burn/engine/core.cpp b/src/burn/engine/core.cpp index da4ea91be..926c327b3 100644 --- a/src/burn/engine/core.cpp +++ b/src/burn/engine/core.cpp @@ -256,6 +256,10 @@ extern "C" HRESULT CoreQueryRegistration( SIZE_T cbBuffer = 0; SIZE_T iBuffer = 0; + // Detect if bundle is already installed. + hr = RegistrationDetectInstalled(&pEngineState->registration, &pEngineState->cache); + ExitOnFailure(hr, "Failed to detect bundle install state."); + // detect resume type hr = RegistrationDetectResumeType(&pEngineState->registration, &pEngineState->command.resumeType); ExitOnFailure(hr, "Failed to detect resume type."); @@ -2280,6 +2284,7 @@ static HRESULT DetectPackagePayloadsCached( LExit: ReleaseStr(sczPayloadCachePath); ReleaseStr(sczCachePath); + return hr; } diff --git a/src/burn/engine/engine.mc b/src/burn/engine/engine.mc index edafef970..72e62fe4b 100644 --- a/src/burn/engine/engine.mc +++ b/src/burn/engine/engine.mc @@ -1055,7 +1055,7 @@ MessageId=371 Severity=Success SymbolicName=MSG_SESSION_UPDATE Language=English -Updating session, registration key: %1!ls!, scope: %2!hs!, resume: %3!hs!, restart initiated: %4!hs!, disable resume: %5!hs! +Updating session, registration key: %1!ls!, scope: %2!hs!, resume: %3!hs!, restart initiated: %4!hs!, disable resume: %5!hs!, registration: %6!hs! . MessageId=372 diff --git a/src/burn/engine/registration.cpp b/src/burn/engine/registration.cpp index e7a9b95fd..855ebf046 100644 --- a/src/burn/engine/registration.cpp +++ b/src/burn/engine/registration.cpp @@ -64,7 +64,7 @@ static HRESULT EnsureRegistrationVariable( ); static HRESULT UpdateResumeMode( __in BURN_REGISTRATION* pRegistration, - __in HKEY hkRegistration, + __in_opt HKEY hkRegistration, __in BURN_RESUME_MODE resumeMode, __in BOOTSTRAPPER_REGISTRATION_TYPE registrationType, __in BOOL fRestartInitiated @@ -461,13 +461,7 @@ extern "C" HRESULT RegistrationSetDynamicVariables( ) { HRESULT hr = S_OK; - LONGLONG llInstalled = 0; - - // Detect if bundle is already installed. - hr = RegistrationDetectInstalled(pRegistration); - ExitOnFailure(hr, "Failed to detect bundle install state."); - - llInstalled = BOOTSTRAPPER_REGISTRATION_TYPE_FULL == pRegistration->detectedRegistrationType ? 1 : 0; + LONGLONG llInstalled = BOOTSTRAPPER_REGISTRATION_TYPE_FULL == pRegistration->detectedRegistrationType ? 1 : 0; hr = VariableSetNumeric(pVariables, BURN_BUNDLE_INSTALLED, llInstalled, TRUE); ExitOnFailure(hr, "Failed to set the bundle installed built-in variable."); @@ -483,7 +477,8 @@ extern "C" HRESULT RegistrationSetDynamicVariables( } extern "C" HRESULT RegistrationDetectInstalled( - __in BURN_REGISTRATION* pRegistration + __in BURN_REGISTRATION* pRegistration, + __in BURN_CACHE* pCache ) { HRESULT hr = S_OK; @@ -496,14 +491,28 @@ extern "C" HRESULT RegistrationDetectInstalled( { // For PUOM/PMOU bundles, check per-machine then fall back to per-user. hr = DetectInstalled(pRegistration, HKEY_LOCAL_MACHINE); + if (SUCCEEDED(hr)) + { + pRegistration->fPerMachine = TRUE; - if (FAILED(hr)) + hr = RegistrationSetPaths(pRegistration, pCache); + ExitOnFailure(hr, "Failed to set registration paths for per-machine configurable scope."); + } + else { hr = DetectInstalled(pRegistration, HKEY_CURRENT_USER); + + if (SUCCEEDED(hr)) + { + pRegistration->fPerMachine = FALSE; + + hr = RegistrationSetPaths(pRegistration, pCache); + ExitOnFailure(hr, "Failed to set registration paths for per-user configurable scope."); + } } } -//LExit: +LExit: // Not finding the key or value is okay. if (E_FILENOTFOUND == hr || E_PATHNOTFOUND == hr) { @@ -1086,6 +1095,8 @@ extern "C" HRESULT RegistrationSetPaths( hr = PathConcatRelativeToFullyQualifiedBase(sczCacheDirectory, pRegistration->sczExecutableName, &pRegistration->sczCacheExecutablePath); ExitOnFailure(hr, "Failed to build cached executable path."); + pRegistration->fCached = pRegistration->sczCacheExecutablePath && FileExistsEx(pRegistration->sczCacheExecutablePath, NULL); + // build state file path hr = StrAllocFormatted(&pRegistration->sczStateFile, L"%ls\\state.rsm", sczCacheDirectory); ExitOnFailure(hr, "Failed to build state file path."); @@ -1242,7 +1253,7 @@ static HRESULT EnsureRegistrationVariable( static HRESULT UpdateResumeMode( __in BURN_REGISTRATION* pRegistration, - __in HKEY hkRegistration, + __in_opt HKEY hkRegistration, __in BURN_RESUME_MODE resumeMode, __in BOOTSTRAPPER_REGISTRATION_TYPE registrationType, __in BOOL fRestartInitiated @@ -1254,7 +1265,7 @@ static HRESULT UpdateResumeMode( LPWSTR sczRunOnceCommandLine = NULL; LPCWSTR sczResumeKey = REGISTRY_RUN_ONCE_KEY; - LogId(REPORT_STANDARD, MSG_SESSION_UPDATE, pRegistration->sczRegistrationKey, LoggingInstallScopeToString(pRegistration->fPerMachine), LoggingResumeModeToString(resumeMode), LoggingBoolToString(fRestartInitiated), LoggingBoolToString(pRegistration->fDisableResume)); + LogId(REPORT_STANDARD, MSG_SESSION_UPDATE, pRegistration->sczRegistrationKey, LoggingInstallScopeToString(pRegistration->fPerMachine), LoggingResumeModeToString(resumeMode), LoggingBoolToString(fRestartInitiated), LoggingBoolToString(pRegistration->fDisableResume), LoggingRegistrationTypeToString(registrationType)); // write resume information if (hkRegistration) @@ -1776,7 +1787,6 @@ static HRESULT DetectInstalled( DWORD dwInstalled = 0; DWORD dwScope = 0; - pRegistration->fCached = pRegistration->sczCacheExecutablePath && FileExistsEx(pRegistration->sczCacheExecutablePath, NULL); pRegistration->detectedRegistrationType = BOOTSTRAPPER_REGISTRATION_TYPE_NONE; pRegistration->detectedScope = BOOTSTRAPPER_SCOPE_DEFAULT; diff --git a/src/burn/engine/registration.h b/src/burn/engine/registration.h index 97c6951b1..050a1a4b1 100644 --- a/src/burn/engine/registration.h +++ b/src/burn/engine/registration.h @@ -186,7 +186,8 @@ HRESULT RegistrationSetDynamicVariables( __in BURN_VARIABLES* pVariables ); HRESULT RegistrationDetectInstalled( - __in BURN_REGISTRATION* pRegistration + __in BURN_REGISTRATION* pRegistration, + __in BURN_CACHE* pCache ); HRESULT RegistrationDetectResumeType( __in BURN_REGISTRATION* pRegistration, diff --git a/src/test/burn/WixToolsetTest.BurnE2E/ConfigurableScopeTests.cs b/src/test/burn/WixToolsetTest.BurnE2E/ConfigurableScopeTests.cs index 049cefc3c..9e8870fc1 100644 --- a/src/test/burn/WixToolsetTest.BurnE2E/ConfigurableScopeTests.cs +++ b/src/test/burn/WixToolsetTest.BurnE2E/ConfigurableScopeTests.cs @@ -108,7 +108,12 @@ public void BundleUpgradeIsLockedToFirstBundlesScope() Assert.True(LogVerifier.MessageInLogFile(log, "Plan begin, 3 packages, action: Install, planned scope: Default")); + log = bundle.Modify(); + Assert.True(LogVerifier.MessageInLogFile(log, "TESTBA: OnDetectBegin: Cached=True")); + Assert.False(LogVerifier.MessageInLogFile(log, "Could not load or read state file:")); + log = bundle.Repair(); + Assert.False(LogVerifier.MessageInLogFile(log, "Could not load or read state file:")); Assert.True(LogVerifier.MessageInLogFile(log, "Bundle was already installed with scope: PerUser. Scope cannot change during maintenance.")); var bundleV2 = this.CreateBundleInstaller("AllPuomBundleTestBAv2"); @@ -132,6 +137,8 @@ public void BundleUpgradeWithSameScopeSucceeds() Assert.True(LogVerifier.MessageInLogFile(log, "Plan begin, 3 packages, action: Install, planned scope: Default")); log = bundle.Repair(); + Assert.True(LogVerifier.MessageInLogFile(log, "TESTBA: OnDetectBegin: Cached=True")); + Assert.False(LogVerifier.MessageInLogFile(log, "Could not load or read state file:")); Assert.True(LogVerifier.MessageInLogFile(log, "Bundle was already installed with scope: PerUser. Scope cannot change during maintenance.")); var bundleV2 = this.CreateBundleInstaller("AllPuomBundleTestBAv2"); @@ -164,6 +171,8 @@ public void PMOU_Bundle_Default_Plan_Installs_PerMachine() Assert.True(LogVerifier.MessageInLogFile(log, "Planned package: PmouPkg2.msi, state: Absent, default requested: Present, ba requested: Present, execute: Install, rollback: Uninstall, scope: PerMachine,")); log = bundle.Repair(); + Assert.True(LogVerifier.MessageInLogFile(log, "TESTBA: OnDetectBegin: Cached=True")); + Assert.False(LogVerifier.MessageInLogFile(log, "Could not load or read state file:")); Assert.True(LogVerifier.MessageInLogFile(log, "Bundle was already installed with scope: PerMachine")); Assert.True(LogVerifier.MessageInLogFile(log, "Detected package: PmouPkg1.msi, state: Present, authored scope: PerMachineOrUser, detected scope: PerMachine,")); Assert.True(LogVerifier.MessageInLogFile(log, "Detected package: PmouPkg2.msi, state: Present, authored scope: PerMachineOrUser, detected scope: PerMachine,")); @@ -508,6 +517,8 @@ public void PM_PU_PMOU_Bundle_Default_Plan_Installs_PerMachine() Assert.True(LogVerifier.MessageInLogFile(log, "Planned package: PerUserPkg.msi, state: Absent, default requested: Present, ba requested: Present, execute: Install, rollback: Uninstall, scope: PerUser")); log = bundle.Repair(); + Assert.True(LogVerifier.MessageInLogFile(log, "TESTBA: OnDetectBegin: Cached=True")); + Assert.False(LogVerifier.MessageInLogFile(log, "Could not load or read state file:")); Assert.True(LogVerifier.MessageInLogFile(log, "Detected package: PerMachinePkg.msi, state: Present, authored scope: PerMachine, detected scope: PerMachine,")); Assert.True(LogVerifier.MessageInLogFile(log, "Detected package: PmouPkg1.msi, state: Present, authored scope: PerMachineOrUser, detected scope: PerMachine,")); Assert.True(LogVerifier.MessageInLogFile(log, "Detected package: PmouPkg2.msi, state: Present, authored scope: PerMachineOrUser, detected scope: PerMachine,"));