Skip to content

fix(conventions): report 3rdparty submodules without a .gitmodules entry - #4304

Merged
oneandonlydean merged 1 commit into
boykopovar:mainfrom
SandstromPL:fix/conventions-thirdparty-gitlinks
Oct 12, 2026
Merged

oneandonlydean merged 1 commit into
boykopovar:mainfrom
SandstromPL:fix/conventions-thirdparty-gitlinks

Conversation

@SandstromPL

@SandstromPL SandstromPL commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

What

repository() in tools/check_conventions.py skips every changed path under 3rdparty/. That includes gitlinks, so a submodule added there without a .gitmodules entry passes. That is how the stray 3rdparty/imgui gitlink from ddf6b8c98 (#3470) reached main: Conventions run 38137041751 passed on that head at 11:50Z, and the PR merged at 11:51Z. Until #3511 removed it at 12:42Z, checking out main failed even with submodules: false. actions/checkout runs git submodule foreach, which stopped with fatal: No url found for submodule path '3rdparty/imgui' in .gitmodules. That broke the Conventions and relinker jobs of other pull requests, for example Conventions run 38137872909 on #3509.

A gitlink under 3rdparty/ that a pull request adds or changes now needs a .gitmodules entry with a path and a non-empty url, which is what a checkout with submodules needs. When the pull request changes .gitmodules, every gitlink under 3rdparty/ in the head is checked, so dropping an entry or its url is caught too. A missing one is reported as system-dependency ("submodule without a url in .gitmodules"). Gitlinks outside 3rdparty/ keep their existing report from #3667.

.gitmodules is read with git config --no-includes --blob <head>:.gitmodules, since git ignores includes there. A .gitmodules that git cannot parse leaves the changed gitlinks reported instead of crashing the check. On a pull request that touches neither, the check makes no extra git call.

Tested

Ubuntu 24.04, Python 3.12.3, git 2.43:

Not covered: a gitlink whose entry is unchanged but whose URL is unreachable.

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 (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
  • AI-assisted: yes (Claude Code); I reviewed and tested every line

Generated with Claude Code

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@oneandonlydean
oneandonlydean merged commit 436e310 into boykopovar:main Oct 12, 2026
15 checks passed
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.

2 participants