Fix MrmGetFilePathFromName to return S_OK with a valid fallback path (supersedes #5828)#6603
Merged
Merged
Conversation
Co-authored-by: Rafael Rivera <rafael@withinwindows.com>
PR #5828 added a DefaultFallback search pass to restore the documented "always succeed, return a best-effort path" contract of MrmGetFilePathFromName (regressed in #4965 to return ERROR_FILE_NOT_FOUND, breaking ResourceManager creation when no resources.pri exists - #5814 / #5940). However DefaultFallback was unreachable: both terminal passes (ParentPathForFileName, exeDirForModulePri) jumped to Final, skipping it. The function therefore returned S_OK with *filePath == nullptr, which crashes callers that dereference the path (e.g. MrmCreateResourceManager rejects an empty path with E_INVALIDARG, and the GetFilePath unit test AVs on wcsstr). Changes: - MRM.cpp: route both terminal passes into DefaultFallback so it runs as the final pass; return S_OK with a non-null best-effort path (the provided filename, else resources.pri) under the base directory (if set) otherwise the module directory - matching pre-#4965 (legacy) behavior. Preserve the module directory since the parent-folder search mutates exeDir in place. - Helper.cpp (GetDefaultPriFile): the sparse-packaged [modulename].pri fallback gated on IsResourceNotFound(hr); since MrmGetFilePathFromName now returns S_OK, gate on actual file existence (CheckFile) instead, and only adopt the unpackaged result if it resolves to an existing file (avoids re-regressing #6376 / microsoft-ui-xaml#10856). - MrmTests.cpp: strengthen GetFilePath to assert the fallback path is under the module directory. - BypassTests.json: remove the now-passing UnitTest::BasicTest::GetFilePath and MrtCoreUnpackagedTests.TestClass.DefaultResourceManager entries so they gate again (they were bypassed in #5498 / #5919 after the #4965 regression, which is why the regression shipped undetected). Verified locally (x64 Debug): MrmUnitTest 21/21, MrtCoreUnpackagedTests 27/27. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Dreynor87
approved these changes
Jul 10, 2026
ssparach
approved these changes
Jul 10, 2026
Member
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
This was referenced Jul 10, 2026
Scottj1s
added a commit
that referenced
this pull request
Jul 17, 2026
…6603) (#6612) * Cherry-pick MrmGetFilePathFromName S_OK fallback (gated) into release/2.0-stable Cherry-picks PR #6603 into release/2.0-stable, but gates the behavior change behind a RuntimeCompatibilityOptions containment check so apps can disable it. MrmGetFilePathFromName is documented to always succeed and return a best-effort path even when no PRI file exists. On 2.0-stable it fails with ERROR_FILE_NOT_FOUND, which makes `new ResourceManager()` throw when there is no resources.pri (the XAML-island / no-PRI scenario). Because this changes an HRESULT from a failure to S_OK, the change is contained under WINAPPSDK_CHANGEID_63048673: - Enabled (default): MrmGetFilePathFromName returns S_OK with a non-null best-effort path (the provided filename, else resources.pri) under the base directory (if set) otherwise the module directory. - Disabled (via RuntimeCompatibilityOptions): behavior reverts byte-for-byte to the current 2.0-stable behavior (ERROR_FILE_NOT_FOUND). Both terminal search passes jump to Final, skipping the DefaultFallback pass. Changes: - MRM.cpp: gate the reachable DefaultFallback pass + S_OK return on IsChangeEnabled<WINAPPSDK_CHANGEID_63048673>(). Add #include <FrameworkUdk/Containment.h> and an inline #define for the change ID, following the existing 62382643 precedent in Helper.cpp. Preserve the module directory since the parent-folder search mutates exeDir in place. - MRM.vcxproj: link the FrameworkUdk Containment worker (targets import + EnsureNuGetPackageBuildImports). - MrmUnitTest.vcxproj: deploy Microsoft.Internal.FrameworkUdk.dll next to the test host (mrm.dll now imports it). - Helper.cpp (GetDefaultPriFile): inside the existing 62382643 block, gate the sparse-packaged [modulename].pri fallback on actual file existence (CheckFile) so it still fires once MrmGetFilePathFromName returns S_OK. Correct whether 63048673 is enabled or disabled; preserves 62382643 behavior. - MrmTests.cpp: strengthen GetFilePath to assert the fallback path is under the module directory. - BypassTests.json: remove the now-passing UnitTest::BasicTest::GetFilePath and MrtCoreUnpackagedTests.TestClass.DefaultResourceManager entries so they gate. Note for release engineering: per the ContainmentV2 model this PR only defines and gates the change ID; associating 63048673 to a release group in RuntimeCompatibilityOptions.cpp's s_catalogGroupsProd[] is the separate branch-cut step. Verified locally (x64 Debug): MrmUnitTest 21/21 (change enabled by default). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add unit test confirming pre-fix behavior when fallback change is disabled Adds ContainmentDisabledTest::GetFilePathFallbackDisabled to MrmUnitTest. It disables WINAPPSDK_CHANGEID_63048673 via the FrameworkUdk containment worker (WinAppSdk::Containment::SetConfiguration) and verifies MrmGetFilePathFromName reverts to the previous behavior: ERROR_FILE_NOT_FOUND (null path) for a non-existent PRI file, instead of S_OK with a best-effort path. The class uses IsolationLevel=Method so it runs in its own process. This is required because IsChangeEnabled<>() caches its result in a process-lifetime function-local static, so the disabled-state test must not share a process with BasicTest::GetFilePath (which exercises the enabled default state). MrmUnitTest.vcxproj: import the FrameworkUdk targets so the test can call the Containment worker (mrm.dll already imports Microsoft.Internal.FrameworkUdk.dll, deployed next to the test host). Verified locally (x64 Debug): MrmUnitTest 22/22, including both the enabled (GetFilePath) and disabled (GetFilePathFallbackDisabled) cases. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Scottj1s
added a commit
that referenced
this pull request
Jul 17, 2026
…6603) (#6617) Ports the MrmGetFilePathFromName fix (main #6603, also cherry-picked to release/2.0-stable in #6612) into release/1.8-stable, gated behind a runtime-compatibility containment change so apps can disable it. MrmGetFilePathFromName is documented to always succeed and return a best-effort path even when no PRI file exists. On 1.8-stable it fails with ERROR_FILE_NOT_FOUND, which makes `new ResourceManager()` throw when there is no resources.pri (the XAML-island / no-PRI scenario - issue #5814). 1.8 uses the ContainmentV1 model where IsChangeEnabled requires a target patch version, so the change ID macro bakes it in: #define WINAPPSDK_CHANGEID_63048673 63048673, WinAppSDK_1_8_11 - Enabled (runtime at/after 1.8.11, default): MrmGetFilePathFromName returns S_OK with a non-null best-effort path (the provided filename, else resources.pri) under the base directory (if set) otherwise the module directory. - Disabled (via RuntimeCompatibilityOptions) or runtime pinned before 1.8.11: behavior reverts byte-for-byte to current 1.8-stable (ERROR_FILE_NOT_FOUND). Both terminal search passes jump to Final, skipping the DefaultFallback pass. Changes: - MRM.cpp: gate the reachable DefaultFallback pass + S_OK return on IsChangeEnabled<WINAPPSDK_CHANGEID_63048673>() (2-arg ContainmentV1 form). Add #include <FrameworkUdk/Containment.h> and the inline #define. Preserve the module directory since the parent-folder search mutates exeDir in place. - MRM.vcxproj: link the FrameworkUdk Containment worker (targets import + EnsureNuGetPackageBuildImports). - MrmUnitTest.vcxproj: link FrameworkUdk and deploy Microsoft.Internal.FrameworkUdk.dll next to the test host. - MrmTests.cpp: strengthen GetFilePath to assert the fallback path is under the module directory; add ContainmentDisabledTest (IsolationLevel=Method) that disables 63048673 via SetConfiguration and verifies the pre-fix ERROR_FILE_NOT_FOUND behavior. - BypassTests.json: remove the now-passing UnitTest::BasicTest::GetFilePath and MrtCoreUnpackagedTests.TestClass.DefaultResourceManager entries so they gate. Helper.cpp is intentionally NOT changed: 1.8 has no sparse-packaged PRI fallback block (that came later), so the S_OK result flows through GetDefaultPriFile without adjustment. Verified locally (x64 Debug): MrmUnitTest 22/22, including both the enabled (GetFilePath) and disabled (GetFilePathFallbackDisabled) cases. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #5828 (this branch includes @Berrysoft's commits, preserving authorship). Fixes #5940. Addresses #5814.
Background
MrmGetFilePathFromNameis documented to always succeed and return a best-effort path even when no PRI file exists. #4965 regressed it to returnERROR_FILE_NOT_FOUND, which makesnew ResourceManager()throw when there is noresources.pri(#5814 / #5940).#5828 added a
DefaultFallbacksearch pass to restore this, but it was unreachable: both terminal passes (ParentPathForFileName,exeDirForModulePri) jumped toFinal, skipping it, so the function returnedS_OKwith*filePath == nullptr. That crashes callers that dereference the path, andMrmCreateResourceManagerrejects an empty path withE_INVALIDARG— so the fix was incomplete.Changes
DefaultFallbackso it runs as the final pass; returnS_OKwith a non-null best-effort path (the provided filename, elseresources.pri) under the base directory (if set) otherwise the module directory — matching pre-Rewrote MrmGetFilePathFromName for clarity/robustness and expanded search to include exe path even with a baseDir #4965 (legacy) behavior. Preserve the module directory since the parent-folder search mutatesexeDirin place.GetDefaultPriFile) — the sparse-packaged[modulename].prifallback gated onIsResourceNotFound(hr); sinceMrmGetFilePathFromNamenow returnsS_OK, gate on actual file existence (CheckFile) instead, and only adopt the unpackaged result if it resolves to an existing file (avoids re-regressing Fix sparse-packaged apps unable to discover module-specific PRI files #6376 / microsoft-ui-xaml#10856).GetFilePathto assert the fallback path is under the module directory.UnitTest::BasicTest::GetFilePathandMrtCoreUnpackagedTests.TestClass.DefaultResourceManagerentries so they gate again. These tests already covered the contract but were bypassed in Upload TAEF test results #5498 / Disabling fail on alert and adding new tests to bypass list #5919 after the Rewrote MrmGetFilePathFromName for clarity/robustness and expanded search to include exe path even with a baseDir #4965 regression, which is why it shipped undetected.Validation
Built
MrtCore.sln(x64 Debug) and ran locally:MrmUnitTest— 21/21MrtCoreUnpackagedTests— 27/27 (incl.DefaultResourceManagerno-PRI: ctor succeeds, 0 resources,GetValuereturns0x80070490)