Skip to content

fix(tools): name a missing head instead of reporting no common ancestor in check_main_sync - #10747

Merged
oneandonlydean merged 1 commit into
boykopovar:mainfrom
roboterspinne:fix/tools-check-main-sync-missing-head
Oct 25, 2026
Merged

oneandonlydean merged 1 commit into
boykopovar:mainfrom
roboterspinne:fix/tools-check-main-sync-missing-head

Conversation

@roboterspinne

Copy link
Copy Markdown
Contributor

What

report() in tools/check_main_sync.py raised <head> and <base> have no common ancestor for every nonzero exit of git merge-base, but git returns 1 only when the histories share no commit and dies with 128 when a revision does not resolve to a commit. So --head typo printed error: typo and main have no common ancestor while git merge-base main typo says fatal: Not a valid object name typo, and a tree or a blob as --head got the same message; nothing checks --head before report(), since resolve_base checks only the base.

  • Rule. Exit 1 keeps the no common ancestor message; any other nonzero exit raises git merge-base failed: <stderr>, the format conflicts() already uses for git merge-tree. The tool still exits 2 through parser.error in both cases.
  • Tests. test_unrelated_histories_are_an_error keeps its orphan-branch check (--head island still gives no common ancestor) and now also runs the tool with --head missing and asserts exit 2, git merge-base failed: followed by the stderr of git merge-base main missing from the same repository, and no no common ancestor. It compares with git's own output instead of fixed wording, which depends on the git version and locale. The assertions are folded into that test, so the case shares its setUp and adds no test method.
  • Overlap. pr_overlap.py reports no overlap with open pull requests. fix(tools): report squash-merged PRs in main sync checks #10380 edits both files too; git merge-tree of this branch with its head is clean, and the merged test file passes (16 tests).

Tested

Windows 11, Python 3.13.9 (Anaconda), git 2.55.0, GCC 15.2.0 (WinLibs UCRT), relinker-only Release build (-DANYPS5_RELINKER_ONLY=ON -DBUILD_TESTING=ON); synthetic fixtures only, no title or homebrew, relinked outputs not run.

  • Counter-check: against the untouched tool on main (2dc8560) the amended test_unrelated_histories_are_an_error fails with AssertionError: 'git merge-base failed: fatal: Not a valid object name missing' not found in '... check_main_sync.py: error: missing and main have no common ancestor\n'; with the change it passes.
  • tools/tests/test_check_main_sync.py run directly: 13 tests OK in 15.2 s; ctest --test-dir build-relinker -R '^check_main_sync$': passed in 15.5 s. Merged with the head of fix(tools): report squash-merged PRs in main sync checks #10380 (c9ace48), the file runs 16 tests OK in 20.4 s. On the Windows CI runners the file as on main already takes 31.0-32.6 s (four of five runs on 2026-10-24, --parallel 4) and gets the slow-test warning; the measured cost of the added assertions is in the next bullet.
  • Cost on Windows CI: check_main_sync takes 31.68 s on the Windows runner with the files as on main (#42: check_main_sync ... Passed 31.68 sec, CI run of fix(relinker): validate every entry of the Linux run path #10607 on d2a2889). Locally, the folded assertions add about 0.17 s to test_unrelated_histories_are_an_error (0.74 s against 0.58 s, mean of three runs each) and nothing distinguishable from noise to the file (15.22 s on this commit against 15.46 s on 2dc8560, mean of three interleaved runs each, 13 tests in both). Splitting the check_main_sync ctest entry is a separate topic and stays out of this PR.
  • Scratch repository with git 2.55.0 and an orphan branch, --base main: --head typo gives git merge-base failed: fatal: Not a valid object name typo, --head 'HEAD^{tree}' gives git merge-base failed: error: object ... is a tree, not a commit followed by fatal: Not a valid commit name HEAD^{tree}, both exit 2; the orphan branch still gives no common ancestor, exit 2; --head HEAD gives merge: clean, exit 0.
  • ctest --test-dir build-relinker -j 4: 244 tests, 241 passed, long_paths skipped; select_tests (Windows symlink fixture, FileExistsError in tools/tests/test_select_tests.py) and patch_advisor_unit (a temporary directory path with a space or an 8.3 short name; fix open in test(tools): compare quoted and resolved paths in the patch advisor test #10667) fail on this machine on main as well.
  • python tools/check_conventions.py --base upstream/main with this title and description passed.
  • Not tested on Linux or macOS; CI runs the tests on all three hosts.

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 (Claude Code)

…or in check_main_sync

report() turned every nonzero exit of git merge-base into "<head> and
<base> have no common ancestor", so a mistyped or unfetched --head, a
tree or a blob was reported as unrelated history and git's own error was
dropped. Now only exit status 1 keeps that message, and any other
failure raises "git merge-base failed: <stderr>" like the merge-tree
call in conflicts(). git merge-base returns 1 when the histories share
no commit and dies with 128 when a revision does not resolve to a
commit. The tool still exits 2 in both cases, and
test_unrelated_histories_are_an_error now also checks that an invalid
head reports git's own merge-base error and not a missing common
ancestor.

@MarcZX10 MarcZX10 left a comment

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.

Reviewed exact head dfd99b7cb5fcfcf0b1e3275fa888e39a9450acb3. The change distinguishes merge-base exit 1 (unrelated histories) from other failures, and the regression checks a missing revision against Git's actual stderr. I ran python3 tools/tests/test_check_main_sync.py -v (13/13 passed), python3 tools/check_conventions.py --base upstream/main --head upstream/pr-10747, and git diff --check (all passed). Hosted eligibility/conventions/intake/labels and Ubuntu/Windows/macOS relinker plus ASan checks are green; product jobs are skipped by path policy. I found no blocker.

@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 dfd99b7cb5fc against c8d34aa8f4d8. No actionable findings in this focused review.

Source review

report() now distinguishes unrelated histories (git merge-base exit 1) from other Git failures, including a missing head. Those failures retain Git’s diagnostic under git merge-base failed:, while both error paths preserve CLI exit 2. Successful reporting remains unchanged. The test preserves the orphan-history assertions and adds a real CLI invocation for the missing head, comparing its diagnostic with Git’s output from the same temporary repository.

Validation

  • Controller-recorded Windows/Python 3.11.9 runs of tools/tests/test_check_main_sync.py: baseline, pristine PR, and candidate each passed 13 tests with zero skips.
  • Candidate tests against baseline code failed at the intended missing-head assertion: old code incorrectly reported “no common ancestor.” There were no setup errors.
  • The recorded local whitespace and repository-conventions checks returned 0.
  • GitHub-reported CI succeeded for the four named relinker matrix jobs, relinker-asan, eligibility, conventions, intake, and labels. linux, windows, write-watch, and register-context were skipped; those skips do not establish product-test success.

Scope

Local validation covers the Python tool. The author’s broader build and timing claims were not independently reproduced; full application behavior was not tested.

@oneandonlydean oneandonlydean self-assigned this Oct 25, 2026
@oneandonlydean
oneandonlydean merged commit effe283 into boykopovar:main Oct 25, 2026
13 checks passed
@roboterspinne
roboterspinne deleted the fix/tools-check-main-sync-missing-head branch October 25, 2026 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants