diff --git a/docs/local-ci.md b/docs/local-ci.md index 6a0f13a9..eb3c885d 100644 --- a/docs/local-ci.md +++ b/docs/local-ci.md @@ -93,7 +93,7 @@ tools/ci-local-merge.sh # --dry-run to preview - ran on the PR's exact head commit, with a clean tree; - comes from a head that contains the current `origin/main`; -- covers each workflow GitHub would run for the PR's files (their `pull_request` `paths`), with no FAIL, NOT RUN or SKIPPED step in them or in the drift check. Other workflows don't count, so `--only desktop,phone` is enough for a PR that triggers only those two; +- covers each workflow GitHub would run for the PR's files (their `pull_request` `paths`), with no FAIL, NOT RUN or SKIPPED step in them or in the drift check. Other workflows don't count, so `--only desktop,phone` is enough for a PR that triggers only those two. The files come from GitHub's paginated files listing, a renamed file's old path included, so a PR whose diff is too large for `gh pr diff` (generated images, evidence frames) merges the same way; a PR GitHub lists only partly (over 3,000 files) is refused; - binds every `--linux-host` step to that same commit; a remote result for another commit is stale. The `linux-host / sandbox` checks count like the workflows they cover when present, but a run without them is not refused. It also refuses while the newest finished push run of a workflow on `main` has failed (cancelled runs don't count). Pass `--fixes-main` only for the PR that fixes it. Then it posts the summary table as a PR comment ("Local CI passed on `` …") and runs `gh pr merge --admin --merge --match-head-commit `, with the subject `Merge pull request # from /`. `--dry-run` prints the comment instead of posting it, and merges nothing. diff --git a/tools/ci_local_merge.py b/tools/ci_local_merge.py index df321a1f..c348cf87 100644 --- a/tools/ci_local_merge.py +++ b/tools/ci_local_merge.py @@ -53,6 +53,22 @@ def gh_json(argv): return json.loads(run(["gh", *argv]).stdout) +def pr_files(pr): + """The paths pull request `pr` changes, as its workflows' `pull_request` + path filters see them (each file's path, and a renamed file's old path + too), and how many files GitHub listed. From the paginated files + listing: `gh pr diff --name-only` refuses a diff over 20,000 lines, as a + PR that adds generated designs or evidence frames has.""" + out = run(["gh", "api", "--paginate", f"repos/{{owner}}/{{repo}}/pulls/{pr}/files?per_page=100", + "--jq", '.[] | [.filename, (.previous_filename // "")] | @tsv']).stdout + paths, files = [], 0 + for line in out.splitlines(): + if line: + files += 1 + paths.extend(path for path in line.split("\t") if path) + return paths, files + + def evidence_problems(last, head, changed_files): """Why `last` (target/ci-local/last.json) is not evidence for merging `head` touching `changed_files`; [] when it is.""" @@ -164,7 +180,7 @@ def main(argv=None): args = parser.parse_args(argv) try: info = gh_json(["pr", "view", str(args.pr), "--json", - "number,state,isDraft,baseRefName,headRefName,headRefOid,headRepositoryOwner,url"]) + "number,state,isDraft,baseRefName,headRefName,headRefOid,headRepositoryOwner,url,changedFiles"]) if info["state"] != "OPEN": raise Refused(f"PR #{args.pr} is {info['state']}") if info["isDraft"]: @@ -184,7 +200,10 @@ def main(argv=None): if run(["git", "merge-base", "--is-ancestor", main_sha, head], check=False).returncode != 0: raise Refused(f"the head {head[:12]} does not contain origin/main {main_sha[:12]}: rebase or merge " f"main into the branch, push, run tools/ci-local.sh again, then retry") - changed = run(["gh", "pr", "diff", str(args.pr), "--name-only"]).stdout.split() + changed, listed = pr_files(args.pr) + if listed < info.get("changedFiles", 0): + raise Refused(f"GitHub listed {listed} of the {info['changedFiles']} files PR #{args.pr} changes (its " + f"files listing stops at 3,000), so the workflows it triggers are unknown: split the PR") problems, required = evidence_problems(last, head, changed) if problems: raise Refused("the local run is not evidence for this merge:\n - " + "\n - ".join(problems)) diff --git a/tools/test_ci_local.py b/tools/test_ci_local.py index d3f5ffea..76bc38f7 100644 --- a/tools/test_ci_local.py +++ b/tools/test_ci_local.py @@ -296,6 +296,21 @@ def test_the_comment_names_the_commit(self): self.assertIn("| phone.yml / home | Test | PASS |", body) +class PullRequestFiles(unittest.TestCase): + def test_files_come_from_the_paginated_listing_with_renamed_paths(self): + # A diff over 20,000 lines made `gh pr diff --name-only` fail (#473): + # the files listing has no such limit, and a name may hold spaces. + listing = "apps/pdftools/bundle/main.splash\t\napps/x/new name.png\tapps/x/old name.png\n" + done = SimpleNamespace(returncode=0, stdout=listing, stderr="") + with patch.object(merge, "run", return_value=done) as call: + paths, files = merge.pr_files(473) + self.assertEqual(files, 2) + self.assertEqual(paths, ["apps/pdftools/bundle/main.splash", "apps/x/new name.png", "apps/x/old name.png"]) + argv = call.call_args[0][0] + self.assertEqual(argv[:3], ["gh", "api", "--paginate"]) + self.assertIn("repos/{owner}/{repo}/pulls/473/files?per_page=100", argv) + + class NativeRemoteEvidence(unittest.TestCase): def test_unrelated_changes_do_not_query_native_runner(self): with patch.object(merge, "gh_json") as query: