From 9a5e3bd817833545f8bc0ebdcbde5c86051c8bc6 Mon Sep 17 00:00:00 2001 From: Edward Lemur Date: Tue, 2 Apr 2019 23:37:45 +0000 Subject: [PATCH] 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 Reviewed-by: Andrii Shyshkalov --- gclient_scm.py | 28 +++++++++++------------ scm.py | 24 ++++++++++++++++---- tests/scm_unittest.py | 53 ++++++++++++++++++++++--------------------- 3 files changed, 59 insertions(+), 46 deletions(-) diff --git a/gclient_scm.py b/gclient_scm.py index 9766e1851e..e102e80734 100644 --- a/gclient_scm.py +++ b/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, diff --git a/scm.py b/scm.py index 43a68fd2df..b30b14cfdb 100644 --- a/scm.py +++ b/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 diff --git a/tests/scm_unittest.py b/tests/scm_unittest.py index 16a308774c..2cb86c0b14 100755 --- a/tests/scm_unittest.py +++ b/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):