init: allow REPO_REV/--repo-rev to specify commits/tags

While the help/usage suggested that revisions would work, they never
actually did, and just throw confusing errors.  Now that we warn if
the checkout isn't tracking a branch, allow people to specify commits
or tags explicitly.  Hopefully our nags will be sufficient to keep
most people on the right path.

Bug: https://crbug.com/gerrit/11045
Change-Id: I6ea32c677912185f55ab20faaa23c6c0a4c483b3
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/259492
Tested-by: Mike Frysinger <vapier@google.com>
Reviewed-by: Jonathan Nieder <jrn@google.com>

repo

@@ -475,13 +475,7 @@ def _Init(args, gitc_init=False):
   opt.verbose = opt.output_mode is True
 
   url = opt.repo_url or REPO_URL
-  branch = opt.repo_rev or REPO_REV
+  rev = opt.repo_rev or REPO_REV
-
-  if branch.startswith('refs/heads/'):
-    branch = branch[len('refs/heads/'):]
-  if branch.startswith('refs/'):
-    print("fatal: invalid branch name '%s'" % branch, file=sys.stderr)
-    raise CloneFailure()
 
   try:
     if gitc_init:
@@ -532,12 +526,15 @@ def _Init(args, gitc_init=False):
     dst = os.path.abspath(os.path.join(repodir, S_repo))
     _Clone(url, dst, opt.clone_bundle, opt.quiet, opt.verbose)
 
+    remote_ref, local_rev = resolve_repo_rev(dst, rev)
+    if not opt.quiet and not remote_ref.startswith('refs/heads/'):
+      print('warning: repo is not tracking a remote branch, so it will not '
+            'receive updates', file=sys.stderr)
     if do_verify:
-      rev = _Verify(dst, branch, opt.quiet)
+      rev = _Verify(dst, remote_ref, local_rev, opt.quiet)
     else:
