Skip to content

Commit 8dd14dd

Browse files
authored
Merge pull request #10466 from The-OpenROAD-Project-staging/ci-codeowners-three-dot-compare
ci: route CODEOWNERS reviewers via three-dot compare
2 parents d783371 + 7e10730 commit 8dd14dd

1 file changed

Lines changed: 41 additions & 57 deletions

File tree

.github/workflows/github-actions-on-label-create.yml

Lines changed: 41 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -101,43 +101,30 @@ jobs:
101101
python3 -m venv /tmp/codeowners-venv
102102
/tmp/codeowners-venv/bin/pip install --quiet pathspec
103103
/tmp/codeowners-venv/bin/python <<'PY'
104-
import base64, json, os, subprocess, sys, time
104+
import base64, json, os, subprocess, sys
105105
from pathspec import GitIgnoreSpec
106106
107107
pr = os.environ["PR"]
108108
upstream = os.environ["UPSTREAM"]
109109
110-
# GitHub computes PR mergeability asynchronously, so `mergeable` and
111-
# `merge_commit_sha` are often null on the first read after PR
112-
# creation. Poll until the background job finishes.
113-
attempts = 10
114-
delay = 3
115-
for attempt in range(1, attempts + 1):
116-
pr_json = subprocess.check_output(
117-
["gh", "api", f"repos/{upstream}/pulls/{pr}"], text=True)
118-
pr_data = json.loads(pr_json)
119-
if pr_data.get("mergeable") is not None \
120-
and pr_data.get("merge_commit_sha"):
121-
break
122-
print(f"Mergeability not yet computed "
123-
f"(attempt {attempt}/{attempts}); sleeping {delay}s")
124-
time.sleep(delay)
125-
else:
126-
raise RuntimeError(
127-
"PR merge commit SHA is unavailable after polling; refusing "
128-
"to request CODEOWNERS reviewers from an incomplete "
129-
"effective diff.")
110+
pr_json = subprocess.check_output(
111+
["gh", "api", f"repos/{upstream}/pulls/{pr}"], text=True)
112+
pr_data = json.loads(pr_json)
130113
131114
author = pr_data["user"]["login"]
132115
base_ref = pr_data["base"]["ref"]
133-
base_repo = pr_data["base"]["repo"]["full_name"]
134116
base_sha = pr_data["base"]["sha"]
135117
head_repo = pr_data["head"]["repo"]["full_name"]
136118
head_sha = pr_data["head"]["sha"]
137-
merge_sha = pr_data["merge_commit_sha"]
138119
139-
print(f"Computing effective PR merge diff: {base_repo}@{base_sha}.."
140-
f"{base_repo}@{merge_sha} (head {head_repo}@{head_sha})")
120+
# The compare endpoint accepts cross-fork heads as "{owner}:{sha}".
121+
upstream_owner = upstream.split("/", 1)[0]
122+
head_owner = head_repo.split("/", 1)[0]
123+
head_ref = (head_sha if head_owner == upstream_owner
124+
else f"{head_owner}:{head_sha}")
125+
126+
print(f"Computing PR diff: {upstream}@{base_sha}..."
127+
f"{head_repo}@{head_sha}")
141128
142129
# Authoritative CODEOWNERS is the one on the PR base branch.
143130
raw = subprocess.check_output(
@@ -154,42 +141,39 @@ jobs:
154141
pattern, *rule_owners = line.split()
155142
rules.append((GitIgnoreSpec.from_lines([pattern]), rule_owners))
156143
157-
def commit_tree(repo, sha):
158-
commit_json = subprocess.check_output(
159-
["gh", "api", f"repos/{repo}/git/commits/{sha}"],
160-
text=True)
161-
commit_data = json.loads(commit_json)
162-
tree_sha = commit_data["tree"]["sha"]
163-
tree_json = subprocess.check_output(
144+
# Use the three-dot compare (merge_base(base, head)...head). This is
145+
# what GitHub's "Files changed" tab shows and is a deterministic
146+
# function of the two SHAs, so it does not race with master moving.
147+
# Previously we diffed base.sha against merge_commit_sha, but the
148+
# async-computed merge_commit_sha can lag behind base.sha, making
149+
# files only touched on master after the PR was opened appear as
150+
# PR-introduced changes and falsely route their CODEOWNERS teams.
151+
files = []
152+
page = 1
153+
per_page = 100
154+
while True:
155+
cmp_json = subprocess.check_output(
164156
["gh", "api",
165-
f"repos/{repo}/git/trees/{tree_sha}?recursive=1"],
166-
text=True)
167-
tree_data = json.loads(tree_json)
168-
if tree_data.get("truncated"):
157+
f"repos/{upstream}/compare/{base_sha}...{head_ref}"
158+
f"?per_page={per_page}&page={page}"], text=True)
159+
cmp_data = json.loads(cmp_json)
160+
page_files = cmp_data.get("files") or []
161+
for f in page_files:
162+
files.append(f["filename"])
163+
# Renames carry the source in previous_filename; route to
164+
# CODEOWNERS for both the old and new paths so moves out of
165+
# an owned directory still notify the original owner.
166+
if f.get("previous_filename"):
167+
files.append(f["previous_filename"])
168+
if len(page_files) < per_page:
169+
break
170+
page += 1
171+
if page > 30:
169172
raise RuntimeError(
170-
f"Recursive tree for {repo}@{sha} was truncated; "
173+
"PR comparison exceeded pagination cap (>3000 files); "
171174
"refusing to request partial CODEOWNERS reviewers.")
172175
173-
entries = {}
174-
for entry in tree_data["tree"]:
175-
if entry["type"] == "tree":
176-
continue
177-
entries[entry["path"]] = (
178-
entry.get("sha"), entry.get("mode"), entry.get("type"))
179-
return entries
180-
181-
# Use the effective merge-result delta rather than GitHub's PR file
182-
# list or a direct base -> head tree comparison. The PR file list is
183-
# merge-base based, so it can include already-landed upstream changes
184-
# brought in by a merge from the base branch. A direct base -> head
185-
# comparison has the opposite problem when the PR branch is behind the
186-
# base branch: it reports base-only changes as if the PR changed them.
187-
# Comparing base -> test-merge tree matches the content that would land
188-
# if this PR merged now, including real conflict-resolution edits.
189-
base_tree = commit_tree(base_repo, base_sha)
190-
merge_tree = commit_tree(base_repo, merge_sha)
191-
files = sorted(path for path in set(base_tree) | set(merge_tree)
192-
if base_tree.get(path) != merge_tree.get(path))
176+
files = sorted(set(files))
193177
194178
print("Effective changed files:")
195179
for path in files:

0 commit comments

Comments
 (0)