Add fallback for MrmGetFilePathFromName by Berrysoft · Pull Request #5828 · microsoft/WindowsAppSDK

Yuyi Wang (Berrysoft) · 2025-09-12T16:06:18Z

Closes #5814

Closes #5940

According to the docs, this method should success even if no resource file is found.

Scott Jones (Scottj1s) · 2026-06-09T14:38:50Z

/azp run

azure-pipelines · 2026-06-09T14:39:09Z

Azure Pipelines successfully started running 1 pipeline(s).

Scott Jones (Scottj1s) · 2026-06-09T14:50:39Z

/azp run

azure-pipelines · 2026-06-09T14:51:00Z

Azure Pipelines successfully started running 1 pipeline(s).

Rafael Rivera (riverar) · 2026-06-09T15:46:46Z

+        if ((pass == SearchPass::DefaultFallback) ||
+            ((attributes != INVALID_FILE_ATTRIBUTES) && !(attributes & FILE_ATTRIBUTE_DIRECTORY)))


I think this is going to need an audit of all the code in MUX and WAS where MRM was expected to fail right? (e.g., https://github.com/microsoft/WindowsAppSDK/blob/main/dev/MRTCore/mrt/Microsoft.Windows.ApplicationModel.Resources/src/Helper.cpp#L77-L103)

There are two paths calling this method. One is the ResourceManager:

WindowsAppSDK/dev/MRTCore/mrt/Microsoft.Windows.ApplicationModel.Resources/src/ResourceManager.cpp

Line 22 in 9bde038

if (!IsResourceNotFound(hr))

which is OK if the file not found, and is expected (

WindowsAppSDK/dev/MRTCore/mrt/Core/src/MRM.cpp

Lines 966 to 967 in 9bde038

// provided) under module file path. The reason is that we don't want to fail the creation of

// ResourceManager even if an app doesn't have PRI file.

). The other one is the ResourceLoader:

WindowsAppSDK/dev/MRTCore/mrt/Microsoft.Windows.ApplicationModel.Resources/src/ResourceLoader.cpp

Line 14 in 9bde038

winrt::check_hresult(GetDefaultPriFile(fileName));

which will fail if it cannot find the pri file.

Indeed - thanks for the heads up. Code like that will now need to test for nonexistent path as well (not just S_OK).

Do I need to add tests in this PR?

that would be great, Yuyi Wang (@Berrysoft) - thanks

Well. I have to admit that the PR was created more than half a year before. I need some time to investigate how to add the tests. If it is a must, I'll do it later. But if it is not, I think I'll keep the PR as-is.

Before 1.8, all code works well with the nonexistent path. Furthermore, I have been live-patching the MRM.dll since 1.8 to the latest 2.2. The patch always works well with all versions' WinUI. So I think this PR doesn't break the code.

Hello, Scott Jones (@Scottj1s) Rafael Rivera (@riverar) . I read the test code in this repo and found that it already contains tests for this case:

WindowsAppSDK/dev/MRTCore/mrt/Core/unittests/MrmTests.cpp

Lines 687 to 690 in e858676

VERIFY_ARE_EQUAL(MrmGetFilePathFromName(L"something.pri", &path), S_OK);

// Even if the file doesn't exist, we will still return a path. This is for those don't use PRI for resource.

VERIFY_IS_NOT_NULL(wcsstr(path, L"something.pri"));

MrmFreeResource(path);

But I don't know why it didn't fail before this PR. I think I don't need to add other tests. They already fill the need.

Did the audit. Both callers of MrmGetFilePathFromName are fine: ResourceManager already tolerates a missing PRI (only its check_hresult(GetDefaultPriFile) gate was breaking on the 1.8 ERROR_FILE_NOT_FOUND regression), and ResourceLoader throws with or without the change by design. The one internal path that keyed off the failure was GetDefaultPriFile's sparse-packaged [modulename].pri fallback (added in #6376), which gated on IsResourceNotFound(hr) — since the fix makes MrmGetFilePathFromName return S_OK, that now gates on actual file existence (CheckFile) instead. Handled in #6603.

On tests: MrmTests::GetFilePath and MrtCoreUnpackagedTests.TestClass.DefaultResourceManager already cover this — they just didn't catch the original regression because they were on the BypassTests.json allowlist (added in #5498, extended in #5919, after the regression). #6603 un-bypasses both and strengthens GetFilePath. Verified locally: MrmUnitTest 21/21, MrtCoreUnpackagedTests 27/27.

Awesome work Scott Jones (@Scottj1s), thank you. Squeaking a little more to get this landed too.

Co-authored-by: Rafael Rivera <rafael@withinwindows.com>

Rafael Rivera (riverar) · 2026-06-16T04:47:43Z

Can we slot this in for the next preview build Scott Jones (@Scottj1s) Roy (@RDMacLachlan)? This is impacting reactor projects downstream.

Yuyi Wang (Berrysoft) · 2026-07-07T14:37:36Z

Hello Scott Jones (@Scottj1s) , is it still possible to merge this PR?

Scott Jones (Scottj1s) · 2026-07-09T18:45:07Z

Thanks for this, Yuyi Wang (@Berrysoft) — I've opened #6603 to wrap it up on top of your change. The DefaultFallback pass added here isn't actually reachable (both terminal passes jump to Final), so the function returns S_OK with a null *filePath. #6603 makes DefaultFallback the reachable final pass returning a non-null best-effort path (legacy parity), fixes the dependent GetDefaultPriFile sparse-packaged fallback, and re-enables the two tests that were bypassed. Your commits are included there, so this supersedes the PR.

Yuyi Wang (Berrysoft) · 2026-07-09T18:53:28Z

Thanks!

Scott Jones (Scottj1s) · 2026-07-10T14:50:26Z

Thanks!

Thanks for opening the issue, suggesting a fix, and waiting patiently - much appreciated.

…(supersedes #5828) (#6603) * Add fallback for MrmGetFilePathFromName * Update dev/MRTCore/mrt/Core/src/MRM.cpp Co-authored-by: Rafael Rivera <rafael@withinwindows.com> * Complete MrmGetFilePathFromName fallback: return S_OK with a valid path 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> --------- Co-authored-by: 王宇逸 <Strawberry_Str@hotmail.com> Co-authored-by: Rafael Rivera <rafael@withinwindows.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Yuyi Wang (Berrysoft) · 2026-07-15T18:56:12Z

This PR is not needed now, I think? Regarding that #6603 has been merged.

Rafael Rivera (riverar) · 2026-07-18T04:12:07Z

Did that fix ship in a release we can test with?

Rafael Rivera (riverar) · 2026-07-21T21:44:28Z

Any updates on this? cc: Scott Jones (@Scottj1s)

Yuyi Wang (Berrysoft) · 2026-07-22T05:50:44Z

I don't know if there's a release, but it seems that this PR is not needed.

Rafael Rivera (riverar) · 2026-07-22T05:59:00Z

I'm confused because it seems like Scott Jones (@Scottj1s) was building on top of this PR, not replacing it.

Rafael Rivera (riverar) · 2026-07-22T06:18:49Z

I see now, you're right. This PR was superseded and never cleaned up. The lack of normal GitHub usage patterns by this team continues to frustrate me.

Scott Jones (Scottj1s) · 2026-07-22T13:51:08Z

Yuyi Wang (@Berrysoft) , Rafael Rivera (@riverar) - fix has been cherry picked for August's 1.8 and 2.0 servicing releases (note: usual caveats, schedules always subject to change)

Scott Jones (Scottj1s) · 2026-07-22T13:52:41Z

I see now, you're right. This PR was superseded and never cleaned up. The lack of normal GitHub usage patterns by this team continues to frustrate me.

fair - I considered building on top of this PR, and do appreciate your contribution, Yuyi Wang (@Berrysoft). there was enough divergence to warrant a new PR.

Yuyi Wang (Berrysoft) · 2026-07-22T14:49:29Z

fix has been cherry picked for August's 1.8 and 2.0 servicing releases

Thanks, Scott Jones (@Scottj1s) ! However, when I'm using the latest WinUI3 (2.3.1.0), MrmGetFilePathFromName still returns ERROR_FILE_NOT_FOUND (0x80070002). I detoured this method and get this return code. I watch my process through Process Explorer, and confirmed that the process is really loading the MRM.dll from C:\Program Files\WindowsApps\Microsoft.WindowsAppRuntime.2_2.3.1.0_x64__8wekyb3d8bbwe\MRM.dll. So, does winappsdk 2.3.1 really contain this fix?

Rafael Rivera (riverar) · 2026-07-23T18:35:31Z

I was confused too because at one point the PR or commits were tagged 2.3.1.0. ~~But seems that's gone now, I can't find it.~~

Update: This commit is tagged 2.3.1.0. 91160b0

So now we wait for August release!

Add fallback for MrmGetFilePathFromName

d4d425f

microsoft-github-policy-service Bot added the needs-triage label Sep 12, 2025

Yuyi Wang (Berrysoft) mentioned this pull request Oct 22, 2025

MrmGetFilePathFromName violates its document #5940

Closed

Yuyi Wang (Berrysoft) mentioned this pull request Jun 9, 2026

windows-reactor-setup: not good to copy resources.pri microsoft/windows-rs#4546

Closed

Merge branch 'main' into fix/res-not-found

b3274ee

Rafael Rivera (riverar) reviewed Jun 9, 2026

View reviewed changes

Yuyi Wang (Berrysoft) and others added 2 commits June 10, 2026 00:07

Update dev/MRTCore/mrt/Core/src/MRM.cpp

077ef23

Co-authored-by: Rafael Rivera <rafael@withinwindows.com>

Merge branch 'main' into fix/res-not-found

d111a1b

Rafael Rivera (riverar) mentioned this pull request Jun 11, 2026

windows-reactor-setup replace framework-dependent resources.pri microsoft/windows-rs#4562

Merged

Aalivexy (Aalivexy) mentioned this pull request Jun 29, 2026

Track upstream PR merge to remove temporary resources.pri copy workaround microsoft/windows-rs#4662

Closed

Scott Jones (Scottj1s) mentioned this pull request Jul 9, 2026

Fix MrmGetFilePathFromName to return S_OK with a valid fallback path (supersedes #5828) #6603

Merged

Yuyi Wang (Berrysoft) closed this Jul 22, 2026

		if ((pass == SearchPass::DefaultFallback) \|\|
		((attributes != INVALID_FILE_ATTRIBUTES) && !(attributes & FILE_ATTRIBUTE_DIRECTORY)))

	// provided) under module file path. The reason is that we don't want to fail the creation of
	// ResourceManager even if an app doesn't have PRI file.

	VERIFY_ARE_EQUAL(MrmGetFilePathFromName(L"something.pri", &path), S_OK);
	// Even if the file doesn't exist, we will still return a path. This is for those don't use PRI for resource.
	VERIFY_IS_NOT_NULL(wcsstr(path, L"something.pri"));
	MrmFreeResource(path);

Conversation

Yuyi Wang (Berrysoft) commented Sep 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Scott Jones (Scottj1s) commented Jun 9, 2026

Uh oh!

azure-pipelines Bot commented Jun 9, 2026

Uh oh!

Scott Jones (Scottj1s) commented Jun 9, 2026

Uh oh!

azure-pipelines Bot commented Jun 9, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Rafael Rivera (riverar) commented Jun 16, 2026

Uh oh!

Yuyi Wang (Berrysoft) commented Jul 7, 2026

Uh oh!

Scott Jones (Scottj1s) commented Jul 9, 2026

Uh oh!

Yuyi Wang (Berrysoft) commented Jul 9, 2026

Uh oh!

Scott Jones (Scottj1s) commented Jul 10, 2026

Uh oh!

Yuyi Wang (Berrysoft) commented Jul 15, 2026

Uh oh!

Rafael Rivera (riverar) commented Jul 18, 2026

Uh oh!

Rafael Rivera (riverar) commented Jul 21, 2026

Uh oh!

Yuyi Wang (Berrysoft) commented Jul 22, 2026

Uh oh!

Rafael Rivera (riverar) commented Jul 22, 2026

Uh oh!

Rafael Rivera (riverar) commented Jul 22, 2026

Uh oh!

Scott Jones (Scottj1s) commented Jul 22, 2026

Uh oh!

Scott Jones (Scottj1s) commented Jul 22, 2026

Uh oh!

Yuyi Wang (Berrysoft) commented Jul 22, 2026

Uh oh!

Rafael Rivera (riverar) commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Yuyi Wang (Berrysoft) commented Sep 12, 2025 •

edited

Loading

Rafael Rivera (riverar) commented Jul 23, 2026 •

edited

Loading