Skip to content

Commit 6106f58

Browse files
committed
Revert "Push multiple branches in the stack atomically: it's faster, plus possible improves CODEOWNERS processing by GitHub"
This reverts commit fb9c1b1.
1 parent fb9c1b1 commit 6106f58

File tree

1 file changed

+39
-89
lines changed

1 file changed

+39
-89
lines changed

git-grok

Lines changed: 39 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -86,16 +86,6 @@ 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-
9989
#
10090
# Some data passed from the main process to each individual child process call
10191
# within "git rebase -i", for each commit in the stack.
@@ -237,8 +227,6 @@ class Main:
237227
commit_with_no_url = commit
238228
break
239229
else:
240-
# Push this branch and see whether GitHub says whether it
241-
# was up to date or not.
242230
commit_hashes_to_push_branch.append(commit.hash)
243231

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

283271
self.print_header(f"Processing commit: {self.clean_title(commit.title)}")
284272

285-
to_push: list[Branch] = []
286-
287273
if prev_commit.hash != remote_commit.hash:
288-
prev_commit.branch = self.process_commit_infer_branch(commit=prev_commit)
289-
to_push.append(Branch(hash=prev_commit.hash, branch=prev_commit.branch))
290-
else:
291-
prev_commit.branch = None
292-
293-
commit.branch = self.process_commit_infer_branch(commit=commit)
294-
to_push.append(Branch(hash=commit.hash, branch=commit.branch))
295-
296-
push_results = self.git_push_branches(branches=to_push)
297-
298-
if prev_commit.branch:
274+
prev_commit, result = self.process_commit_push_branch(commit=prev_commit)
299275
self.print_branch_result(
300276
type="base",
301-
branch=prev_commit.branch,
302-
result=push_results[prev_commit.branch],
277+
branch=str(prev_commit.branch),
278+
result=result,
303279
)
304280
else:
305281
self.print_branch_result(
306282
type="base",
307283
branch=self.remote_base_branch,
308284
result="up-to-date",
309285
)
310-
self.print_branch_result(
311-
type="head",
312-
branch=commit.branch,
313-
result=push_results[commit.branch],
314-
)
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)
315290

316291
new_pr_title = commit.title
317292
new_pr_body = None
@@ -367,11 +342,11 @@ class Main:
367342
commits: list[Commit],
368343
prs_by_url: dict[str, Pr],
369344
):
370-
commits_old_to_new = list(reversed(commits))
371-
for i, commit in list(enumerate(commits_old_to_new))[1:]:
345+
commits_chronological = list(reversed(commits))
346+
for i, commit in list(enumerate(commits_chronological))[1:]:
372347
pr = prs_by_url.get(commit.url, None) if commit.url else None
373348
if pr and pr.auto_merge_status == "ENABLED":
374-
prev_commit = commits_old_to_new[i - 1]
349+
prev_commit = commits_chronological[i - 1]
375350
prev_pr = (
376351
prs_by_url.get(prev_commit.url, None) if prev_commit.url else None
377352
)
@@ -451,35 +426,25 @@ class Main:
451426
):
452427
# We must iterate from the oldest commit to the newest one, because
453428
# previous commit PR's branch becomes the next commit PR's base branch.
454-
commits_old_to_new = list(reversed(commits))
455-
456-
# Push all branches atomically, in bulk. This is meant to prevent
457-
# useless CODEOWNERS reviewers addition in case the commits were
458-
# reordered or rebased, and is faster in general.
459-
to_push: list[Branch] = []
460-
for commit in commits_old_to_new:
461-
if (
462-
self.debug_force_push_branches
463-
or commit.hash in commit_hashes_to_push_branch
464-
):
465-
commit.branch = self.process_commit_infer_branch(commit=commit)
466-
to_push.append(Branch(hash=commit.hash, branch=commit.branch))
467-
push_results = self.git_push_branches(branches=to_push)
468-
469-
for i, commit in enumerate(commits_old_to_new):
429+
commits_chronological = list(reversed(commits))
430+
for i, commit in enumerate(commits_chronological):
470431
self.print_header(f"Updating PR: {self.clean_title(commit.title)}")
471432

472-
if commit.branch in push_results:
473-
result = push_results[commit.branch]
433+
if commit.hash in commit_hashes_to_push_branch:
434+
commit, result = self.process_commit_push_branch(commit=commit)
474435
if result == "pushed":
475436
self.print_branch_result(
476437
type="head",
477-
branch=commit.branch,
438+
branch=str(commit.branch),
478439
result=result,
479440
)
441+
commits_chronological[i] = commit
480442

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"
481446
pr, result = self.process_update_pr(
482-
prev_commit=commits_old_to_new[i - 1] if i > 0 else None,
447+
prev_commit=commits_chronological[i - 1] if i > 0 else None,
483448
commit=commit,
484449
commits=commits,
485450
)
@@ -488,26 +453,27 @@ class Main:
488453
result=result,
489454
review_decision=pr.review_decision,
490455
)
491-
commits_old_to_new[i].branch = pr.head_branch
456+
commits_chronological[i].branch = pr.head_branch
492457

493458
#
494-
# For a commit, infers its corresponding remote branch name by either
495-
# querying it from the PR (when commit.url is set), or by building it from
496-
# the commit title and hash.
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.
497461
#
498-
def process_commit_infer_branch(
462+
def process_commit_push_branch(
499463
self,
500464
*,
501465
commit: Commit,
502-
) -> str:
466+
) -> tuple[Commit, BranchPushResult]:
503467
if commit.url:
504-
pr = self.gh_get_pr(url=commit.url) # likely a cache hit
505-
return pr.head_branch
468+
pr = self.gh_get_pr(url=commit.url)
469+
commit.branch = pr.head_branch
506470
else:
507-
return self.build_branch_name(
471+
commit.branch = self.build_branch_name(
508472
title=commit.title,
509473
commit_hash=commit.hash,
510474
)
475+
pushed = self.git_push_branch(branch=commit.branch, hash=commit.hash)
476+
return commit, pushed
511477

512478
#
513479
# Updates PR fields:
@@ -898,14 +864,9 @@ class Main:
898864
return commits
899865

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

940890
#
941891
# Runs an interactive rebase with the provided shell command.
@@ -1197,7 +1147,7 @@ class Main:
11971147
# Prints a status after a commit message was updated locally.
11981148
#
11991149
def print_commit_message_updated(self):
1200-
print(f" ── added PR URL to commit's message to bind them")
1150+
print(f" ── updated commit message")
12011151
sys.stdout.flush()
12021152

12031153
#

0 commit comments

Comments
 (0)