Skip to content

fix(tools): compare explicit APS5_EXPORT NIDs with named exports in pr_overlap - #9936

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
babyhuey:fix/pr-overlap-explicit-nids
Oct 23, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
babyhuey:fix/pr-overlap-explicit-nids

Conversation

@babyhuey

@babyhuey babyhuey commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

What

tools/pr_overlap.py now compares exports by NID, so an explicit APS5_EXPORT("<nid>", <function>) in one pull request is reported against a named export with the same NID in another (#9921).

exports() collects APS5_VABI definitions by name and skips _nid_no_patch functions, so APS5_EXPORT("Pz262plj20w", placeholder_nid_no_patch) added nothing. A pull request implementing FT_Outline_GetInsideBorder_nid_postfix, whose NID is Pz262plj20w, then showed no shared export with it, and merging both fails the NID patch step with duplicate patched export name: Pz262plj20w.

  • New aliases(diff) collects the NIDs of added APS5_EXPORT lines, using nid_names.source_aliases, the parser check_conventions.py already uses. scan() stores them in pr["aliases"].
  • overlaps() now calls same_exports(), which compares export_ids(): compute_nid of each named export plus the aliases. A match is listed by its function name when either side has the name, and by NID when both sides are aliases. NIDs that a shared dependency adds are still left out.
  • exports() is unchanged. An _nid_no_patch helper without an APS5_EXPORT line still adds nothing.

Like the existing name comparison, the NID comparison is not scoped to a module. I tried scoping it by source directory, and that misses the case in this issue: #9922 implements FT_Outline_GetInsideBorder in core/libs/prx/libSceFreeType/src/OutlineBorders.cpp, which its CMake builds into the provider that has #9462's placeholder:

add_library(libSceFreeTypeOt SHARED EXCLUDE_FROM_ALL Export.cpp ../libSceFreeType/src/OutlineBorders.cpp)
add_library(libSceFreeTypeOl SHARED EXCLUDE_FROM_ALL Export.cpp ../libSceFreeType/src/OutlineBorders.cpp)

Scoping by the built PRX would need the CMake source lists, and many use variables, included .cmake files and globs. So a match in different providers is reported too. In the open-PR scan below, those are 43 pairs: the four bulk declaration PRs (#9451, #9453, #9462, #9498), or #9922 which carries #9462's declarations, against implementations in sibling providers (for example #9453's libSceFreeTypeOl placeholder for FT_Get_Kerning against #8558's libSceFreeType implementation). They don't fail the NID patch step on their own, but each is the same function declared as not implemented in one PR and implemented in the other.

#8970 also edits tools/pr_overlap.py (comparison against exports already on main, as noted on #9921). The two touch neighbouring lines, so whichever merges second needs a small rebase.

Tested

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 under Silent stubs in their library's technical debt file
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Fixes: Fixes bug(pr-overlap): explicit APS5_EXPORT NIDs are omitted from overlap checks #9921
  • Unblocks: none
  • GPUs tested: n/a
  • AI-assisted: yes

@github-actions github-actions Bot added the fix error Closes an issue (Fixes #N) label Oct 22, 2026
@adriadam10 adriadam10 self-assigned this Oct 22, 2026
@MarcZX10

Copy link
Copy Markdown
Contributor

I reviewed the alias parser and NID comparison path. It normalizes named _nid_postfix exports through compute_nid, collects only added APS5_EXPORT aliases through the shared C++ token parser, suppresses identities inherited from common dependencies, and preserves the no-alias behavior for _nid_no_patch helpers. The unit cases cover alias collection/removal, alias-to-name matching, alias-to-alias output and shared-dependency suppression. I also ran the exact-head test_pr_overlap*.py suite from commit 8f2261b14bf6d8bc6a888e04832ead9b318ea1a6: 69 tests passed. The first local attempt lacked the workflow fixture in my /dev/shm snapshot; after adding that exact-head fixture, the suite passed. Hosted eligibility, conventions, intake and the completed relinker/ASan checks pass; Windows relinker is still running and product jobs are skipped.

The live base has advanced: this head is 1 commit ahead/11 behind current main caa089fd735d0a5d615279e0f567cb638ad65690, and the open queue is now 1,287 PRs. Please sync to current main and refresh the full overlap scan against the live queue before merge. The published 44-pair scan is useful and transparent, but it ran on main 5366b8b91 with 1,285 PRs. I found no implementation defect in the reviewed diff; this follow-up is for current-base/current-queue validation.

@MarcZX10

Copy link
Copy Markdown
Contributor

Follow-up with exact-head evidence: I reran the test_pr_overlap*.py suite on 8f2261b14bf6d8bc6a888e04832ead9b318ea1a6 (69/69 pass), then ran the PR branch scan against the live main and open-PR queue. It reported no confirmed overlapping PR pair; two comparisons remain manual because the shared files conflict with the base: #988 (tools/pr_overlap.py) and #8970 (tools/pr_overlap.py, tools/tests/test_pr_overlap_exports.py). I inspected those diffs: #988 adds scan/job-summary logging, while #8970 handles export collisions against base main; they cover distinct behavior from this PR’s PR-to-PR explicit-alias comparison. Please preserve both changes when integrating. Current hosted eligibility/conventions/intake/labels and cross-platform relinker/ASAN checks pass; product builds are skipped.

@github-actions

Copy link
Copy Markdown

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.

Pull request Shared files not compared
PR 988 tools/pr_overlap.py
PR 8970 tools/pr_overlap.py
tools/tests/test_pr_overlap_exports.py

@babyhuey
babyhuey force-pushed the fix/pr-overlap-explicit-nids branch from 8f2261b to 957b65f Compare October 22, 2026 23:47
@babyhuey

Copy link
Copy Markdown
Contributor Author

@MarcZX10 Synced and rescanned. The head is now 957b65fa2f, rebased onto main 895f47f4ee, the same patch (it picks up #9666's change to the same file without a conflict).

#988 and #8970 keep their behaviour; whichever of #8970 and this lands second needs the small rebase noted in the description.

@adriadam10
adriadam10 merged commit 7e69be6 into boykopovar:main Oct 23, 2026
14 checks passed
@adriadam10

Copy link
Copy Markdown
Collaborator

Checked aliases()/same_exports() against nid_names.compute_nid and source_aliases (only added APS5_EXPORT lines are collected, shared-dependency NIDs are still excluded, exports() is unchanged). Both new tests need the new functions, so they fail on main. Merged.

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

Labels

fix error Closes an issue (Fixes #N)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(pr-overlap): explicit APS5_EXPORT NIDs are omitted from overlap checks

3 participants