Skip to content

Commit 0010548

Browse files
jgmelberclaude
andcommitted
Generalize submodule-pin check to cover all submodules, not just aie_api
Replace the aie_api-specific "must be an ancestor of upstream main" check with a generic ratchet: for every submodule in .gitmodules, HEAD's pin must be unchanged or a descendant of origin/main's pin. This needs no per-submodule knowledge of which branch/tag it tracks (cmake/modulesXilinx, platforms/boards, third_party/aie-rt, and third_party/bootgen each track a different one), so it generalizes cleanly across all five submodules instead of just the one that has regressed twice so far. A submodule whose .gitmodules url changes in the same diff is skipped, since a deliberate remote switch (see #3262) can legitimately be a non-fast-forward change, and the url edit is itself a visible, reviewed part of the diff. Verified locally against a synthetic submodule+superproject: forward bumps pass, url switches are skipped, and a reverted pin is caught with exit 1. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 05b7b1d commit 0010548

3 files changed

Lines changed: 145 additions & 72 deletions

File tree

.github/workflows/lintAndFormat.yml

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,9 @@ jobs:
8484
- name: Check shared dependency versions agree
8585
run: python3 utils/check_shared_dep_versions.py
8686

87-
aie-api-submodule:
87+
submodule-regressions:
8888

89-
name: aie_api submodule pin is not stale
89+
name: Submodule pins haven't regressed
9090

9191
runs-on: ubuntu-latest
9292

@@ -96,12 +96,13 @@ jobs:
9696
steps:
9797
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
9898

99-
# third_party/aie_api tracks stock upstream Xilinx/aie_api directly (no
100-
# fork, see #3262). The pin has twice been silently reverted to a stale
101-
# pre-#3262 commit by a merge/rebase from a branch based before the fix
102-
# landed (#3292, #3330). Fail fast on a third recurrence.
103-
- name: Verify aie_api pin is reachable from upstream main
104-
run: python3 utils/check_aie_api_submodule.py
99+
# A PR branch based on an old commit of main can silently reset a
100+
# submodule's gitlink back to an older pin when merged, undoing a bump
101+
# that has since landed on main -- third_party/aie_api hit this twice
102+
# (#3292, then again via #3296, fixed by #3330). Fail fast on a repeat,
103+
# for any submodule, not just aie_api.
104+
- name: Verify no submodule pin moved backward relative to main
105+
run: python3 utils/check_submodule_regressions.py
105106

106107
clang-tidy-pylint:
107108

utils/check_aie_api_submodule.py

Lines changed: 0 additions & 64 deletions
This file was deleted.
Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
#!/usr/bin/env python3
2+
3+
# Copyright (C) 2026 Advanced Micro Devices, Inc.
4+
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
5+
6+
"""Fail if any submodule's pin moves backward relative to origin/main.
7+
8+
Submodule gitlinks are merged like ordinary blobs: a PR branch based on an
9+
old commit of main that never rebases can silently reset a submodule pin to
10+
whatever it was on that old base, undoing a bump that has since landed on
11+
main. third_party/aie_api hit exactly this twice (#3292, then again via
12+
#3296, fixed by #3330).
13+
14+
This check is deliberately generic across every submodule in .gitmodules,
15+
not just aie_api, and it does not need to know which branch/tag each one is
16+
meant to track (cmake/modulesXilinx, platforms/boards, third_party/aie-rt,
17+
and third_party/bootgen all track different release branches/tags of their
18+
own). It only cares whether, for each path, HEAD's pin is equal to or a
19+
descendant of origin/main's pin -- which is exactly what a silent revert
20+
from a stale-branch merge is not.
21+
22+
A submodule whose .gitmodules url changed in this diff is skipped: a
23+
deliberate remote switch (see #3262) can legitimately be a non-fast-forward
24+
change in commit history, and the url edit itself is a visible, reviewed
25+
part of the diff.
26+
"""
27+
28+
import subprocess
29+
import sys
30+
31+
BASE_REF = "origin/main"
32+
33+
34+
def run(*args):
35+
return subprocess.run(
36+
args, check=True, capture_output=True, text=True
37+
).stdout.strip()
38+
39+
40+
def try_run(*args):
41+
result = subprocess.run(args, capture_output=True, text=True)
42+
return result.returncode == 0, result.stdout.strip()
43+
44+
45+
def submodule_paths_and_urls(rev):
46+
ok, paths_out = try_run(
47+
"git", "config", f"--blob={rev}:.gitmodules",
48+
"--get-regexp", r"^submodule\..*\.path$",
49+
)
50+
if not ok:
51+
return {}
52+
53+
urls = dict(
54+
line.split(" ", 1)
55+
for line in run(
56+
"git", "config", f"--blob={rev}:.gitmodules",
57+
"--get-regexp", r"^submodule\..*\.url$",
58+
).splitlines()
59+
)
60+
61+
path_by_url_key = {}
62+
for line in paths_out.splitlines():
63+
key, path = line.split(" ", 1)
64+
url_key = key[: -len(".path")] + ".url"
65+
path_by_url_key[path] = urls.get(url_key)
66+
67+
return path_by_url_key
68+
69+
70+
def pinned_commit(rev, path):
71+
ok, out = try_run("git", "ls-tree", rev, "--", path)
72+
if not ok or not out.strip():
73+
return None
74+
return out.split()[2]
75+
76+
77+
def check_submodule(path, url, old_pin, new_pin):
78+
old_ref = f"refs/tmp/submodule-check-old-{abs(hash(path))}"
79+
new_ref = f"refs/tmp/submodule-check-new-{abs(hash(path))}"
80+
run("git", "fetch", "--quiet", "--force", url, f"{old_pin}:{old_ref}")
81+
run("git", "fetch", "--quiet", "--force", url, f"{new_pin}:{new_ref}")
82+
return subprocess.run(
83+
["git", "merge-base", "--is-ancestor", old_ref, new_ref]
84+
).returncode == 0
85+
86+
87+
def main():
88+
run("git", "fetch", "--quiet", "origin", "main")
89+
90+
head_submodules = submodule_paths_and_urls("HEAD")
91+
base_submodules = submodule_paths_and_urls(BASE_REF)
92+
93+
failures = []
94+
for path, url in head_submodules.items():
95+
new_pin = pinned_commit("HEAD", path)
96+
old_pin = pinned_commit(BASE_REF, path)
97+
98+
if new_pin is None or old_pin is None:
99+
print(f"OK: {path} added or removed on this branch, nothing to compare.")
100+
continue
101+
102+
if old_pin == new_pin:
103+
print(f"OK: {path} unchanged ({new_pin}).")
104+
continue
105+
106+
if base_submodules.get(path) != url:
107+
print(
108+
f"NOTE: {path} changed its .gitmodules url in this diff; "
109+
"skipping the ancestry check (the remote switch is itself "
110+
"a visible, reviewed part of the diff)."
111+
)
112+
continue
113+
114+
if check_submodule(path, url, old_pin, new_pin):
115+
print(f"OK: {path} moved forward {old_pin} -> {new_pin}")
116+
else:
117+
failures.append((path, old_pin, new_pin))
118+
119+
if failures:
120+
for path, old_pin, new_pin in failures:
121+
print(
122+
f"::error::{path} moved from {old_pin} (on {BASE_REF}) to "
123+
f"{new_pin}, which is not a descendant of it. This is what a "
124+
"silent regression from a stale-branch merge looks like (see "
125+
"#3292, #3330). If this is an intentional downgrade, rebase "
126+
"onto the latest main before merging so the pin doesn't "
127+
"silently clobber a bump that landed after this branch was "
128+
"created."
129+
)
130+
sys.exit(1)
131+
132+
print("All submodule pins are unchanged or moved forward relative to origin/main.")
133+
134+
135+
if __name__ == "__main__":
136+
main()

0 commit comments

Comments
 (0)