Skip to content

Commit 0ae73db

Browse files
committed
Push multiple branches in the stack atomically: it's faster, plus possible improves CODEOWNERS processing by GitHub
Pull Request: #26 (main)
1 parent d923073 commit 0ae73db

File tree

1 file changed

+105
-44
lines changed

1 file changed

+105
-44
lines changed

git-grok

+105-44
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,16 @@ class Pr:
8686
labels: list[str]
8787

8888

89+
#
90+
# A pair of commit hash and branch name.
91+
#
92+
@dataclass
93+
class Branch:
94+
hash: str
95+
branch: str
96+
push_result: BranchPushResult | None = None
97+
98+
8999
#
90100
# Some data passed from the main process to each individual child process call
91101
# within "git rebase -i", for each commit in the stack.
@@ -127,6 +137,7 @@ class Main:
127137
login: str
128138
remote: str
129139
remote_base_branch: str = "master"
140+
in_rebase_interactive: InRebaseInteractiveData | None = None
130141

131142
#
132143
# Main entry point.
@@ -151,14 +162,14 @@ class Main:
151162
)
152163
args = parser.parse_args()
153164

154-
in_rebase_interactive = InRebaseInteractiveData.parse(
165+
self.in_rebase_interactive = InRebaseInteractiveData.parse(
155166
os.environ.get(INTERNAL_IN_REBASE_INTERACTIVE_VAR, "")
156167
)
157168

158169
self.debug = args.debug
159170
self.debug_force_push_branches = args.debug_force_push_branches
160171

161-
if not in_rebase_interactive:
172+
if not self.in_rebase_interactive:
162173
self.gh_verify_version()
163174
self.git_verify_version()
164175
self.self_update()
@@ -179,10 +190,10 @@ class Main:
179190
"git_get_current_remote_base_branch",
180191
self.git_get_current_remote_base_branch,
181192
)
182-
if not in_rebase_interactive:
193+
if not self.in_rebase_interactive:
183194
self.run_all()
184195
else:
185-
self.run_in_rebase_interactive(data=in_rebase_interactive)
196+
self.run_in_rebase_interactive(data=self.in_rebase_interactive)
186197

187198
#
188199
# Assuming all PRs in the stack already have PR URLs in their description,
@@ -227,6 +238,8 @@ class Main:
227238
commit_with_no_url = commit
228239
break
229240
else:
241+
# Push this branch and see whether GitHub says whether it
242+
# was up to date or not.
230243
commit_hashes_to_push_branch.append(commit.hash)
231244

232245
# Some commits have no related PRs (no GitHub URLs in the message)?
@@ -270,23 +283,36 @@ class Main:
270283

271284
self.print_header(f"Processing commit: {self.clean_title(commit.title)}")
272285

286+
to_push: list[Branch] = []
287+
273288
if prev_commit.hash != remote_commit.hash:
274-
prev_commit, result = self.process_commit_push_branch(commit=prev_commit)
289+
prev_commit.branch = self.process_commit_infer_branch(commit=prev_commit)
290+
to_push.append(Branch(hash=prev_commit.hash, branch=prev_commit.branch))
291+
else:
292+
prev_commit.branch = None
293+
294+
commit.branch = self.process_commit_infer_branch(commit=commit)
295+
to_push.append(Branch(hash=commit.hash, branch=commit.branch))
296+
297+
push_results = self.git_push_branches(branches=to_push)
298+
299+
if prev_commit.branch:
275300
self.print_branch_result(
276301
type="base",
277-
branch=str(prev_commit.branch),
278-
result=result,
302+
branch=prev_commit.branch,
303+
result=push_results[prev_commit.branch],
279304
)
280305
else:
281306
self.print_branch_result(
282307
type="base",
283308
branch=self.remote_base_branch,
284309
result="up-to-date",
285310
)
286-
prev_commit.branch = None
287-
288-
commit, result = self.process_commit_push_branch(commit=commit)
289-
self.print_branch_result(type="head", branch=str(commit.branch), result=result)
311+
self.print_branch_result(
312+
type="head",
313+
branch=commit.branch,
314+
result=push_results[commit.branch],
315+
)
290316

291317
new_pr_title = commit.title
292318
new_pr_body = None
@@ -342,11 +368,11 @@ class Main:
342368
commits: list[Commit],
343369
prs_by_url: dict[str, Pr],
344370
):
345-
commits_chronological = list(reversed(commits))
346-
for i, commit in list(enumerate(commits_chronological))[1:]:
371+
commits_old_to_new = list(reversed(commits))
372+
for i, commit in list(enumerate(commits_old_to_new))[1:]:
347373
pr = prs_by_url.get(commit.url, None) if commit.url else None
348374
if pr and pr.auto_merge_status == "ENABLED":
349-
prev_commit = commits_chronological[i - 1]
375+
prev_commit = commits_old_to_new[i - 1]
350376
prev_pr = (
351377
prs_by_url.get(prev_commit.url, None) if prev_commit.url else None
352378
)
@@ -426,25 +452,35 @@ class Main:
426452
):
427453
# We must iterate from the oldest commit to the newest one, because
428454
# previous commit PR's branch becomes the next commit PR's base branch.
429-
commits_chronological = list(reversed(commits))
430-
for i, commit in enumerate(commits_chronological):
455+
commits_old_to_new = list(reversed(commits))
456+
457+
# Push all branches atomically, in bulk. This is meant to prevent
458+
# useless CODEOWNERS reviewers addition in case the commits were
459+
# reordered or rebased, and is faster in general.
460+
to_push: list[Branch] = []
461+
for commit in commits_old_to_new:
462+
if (
463+
self.debug_force_push_branches
464+
or commit.hash in commit_hashes_to_push_branch
465+
):
466+
commit.branch = self.process_commit_infer_branch(commit=commit)
467+
to_push.append(Branch(hash=commit.hash, branch=commit.branch))
468+
push_results = self.git_push_branches(branches=to_push)
469+
470+
for i, commit in enumerate(commits_old_to_new):
431471
self.print_header(f"Updating PR: {self.clean_title(commit.title)}")
432472

433-
if commit.hash in commit_hashes_to_push_branch:
434-
commit, result = self.process_commit_push_branch(commit=commit)
473+
if commit.branch in push_results:
474+
result = push_results[commit.branch]
435475
if result == "pushed":
436476
self.print_branch_result(
437477
type="head",
438-
branch=str(commit.branch),
478+
branch=commit.branch,
439479
result=result,
440480
)
441-
commits_chronological[i] = commit
442481

443-
assert (
444-
commit.url is not None
445-
), f"commit {commit.hash} PR url is expected to be in the message at this point"
446482
pr, result = self.process_update_pr(
447-
prev_commit=commits_chronological[i - 1] if i > 0 else None,
483+
prev_commit=commits_old_to_new[i - 1] if i > 0 else None,
448484
commit=commit,
449485
commits=commits,
450486
)
@@ -453,27 +489,26 @@ class Main:
453489
result=result,
454490
review_decision=pr.review_decision,
455491
)
456-
commits_chronological[i].branch = pr.head_branch
492+
commits_old_to_new[i].branch = pr.head_branch
457493

458494
#
459-
# Pushes an existing branch (it we know this commit's PR URL by querying
460-
# GitHub), or creates a new branch based on commit title and pushes it.
495+
# For a commit, infers its corresponding remote branch name by either
496+
# querying it from the PR (when commit.url is set), or by building it from
497+
# the commit title and hash.
461498
#
462-
def process_commit_push_branch(
499+
def process_commit_infer_branch(
463500
self,
464501
*,
465502
commit: Commit,
466-
) -> tuple[Commit, BranchPushResult]:
503+
) -> str:
467504
if commit.url:
468-
pr = self.gh_get_pr(url=commit.url)
469-
commit.branch = pr.head_branch
505+
pr = self.gh_get_pr(url=commit.url) # likely a cache hit
506+
return pr.head_branch
470507
else:
471-
commit.branch = self.build_branch_name(
508+
return self.build_branch_name(
472509
title=commit.title,
473510
commit_hash=commit.hash,
474511
)
475-
pushed = self.git_push_branch(branch=commit.branch, hash=commit.hash)
476-
return commit, pushed
477512

478513
#
479514
# Updates PR fields:
@@ -864,9 +899,16 @@ class Main:
864899
return commits
865900

866901
#
867-
# Pushes a branch to remote GitHub.
902+
# Pushes multiple branches atomically to remote GitHub. Returns a dict
903+
# mapping branch names to their push results.
868904
#
869-
def git_push_branch(self, *, branch: str, hash: str) -> BranchPushResult:
905+
def git_push_branches(
906+
self,
907+
*,
908+
branches: list[Branch],
909+
) -> dict[str, BranchPushResult]:
910+
if not branches:
911+
return {}
870912
# Git push is a quick no-op on GitHub end if the branch isn't changed
871913
# (it prints "Everything up-to-date"), so we always push and then verify
872914
# the output for the status (instead of fetching from the remote and
@@ -877,15 +919,26 @@ class Main:
877919
"push",
878920
"-f",
879921
self.remote,
880-
f"{hash}:refs/heads/{branch}",
922+
*[f"{branch.hash}:refs/heads/{branch.branch}" for branch in branches],
881923
],
882924
stderr_to_stdout=True,
883925
)
884-
return (
885-
"up-to-date"
886-
if re.match(r"^[^\n]+up-to-date", out, flags=re.S)
887-
else "pushed"
888-
)
926+
# If the hash is NOT mentioned in the output, it's either a short
927+
# "Everything up-to-date" message (which means that ALL branches are
928+
# unchanged), or THIS particular branch is up-to-date. I.e. if a branch
929+
# is changed, git always prints its hash in the output, on one of the
930+
# following formats:
931+
# 1. * [new branch] 10dc4f6 -> grok/...
932+
# 2. + 10dc4f6...b28d03e 10dc4f6 -> grok/... (forced update)
933+
results: dict[str, BranchPushResult] = {
934+
branch.branch: "up-to-date" for branch in branches
935+
}
936+
for branch in branches:
937+
for line in out.splitlines():
938+
if branch.hash in line:
939+
results[branch.branch] = "pushed"
940+
break
941+
return results
889942

890943
#
891944
# Runs an interactive rebase with the provided shell command.
@@ -1147,7 +1200,7 @@ class Main:
11471200
# Prints a status after a commit message was updated locally.
11481201
#
11491202
def print_commit_message_updated(self):
1150-
print(f" ── updated commit message")
1203+
print(f" ── added PR URL to commit's message to bind them")
11511204
sys.stdout.flush()
11521205

11531206
#
@@ -1282,7 +1335,15 @@ class Main:
12821335
def debug_log_text(self, *, text: str):
12831336
try:
12841337
with open(DEBUG_FILE, "a+") as file:
1285-
file.write(f"=== {datetime.now()}\n")
1338+
file.write(
1339+
f"=== {datetime.now()}"
1340+
+ (
1341+
f" (in rebase interactive, {self.in_rebase_interactive.commit_index_one_based}/{self.in_rebase_interactive.total_commits_in_stack})"
1342+
if self.in_rebase_interactive
1343+
else ""
1344+
)
1345+
+ "\n"
1346+
)
12861347
file.write(f"{text.rstrip()}\n")
12871348
file.write("\n")
12881349
except:

0 commit comments

Comments
 (0)