Skip to content

fix(relinker): relink the modules that found needed modules need - #1782

Closed
MathiasDevelopes wants to merge 1 commit into
boykopovar:mainfrom
MathiasDevelopes:fix/relinker-transitive-needed-modules
Closed

MathiasDevelopes wants to merge 1 commit into
boykopovar:mainfrom
MathiasDevelopes:fix/relinker-transitive-needed-modules

Conversation

@MathiasDevelopes

Copy link
Copy Markdown
Contributor

What

When the executable needs a module that is not in sce_module, sce_modules or prx, the relinker searches the input tree for it and relinks it. The modules that this found module needs in turn were not looked up. They were left as host libraries, which no host provides, so the title failed to load.

The needed names of every module found this way are now searched for the same way, round by round, until no new module turns up. Each name is searched once, so a dependency cycle ends the search. Names that nothing in the input provides stay host libraries, as before. Ambiguity, exclusion (--exclude-sce-module) and the skipped module directories work as before. Each found module is parsed once, and the parsed image is reused when the modules are relinked.

The scope is kept narrow. Only modules found by this search have their needs followed. Modules in sce_module, sce_modules and prx are unchanged, and their needs resolve as before.

Example: Planet Coaster (PPSA01736) needs libcohtml.Prospero.prx at the package root, and that module needs libChakraCore.prx and libRenoirCore.PS5.prx beside it. Before, only cohtml was relinked; now all three are.

Tested

Windows 11, MinGW-w64 15.2.0. test_guest_needed_modules.py adds two cases, each for both Linux ELF and Windows PE output:

  • A found module needs a second module elsewhere in the tree: both are relinked. The Linux DT_NEEDED order puts the dependency first, and the Windows output runs and returns 42.
  • The second module needs a library the input lacks: it stays a host library in DT_NEEDED.

The first case fails without the fix. The relinker-only build passes its tests (36/36), and so does the full build (476/476). tools/check_conventions.py --base origin/main passes. Converting PPSA01736 now relinks cohtml, ChakraCore and RenoirCore, and the title loads them. Not built or run on Linux or macOS.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt
  • No notes, investigation .md files or images added to the repository
  • New third-party code is a submodule built from source (none added)
  • Generic behaviour, not specific to one title
  • Depends on: none. It touches the same lines as fix(relinker): retain declared import library identities #1617 (module identities for needed names); whichever lands second needs a rebase.
  • AI-assisted: yes

Copilot AI balanced review requested due to automatic review settings October 8, 2026 19:06

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@oneandonlydean

Copy link
Copy Markdown
Collaborator

This conflicts with main after today's merges; could you rebase? It passed testing and is otherwise ready.

@adriadam10 adriadam10 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The rebase isn't mechanical: since this PR was opened, #1503/#1830 changed the same block of GuestModuleBuilder.cpp (needed names matched by stem/alias and by module identity, images preloaded into discovered), so both GuestModuleBuilder.cpp and test_guest_needed_modules.py conflict.

Please rebase on current main and carry the round-by-round search over the new logic: the dependencies of each module found this way must go through the same exact/stem/identity matching, each name searched once, and the parsed image still reused when the modules are relinked. Keep both new test cases (transitive, and transitive with a host library), and check they still fail without the fix.

Dean already said it passed testing; once rebased with CI green it can go in.

@SCD28

SCD28 commented Oct 10, 2026

Copy link
Copy Markdown
Contributor

Rebuilt the PR changes on current main, preserving the newer exact-name, stem/alias and module-identity resolution already present in GuestModuleBuilder.
The transitive lookup now runs round by round, tracks searched dependency names so cycles terminate, and reuses parsed images already present in discovered. Missing package modules still remain host libraries.
The existing regression coverage now includes:

  • a transitive guest module found under Media/Modules;
  • a transitive dependency that is absent from the package and remains a host library.
    I also verified the regression directly: with only the transitive loop removed, guest_needed_nested_modules fails because libTransitive.prx.guest.prx is not produced. With the fix restored, the same test passes.
    Validation on Linux / WSL2 Ubuntu 24.04:
  • targeted relinker tests: 3/3 pass
  • guest_needed_nested_modules --repeat until-fail:5: 5/5 pass
  • full relinker-only suite: 96/96 pass
  • tools/check_conventions.py --base upstream/main: pass
  • tools/check_test_registration.py: pass
  • git diff --check: pass
    AI-assisted: yes. (Codex)
    Clean branch:
    SCD28/AnyPS5:fix/relinker-transitive-needed-modules
    Feel free to cherry-pick or reuse the branch if useful.

A module named by the executable's DT_NEEDED and found outside
sce_module, sce_modules and prx was relinked, but the modules it needed
in turn were not looked up. They were left as host libraries, which no
host provides, so the title failed to load. The needed names of each
module found this way are now searched for in the same way, until no new
module is found; names nothing in the input provides stay host
libraries. Each found module is parsed once.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@MathiasDevelopes
MathiasDevelopes force-pushed the fix/relinker-transitive-needed-modules branch from 8fde4cb to d62048e Compare October 10, 2026 09:08
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 10, 2026
@MathiasDevelopes

Copy link
Copy Markdown
Contributor Author

@oneandonlydean @adriadam10 Rebased onto current main. Since the needed-module search now also matches by stem and module identity, I moved that whole search into a step that reruns on the dependencies of each module it finds, until nothing new turns up. The first pass behaves exactly as on main. The two new tests (transitive, transitive-host) fail on main and pass here, and all relinker tests pass.

@github-actions github-actions Bot mentioned this pull request Oct 10, 2026
8 tasks done
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 10, 2026
@MathiasDevelopes

Copy link
Copy Markdown
Contributor Author

Closing this one. 41126bf removed the search for modules outside sce_module/sce_modules/prx, which is the code this PR extended. Now every module directly in those folders is relinked, so a module's own dependencies are picked up when they sit there too. I checked this on current main: a module in sce_module that needs another one relinks both in the right order, and a dependency nothing provides stays a host library. If a title turns up with modules outside those folders, I'll open a new issue for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs rebase Conflicts with main, rebase needed relinker

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants