Skip to content

Rewrote MrmGetFilePathFromName for clarity/robustness and expanded search to include exe path even with a baseDir - #4965

Merged
Scott Jones (Scottj1s) merged 2 commits into
mainfrom
user/sjones/search_exedir_and_basedir
May 27, 2025
Merged

Scott Jones (Scottj1s) merged 2 commits into
mainfrom
user/sjones/search_exedir_and_basedir

Conversation

@Scottj1s

Copy link
Copy Markdown
Member

This work supports A/B runtime selection

…arch to include exe path even with a baseDir
break;
case SearchPass::ParentPathForFileName:
// move to parent folder of previous search
RETURN_IF_FAILED(PathCchRemoveFileSpec(searchDir, searchDirCount));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like searchDir and searchDir count will always be zero here since they are both local to the loop body scope?

@JesseCol

Copy link
Copy Markdown
Contributor

It seems important that we don't accidentally change the logic later -- is it possible to add a test case somewhere?

@Scottj1s
Scott Jones (Scottj1s) merged commit 4d11c40 into main May 27, 2025
@Scottj1s
Scott Jones (Scottj1s) deleted the user/sjones/search_exedir_and_basedir branch May 27, 2025 20:30
Scott Jones (Scottj1s) added a commit that referenced this pull request Jul 10, 2026
…(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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants