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>
This commit is contained in:
Mike Frysinger 2019-09-11 18:43:17 -04:00
parent 2ba5a1e963
commit 6da17751ca
3 changed files with 114 additions and 12 deletions

View File

@ -134,6 +134,7 @@ class DownloadedChange(object):
class ReviewableBranch(object): class ReviewableBranch(object):
_commit_cache = None _commit_cache = None
_base_exists = None
def __init__(self, project, branch, base): def __init__(self, project, branch, base):
self.project = project self.project = project
@ -147,14 +148,19 @@ class ReviewableBranch(object):
@property @property
def commits(self): def commits(self):
if self._commit_cache is None: if self._commit_cache is None:
self._commit_cache = self.project.bare_git.rev_list('--abbrev=8', args = ('--abbrev=8', '--abbrev-commit', '--pretty=oneline', '--reverse',
'--abbrev-commit', '--date-order', not_rev(self.base), R_HEADS + self.name, '--')
'--pretty=oneline', try:
'--reverse', self._commit_cache = self.project.bare_git.rev_list(*args)
'--date-order', except GitError:
not_rev(self.base), # We weren't able to probe the commits for this branch. Was it tracking
R_HEADS + self.name, # 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 return self._commit_cache
@property @property
@ -173,6 +179,23 @@ class ReviewableBranch(object):
R_HEADS + self.name, 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, def UploadForReview(self, people,
auto_topic=False, auto_topic=False,
draft=False, draft=False,

View File

@ -51,11 +51,16 @@ class Prune(PagedCommand):
out.project('project %s/' % project.relpath) out.project('project %s/' % project.relpath)
out.nl() out.nl()
print('%s %-33s ' % (
branch.name == project.CurrentBranch and '*' or ' ',
branch.name), end='')
if not branch.base_exists:
print('(ignoring: tracking branch is gone: %s)' % (branch.base,))
else:
commits = branch.commits commits = branch.commits
date = branch.date date = branch.date
print('%s %-33s (%2d commit%s, %s)' % ( print('(%2d commit%s, %s)' % (
branch.name == project.CurrentBranch and '*' or ' ',
branch.name,
len(commits), len(commits),
len(commits) != 1 and 's' or ' ', len(commits) != 1 and 's' or ' ',
date)) date))

View File

@ -18,11 +18,30 @@
from __future__ import print_function from __future__ import print_function
import contextlib
import os
import shutil
import subprocess
import tempfile
import unittest import unittest
import git_config
import project 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): class RepoHookShebang(unittest.TestCase):
"""Check shebang parsing in RepoHook.""" """Check shebang parsing in RepoHook."""
@ -60,3 +79,58 @@ class RepoHookShebang(unittest.TestCase):
for shebang, interp in DATA: for shebang, interp in DATA:
self.assertEqual(project.RepoHook._ExtractInterpFromShebang(shebang), self.assertEqual(project.RepoHook._ExtractInterpFromShebang(shebang),
interp) 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)