Skip to content

Commit 8dd2f02

Browse files
alexreinkingclaude
andcommitted
Build PRs targeting main, recognized GitHub Stacks, or other tracked open PRs
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 8dd2f02

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)