Repository navigation
fix(tools): fold Windows PRX suffix case - #11473
Conversation
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
7d802ef to
e03d183
Compare
|
Synced the branch onto current main. The branch is now 2 commits ahead and 0 behind, the merge preview is clean, and current-main conventions and diff checks pass. I reran |
e03d183 to
2f2247e
Compare
|
Rebased again onto current main 48c160c after it advanced. The branch is 2 commits ahead and 0 behind; current-main sync, conventions, diff check, and merge preview pass. I reran The overlap scan still cannot compare #6171 and #7454 because their branches conflict with main. I inspected both current diffs: #6171 changes ELF module identity parsing and |
roboterspinne
left a comment
There was a problem hiding this comment.
Reviewed 7d802ef merged onto main 570089a (2026-10-26); the merge is clean. The rebased head 2f2247e has the same changes, and main 88795ed hasn't touched these files or ImportModule.hpp since. Windows 11, Python 3.13.9.
Results
test_import_audit.py: 151 tests OK, 3 skipped.- Counter-check: with
main'simport_audit.py,test_windows_prx_suffix_lookup_folds_the_suffix_casefails in 5 of 8 subtests (.SPRX,.SUPRX,.DEBUG_PRX,.prx,.PRX)..sprx,.suprxand.debug_prxalready pass onmain.
Should fix: see the inline comment. Folding the console suffixes hides a library the Windows output cannot load.
Nits
- The lowercase
.prxsubtest was red onmainbecause of the stem case (libsceaagainstLibSceA), not the extension case:mainfolds the stem only for lowercase.sprx,.suprxand.debug_prx. That part of the fix is right for Windows, and the What could say so. - Deriving
normalizedfromfoldedinstead ofname(line 726) passes the whole suite. It would turn a match into a provider mismatch when two built providers differ only in case, for exampleLibSceA.prxin one--libsdirectory andlibscea.prxin another. The exact-name preference at line 728 has no test, also not onmain.
Note: nothing changes for non-Windows outputs, since windows_names is empty there.
Not checked: Linux, and whether any title has uppercase suffixes in its needed libraries or import modules.
For disclosure: none of my open PRs touches these files.
Written with AI assistance (Claude Code). I ran the tests, the counter-check and the probes above on my Windows 11 machine.
| if name.endswith(suffix): | ||
| normalized = name.removesuffix(suffix) + ".prx" | ||
| folded = name.casefold() | ||
| for suffix in (".sprx", ".suprx", ".debug_prx", ".prx"): |
There was a problem hiding this comment.
Should fix: folding .SPRX, .SUPRX and .DEBUG_PRX makes the audit report a library as available that the Windows output can't load. The relinker maps those suffixes with a case-sensitive ends_with in Domain::NeededLibraryName (core/relinker/domain/include/domain/ImportModule.hpp:14-23): libscea.sprx becomes libscea.prx, but libscea.SPRX stays libscea.SPRX. That is the name the Windows output loads (WindowsImportBuilder.cpp:47, LoadLibraryExW in WindowsDependencyStubBuilder.cpp:251).
With only LibSceA.prx in a directory, LoadLibrary finds it as libscea.prx, libscea.PRX and LIBSCEA.PRX, and fails with error 126 for libscea.SPRX, libscea.SUPRX and libscea.DEBUG_PRX. So main's exit 1 for libscea.SPRX was right, the example in What describes correct behaviour as a bug, and the subtests for .SPRX, .SUPRX and .DEBUG_PRX assert the wrong outcome.
Folding only .prx keeps the half that is right:
if (folded if suffix == ".prx" else name).endswith(suffix):That fails exactly those three subtests and passes everything else, so they would need to expect a missing library (or go). If uppercase console suffixes should load, NeededLibraryName has to fold them first, in its own PR.
THEMANnt
left a comment
There was a problem hiding this comment.
Reviewed 2f2247eb8b043e9e55668d98ad6c8aaac722bb14. Approve; no actionable finding in this focused review.
Windows system-library matching folds the suffix as well as the lookup key, covering uppercase PRX, SPRX, SUPRX and DEBUG_PRX while preserving exact provider choices and unresolved-library reporting.
Validation: Windows/Python 3.11. Candidate: test_import_audit: 151 run, 3 skip reports; no failures. Comparison base: test_import_audit: 150 run, 3 skip reports. Old production fails five suffix-resolution assertions.
Scope: Synthetic PE/library resolution; no Windows loader or guest execution. Three candidate platform cases skip. Current head and effective hosted checks are verified again before submission.
|
@MarcZX10 Please fold only the .prx suffix (roboterspinne's one-line change at import_audit.py:724). The relinker maps .sprx/.suprx/.debug_prx case-sensitively in Domain::NeededLibraryName, so libscea.SPRX stays unmapped and Windows can't load it. Main's report of a missing library for it was correct. The .SPRX/.SUPRX/.DEBUG_PRX subtests should then expect a missing library (or go). If uppercase console suffixes should load, that needs NeededLibraryName to fold them first, in its own PR. |
2f2247e to
edfca59
Compare
|
Thanks for the correction, @oneandonlydean. I narrowed case folding to |
What
Windows module names are case-insensitive, but the audit only matched
.sprx,.suprxand.debug_prxaliases when their suffix was lowercase. For example,libscea.SPRXwas reported as a missing library and as a provider mismatch even whenLibSceA.prxwas present.Fold the suffix case before alias matching and include
.prxitself so filenames with differently cased extensions resolve to the built PE provider. The regression covers lower and upper case for all four recognized suffixes, in both registry imports and module dependencies.Tested
Linux, synthetic PE and ELF fixtures:
libscea.SPRX.python3 -m unittest tools.tests.test_import_audit: 151 tests passed, 1 skipped.python3 tools/check_conventions.py --base upstream/main: passed.git diff --check: passed.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)