[llvm][utils] Add a script to use PRs over pushing to main #166473

ilovepi · 2025-11-05T00:07:46Z

As discussed on discourse
https://discourse.llvm.org/t/rfc-require-pull-requests-for-all-llvm-project-commits/88164

This is an attempt at a script to automatically make and land PRs.

It creates a branch (or a series of branches for stacks), and lands them one by one, without waiting for review or checks to pass.

It supports --auto-merge, for single PRs. For stacks, use Graphite or some other tool.

It can work with GitHub tokens for https or ssh keys.

Example:

GITHUB_TOKEN=$(gh auth token) python3 llvm_push_pr.py --upstream-remote origin

As discussed on discourse https://discourse.llvm.org/t/rfc-require-pull-requests-for-all-llvm-project-commits/88164 This is an attempt at a script to automatically make and land PRs. It creates a branch (or a series of branches for stacks), and lands them one by one, without waiting for review or checks to pass. It supports --auto-merge, for single PRs. For stacks, use Graphite or some other tool. It can work with GitHub tokens for https or ssh keys. Example: ```console GITHUB_TOKEN=$(gh auth token) python3 llvm_push_pr.py --upstream-remote origin ```

joker-eph · 2025-11-05T00:39:44Z

llvm/utils/llvm_push_pr.py

+
+    printer = Printer()
+    token = os.getenv("GITHUB_TOKEN")
+    default_prefix = "dev/"


Suggested change

default_prefix = "dev/"

default_prefix = "users/"

seems like a better default to have it work seamlessly with upstream branch restriction

joker-eph · 2025-11-05T00:41:37Z

llvm/utils/llvm_push_pr.py

+        self.printer.print(f"On branch: {self.original_branch}")
+
+        try:
+            self._rebase_current_branch()


Why is this part of the flow?

joker-eph · 2025-11-05T00:43:06Z

llvm/utils/llvm_push_pr.py

+                    "For stacks, the script must merge sequentially.",
+                    file=sys.stderr,
+                )
+                sys.exit(1)


Why is that? When pushing branches to upstream, you can cascade pull-requests to create your stack.

boomanaiden154

I'm assuming you've tested this somewhere?

boomanaiden154 · 2025-11-05T00:27:20Z

llvm/utils/llvm_push_pr.py

+import subprocess
+import sys
+import time
+from typing import List, Optional


Can we add a TODO that we should remove this once we move to Python 3.10 and can move to the native type annotations?

boomanaiden154 · 2025-11-05T00:27:38Z

llvm/utils/llvm_push_pr.py

+import time
+from typing import List, Optional
+
+import requests


Is there a reason to prefer requests (third party library) over the stdlib urllib.request?

boomanaiden154 · 2025-11-05T00:28:55Z

llvm/utils/llvm_push_pr.py

+                if e.stderr:
+                    self.print(f"--- stderr ---\n{e.stderr}", file=sys.stderr)
+                raise e
+            return e


We're returning an exception here when the type is specified as subprocess.CompletedProcess?

boomanaiden154 · 2025-11-05T00:29:50Z

llvm/utils/llvm_push_pr.py

+    def __init__(
+        self, dry_run: bool = False, verbose: bool = False, quiet: bool = False
+    ):
+        """Initializes the Printer with dry_run, verbose, and quiet settings."""


This docstring seems pretty redundant. They're also not required by most style guides (including the Google one) for short functions.

boomanaiden154 · 2025-11-05T00:29:54Z

llvm/utils/llvm_push_pr.py

+        self.quiet = quiet
+
+    def print(self, message: str, file=sys.stdout):
+        """Prints a message to the specified file, respecting quiet mode."""


boomanaiden154 · 2025-11-05T00:32:09Z

llvm/utils/llvm_push_pr.py

+
+    printer = Printer()
+    token = os.getenv("GITHUB_TOKEN")
+    default_prefix = "dev/"


users/ given the monorepo will reject branches without a users/ prefix?

boomanaiden154 · 2025-11-05T00:34:29Z

llvm/utils/llvm_push_pr.py

+        """Wrapper for run_command that passes the dry_run flag."""
+        return self.printer.run_command(command, read_only=read_only, **kwargs)
+
+    def _get_repo_slug(self) -> str:


This duplicates functionality below and should also be constant given we only need to support the monorepo.

boomanaiden154 · 2025-11-05T00:35:25Z

llvm/utils/llvm_push_pr.py

+        self.printer.print(f"On branch: {self.original_branch}")
+
+        try:
+            self._rebase_current_branch()


