Skip to content

Commit 96a9587

Browse files
alexreinkingclaude
andauthored
Build PRs targeting main, recognized GitHub Stacks, or other tracked open PRs (#361)
Previously the scheduler only built PRs whose base branch was literally "main", so a PR whose base is another PR's branch (a stacked PR, e.g. halide/Halide#9253 based on #9251) was silently never scheduled. Move the gating into SafeGitHubEventHandler.handle_pull_request, where the full PR payload is available: - base == "main": build, as before. - base is a release/N.x branch: skip, as before. - PR is part of a recognized GitHub Stack (the `stack` payload field): build. refs/pull/<N>/merge for a stacked PR is chained through every lower PR's own merge ref, so building it already transitively tests against stack.base.ref (main) -- confirmed by inspecting the actual merge commit parents on halide/Halide#9249-#9253. - base is otherwise the head of another open PR in the same repo (an informal/pre-Stacks-feature chain): build. - anything else (an untracked, possibly short-lived branch): skip, to avoid wasting CI on builds that can be invalidated without warning. Fixes #148 Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 7bd9955 commit 96a9587

1 file changed

Lines changed: 78 additions & 4 deletions

File tree

master/master.cfg

Lines changed: 78 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ from buildbot.changes.filter import ChangeFilter
1616
from buildbot.config import BuilderConfig
1717
from buildbot.locks import WorkerLock
1818
from buildbot.process.factory import BuildFactory
19-
from buildbot.process.properties import Interpolate, Property, Transform, renderer
19+
from buildbot.process.properties import Interpolate, Properties, Property, Transform, renderer
2020
from buildbot.reporters.generators.build import BuildStartEndStatusGenerator
2121
from buildbot.reporters.github import GitHubStatusPush
2222
from buildbot.reporters.message import MessageFormatterRenderable
@@ -28,6 +28,7 @@ from buildbot.steps.master import SetProperties
2828
from buildbot.steps.shell import SetPropertyFromCommand, ShellCommand
2929
from buildbot.steps.source.github import GitHub
3030
from buildbot.steps.worker import MakeDirectory, SetPropertiesFromEnv, RemoveDirectory
31+
from buildbot.util import httpclientservice
3132
from buildbot.worker import Worker
3233
from buildbot.www.auth import UserPasswordAuth
3334
from buildbot.www.authz import Authz
@@ -1083,7 +1084,6 @@ c["schedulers"] = [
10831084
change_filter=ChangeFilter(
10841085
category="pull",
10851086
branch_fn=lambda br: br != "main",
1086-
filter_fn=lambda ch: ch.properties.getProperty("basename") == "main",
10871087
),
10881088
builderNames=[b1.name for b1 in c["builders"]],
10891089
),
@@ -1110,6 +1110,72 @@ class SafeGitHubEventHandler(GitHubEventHandler):
11101110
self._log(f"ignoring push event for ref: {ref}")
11111111
return self.skip()
11121112

1113+
@staticmethod
1114+
def _is_release_branch(basename):
1115+
# Matches the branches accepted by handle_push, above.
1116+
return bool(re.match(r"^release/\d+\.x$", basename or ""))
1117+
1118+
@inlineCallbacks
1119+
def _base_is_open_pr_head(self, base_repo_full_name, basename):
1120+
# Fallback for PRs based on another branch that GitHub hasn't
1121+
# registered as a formal Stack (see _base_branch_is_buildable):
1122+
# true if `basename` is itself the head of another open PR in the
1123+
# same repo, i.e. it's an informally-stacked/tracked branch rather
1124+
# than an untracked one that could vanish without warning.
1125+
owner = base_repo_full_name.split("/")[0]
1126+
headers = {"User-Agent": "Buildbot"}
1127+
if self._token:
1128+
p = Properties()
1129+
p.master = self.master
1130+
p.setProperty("full_name", base_repo_full_name, "change_hook")
1131+
token = yield p.render(self._token)
1132+
headers["Authorization"] = "token " + token
1133+
1134+
http = yield httpclientservice.HTTPSession(
1135+
self.master.httpservice, # ty: ignore[possibly-missing-attribute]
1136+
self.github_api_endpoint,
1137+
headers=headers,
1138+
debug=self.debug,
1139+
verify=self.verify,
1140+
)
1141+
res = yield http.get(
1142+
f"/repos/{base_repo_full_name}/pulls",
1143+
params={"head": f"{owner}:{basename}", "state": "open"},
1144+
)
1145+
if 200 <= res.code < 300:
1146+
data = yield res.json()
1147+
return len(data) > 0
1148+
1149+
self._log(f"Failed checking open PRs based on '{basename}': response code {res.code}")
1150+
return False
1151+
1152+
@inlineCallbacks
1153+
def _base_branch_is_buildable(self, pr):
1154+
# Buildbot builds refs/pull/<N>/merge, which GitHub computes against
1155+
# the PR's immediate base branch. For a PR that is part of a
1156+
# recognized GitHub Stack (the `stack` payload field), that merge
1157+
# ref is chained through every lower PR's own merge ref, so building
1158+
# it already transitively tests against `stack.base.ref` (typically
1159+
# "main") -- confirmed by walking the actual parent commits of
1160+
# halide/Halide#9249-#9253's refs/pull/*/merge. For a PR based on
1161+
# some other, untracked branch, the merge ref is just base-tip vs.
1162+
# head-tip with no relation to main, and that base branch may be
1163+
# force-pushed or deleted at any time, wasting the build. Only
1164+
# build PRs against main, recognized stacks, or (as a fallback for
1165+
# informal/pre-Stacks-feature chains) a base that is itself the
1166+
# head of another open PR.
1167+
basename = pr["base"]["ref"]
1168+
if basename == "main":
1169+
return True
1170+
if self._is_release_branch(basename):
1171+
return False
1172+
if pr.get("stack"):
1173+
return True
1174+
1175+
buildable = yield self._base_is_open_pr_head(pr["base"]["repo"]["full_name"], basename)
1176+
return buildable
1177+
1178+
@inlineCallbacks
11131179
def handle_pull_request(self, payload, event):
11141180
pr = payload["pull_request"]
11151181
try:
@@ -1125,7 +1191,8 @@ class SafeGitHubEventHandler(GitHubEventHandler):
11251191
# Pretend it's a 'synchronize' event instead since private buildbot code
11261192
# rejects review_requested for no clear reason.
11271193
payload["action"] = "synchronize"
1128-
return super().handle_pull_request(payload, event)
1194+
result = yield super().handle_pull_request(payload, event)
1195+
return result
11291196

11301197
# Skip external pull requests that originate from untrusted forks
11311198
trusted_repos = (
@@ -1137,8 +1204,15 @@ class SafeGitHubEventHandler(GitHubEventHandler):
11371204
self._log(f"PR {pr['head']['repo']['full_name']} was skipped due to being external:")
11381205
return self.skip()
11391206

1207+
# Skip PRs targeting a branch that isn't main, a recognized
1208+
# stack targeting main, or another actively-tracked open PR.
1209+
if not (yield self._base_branch_is_buildable(pr)):
1210+
self._log(f"PR {pr['html_url']} was skipped: base '{pr['base']['ref']}' isn't buildable")
1211+
return self.skip()
1212+
11401213
self._log(f"PR {pr['html_url']} is being handled normally")
1141-
return super().handle_pull_request(payload, event)
1214+
result = yield super().handle_pull_request(payload, event)
1215+
return result
11421216

11431217
except KeyError as e:
11441218
self._log(f'missing key "{e}" in malformed payload: {payload}')

0 commit comments

Comments
 (0)