mirror of
https://github.com/bherbruck/solidcraft.git
synced 2026-11-03 02:25:55 +00:00
CI: carry only from this pull request's runs, compare patches verbatim
From the review: git patch-id --stable ignores whitespace, so a conflict resolution that only respaced the change (inside a string, or Python indentation) counted as no change and was carried; --verbatim compares every byte. And a passing run is now taken only from this pull request (by number, or for a fork, where GitHub lists none on the run, by head repository and branch), not from another pull request with the same head commit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QWhMwdqJYQKQq8RCLSL1LT
This commit is contained in:
@@ -130,6 +130,9 @@ jobs:
|
||||
BASE_SHA: ${{ github.event.pull_request.base.sha }}
|
||||
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
|
||||
PUSH_SHA: ${{ github.sha }}
|
||||
PR_NUMBER: ${{ github.event.pull_request.number }}
|
||||
HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }}
|
||||
HEAD_REF: ${{ github.event.pull_request.head.ref }}
|
||||
REPO: ${{ github.repository }}
|
||||
APPROVED: ${{ contains(github.event.pull_request.labels.*.name, 'approved-for-ci') }}
|
||||
GH_TOKEN: ${{ github.token }}
|
||||
@@ -236,7 +239,8 @@ jobs:
|
||||
# touched the pull request's files, and each restarted the whole run.
|
||||
carried=
|
||||
if [ "$EVENT_NAME" = pull_request ] && [ "$native" = true ]; then
|
||||
carried=$(python3 -I scripts/ci/main_merge.py --base "$BASE_SHA" --head "$HEAD_SHA" --repo "$REPO" | sed -n 's/^carried=//p')
|
||||
carried=$(python3 -I scripts/ci/main_merge.py --base "$BASE_SHA" --head "$HEAD_SHA" --repo "$REPO" \
|
||||
--pr "$PR_NUMBER" --head-repo "$HEAD_REPO" --head-ref "$HEAD_REF" | sed -n 's/^carried=//p')
|
||||
if [ -n "$carried" ]; then
|
||||
echo "Only a merge of main into a change that passed in $carried: repository checks only."
|
||||
native=false
|
||||
|
||||
@@ -1,14 +1,18 @@
|
||||
#!/usr/bin/env python3
|
||||
"""Is this pull request push only a merge of main into a change that already passed CI?
|
||||
|
||||
python3 -I scripts/ci/main_merge.py --base BASE_SHA --head HEAD_SHA --repo OWNER/NAME
|
||||
python3 -I scripts/ci/main_merge.py --base BASE_SHA --head HEAD_SHA --repo OWNER/NAME \
|
||||
--pr NUMBER --head-repo OWNER/NAME --head-ref BRANCH
|
||||
|
||||
Prints `carried=<url of the passing run>` when all of these hold, else `carried=`:
|
||||
- HEAD is a merge commit whose second parent is on main (an ancestor of BASE): "Merge branch
|
||||
'main'", or GitHub's Update branch button;
|
||||
- the pull request's own change is the same before and after the merge (equal `git patch-id`),
|
||||
so the merge resolved no conflict and brought nothing into the change itself;
|
||||
- a ci.yml run of the change before the merge passed with the test shards. When that change was
|
||||
- the pull request's own change is the same before and after the merge (equal `git patch-id
|
||||
--verbatim`, whitespace included), so the merge resolved no conflict and brought nothing into
|
||||
the change itself;
|
||||
- a ci.yml run of this pull request (by number, or for a fork, where GitHub lists no pull
|
||||
request on the run, by head repository and branch) for the change before the merge passed
|
||||
with the test shards. When that change was
|
||||
itself only a merge of main (a run that carried, its shards skipped), the walk goes on down
|
||||
the first parents, through at most MAX_CHAIN merges, to the run where the shards ran.
|
||||
|
||||
@@ -38,7 +42,8 @@ def patch_id(a, b):
|
||||
diff = subprocess.run(["git", "diff", "--no-renames", a, b], capture_output=True, check=True).stdout
|
||||
if not diff.strip():
|
||||
return ""
|
||||
out = subprocess.run(["git", "patch-id", "--stable"], input=diff, capture_output=True, check=True).stdout
|
||||
# --verbatim: --stable ignores whitespace, which matters in strings and indentation.
|
||||
out = subprocess.run(["git", "patch-id", "--verbatim"], input=diff, capture_output=True, check=True).stdout
|
||||
return out.split()[0].decode() if out.split() else ""
|
||||
|
||||
|
||||
@@ -74,10 +79,20 @@ def get(url, token):
|
||||
return json.load(r)
|
||||
|
||||
|
||||
def passing_run(repo, sha, token):
|
||||
"""URL of a successful pull request ci.yml run of `sha` whose test shards all passed."""
|
||||
def same_pull_request(run, pr, head_repo, head_ref):
|
||||
"""Whether a workflow run belongs to this pull request, not another one with the same head."""
|
||||
numbers = [p.get("number") for p in run.get("pull_requests") or []]
|
||||
if numbers:
|
||||
return pr in numbers
|
||||
return (run.get("head_repository") or {}).get("full_name") == head_repo and run.get("head_branch") == head_ref
|
||||
|
||||
|
||||
def passing_run(repo, sha, token, pr, head_repo, head_ref):
|
||||
"""URL of a successful ci.yml run of this pull request at `sha` whose test shards all passed."""
|
||||
runs = get(f"{API}/repos/{repo}/actions/workflows/ci.yml/runs?head_sha={sha}&event=pull_request&status=success&per_page=10", token)
|
||||
for run in runs.get("workflow_runs", []):
|
||||
if not same_pull_request(run, pr, head_repo, head_ref):
|
||||
continue
|
||||
jobs = get(f"{API}/repos/{repo}/actions/runs/{run['id']}/jobs?per_page=100", token).get("jobs", [])
|
||||
shards = [j for j in jobs if j.get("name", "").startswith(SHARD_PREFIX)]
|
||||
if shards and all(j.get("conclusion") == "success" for j in shards):
|
||||
@@ -90,12 +105,15 @@ def main():
|
||||
ap.add_argument("--base", required=True)
|
||||
ap.add_argument("--head", required=True)
|
||||
ap.add_argument("--repo", required=True)
|
||||
ap.add_argument("--pr", required=True, type=int)
|
||||
ap.add_argument("--head-repo", required=True)
|
||||
ap.add_argument("--head-ref", required=True)
|
||||
a = ap.parse_args()
|
||||
carried = ""
|
||||
try:
|
||||
token = os.environ.get("GH_TOKEN", "")
|
||||
for change in merge_chain(a.base, a.head) if token else []:
|
||||
carried = passing_run(a.repo, change, token)
|
||||
carried = passing_run(a.repo, change, token, a.pr, a.head_repo, a.head_ref)
|
||||
if carried:
|
||||
print(f"{a.head} merges main into {change}, which passed: {carried}", file=sys.stderr)
|
||||
break
|
||||
|
||||
@@ -39,6 +39,18 @@ class Repo:
|
||||
LINES = "".join(f"line {i}\n" for i in range(40))
|
||||
|
||||
|
||||
class SamePullRequest(unittest.TestCase):
|
||||
def test_listed_pull_request_must_match(self):
|
||||
self.assertTrue(main_merge.same_pull_request({"pull_requests": [{"number": 7}]}, 7, "x/y", "b"))
|
||||
self.assertFalse(main_merge.same_pull_request({"pull_requests": [{"number": 8}]}, 7, "x/y", "b"))
|
||||
|
||||
def test_fork_run_matches_by_head_repository_and_branch(self):
|
||||
run = {"pull_requests": [], "head_repository": {"full_name": "fork/solidcraft"}, "head_branch": "fix"}
|
||||
self.assertTrue(main_merge.same_pull_request(run, 7, "fork/solidcraft", "fix"))
|
||||
self.assertFalse(main_merge.same_pull_request(run, 7, "fork/solidcraft", "other"))
|
||||
self.assertFalse(main_merge.same_pull_request(run, 7, "someone/solidcraft", "fix"))
|
||||
|
||||
|
||||
class MergeOfMain(unittest.TestCase):
|
||||
def setUp(self):
|
||||
self.tmp = tempfile.TemporaryDirectory()
|
||||
@@ -98,6 +110,16 @@ class MergeOfMain(unittest.TestCase):
|
||||
head = self.merge_main()
|
||||
self.assertIsNone(main_merge.merge_of_main(base, head))
|
||||
|
||||
def test_whitespace_change_in_the_change_is_not_carried(self):
|
||||
# A conflict resolution that only respaces the change's own line still changes the change.
|
||||
base = self.main_commit(**{"a.txt": LINES.replace("line 5\n", "line 5 on main\n")})
|
||||
subprocess.run(["git", "merge", "-q", "main"], cwd=self.tmp.name, capture_output=True)
|
||||
self.r.write("a.txt", LINES.replace("line 5\n", "line five\n"))
|
||||
self.r.git("add", "-A")
|
||||
self.r.git("commit", "-q", "--no-edit")
|
||||
head = self.r.git("rev-parse", "HEAD")
|
||||
self.assertIsNone(main_merge.merge_of_main(base, head))
|
||||
|
||||
def test_ordinary_commit_is_not_carried(self):
|
||||
head = self.r.commit("more", **{"a.txt": LINES.replace("line 5\n", "line 5b\n")})
|
||||
base = self.r.git("rev-parse", "main")
|
||||
|
||||
Reference in New Issue
Block a user