From a155deff4221b4d125858076fb5115b1dbd530c6 Mon Sep 17 00:00:00 2001 From: Mason Freed Date: Tue, 11 Feb 2025 08:48:19 -0800 Subject: [PATCH] Revert "Remove "attempt to squash" behavior from `git rebase-update`" This reverts commit 58625e82c685426d441be5b422c9ad88e4867d20. Reason for revert: Several comments on crbug.com/40264739 that this broke their workflow. I plan to revert this and put patchset #2 back up for review, with an opt-in instead of a change to the default behavior. Original change's description: > Remove "attempt to squash" behavior from `git rebase-update` > > In common workflows, this step only succeeds ~5% of the time, and > in the other 95% of cases, simply adds ~30 seconds of wasted time > to the execution time of `rebase-update`. Therefore, this CL removes > the automated functionality, and leaves squashing as a manual option > to the user. > > Fixed: 40264739 > Bug: 40390274 > Change-Id: Ib2a3ffe4b0e5d0b74323a2a0d34ab0e1b7fafb08 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6104282 > Auto-Submit: Mason Freed > Reviewed-by: Josip Sokcevic > Commit-Queue: Josip Sokcevic Bug: 40390274 Change-Id: I91210a5a21b3751695553433389f346ba443bb1e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6254237 Auto-Submit: Mason Freed Commit-Queue: Yiwei Zhang Reviewed-by: Yiwei Zhang --- git_rebase_update.py | 59 +++++++++++++++++++++++++++++---- man/html/git-rebase-update.html | 6 ++-- man/src/git-rebase-update.txt | 10 +++--- tests/git_rebase_update_test.py | 15 ++------- 4 files changed, 65 insertions(+), 25 deletions(-) diff --git a/git_rebase_update.py b/git_rebase_update.py index 2c99ad4d7..46d16064e 100755 --- a/git_rebase_update.py +++ b/git_rebase_update.py @@ -161,22 +161,68 @@ def rebase_branch(branch, parent, start_hash): if git.hash_one(parent) != start_hash: # Try a plain rebase first print('Rebasing:', branch) - rebase_ret = git.rebase(parent, start_hash, branch, abort=False) + consider_squashing = git.get_num_commits(branch) != 1 + rebase_ret = git.rebase(parent, + start_hash, + branch, + abort=consider_squashing) if not rebase_ret.success: mid_rebase_message = textwrap.dedent("""\ Your working copy is in mid-rebase. Either: * completely resolve like a normal git-rebase; OR - * try squashing your branch first - (git rebase --abort && git squash-branch) and try - again; OR * abort the rebase and mark this branch as dormant: git rebase --abort && \\ git config branch.%s.dormant true And then run `git rebase-update -n` to resume. """ % branch) - print(mid_rebase_message) - return False + if not consider_squashing: + print(mid_rebase_message) + return False + print("Failed! Attempting to squash", branch, "...", end=' ') + sys.stdout.flush() + squash_branch = branch + "_squash_attempt" + git.run('checkout', '-b', squash_branch) + git.squash_current_branch(merge_base=start_hash) + + # Try to rebase the branch_squash_attempt branch to see if it's + # empty. + squash_ret = git.rebase(parent, + start_hash, + squash_branch, + abort=True) + empty_rebase = git.hash_one(squash_branch) == git.hash_one(parent) + git.run('checkout', branch) + git.run('branch', '-D', squash_branch) + if squash_ret.success and empty_rebase: + print('Success!') + git.squash_current_branch(merge_base=start_hash) + git.rebase(parent, start_hash, branch) + else: + print("Failed!") + print() + + # rebase and leave in mid-rebase state. + # This second rebase attempt should always fail in the same + # way that the first one does. If it magically succeeds then + # something very strange has happened. + second_rebase_ret = git.rebase(parent, start_hash, branch) + if second_rebase_ret.success: # pragma: no cover + print("Second rebase succeeded unexpectedly!") + print("Please see: http://crbug.com/425696") + print("First rebased failed with:") + print(rebase_ret.stderr) + else: + print("Here's what git-rebase (squashed) had to say:") + print() + print(squash_ret.stdout) + print(squash_ret.stderr) + print( + textwrap.dedent("""\ + Squashing failed. You probably have a real merge conflict. + """)) + print(mid_rebase_message) + return False else: print('%s up-to-date' % branch) @@ -229,7 +275,6 @@ def main(args=None): '-e', action='store_true', help='Do not automatically delete empty branches.') - opts = parser.parse_args(args) if opts.verbose: # pragma: no cover diff --git a/man/html/git-rebase-update.html b/man/html/git-rebase-update.html index 4c37eefe0..5e01291e0 100644 --- a/man/html/git-rebase-update.html +++ b/man/html/git-rebase-update.html @@ -803,8 +803,10 @@ Rebasing

