Skip to content

fix(tools): include renamed paths in main sync checks - #11805

Merged
oneandonlydean merged 3 commits into
boykopovar:mainfrom
StoneDC:fix/main-sync-renamed-paths
Nov 2, 2026
Merged

oneandonlydean merged 3 commits into
boykopovar:mainfrom
StoneDC:fix/main-sync-renamed-paths

Conversation

@StoneDC

@StoneDC StoneDC commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

What

Renaming a tracked file can hide the old endpoint from the main-sync shared-path inventory and first-parent PR lookup. Use --no-renames in both inventories and the lookup so either direction retains the shared source path and identifies the upstream change. Preserve main's --literal-pathspecs so glob-like filenames remain literal. Git still determines the actual merge result; the inventory is conservative touched-path reporting.

Refresh onto current main and resolve the lookup conflict by retaining both protections. Keep main's five automatically discovered bounded test classes and the four separately registered rename cases. Remove the former per-method registrations and environment selector, which would otherwise duplicate tests and reference methods moved into main's new classes. Main's test_check_main_sync.py is byte-for-byte unchanged in the final Git diff.

Tested

Exact final tested head 16cfabc8a211d131ca69923ebe9bd0f683762452; main snapshot f6fddd60d826831f70f7c62e25423610ee742bcc. The original published PR head and this main are ancestors of the final branch.

Windows: Python 3.14.3, MinGW GCC 15.2, CMake 4.3.1. Ubuntu/WSL: Python 3.14.4, GCC 15.2, CMake 4.2.3. Linux uses tracked source snapshots on its native filesystem; the final six disjoint main-file updates were applied from the exact final archive before the focused rerun.

Configure Ninja Release with BUILD_TESTING=ON and ANYPS5_RELINKER_ONLY=ON. ctest --test-dir <build> -R '^(check_main_sync($|_)|main_sync_renames($|_))' --parallel 2 -V: 9/9 registered groups pass on both Windows and Linux, executing all 20 main methods and 4 rename cases with no skips. Real repositories cover both rename directions, Unicode/space paths, rename detection enabled/disabled, CLI output and actual Git merge results. Disabled detection preserves the real delete/modify conflict result.

Compare actual ctest --show-only=json-v1 commands with normal unittest discovery: all 24 methods are selected exactly once, with zero omissions or duplicates. Keep main's existing class limits and the original four rename limits; no timeout is raised.

Retain current tests and restore only main's production tool: the two enabled-detection rename cases fail at the expected shared-path assertion. Removing only --literal-pathspecs from the resolved tool fails main's literal-history assertion. Both controls execute on Windows and Linux without import/setup failures. Focused input bytes are unchanged by the final unrelated main merge.

This is tool/Git/CMake integration validation; no native relinker build, full product build, browser/title/PS5 or GPU execution is claimed for this Python-only update.

Current-main conventions with this complete description, whitespace, script registration, Python 3.9 grammar where applicable and CTest skip-policy checks pass. The two affected suites were rerun after the final main refresh. Imported unrelated graphics code is outside this PR's diff and focused validation. Hosted checks for the new head start after publication; prior-head results are not substituted for them.

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: none
  • Unblocks: none
  • GPUs tested: n/a
  • AI-assisted: yes

@github-actions

github-actions Bot commented Oct 27, 2026 •

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 1835 CMakeLists.txt
PR 5021 CMakeLists.txt
PR 7642 CMakeLists.txt
PR 9508 CMakeLists.txt
PR 9539 CMakeLists.txt
PR 13578 CMakeLists.txt
PR 14212 CMakeLists.txt
PR 14264 CMakeLists.txt

@babyhuey

Copy link
Copy Markdown
Contributor

Windows run of this head b61fa829cbb, merged onto main 5d52b06ff3d (clean merge, local only), with the selection CI's skipped Windows product job uses: Windows 11, Ryzen 7 5800X, RTX 3080, WinLibs GCC 15.2, Release, BUILD_TESTING=ON. The tree was configured fresh after the merge, then select_tests.py select / ninja -f select.ninja selected / --target libs / select_tests.py ctest --parallel 4 --timeout 120.

AI-assisted: yes (Claude Code)

@github-actions github-actions Bot added the build label Oct 27, 2026

@THEMANnt THEMANnt 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.

Reviewed b61fa829cbb789a09f545dbd99969f69081f1eec — approve. The conservative path inventory now retains both endpoints of renames; it does not replace Git merge/conflict detection.

Windows/Python 3.11: 22/22 focused tests passed (18 existing plus four real-Git rename scenarios). The author base passes its existing 18/18. Current rename tests against old production fail two cases because the shared original path disappears. Reviewed the CTest selector and separate bounded registrations; normal unittest invocation retains the full suite.

The hosted Windows relinker job merged this exact head and completed successfully. Product jobs were skipped; no full product build is claimed.

@oneandonlydean oneandonlydean self-assigned this Oct 28, 2026
@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 29, 2026
@StoneDC

StoneDC commented Oct 30, 2026

Copy link
Copy Markdown
Contributor Author

Updated head: 16cfabc8a211d131ca69923ebe9bd0f683762452, incorporating main f6fddd60d826831f70f7c62e25423610ee742bcc.

Resolved the current-main integration by retaining both --no-renames and --literal-pathspecs. Removed the superseded per-method registrations/environment selector, while preserving main's five bounded test classes and all four rename regressions. Main's existing test module remains unchanged.

On this exact final source, Windows and Linux each pass all 9 focused CTest groups: 20 main methods plus 4 rename methods, no skips. Actual CTest commands match unittest discovery exactly once per method. Restoring main's tool fails the two enabled-detection rename cases; removing only the literal-pathspec flag fails the preserved literal-history case. Conventions, whitespace, script registration and skip-policy checks pass; no timeout was increased.

Both focused suites were rerun after the final main refresh. The PR description records commands and validation limits. Hosted-check snapshot at 2026-10-30T00:57:34+00:00 (UTC): 1 passed, 8 pending, 3 skipped; no failures. Current checks. Pending/skipped checks are excluded from passes.

Please re-review the updated head when convenient. AI-assisted: yes.

@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 30, 2026
@oneandonlydean
oneandonlydean merged commit 720c3e9 into boykopovar:main Nov 2, 2026
13 of 14 checks passed
adriadam10 added a commit that referenced this pull request Nov 2, 2026
…running git

#11805 added test_main_sync_renames; main's push build 41455131775 failed its two detection-disabled cases on the runner's git 2.43, whose merge-tree ignores diff.renames/merge.renames and always detects the rename, so no conflict is reported. The tests now probe merge-tree on the fixture and expect a conflict only when the running git reports one; with detection enabled they still require a clean merge.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants