Fix gclient branch ref mangling and allow --force branch switches.

R=iannucci@chromium.org, maruel@chromium.org
BUG=410959

Review URL: https://codereview.chromium.org/549733002

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@291883 0039d316-1c4b-4281-b951-d872f2087c98

gclient_scm.py

@@ -369,11 +369,14 @@ class GitWrapper(SCMWrapper):
       verbose = ['--verbose']
       printed_path = True
 
-    if revision.startswith('refs/'):
+    remote_ref = scm.GIT.RefToRemoteRef(revision, self.remote)
-      rev_type = "branch"
+    if remote_ref:
-    elif revision.startswith(self.remote + '/'):
       # Rewrite remote refs to their local equivalents.
-      revision = 'refs/remotes/' + revision
+      revision = ''.join(remote_ref)
+      rev_type = "branch"
+    elif revision.startswith('refs/'):
+      # Local branch? We probably don't want to support, since DEPS should
+      # always specify branches as they are in the upstream repo.
       rev_type = "branch"
     else:
       # hash is also a tag, only make a distinction at checkout
@@ -393,10 +396,6 @@ class GitWrapper(SCMWrapper):
       except subprocess2.CalledProcessError:
         self._DeleteOrMove(options.force)
         self._Clone(revision, url, options)
-      if deps_revision and deps_revision.startswith('branch-heads/'):
-        deps_branch = deps_revision.replace('branch-heads/', '')
-        self._Capture(['branch', deps_branch, deps_revision])
-        self._Checkout(options, deps_branch, quiet=True)
       if file_list is not None:
         files = self._Capture(['ls-files']).splitlines()
         file_list.extend([os.path.join(self.checkout_path, f) for f in files])
@@ -451,12 +450,16 @@ class GitWrapper(SCMWrapper):
     # 2) current branch is tracking a remote branch with local committed
     #    changes, but the DEPS file switched to point to a hash
     #   - rebase those changes on top of the hash
-    # 3) current branch is tracking a remote branch w/or w/out changes,
+    # 3) current branch is tracking a remote branch w/or w/out changes, and
-    #    no switch
+    #    no DEPS switch
     #   - see if we can FF, if not, prompt the user for rebase, merge, or stop
-    # 4) current branch is tracking a remote branch, switches to a different
+    # 4) current branch is tracking a remote branch, but DEPS switches to a
-    #    remote branch
+    #    different remote branch, and
-    #   - exit
+    #   a) current branch has no local changes, and --force:
+    #      - checkout new branch
+    #   b) current branch has local changes, and --force and --reset:
+    #      - checkout new branch
+    #   c) otherwise exit
 
     # GetUpstreamBranch returns something like 'refs/remotes/origin/master' for
     # a tracking branch
@@ -534,17 +537,37 @@ class GitWrapper(SCMWrapper):
                           newbase=revision, printed_path=printed_path,
                           merge=options.merge)
       printed_path = True
-    elif revision.replace('heads', 'remotes/' + self.remote) != upstream_branch:
+    elif remote_ref and ''.join(remote_ref) != upstream_branch:
       # case 4
-      new_base = revision.replace('heads', 'remotes/' + self.remote)
+      new_base = ''.join(remote_ref)
       if not printed_path:
         self.Print('_____ %s%s' % (self.relpath, rev_str), timestamp=False)
-      switch_error = ("Switching upstream branch from %s to %s\n"
+      switch_error = ("Could not switch upstream branch from %s to %s\n"
                      % (upstream_branch, new_base) +
-                     "Please merge or rebase manually:\n" +
+                     "Please use --force or merge or rebase manually:\n" +
                      "cd %s; git rebase %s\n" % (self.checkout_path, new_base) +
                      "OR git checkout -b <some new branch> %s" % new_base)
-      raise gclient_utils.Error(switch_error)
+      force_switch = False
+      if options.force:
+        try:
+          self._CheckClean(rev_str)
+          # case 4a
+          force_switch = True
+        except gclient_utils.Error as e:
+          if options.reset:
+            # case 4b
+            force_switch = True
+          else:
+            switch_error = '%s\n%s' % (e.message, switch_error)
+      if force_switch:
+        self.Print("Switching upstream branch from %s to %s" %
+                   (upstream_branch, new_base))
+        switch_branch = 'gclient_' + remote_ref[1]
+        self._Capture(['branch', '-f', switch_branch, new_base])
+        self._Checkout(options, switch_branch, force=True, quiet=True)
+      else:
+        # case 4c
+        raise gclient_utils.Error(switch_error)
     else:
       # case 3 - the default case
       if files is not None:
@@ -870,7 +893,8 @@ class GitWrapper(SCMWrapper):
       if template_dir:
         gclient_utils.rmtree(template_dir)
     self._UpdateBranchHeads(options, fetch=True)
-    self._Checkout(options, revision.replace('refs/heads/', ''), quiet=True)
+    remote_ref = scm.GIT.RefToRemoteRef(revision, self.remote)
+    self._Checkout(options, ''.join(remote_ref or revision), quiet=True)
     if self._GetCurrentBranch() is None:
       # Squelch git's very verbose detached HEAD warning and use our own
       self.Print(

scm.py

@@ -352,12 +352,35 @@ class GIT(object):
             upstream_branch = None
     return remote, upstream_branch
 
+  @staticmethod
+  def RefToRemoteRef(ref, remote=None):
+    """Convert a checkout ref to the equivalent remote ref.
+
+    Returns:
+      A tuple of the remote ref's (common prefix, unique suffix), or None if it
+      doesn't appear to refer to a remote ref (e.g. it's a commit hash).
+    """
+    # TODO(mmoss): This is just a brute-force mapping based of the expected git
+    # config. It's a bit better than the even more brute-force replace('heads',
+    # ...), but could still be smarter (like maybe actually using values gleaned
+    # from the git config).
+    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), ''))
+    return None
+
   @staticmethod
   def GetUpstreamBranch(cwd):
     """Gets the current branch's upstream branch."""
     remote, upstream_branch = GIT.FetchUpstreamTuple(cwd)
     if remote != '.' and upstream_branch:
-      upstream_branch = upstream_branch.replace('heads', 'remotes/' + remote)
+      remote_ref = GIT.RefToRemoteRef(upstream_branch, remote)
+      if remote_ref:
+        upstream_branch = ''.join(remote_ref)
     return upstream_branch
 
   @staticmethod

tests/gclient_scm_test.py

@@ -1561,7 +1561,7 @@ class UnmanagedGitWrapperTestCase(BaseGitWrapperTestCase):
 
     rmtree(origin_root_dir)
 
-  def testUpdateCloneOnDetachedBranch(self):
+  def testUpdateCloneOnFetchedRemoteBranch(self):
     if not self.enabled:
       return
     options = self.Options()
@@ -1593,7 +1593,7 @@ class UnmanagedGitWrapperTestCase(BaseGitWrapperTestCase):
 
     rmtree(origin_root_dir)
 
-  def testUpdateCloneOnBranchHead(self):
+  def testUpdateCloneOnTrueRemoteBranch(self):
     if not self.enabled:
       return
     options = self.Options()
@@ -1618,9 +1618,17 @@ class UnmanagedGitWrapperTestCase(BaseGitWrapperTestCase):
     self.assertEquals(file_list, expected_file_list)
     self.assertEquals(scm.revinfo(options, (), None),
                       '9a51244740b25fa2ded5252ca00a3178d3f665a9')
-    self.assertEquals(self.getCurrentBranch(), 'feature')
+    # @refs/heads/feature is AKA @refs/remotes/origin/feature in the clone, so
-    self.checkNotInStdout(
+    # should be treated as such by gclient.
-      'Checked out refs/heads/feature to a detached HEAD')
+    # TODO(mmoss): Though really, we should only allow DEPS to specify branches
+    # as they are known in the upstream repo, since the mapping into the local
+    # repo can be modified by users (or we might even want to change the gclient
+    # defaults at some point). But that will take more work to stop using
+    # refs/remotes/ everywhere that we do (and to stop assuming a DEPS ref will
+    # always resolve locally, like when passing them to show-ref or rev-list).
+    self.assertEquals(self.getCurrentBranch(), None)
+    self.checkInStdout(
+      'Checked out refs/remotes/origin/feature to a detached HEAD')
 
     rmtree(origin_root_dir)
 

tests/scm_unittest.py

@@ -98,6 +98,7 @@ class GitWrapperTestCase(BaseSCMTestCase):
         'IsWorkTreeDirty',
         'MatchSvnGlob',
         'ParseGitSvnSha1',
+        'RefToRemoteRef',
         'ShortBranchName',
     ]
     # If this test fails, you should add the relevant test.
@@ -122,6 +123,50 @@ class GitWrapperTestCase(BaseSCMTestCase):
         'branches/*:refs/remotes/*',
         True), 'refs/remotes/bleeding_edge')
 
+  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):
+    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,
+                                            'foobar'),
+        '%s/foobar' % remote: ('refs/remotes/%s/' % remote, 'foobar'),
+        # upstream ref for branch
+        'refs/heads/foobar': ('refs/remotes/%s/' % remote, 'foobar'),
+        # could be either local or upstream ref, assumed to refer to
+        # upstream, but probably don't want to encourage refs like this.
+        'heads/foobar': ('refs/remotes/%s/' % remote, 'foobar'),
+        # underspecified, probably intended to refer to a local branch
+        'foobar': 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))
+
 
 class RealGitTest(fake_repos.FakeReposTestBase):
   def setUp(self):