Things get interesting when there are merge conflicts on rebase. The most common cause for conflicts is when your branch has been committed to the upstream in squashed form, ala git-squash-branch(1), which is what -git-cl(1) and the Commit Queue will do.

-

If it does not apply cleanly, then your original branch will be +git-cl(1) and the Commit Queue will do. Because of that, git +rebase-update will attempt to squash your conflicted branch to see if the +squashed version applies cleanly to its upstream.

+

If it does not apply cleanly, then your original (non-squashed) branch will be left in mid-rebase and git rebase-update will exit. You can deal with this like any other conflicted rebase. When you’re done, just git rebase-update again to pick up where you left off.

diff --git a/man/src/git-rebase-update.txt b/man/src/git-rebase-update.txt index 643de3c06..3f07fdc5e 100644 --- a/man/src/git-rebase-update.txt +++ b/man/src/git-rebase-update.txt @@ -40,11 +40,13 @@ Rebasing:: Things get interesting when there are merge conflicts on rebase. The *most common* cause for conflicts is when your branch has been committed to the upstream in squashed form, ala linkgit:git-squash-branch[1], which is what -linkgit:git-cl[1] and the 'Commit Queue' will do. +linkgit:git-cl[1] and the 'Commit Queue' will do. Because of that, `git +rebase-update` will attempt to squash your conflicted branch to see if the +squashed version applies cleanly to its upstream. + -If it does not apply cleanly, then your original branch will be left in -mid-rebase and `git rebase-update` will exit. You can deal with this like -any other conflicted rebase. When you're done, just `git rebase-update` +If it does not apply cleanly, then your original (non-squashed) branch will be +left in mid-rebase and `git rebase-update` will exit. You can deal with this +like any other conflicted rebase. When you're done, just `git rebase-update` again to pick up where you left off. If you'd like to rebase all rebaseable branches in one pass and manually process diff --git a/tests/git_rebase_update_test.py b/tests/git_rebase_update_test.py index 0fb72d249..a69eb88fb 100755 --- a/tests/git_rebase_update_test.py +++ b/tests/git_rebase_update_test.py @@ -191,7 +191,7 @@ class GitRebaseUpdateTest(git_test_utils.GitRepoReadWriteTestBase): self.repo.git('checkout', 'sub_K') output, _ = self.repo.capture_stdio(self.rp.main, ['foobar']) - self.assertIn('try squashing your branch first', output) + self.assertIn('Squashing failed', output) self.assertTrue(self.repo.run(self.gc.in_rebase)) @@ -257,7 +257,7 @@ class GitRebaseUpdateTest(git_test_utils.GitRepoReadWriteTestBase): self.repo.git('add', 'M') self.repo.git_commit('K NOPE') - # Add a commits to branch_L which would work if squashed + # Add a commits to branch_L which will work when squashed self.repo.git('checkout', 'branch_L') self.repo.git('reset', 'branch_L~') with self.repo.open('L', 'w') as f: @@ -282,16 +282,7 @@ class GitRebaseUpdateTest(git_test_utils.GitRepoReadWriteTestBase): self.repo.git('rebase', '--skip') output, _ = self.repo.capture_stdio(self.reup.main) - self.assertIn('try squashing your branch first', output) - - # manually squash the branch - self.repo.git('rebase', '--abort') - self.repo.git('squash-branch',) - - # Try the rebase again - self.repo.git('rebase', '--skip') - - output, _ = self.repo.capture_stdio(self.reup.main) + self.assertIn('Failed! Attempting to squash', output) self.assertIn('Deleted branch branch_G', output) self.assertIn('Deleted branch branch_L', output) self.assertIn('\'branch_G\' was merged', output)