We don't need to rebase on top of main. Github handles the actual merge for us, and I think not rebasing is one of the selling points of using this script.

boomanaiden154 · 2025-11-05T00:36:46Z

llvm/utils/llvm_push_pr.py

+    def _get_commit_stack(self) -> List[str]:
+        """Gets the stack of commits between the current branch's HEAD and its merge base with upstream."""
+        target = f"{self.args.upstream_remote}/{self.args.base}"
+        merge_base_result = self._run_cmd(


Why do we need to find a merge base here? Why can't we just do a three dot diff between main and the user's branch?

I don't think this handles causes like someone rebasing a bunch of merged commits any more nicely.

boomanaiden154 · 2025-11-05T00:42:42Z

llvm/utils/llvm_push_pr.py

+            self.printer.print("Cleaning up temporary local branches...")
+            self._run_cmd(["git", "branch", "-D"] + self.created_branches)
+            self.printer.print("Cleaning up temporary remote branches...")
+            self._run_cmd(


Will this delete branches that have automerge enabled?

joker-eph · 2025-11-05T00:46:06Z

llvm/utils/llvm_push_pr.py

+
+            self.printer.print(f"Found {len(initial_commits)} commit(s) to process.")
+            branch_base_name = self.original_branch
+            if self.original_branch in ["main", "master"]:


I don't quite get why this check exists?

master isn't a thing in LLVM

I almost never push from main, not sure why the current branch name matters?

joker-eph · 2025-11-05T00:49:44Z

llvm/utils/llvm_push_pr.py

+    def _get_commit_details(self, commit_hash: str) -> tuple[str, str]:
+        """Gets the title and body of a commit."""
+        result = self._run_cmd(
+            ["git", "show", "-s", "--format=%s%n%n%b", commit_hash],


Suggested change

["git", "show", "-s", "--format=%s%n%n%b", commit_hash],

["git", "show", "-s", "--format=%B", commit_hash],

Why not just %B?

joker-eph · 2025-11-05T00:50:19Z

llvm/utils/llvm_push_pr.py

+        )
+        parts = result.stdout.strip().split("\n\n", 1)
+        title = parts[0]
+        body = parts[1] if len(parts) > 1 else ""


What is the body contains multiple paragraphs?

joker-eph · 2025-11-05T00:56:00Z

llvm/utils/llvm_push_pr.py

+        self._run_cmd(["git", "branch", "-f", branch_name, commit_hash])
+        push_command = ["git", "push", self.args.remote, branch_name]


Why are we creating local branches? We can just push a sha directly:

Suggested change

self._run_cmd(["git", "branch", "-f", branch_name, commit_hash])

push_command = ["git", "push", self.args.remote, branch_name]

push_command = ["git", "push", self.args.remote, f"{commit_hash}:refs/head/{branch_name}"]

joker-eph · 2025-11-05T00:58:59Z

llvm/utils/llvm_push_pr.py

+    """Checks if git is installed and if inside a git repository."""
+    printer.print("Checking prerequisites...")
+    printer.run_command(["git", "--version"], capture_output=True, read_only=True)
+    if not os.getenv("GITHUB_TOKEN"):


Can we name it GITHUB_LLVM_TOKEN?
In general folks may want to segregate their tokens to be finer grain, and so having a name that is more specific allows them to put it in a .bashrc or similar without colliding with other token needs.

joker-eph · 2025-11-05T00:59:58Z

llvm/utils/llvm_push_pr.py

+    parser = argparse.ArgumentParser(
+        description="Create and land a stack of Pull Requests."
+    )
+    GITHUB_REMOTE_NAME = "origin"


I'm not sure why "origin" is the best name, instead of "fork" for example?

ilovepi requested review from boomanaiden154, joker-eph and petrhosek November 5, 2025 00:07

joker-eph reviewed Nov 5, 2025

View reviewed changes

boomanaiden154 reviewed Nov 5, 2025

View reviewed changes

joker-eph reviewed Nov 5, 2025

View reviewed changes

	["git", "show", "-s", "--format=%s%n%n%b", commit_hash],
	["git", "show", "-s", "--format=%B", commit_hash],

		self._run_cmd(["git", "branch", "-f", branch_name, commit_hash])
		push_command = ["git", "push", self.args.remote, branch_name]

[llvm][utils] Add a script to use PRs over pushing to main #166473

Are you sure you want to change the base?

[llvm][utils] Add a script to use PRs over pushing to main #166473

Conversation

ilovepi commented Nov 5, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

boomanaiden154 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joker-eph Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

joker-eph Nov 5, 2025 •

edited

Loading