-      rev = 'refs/remotes/origin/%s^0' % branch
+      rev = local_rev
-
+    _Checkout(dst, remote_ref, rev, opt.quiet)
-    _Checkout(dst, branch, rev, opt.quiet)
 
     if not os.path.isfile(os.path.join(dst, 'repo')):
       print("warning: '%s' does not look like a git-repo repository, is "
@@ -845,23 +842,83 @@ def _Clone(url, cwd, clone_bundle, quiet, verbose):
   _Fetch(url, cwd, 'origin', quiet, verbose)
 
 
-def _Verify(cwd, branch, quiet):
+def resolve_repo_rev(cwd, committish):
-  """Verify the branch has been signed by a tag.
+  """Figure out what REPO_REV represents.
+
+  We support:
+  * refs/heads/xxx: Branch.
+  * refs/tags/xxx: Tag.
+  * xxx: Branch or tag or commit.
+
+  Args:
+    cwd: The git checkout to run in.
+    committish: The REPO_REV argument to resolve.
+
+  Returns:
+    A tuple of (remote ref, commit) as makes sense for the committish.
+    For branches, this will look like ('refs/heads/stable', <revision>).
+    For tags, this will look like ('refs/tags/v1.0', <revision>).
+    For commits, this will be (<revision>, <revision>).
   """
-  try:
+  def resolve(committish):
-    ret = run_git('describe', 'origin/%s' % branch, cwd=cwd)
+    ret = run_git('rev-parse', '--verify', '%s^{commit}' % (committish,),
-    cur = ret.stdout.strip()
+                  cwd=cwd, check=False)
-  except CloneFailure:
+    return None if ret.returncode else ret.stdout.strip()
-    print("fatal: branch '%s' has not been signed" % branch, file=sys.stderr)
+
-    raise
+  # An explicit branch.
+  if committish.startswith('refs/heads/'):
+    remote_ref = committish
+    committish = committish[len('refs/heads/'):]
+    rev = resolve('refs/remotes/origin/%s' % committish)
+    if rev is None:
+      print('repo: error: unknown branch "%s"' % (committish,),
+            file=sys.stderr)
+      raise CloneFailure()
+    return (remote_ref, rev)
+
+  # An explicit tag.
+  if committish.startswith('refs/tags/'):
+    remote_ref = committish
+    committish = committish[len('refs/tags/'):]
+    rev = resolve(remote_ref)
+    if rev is None:
+      print('repo: error: unknown tag "%s"' % (committish,),
+            file=sys.stderr)
+      raise CloneFailure()
+    return (remote_ref, rev)
+
+  # See if it's a short branch name.
+  rev = resolve('refs/remotes/origin/%s' % committish)
+  if rev:
+    return ('refs/heads/%s' % (committish,), rev)
+
+  # See if it's a tag.
+  rev = resolve('refs/tags/%s' % committish)
+  if rev:
+    return ('refs/tags/%s' % (committish,), rev)
+
+  # See if it's a commit.
+  rev = resolve(committish)
+  if rev and rev.lower().startswith(committish.lower()):
+    return (rev, rev)
+
+  # Give up!
+  print('repo: error: unable to resolve "%s"' % (committish,), file=sys.stderr)
+  raise CloneFailure()
+
+
+def _Verify(cwd, remote_ref, rev, quiet):
+  """Verify the commit has been signed by a tag."""
+  ret = run_git('describe', rev, cwd=cwd)
+  cur = ret.stdout.strip()
 
   m = re.compile(r'^(.*)-[0-9]{1,}-g[0-9a-f]{1,}$').match(cur)
   if m:
     cur = m.group(1)
     if not quiet:
       print(file=sys.stderr)
-      print("info: Ignoring branch '%s'; using tagged release '%s'"
+      print("warning: '%s' is not signed; falling back to signed release '%s'"
-            % (branch, cur), file=sys.stderr)
+            % (remote_ref, cur), file=sys.stderr)
       print(file=sys.stderr)
 
   env = os.environ.copy()
@@ -870,13 +927,13 @@ def _Verify(cwd, branch, quiet):
   return '%s^0' % cur
 
 
-def _Checkout(cwd, branch, rev, quiet):
+def _Checkout(cwd, remote_ref, rev, quiet):
   """Checkout an upstream branch into the repository and track it.
   """
   run_git('update-ref', 'refs/heads/default', rev, cwd=cwd)
 
   _SetConfig(cwd, 'branch.default.remote', 'origin')
-  _SetConfig(cwd, 'branch.default.merge', 'refs/heads/%s' % branch)
+  _SetConfig(cwd, 'branch.default.merge', remote_ref)
 
   run_git('symbolic-ref', 'HEAD', 'refs/heads/default', cwd=cwd)
 

tests/test_wrapper.py

@@ -20,6 +20,8 @@ from __future__ import print_function
 
 import os
 import re
+import shutil
+import tempfile
 import unittest
 
 from pyversion import is_python3
@@ -241,5 +243,90 @@ class CheckGitVersion(RepoWrapperTestCase):
       self.wrapper._CheckGitVersion()
 
 
+class ResolveRepoRev(RepoWrapperTestCase):
+  """Check resolve_repo_rev behavior."""
+
+  GIT_DIR = None
+  REV_LIST = None
+
+  @classmethod
+  def setUpClass(cls):
+    # Create a repo to operate on, but do it once per-class.
+    cls.GIT_DIR = tempfile.mkdtemp(prefix='repo-rev-tests')
+    run_git = wrapper.Wrapper().run_git
+
+    remote = os.path.join(cls.GIT_DIR, 'remote')
+    os.mkdir(remote)
+    run_git('init', cwd=remote)
+    run_git('commit', '--allow-empty', '-minit', cwd=remote)
+    run_git('branch', 'stable', cwd=remote)
+    run_git('tag', 'v1.0', cwd=remote)
+    run_git('commit', '--allow-empty', '-m2nd commit', cwd=remote)
+    cls.REV_LIST = run_git('rev-list', 'HEAD', cwd=remote).stdout.splitlines()
+
+    run_git('init', cwd=cls.GIT_DIR)
+    run_git('fetch', remote, '+refs/heads/*:refs/remotes/origin/*', cwd=cls.GIT_DIR)
+
+  @classmethod
+  def tearDownClass(cls):
+    if not cls.GIT_DIR:
+      return
+
+    shutil.rmtree(cls.GIT_DIR)
+
+  def test_explicit_branch(self):
+    """Check refs/heads/branch argument."""
+    rrev, lrev = self.wrapper.resolve_repo_rev(self.GIT_DIR, 'refs/heads/stable')
+    self.assertEqual('refs/heads/stable', rrev)
+    self.assertEqual(self.REV_LIST[1], lrev)
+
+    with self.assertRaises(wrapper.CloneFailure):
+      self.wrapper.resolve_repo_rev(self.GIT_DIR, 'refs/heads/unknown')
+
+  def test_explicit_tag(self):
+    """Check refs/tags/tag argument."""
+    rrev, lrev = self.wrapper.resolve_repo_rev(self.GIT_DIR, 'refs/tags/v1.0')
+    self.assertEqual('refs/tags/v1.0', rrev)
+    self.assertEqual(self.REV_LIST[1], lrev)
+
+    with self.assertRaises(wrapper.CloneFailure):
+      self.wrapper.resolve_repo_rev(self.GIT_DIR, 'refs/tags/unknown')
+
+  def test_branch_name(self):
+    """Check branch argument."""
+    rrev, lrev = self.wrapper.resolve_repo_rev(self.GIT_DIR, 'stable')
+    self.assertEqual('refs/heads/stable', rrev)
+    self.assertEqual(self.REV_LIST[1], lrev)
+
+    rrev, lrev = self.wrapper.resolve_repo_rev(self.GIT_DIR, 'master')
+    self.assertEqual('refs/heads/master', rrev)
+    self.assertEqual(self.REV_LIST[0], lrev)
+
+  def test_tag_name(self):
+    """Check tag argument."""
+    rrev, lrev = self.wrapper.resolve_repo_rev(self.GIT_DIR, 'v1.0')
+    self.assertEqual('refs/tags/v1.0', rrev)
+    self.assertEqual(self.REV_LIST[1], lrev)
+
+  def test_full_commit(self):
+    """Check specific commit argument."""
+    commit = self.REV_LIST[0]
+    rrev, lrev = self.wrapper.resolve_repo_rev(self.GIT_DIR, commit)
+    self.assertEqual(commit, rrev)
+    self.assertEqual(commit, lrev)
+
+  def test_partial_commit(self):
+    """Check specific (partial) commit argument."""
+    commit = self.REV_LIST[0][0:20]
+    rrev, lrev = self.wrapper.resolve_repo_rev(self.GIT_DIR, commit)
+    self.assertEqual(self.REV_LIST[0], rrev)
+    self.assertEqual(self.REV_LIST[0], lrev)
+
+  def test_unknown(self):
+    """Check unknown ref/commit argument."""
+    with self.assertRaises(wrapper.CloneFailure):
+      self.wrapper.resolve_repo_rev(self.GIT_DIR, 'boooooooya')
+
+
 if __name__ == '__main__':
   unittest.main()