From d421f6a47708871fbbd08d7cec6d966ad610453d Mon Sep 17 00:00:00 2001 From: "sdefresne@chromium.org" Date: Wed, 2 Dec 2015 18:13:54 +0000 Subject: [PATCH] Fix "git rebase-update" when multiple branch are stale. Fix a bug that left branches tracking a dead branch if both their parent and grand-parent were left with no changes after a "rebase-update" step. Given the following initial state: $ git map-branches -v origin/master a b c * [ ahead 1 ] without this patch, a "git rebase-update" on this tree state would leave the branch "c" as tracking a non-existing branch "a": $ git recursive-rebase a up-to-date b up-to-date c up-to-date Reparented c to track a (was tracking b) Deleted branch b (was 448d1da). Deleted branch a (was 448d1da). $ git map-branches -v {a:GONE} c * with the patch, we record that the branch "c" is tracking must be updated twice and we end up in a state were "c" is correctly tracking "origin/master": $ git recursive-rebase a up-to-date b up-to-date c up-to-date Reparented c to track origin/master (was tracking b) Deleted branch b (was 448d1da). Deleted branch a (was 448d1da). $ git map-branches -v origin/master c * [ ahead 1 ] BUG=456806 Review URL: https://codereview.chromium.org/1482753002 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@297792 0039d316-1c4b-4281-b951-d872f2087c98 --- git_rebase_update.py | 44 ++++++++++++++++++++++++--------- tests/git_rebase_update_test.py | 26 +++++++++++++++---- 2 files changed, 54 insertions(+), 16 deletions(-) diff --git a/git_rebase_update.py b/git_rebase_update.py index 992bb195db..9d5755e9f0 100755 --- a/git_rebase_update.py +++ b/git_rebase_update.py @@ -87,32 +87,54 @@ def remove_empty_branches(branch_tree): tag_set = git.tags() ensure_root_checkout = git.once(lambda: git.run('checkout', git.root())) - deletions = set() + deletions = {} + reparents = {} downstreams = collections.defaultdict(list) for branch, parent in git.topo_iter(branch_tree, top_down=False): downstreams[parent].append(branch) + # If branch and parent have the same state, then branch has to be marked + # for deletion and its children and grand-children reparented to parent. if git.hash_one(branch) == git.hash_one(parent): ensure_root_checkout() logging.debug('branch %s merged to %s', branch, parent) + # Mark branch for deletion while remembering the ordering, then add all + # its children as grand-children of its parent and record reparenting + # information if necessary. + deletions[branch] = len(deletions) + for down in downstreams[branch]: if down in deletions: continue - if parent in tag_set: - git.set_branch_config(down, 'remote', '.') - git.set_branch_config(down, 'merge', 'refs/tags/%s' % parent) - print ('Reparented %s to track %s [tag] (was tracking %s)' - % (down, parent, branch)) + # Record the new and old parent for down, or update such a record + # if it already exists. Keep track of the ordering so that reparenting + # happen in topological order. + downstreams[parent].append(down) + if down not in reparents: + reparents[down] = (len(reparents), parent, branch) else: - git.run('branch', '--set-upstream-to', parent, down) - print ('Reparented %s to track %s (was tracking %s)' - % (down, parent, branch)) + order, _, old_parent = reparents[down] + reparents[down] = (order, parent, old_parent) + + # Apply all reparenting recorded, in order. + for branch, value in sorted(reparents.iteritems(), key=lambda x:x[1][0]): + _, parent, old_parent = value + if parent in tag_set: + git.set_branch_config(branch, 'remote', '.') + git.set_branch_config(branch, 'merge', 'refs/tags/%s' % parent) + print ('Reparented %s to track %s [tag] (was tracking %s)' + % (branch, parent, old_parent)) + else: + git.run('branch', '--set-upstream-to', parent, branch) + print ('Reparented %s to track %s (was tracking %s)' + % (branch, parent, old_parent)) - deletions.add(branch) - print git.run('branch', '-d', branch) + # Apply all deletions recorded, in order. + for branch, _ in sorted(deletions.iteritems(), key=lambda x: x[1]): + print git.run('branch', '-d', branch) def rebase_branch(branch, parent, start_hash): diff --git a/tests/git_rebase_update_test.py b/tests/git_rebase_update_test.py index 273e922048..976d9a6d08 100755 --- a/tests/git_rebase_update_test.py +++ b/tests/git_rebase_update_test.py @@ -84,6 +84,14 @@ class GitRebaseUpdateTest(git_test_utils.GitRepoReadWriteTestBase): f.write('totes the Foobar file') self.repo.git_commit('foobar2') + self.repo.run(self.nb.main, ['--upstream-current', 'int1_foobar']) + self.repo.run(self.nb.main, ['--upstream-current', 'int2_foobar']) + self.repo.run(self.nb.main, ['--upstream-current', 'sub_foobar']) + with self.repo.open('foobar', 'w') as f: + f.write('some more foobaring') + self.repo.git('add', 'foobar') + self.repo.git_commit('foobar3') + self.repo.git('checkout', 'branch_K') self.repo.run(self.nb.main, ['--upstream-current', 'sub_K']) with self.repo.open('K', 'w') as f: @@ -103,7 +111,7 @@ class GitRebaseUpdateTest(git_test_utils.GitRepoReadWriteTestBase): self.assertSchema(""" A B H I J K sub_K J L - B C D E foobar1 foobar2 + B C D E foobar1 foobar2 foobar3 E F G A old_file """) @@ -131,12 +139,15 @@ class GitRebaseUpdateTest(git_test_utils.GitRepoReadWriteTestBase): self.assertIn('Deleted branch branch_G', output) self.assertIn('Deleted branch empty_branch', output) self.assertIn('Deleted branch empty_branch2', output) + self.assertIn('Deleted branch int1_foobar', output) + self.assertIn('Deleted branch int2_foobar', output) self.assertIn('Reparented branch_K to track origin/master', output) + self.assertIn('Reparented sub_foobar to track foobar', output) self.assertSchema(""" A B C D E F G M N O H I J K sub_K K L - O foobar1 foobar2 + O foobar1 foobar2 foobar3 A old_file """) @@ -167,6 +178,7 @@ class GitRebaseUpdateTest(git_test_utils.GitRepoReadWriteTestBase): self.assertSchema(""" A B C D E F G M N O foobar1 foobar2 H I J K L + foobar2 foobar3 K sub_K A old_file """) @@ -182,6 +194,7 @@ class GitRebaseUpdateTest(git_test_utils.GitRepoReadWriteTestBase): self.assertSchema(""" A B C D E F G M N O foobar1 foobar2 H I J K L + foobar2 foobar3 A old_file K sub_K """) @@ -190,14 +203,16 @@ class GitRebaseUpdateTest(git_test_utils.GitRepoReadWriteTestBase): branches = self.repo.run(set, self.gc.branches()) self.assertEqual(branches, {'branch_K', 'master', 'sub_K', 'root_A', - 'branch_L', 'old_branch', 'foobar'}) + 'branch_L', 'old_branch', 'foobar', + 'sub_foobar'}) self.repo.git('checkout', 'branch_K') self.repo.run(self.mv.main, ['special_K']) branches = self.repo.run(set, self.gc.branches()) self.assertEqual(branches, {'special_K', 'master', 'sub_K', 'root_A', - 'branch_L', 'old_branch', 'foobar'}) + 'branch_L', 'old_branch', 'foobar', + 'sub_foobar'}) self.repo.git('checkout', 'origin/master') _, err = self.repo.capture_stdio(self.mv.main, ['special_K', 'cool branch']) @@ -207,7 +222,8 @@ class GitRebaseUpdateTest(git_test_utils.GitRepoReadWriteTestBase): branches = self.repo.run(set, self.gc.branches()) # This check fails with git 2.4 (see crbug.com/487172) self.assertEqual(branches, {'cool_branch', 'master', 'sub_K', 'root_A', - 'branch_L', 'old_branch', 'foobar'}) + 'branch_L', 'old_branch', 'foobar', + 'sub_foobar'}) _, branch_tree = self.repo.run(self.gc.get_branch_tree) self.assertEqual(branch_tree['sub_K'], 'foobar')