gclient_scm: Simplify fetching refs.

Bug: 874501, 942229
Change-Id: Ie9896e8a289e32a1b468ed5fa51c95a81970bdf8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1544802
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>

gclient_scm.py

@@ -1299,21 +1299,19 @@ class GitWrapper(SCMWrapper):
   def _Fetch(self, options, remote=None, prune=False, quiet=False,
              refspec=None):
     cfg = gclient_utils.DefaultIndexPackConfig(self.url)
-    # When a mirror is configured, it fetches only the refs/heads, and possibly
-    # the refs/branch-heads and refs/tags, but not the refs/changes. So, if
-    # we're asked to fetch a refs/changes ref from the mirror, it won't have it.
-    # This makes sure that we always fetch refs/changes directly from the
-    # repository and not from the mirror.
-    if refspec and refspec.startswith('refs/changes'):
-      remote, _ = gclient_utils.SplitUrlRevision(self.url)
-      # Make sure that we fetch the (remote) refs/changes/xx ref to the (local)
-      # refs/changes/xx ref.
-      if ':' not in refspec:
-        refspec += ':' + refspec
-    if (refspec and refspec.startswith('refs/remotes/branch-heads')
-        and not getattr(options, 'with_branch_heads', False)):
-      refspec = '%s:%s' % (refspec.replace('/remotes', '', 1), refspec)
-
+    # When updating, the ref is modified to be a remote ref .
+    # (e.g. refs/heads/NAME becomes refs/remotes/REMOTE/NAME).
+    # Try to reverse that mapping.
+    original_ref = scm.GIT.RemoteRefToRef(refspec, self.remote)
+    if original_ref:
+      refspec = original_ref + ':' + refspec
+      # When a mirror is configured, it only fetches
+      # refs/{heads,branch-heads,tags}/*.
+      # If asked to fetch other refs, we must fetch those directly from the
+      # repository, and not from the mirror.
+      if not original_ref.startswith(
+          ('refs/heads/', 'refs/branch-heads/', 'refs/tags/')):
+        remote, _ = gclient_utils.SplitUrlRevision(self.url)
     fetch_cmd =  cfg + [
         'fetch',
         remote or self.remote,

scm.py

@@ -226,7 +226,7 @@ class GIT(object):
     return remote, upstream_branch
 
   @staticmethod
-  def RefToRemoteRef(ref, remote=None):
+  def RefToRemoteRef(ref, remote):
     """Convert a checkout ref to the equivalent remote ref.
 
     Returns:
@@ -240,10 +240,24 @@ class GIT(object):
     m = re.match('^(refs/(remotes/)?)?branch-heads/', ref or '')
     if m:
       return ('refs/remotes/branch-heads/', ref.replace(m.group(0), ''))
-    if remote:
-      m = re.match('^((refs/)?remotes/)?%s/|(refs/)?heads/' % remote, ref or '')
-      if m:
-        return ('refs/remotes/%s/' % remote, ref.replace(m.group(0), ''))
+
+    m = re.match('^((refs/)?remotes/)?%s/|(refs/)?heads/' % remote, ref or '')
+    if m:
+      return ('refs/remotes/%s/' % remote, ref.replace(m.group(0), ''))
+
+    return None
+
+  @staticmethod
+  def RemoteRefToRef(ref, remote):
+    assert remote, 'A remote must be given'
+    if not ref or not ref.startswith('refs/'):
+      return None
+    if not ref.startswith('refs/remotes/'):
+      return ref
+    if ref.startswith('refs/remotes/branch-heads/'):
+      return 'refs' + ref[len('refs/remotes'):]
+    if ref.startswith('refs/remotes/%s/' % remote):
+      return 'refs/heads' + ref[len('refs/remotes/%s' % remote):]
     return None
 
   @staticmethod

tests/scm_unittest.py

@@ -96,6 +96,7 @@ class GitWrapperTestCase(BaseSCMTestCase):
         'IsValidRevision',
         'IsWorkTreeDirty',
         'RefToRemoteRef',
+        'RemoteRefToRef',
         'ShortBranchName',
     ]
     # If this test fails, you should add the relevant test.
@@ -108,33 +109,9 @@ class GitWrapperTestCase(BaseSCMTestCase):
     self.mox.ReplayAll()
     self.assertEqual(scm.GIT.GetEmail(self.root_dir), 'mini@me.com')
 
-  def testRefToRemoteRefNoRemote(self):
-    refs = {
-        # local ref for upstream branch-head
-        'refs/remotes/branch-heads/1234': ('refs/remotes/branch-heads/',
-                                           '1234'),
-        # upstream ref for branch-head
-        'refs/branch-heads/1234': ('refs/remotes/branch-heads/', '1234'),
-        # could be either local or upstream ref, assumed to refer to
-        # upstream, but probably don't want to encourage refs like this.
-        'branch-heads/1234': ('refs/remotes/branch-heads/', '1234'),
-        # actively discouraging refs like this, should prepend with 'refs/'
-        'remotes/branch-heads/1234': None,
-        # might be non-"branch-heads" upstream branches, but can't resolve
-        # without knowing the remote.
-        'refs/heads/1234': None,
-        'heads/1234': None,
-        # underspecified, probably intended to refer to a local branch
-        '1234': None,
-        }
-    for k, v in refs.items():
-      r = scm.GIT.RefToRemoteRef(k)
-      self.assertEqual(r, v, msg='%s -> %s, expected %s' % (k, r, v))
-
-  def testRefToRemoteRefWithRemote(self):
+  def testRefToRemoteRef(self):
     remote = 'origin'
     refs = {
-        # This shouldn't be any different from the NoRemote() version.
         'refs/branch-heads/1234': ('refs/remotes/branch-heads/', '1234'),
         # local refs for upstream branch
         'refs/remotes/%s/foobar' % remote: ('refs/remotes/%s/' % remote,
@@ -147,11 +124,35 @@ class GitWrapperTestCase(BaseSCMTestCase):
         'heads/foobar': ('refs/remotes/%s/' % remote, 'foobar'),
         # underspecified, probably intended to refer to a local branch
         'foobar': None,
-        }
+        # tags and other refs
+        'refs/tags/TAG': None,
+        'refs/changes/34/1234': None,
+    }
     for k, v in refs.items():
       r = scm.GIT.RefToRemoteRef(k, remote)
       self.assertEqual(r, v, msg='%s -> %s, expected %s' % (k, r, v))
 
+  def testRemoteRefToRef(self):
+    remote = 'origin'
+    refs = {
+        'refs/remotes/branch-heads/1234': 'refs/branch-heads/1234',
+        # local refs for upstream branch
+        'refs/remotes/origin/foobar': 'refs/heads/foobar',
+        # tags and other refs
+        'refs/tags/TAG': 'refs/tags/TAG',
+        'refs/changes/34/1234': 'refs/changes/34/1234',
+        # different remote
+        'refs/remotes/other-remote/foobar': None,
+        # underspecified, probably intended to refer to a local branch
+        'heads/foobar': None,
+        'origin/foobar': None,
+        'foobar': None,
+        None: None,
+      }
+    for k, v in refs.items():
+      r = scm.GIT.RemoteRefToRef(k, remote)
+      self.assertEqual(r, v, msg='%s -> %s, expected %s' % (k, r, v))
+
 
 class RealGitTest(fake_repos.FakeReposTestBase):
   def setUp(self):