prune: handle branches that track missing branches
Series of steps:
* Create a local "b1" branch with `repo start b1` that tracks a remote
branch (totally fine)
* Manually create a local "b2" branch with `git branch --track b1 b2`
that tracks the local "b1" (uh-oh...)
* Delete the local "b1" branch manually or via `repo prune` (....)
* Try to process the "b2" branch with `repo prune`
Since b2 tracks a branch that no longer exists, everything blows up
at this point as we try to probe the non-existent ref. Instead, we
should flag this as unknown and leave it up to the user to resolve.
This probably could come up if a local branch was tracking a remote
branch that was deleted from the server, and users ran something like
`repo sync --prune` which cleaned up the remote refs.
Bug: https://crbug.com/gerrit/11485
Change-Id: I6b6b6041943944b8efa6e2ad0b8b10f13a75a5c2
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/236793
Reviewed-by: David Pursehouse <dpursehouse@collab.net>
Reviewed-by: Kirtika Ruchandani <kirtika@google.com>
Reviewed-by: Mike Frysinger <vapier@google.com>
Tested-by: Mike Frysinger <vapier@google.com>
diff --git a/project.py b/project.py
index 5ffa542..7811d6b 100755
--- a/project.py
+++ b/project.py
@@ -134,6 +134,7 @@
class ReviewableBranch(object):
_commit_cache = None
+ _base_exists = None
def __init__(self, project, branch, base):
self.project = project
@@ -147,14 +148,19 @@
@property
def commits(self):
if self._commit_cache is None:
- self._commit_cache = self.project.bare_git.rev_list('--abbrev=8',
- '--abbrev-commit',
- '--pretty=oneline',
- '--reverse',
- '--date-order',
- not_rev(self.base),
- R_HEADS + self.name,
- '--')
+ args = ('--abbrev=8', '--abbrev-commit', '--pretty=oneline', '--reverse',
+ '--date-order', not_rev(self.base), R_HEADS + self.name, '--')
+ try:
+ self._commit_cache = self.project.bare_git.rev_list(*args)
+ except GitError:
+ # We weren't able to probe the commits for this branch. Was it tracking
+ # a branch that no longer exists? If so, return no commits. Otherwise,
+ # rethrow the error as we don't know what's going on.
+ if self.base_exists:
+ raise
+
+ self._commit_cache = []
+
return self._commit_cache
@property
@@ -173,6 +179,23 @@
R_HEADS + self.name,
'--')
+ @property
+ def base_exists(self):
+ """Whether the branch we're tracking exists.
+
+ Normally it should, but sometimes branches we track can get deleted.
+ """
+ if self._base_exists is None:
+ try:
+ self.project.bare_git.rev_parse('--verify', not_rev(self.base))
+ # If we're still here, the base branch exists.
+ self._base_exists = True
+ except GitError:
+ # If we failed to verify, the base branch doesn't exist.
+ self._base_exists = False
+
+ return self._base_exists
+
def UploadForReview(self, people,
auto_topic=False,
draft=False,
diff --git a/subcmds/prune.py b/subcmds/prune.py
index dc8b8c9..ff2fba1 100644
--- a/subcmds/prune.py
+++ b/subcmds/prune.py
@@ -51,11 +51,16 @@
out.project('project %s/' % project.relpath)
out.nl()
- commits = branch.commits
- date = branch.date
- print('%s %-33s (%2d commit%s, %s)' % (
+ print('%s %-33s ' % (
branch.name == project.CurrentBranch and '*' or ' ',
- branch.name,
+ branch.name), end='')
+
+ if not branch.base_exists:
+ print('(ignoring: tracking branch is gone: %s)' % (branch.base,))
+ else:
+ commits = branch.commits
+ date = branch.date
+ print('(%2d commit%s, %s)' % (
len(commits),
len(commits) != 1 and 's' or ' ',
date))
diff --git a/tests/test_project.py b/tests/test_project.py
index 9b289e1..77126df 100644
--- a/tests/test_project.py
+++ b/tests/test_project.py
@@ -18,11 +18,30 @@
from __future__ import print_function
+import contextlib
+import os
+import shutil
+import subprocess
+import tempfile
import unittest
+import git_config
import project
+@contextlib.contextmanager
+def TempGitTree():
+ """Create a new empty git checkout for testing."""
+ # TODO(vapier): Convert this to tempfile.TemporaryDirectory once we drop
+ # Python 2 support entirely.
+ try:
+ tempdir = tempfile.mkdtemp(prefix='repo-tests')
+ subprocess.check_call(['git', 'init'], cwd=tempdir)
+ yield tempdir
+ finally:
+ shutil.rmtree(tempdir)
+
+
class RepoHookShebang(unittest.TestCase):
"""Check shebang parsing in RepoHook."""
@@ -60,3 +79,58 @@
for shebang, interp in DATA:
self.assertEqual(project.RepoHook._ExtractInterpFromShebang(shebang),
interp)
+
+
+class FakeProject(object):
+ """A fake for Project for basic functionality."""
+
+ def __init__(self, worktree):
+ self.worktree = worktree
+ self.gitdir = os.path.join(worktree, '.git')
+ self.name = 'fakeproject'
+ self.work_git = project.Project._GitGetByExec(
+ self, bare=False, gitdir=self.gitdir)
+ self.bare_git = project.Project._GitGetByExec(
+ self, bare=True, gitdir=self.gitdir)
+ self.config = git_config.GitConfig.ForRepository(gitdir=self.gitdir)
+
+
+class ReviewableBranchTests(unittest.TestCase):
+ """Check ReviewableBranch behavior."""
+
+ def test_smoke(self):
+ """A quick run through everything."""
+ with TempGitTree() as tempdir:
+ fakeproj = FakeProject(tempdir)
+
+ # Generate some commits.
+ with open(os.path.join(tempdir, 'readme'), 'w') as fp:
+ fp.write('txt')
+ fakeproj.work_git.add('readme')
+ fakeproj.work_git.commit('-mAdd file')
+ fakeproj.work_git.checkout('-b', 'work')
+ fakeproj.work_git.rm('-f', 'readme')
+ fakeproj.work_git.commit('-mDel file')
+
+ # Start off with the normal details.
+ rb = project.ReviewableBranch(
+ fakeproj, fakeproj.config.GetBranch('work'), 'master')
+ self.assertEqual('work', rb.name)
+ self.assertEqual(1, len(rb.commits))
+ self.assertIn('Del file', rb.commits[0])
+ d = rb.unabbrev_commits
+ self.assertEqual(1, len(d))
+ short, long = next(iter(d.items()))
+ self.assertTrue(long.startswith(short))
+ self.assertTrue(rb.base_exists)
+ # Hard to assert anything useful about this.
+ self.assertTrue(rb.date)
+
+ # Now delete the tracking branch!
+ fakeproj.work_git.branch('-D', 'master')
+ rb = project.ReviewableBranch(
+ fakeproj, fakeproj.config.GetBranch('work'), 'master')
+ self.assertEqual(0, len(rb.commits))
+ self.assertFalse(rb.base_exists)
+ # Hard to assert anything useful about this.
+ self.assertTrue(rb.date)
