gclient: Add support for the branch:revision format.

Bug: 850812, 853032
Change-Id: I597acbde2b3c229813b7eba8fcba52d5877130b2
Reviewed-on: https://chromium-review.googlesource.com/1119235
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Aaron Gable <agable@chromium.org>

gclient_scm.py

@@ -303,12 +303,12 @@ class GitWrapper(SCMWrapper):
         except OSError:
           pass
 
-  def _FetchAndReset(self, revision, file_list, options):
+  def _FetchAndReset(self, ref, remote_ref, revision, file_list, options):
     """Equivalent to git fetch; git reset."""
-    self._UpdateBranchHeads(options, fetch=False)
+    self._UpdateBranchHeads(options, ref, remote_ref, fetch=False)
 
     self._Fetch(options, prune=True, quiet=options.verbose)
-    self._Scrub(revision, options)
+    self._Scrub(revision or remote_ref, options)
     if file_list is not None:
       files = self._Capture(
           ['-c', 'core.quotePath=false', 'ls-files']).splitlines()
@@ -384,11 +384,12 @@ class GitWrapper(SCMWrapper):
 
     self._CheckMinVersion("1.6.6")
 
-    # If a dependency is not pinned, track the default remote branch.
-    default_rev = 'refs/remotes/%s/master' % self.remote
+    # If a dependency is not pinned, track refs/heads/master by default.
+    default_rev = 'refs/heads/master'
     url, deps_revision = gclient_utils.SplitUrlRevision(self.url)
     revision = deps_revision
     managed = True
+
     if options.revision:
       # Override the revision number.
       revision = str(options.revision)
@@ -410,20 +411,37 @@ class GitWrapper(SCMWrapper):
       verbose = ['--verbose']
       printed_path = True
 
-    remote_ref = scm.GIT.RefToRemoteRef(revision, self.remote)
-    if remote_ref:
-      # Rewrite remote refs to their local equivalents.
-      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
-      rev_type = "hash"
+    ref = remote_ref = None
+    # Support the 'branch:revision' syntax.
+    if ':' in revision:
+      ref, revision = revision.split(':')
+      if not gclient_utils.IsFullGitSha(revision):
+        raise gclient_utils.Error(
+            'Invalid format: %s:%s. revision must be a git hash.' % (
+                remote_ref, revision))
+    elif not gclient_utils.IsFullGitSha(revision):
+      ref = revision
+      revision = None
+
+    if ref:
+      if ref.startswith('origin/'):
+        ref = ref[len('origin/'):]
+      if not ref.startswith('refs/'):
+        ref = 'refs/heads/' + ref
+      remote_ref = scm.GIT.RefToRemoteRef(ref, self.remote)
+      if remote_ref:
+        # If there is a corresponding remote ref for |ref|,  RefToRemoteRef
+        # returns a tuple, so we need to join it to get the actual remote ref.
+        # E.g. ('refs/remotes/origin/', 'branch-name')
+        #         -> 'refs/remotes/origin/branch-name
+        remote_ref = ''.join(remote_ref)
+      else:
+        # Otherwise, it returns None, so we use |ref|.
+        remote_ref = ref
 
-    mirror = self._GetMirror(url, options)
+    # If we're using a mirror, make sure it contains the ref we are asked to
+    # sync.
+    mirror = self._GetMirror(url, options, ref)
     if mirror:
       url = mirror.mirror_path
 
@@ -445,12 +463,12 @@ class GitWrapper(SCMWrapper):
         (os.path.isdir(self.checkout_path) and
          not os.path.exists(os.path.join(self.checkout_path, '.git')))):
       if mirror:
-        self._UpdateMirrorIfNotContains(mirror, options, rev_type, revision)
+        self._UpdateMirrorIfNotContains(mirror, options, revision)
       try:
-        self._Clone(revision, url, options)
+        self._Clone(ref, remote_ref, revision, url, options)
       except subprocess2.CalledProcessError:
         self._DeleteOrMove(options.force)
-        self._Clone(revision, url, options)
+        self._Clone(ref, remote_ref, revision, url, options)
       if file_list is not None:
         files = self._Capture(
           ['-c', 'core.quotePath=false', 'ls-files']).splitlines()
@@ -462,14 +480,14 @@ class GitWrapper(SCMWrapper):
       return self._Capture(['rev-parse', '--verify', 'HEAD'])
 
     if not managed:
-      self._UpdateBranchHeads(options, fetch=False)
+      self._UpdateBranchHeads(options, ref, remote_ref, fetch=False)
       self.Print('________ unmanaged solution; skipping %s' % self.relpath)
       return self._Capture(['rev-parse', '--verify', 'HEAD'])
 
     self._maybe_break_locks(options)
 
     if mirror:
-      self._UpdateMirrorIfNotContains(mirror, options, rev_type, revision)
+      self._UpdateMirrorIfNotContains(mirror, options, revision)
 
     # See if the url has changed (the unittests use git://foo for the url, let
     # that through).
@@ -496,12 +514,14 @@ class GitWrapper(SCMWrapper):
             self.checkout_path, '.git', 'objects', 'info', 'alternates'),
             'w') as fh:
           fh.write(os.path.join(url, 'objects'))
-      self._EnsureValidHeadObjectOrCheckout(revision, options, url)
-      self._FetchAndReset(revision, file_list, options)
+      self._EnsureValidHeadObjectOrCheckout(ref, remote_ref, revision, options,
+                                            url)
+      self._FetchAndReset(ref, remote_ref, revision, file_list, options)
 
       return_early = True
     else:
-      self._EnsureValidHeadObjectOrCheckout(revision, options, url)
+      self._EnsureValidHeadObjectOrCheckout(ref, remote_ref, revision, options,
+                                            url)
 
     if return_early:
       return self._Capture(['rev-parse', '--verify', 'HEAD'])
@@ -547,16 +567,17 @@ class GitWrapper(SCMWrapper):
       else:
         raise gclient_utils.Error('Invalid Upstream: %s' % upstream_branch)
 
-    if not scm.GIT.IsValidRevision(self.checkout_path, revision, sha_only=True):
+    if revision and not scm.GIT.IsValidRevision(
+        self.checkout_path, revision, sha_only=True):
       # Update the remotes first so we have all the refs.
       remote_output = scm.GIT.Capture(['remote'] + verbose + ['update'],
               cwd=self.checkout_path)
       if verbose:
         self.Print(remote_output)
 
-    self._UpdateBranchHeads(options, fetch=True)
+    self._UpdateBranchHeads(options, ref, remote_ref, fetch=True)
 
-    revision = self._AutoFetchRef(options, revision)
+    revision = self._AutoFetchRevision(options, revision)
 
     # This is a big hammer, debatable if it should even be here...
     if options.force or options.reset:
@@ -573,8 +594,8 @@ class GitWrapper(SCMWrapper):
       # to actually "Clean" the checkout; that commit is uncheckoutable on this
       # system. The best we can do is carry forward to the checkout step.
       if not (options.force or options.reset):
-        self._CheckClean(revision)
-      self._CheckDetachedHead(revision, options)
+        self._CheckClean(revision or ref)
+      self._CheckDetachedHead(revision or ref, options)
       if self._Capture(['rev-list', '-n', '1', 'HEAD']) == revision:
         self.Print('Up-to-date; skipping checkout.')
       else:
@@ -582,43 +603,35 @@ class GitWrapper(SCMWrapper):
         # it only when nuclear options are enabled.
         self._Checkout(
             options,
-            revision,
+            revision or ref,
             force=(options.force and options.delete_unversioned_trees),
             quiet=True,
         )
       if not printed_path:
-        self.Print('_____ %s at %s' % (self.relpath, revision), timestamp=False)
+        self.Print('_____ %s at %s' % (self.relpath, revision or ref),
+                   timestamp=False)
     elif current_type == 'hash':
       # case 1
       # Can't find a merge-base since we don't know our upstream. That makes
       # this command VERY likely to produce a rebase failure. For now we
       # assume origin is our upstream since that's what the old behavior was.
-      upstream_branch = self.remote
-      if options.revision or deps_revision:
-        upstream_branch = revision
+      upstream_branch = revision or ref or self.remote
       self._AttemptRebase(upstream_branch, file_list, options,
                           printed_path=printed_path, merge=options.merge)
       printed_path = True
-    elif rev_type == 'hash':
-      # case 2
-      self._AttemptRebase(upstream_branch, file_list, options,
-                          newbase=revision, printed_path=printed_path,
-                          merge=options.merge)
-      printed_path = True
-    elif remote_ref and ''.join(remote_ref) != upstream_branch:
+    elif remote_ref and remote_ref != upstream_branch:
       # case 4
-      new_base = ''.join(remote_ref)
       if not printed_path:
-        self.Print('_____ %s at %s' % (self.relpath, revision), timestamp=False)
+        self.Print('_____ %s at %s' % (self.relpath, ref), timestamp=False)
       switch_error = ("Could not switch upstream branch from %s to %s\n"
-                     % (upstream_branch, new_base) +
+                     % (upstream_branch, ref) +
                      "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)
+                     "cd %s; git rebase %s\n" % (self.checkout_path, ref) +
+                     "OR git checkout -b <some new branch> %s" % ref)
       force_switch = False
       if options.force:
         try:
-          self._CheckClean(revision)
+          self._CheckClean(ref)
           # case 4a
           force_switch = True
         except gclient_utils.Error as e:
@@ -629,15 +642,25 @@ class GitWrapper(SCMWrapper):
             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])
+                   (upstream_branch, ref))
+        switch_branch = 'gclient_' + re.sub('[^A-Za-z0-9]', '_', ref)
+        self._Capture(['branch', '-f', switch_branch, ref])
         self._Checkout(options, switch_branch, force=True, quiet=True)
+        if revision:
+          self._Scrub(revision, options)
       else:
         # case 4c
         raise gclient_utils.Error(switch_error)
+    elif revision:
+      # case 2
+      self._AttemptRebase(upstream_branch, file_list, options,
+                          newbase=revision, printed_path=printed_path,
+                          merge=options.merge)
+      printed_path = True
     else:
       # case 3 - the default case
+      # The same ref as |upstream_branch| was specified, and no revision was
+      # used.
       rebase_files = self._GetDiffFilenames(upstream_branch)
       if verbose:
         self.Print('Trying fast-forward merge to branch : %s' % upstream_branch)
@@ -653,7 +676,7 @@ class GitWrapper(SCMWrapper):
         rebase_files = []
         if re.match('fatal: Not possible to fast-forward, aborting.', e.stderr):
           if not printed_path:
-            self.Print('_____ %s at %s' % (self.relpath, revision),
+            self.Print('_____ %s at %s' % (self.relpath, ref),
                        timestamp=False)
             printed_path = True
           while True:
@@ -686,7 +709,7 @@ class GitWrapper(SCMWrapper):
                       "changes or stash them before you can merge.\n",
                       e.stderr):
           if not printed_path:
-            self.Print('_____ %s at %s' % (self.relpath, revision),
+            self.Print('_____ %s at %s' % (self.relpath, ref),
                        timestamp=False)
             printed_path = True
           raise gclient_utils.Error(e.stderr)
@@ -835,13 +858,16 @@ class GitWrapper(SCMWrapper):
     return os.path.join(self._root_dir,
                         'old_' + self.relpath.replace(os.sep, '_')) + '.git'
 
-  def _GetMirror(self, url, options):
+  def _GetMirror(self, url, options, ref=None):
     """Get a git_cache.Mirror object for the argument url."""
     if not self.cache_dir:
       return None
+    # Don't try to fetch local refs in the mirror.
+    if ref and ref.startswith('refs/remotes'):
+      ref = None
     mirror_kwargs = {
         'print_func': self.filter,
-        'refs': []
+        'refs': [ref] if ref else [],
     }
     if hasattr(options, 'with_branch_heads') and options.with_branch_heads:
       mirror_kwargs['refs'].append('refs/branch-heads/*')
@@ -849,11 +875,11 @@ class GitWrapper(SCMWrapper):
       mirror_kwargs['refs'].append('refs/tags/*')
     return git_cache.Mirror(url, **mirror_kwargs)
 
-  def _UpdateMirrorIfNotContains(self, mirror, options, rev_type, revision):
+  def _UpdateMirrorIfNotContains(self, mirror, options, revision):
     """Update a git mirror by fetching the latest commits from the remote,
     unless mirror already contains revision whose type is sha1 hash.
     """
-    if rev_type == 'hash' and mirror.contains_revision(revision):
+    if revision and mirror.contains_revision(revision):
       if options.verbose:
         self.Print('skipping mirror update, it has rev=%s already' % revision,
                    timestamp=False)
@@ -874,7 +900,7 @@ class GitWrapper(SCMWrapper):
                     lock_timeout=getattr(options, 'lock_timeout', 0))
     mirror.unlock()
 
-  def _Clone(self, revision, url, options):
+  def _Clone(self, ref, remote_ref, revision, url, options):
     """Clone a git repository from the given URL.
 
     Once we've cloned the repo, we checkout a working branch if the specified
@@ -900,7 +926,7 @@ class GitWrapper(SCMWrapper):
 
     template_dir = None
     if hasattr(options, 'no_history') and options.no_history:
-      if gclient_utils.IsGitSha(revision):
+      if revision and gclient_utils.IsGitSha(revision):
         # In the case of a subproject, the pinned sha is not necessarily the
         # head of the remote branch (so we can't just use --depth=N). Instead,
         # we tell git to fetch all the remote objects from SHA..HEAD by means of
@@ -941,17 +967,18 @@ class GitWrapper(SCMWrapper):
       gclient_utils.rmtree(tmp_dir)
       if template_dir:
         gclient_utils.rmtree(template_dir)
-    self._UpdateBranchHeads(options, fetch=True)
-    revision = self._AutoFetchRef(options, revision)
-    remote_ref = scm.GIT.RefToRemoteRef(revision, self.remote)
-    self._Checkout(options, ''.join(remote_ref or revision), quiet=True)
+    self._UpdateBranchHeads(options, ref, remote_ref, fetch=True)
+    if revision:
+      revision = self._AutoFetchRevision(options, revision)
+    self._Checkout(options, revision or remote_ref, quiet=True)
     if self._GetCurrentBranch() is None:
       # Squelch git's very verbose detached HEAD warning and use our own
       self.Print(
         ('Checked out %s to a detached HEAD. Before making any commits\n'
          'in this repo, you should use \'git checkout <branch>\' to switch to\n'
          'an existing branch or use \'git checkout %s -b <branch>\' to\n'
-         'create a new branch for your work.') % (revision, self.remote))
+         'create a new branch for your work.') % (
+             revision or remote_ref, self.remote))
 
   def _AskForData(self, prompt, options):
     if options.jobs > 1:
@@ -1051,7 +1078,8 @@ class GitWrapper(SCMWrapper):
       raise gclient_utils.Error('git version %s < minimum required %s' %
                                 (current_version, min_version))
 
-  def _EnsureValidHeadObjectOrCheckout(self, revision, options, url):
+  def _EnsureValidHeadObjectOrCheckout(self, ref, remote_ref, revision, options,
+                                       url):
     # Special case handling if all 3 conditions are met:
     #   * the mirros have recently changed, but deps destination remains same,
     #   * the git histories of mirrors are conflicting.
@@ -1070,7 +1098,7 @@ class GitWrapper(SCMWrapper):
           '%s' % e)
         )
         self._DeleteOrMove(options.force)
-        self._Clone(revision, url, options)
+        self._Clone(ref, remote_ref, revision, url, options)
       else:
         raise
 
@@ -1170,6 +1198,7 @@ class GitWrapper(SCMWrapper):
     if quiet:
       checkout_args.append('--quiet')
     checkout_args.append(ref)
+    checkout_args.append('--')
     return self._Capture(checkout_args)
 
   def _Fetch(self, options, remote=None, prune=False, quiet=False,
@@ -1204,7 +1233,7 @@ class GitWrapper(SCMWrapper):
     # Return the revision that was fetched; this will be stored in 'FETCH_HEAD'
     return self._Capture(['rev-parse', '--verify', 'FETCH_HEAD'])
 
-  def _UpdateBranchHeads(self, options, fetch=False):
+  def _UpdateBranchHeads(self, options, ref, remote_ref, fetch=False):
     """Adds, and optionally fetches, "branch-heads" and "tags" refspecs
     if requested."""
     need_fetch = fetch
@@ -1220,16 +1249,20 @@ class GitWrapper(SCMWrapper):
                     '^\\+refs/tags/\\*:.*$']
       self._Run(config_cmd, options)
       need_fetch = True
+    # Make sure we fetch the ref we're asked to sync, if any.
+    if ref and not ref.startswith(('refs/remotes',)):
+      config_cmd = ['config', 'remote.%s.fetch' % self.remote,
+                    '+%s:%s' % (ref, remote_ref), '--add']
+      self._Run(config_cmd, options)
+      need_fetch = True
     if fetch and need_fetch:
       self._Fetch(options, prune=options.force)
 
-  def _AutoFetchRef(self, options, revision):
+  def _AutoFetchRevision(self, options, revision):
     """Attempts to fetch |revision| if not available in local repo.
 
     Returns possibly updated revision."""
-    try:
-      self._Capture(['rev-parse', revision])
-    except subprocess2.CalledProcessError:
+    if revision and not scm.GIT.IsValidRevision(self.checkout_path, revision):
       self._Fetch(options, refspec=revision)
       revision = self._Capture(['rev-parse', 'FETCH_HEAD'])
     return revision

testing_support/fake_repos.py

@@ -382,10 +382,8 @@ deps_os = {
 }""" % {
             'git_base': self.git_base,
             # See self.__init__() for the format. Grab's the hash of the first
-            # commit in repo_2. Only keep the first 7 character because of:
-            # TODO(maruel): http://crosbug.com/3591 We need to strip the hash..
-            # duh.
-            'hash3': self.git_hashes['repo_3'][1][0][:7]
+            # commit in repo_3.
+            'hash3': self.git_hashes['repo_3'][1][0]
         },
         'origin': 'git/repo_1@1\n',
     })
@@ -446,9 +444,8 @@ hooks = [
 """ % {
         'git_base': self.git_base,
         # See self.__init__() for the format. Grab's the hash of the first
-        # commit in repo_2. Only keep the first 7 character because of:
-        # TODO(maruel): http://crosbug.com/3591 We need to strip the hash.. duh.
-        'hash': self.git_hashes['repo_2'][1][0][:7]
+        # commit in repo_2.
+        'hash': self.git_hashes['repo_2'][1][0]
       },
       'origin': 'git/repo_1@2\n',
     })
@@ -471,8 +468,8 @@ pre_deps_hooks = [
 ]
 """ % {
          'git_base': self.git_base,
-         'hash1': self.git_hashes['repo_1'][2][0][:7],
-         'hash2': self.git_hashes['repo_2'][1][0][:7],
+         'hash1': self.git_hashes['repo_1'][2][0],
+         'hash2': self.git_hashes['repo_2'][1][0],
       },
     'origin': 'git/repo_5@2\n',
     })
@@ -496,8 +493,8 @@ pre_deps_hooks = [
 ]
 """ % {
          'git_base': self.git_base,
-         'hash1': self.git_hashes['repo_1'][2][0][:7],
-         'hash2': self.git_hashes['repo_2'][1][0][:7],
+         'hash1': self.git_hashes['repo_1'][2][0],
+         'hash2': self.git_hashes['repo_2'][1][0],
       },
     'origin': 'git/repo_5@3\n',
     })
@@ -594,7 +591,7 @@ recursedeps = [
   'src/repo8',
 ]""" % {
         'git_base': self.git_base,
-        'hash': self.git_hashes['repo_2'][1][0][:7]
+        'hash': self.git_hashes['repo_2'][1][0]
       },
       'origin': 'git/repo_6@1\n',
     })
@@ -638,7 +635,7 @@ deps_os ={
 vars = {
   'str_var': 'xyz',
 }
-gclient_gn_args_file = 'src/repo2/gclient.args'
+gclient_gn_args_file = 'src/repo9/gclient.args'
 gclient_gn_args = [
   'str_var',
 ]

tests/gclient_scm_test.py

@@ -184,6 +184,20 @@ Add C
 from :3
 M 100644 :7 c
 
+blob
+mark :9
+data 4
+foo
+
+commit refs/heads/feature
+mark :10
+author Alice <alice@example.com> 1490311986 -0700
+committer Alice <alice@example.com> 1490311986 -0700
+data 6
+Add D
+from :8
+M 100644 :9 d
+
 reset refs/heads/master
 from :3
 """
@@ -213,7 +227,8 @@ from :3
         stderr=STDOUT, cwd=path).communicate()
     Popen(['git', 'checkout', '-b', 'new', 'origin/master', '-q'], stdout=PIPE,
         stderr=STDOUT, cwd=path).communicate()
-    Popen(['git', 'push', 'origin', 'origin/origin:origin/master', '-q'],
+    Popen(['git', 'push', 'origin',
+           'refs/heads/origin:refs/heads/master', '-q'],
         stdout=PIPE, stderr=STDOUT, cwd=path).communicate()
     Popen(['git', 'config', '--unset', 'remote.origin.fetch'], stdout=PIPE,
         stderr=STDOUT, cwd=path).communicate()
@@ -383,6 +398,21 @@ class ManagedGitWrapperTestCase(BaseGitWrapperTestCase):
                       'a7142dc9f0009350b96a11f372b6ea658592aa95')
     sys.stdout.close()
 
+  def testUpdateRefRevision(self):
+    if not self.enabled:
+      return
+    options = self.Options()
+    options.force = True
+    options.revision = ('refs/heads/feature'
+                        ':9a51244740b25fa2ded5252ca00a3178d3f665a9')
+    scm = gclient_scm.GitWrapper(self.url, self.root_dir,
+                                 self.relpath)
+    file_list = []
+    scm.update(options, (), file_list)
+    self.assertEquals(scm.revinfo(options, (), None),
+                      '9a51244740b25fa2ded5252ca00a3178d3f665a9')
+    sys.stdout.close()
+
   def testUpdateMerge(self):
     if not self.enabled:
       return
@@ -395,11 +425,11 @@ class ManagedGitWrapperTestCase(BaseGitWrapperTestCase):
     file_list = []
     scm.update(options, (), file_list)
     self.assertEquals(file_list, [join(self.base_path, x)
-                                  for x in ['a', 'b', 'c']])
+                                  for x in ['a', 'b', 'c', 'd']])
     # The actual commit that is created is unstable, so we verify its tree and
     # parents instead.
     self.assertEquals(scm._Capture(['rev-parse', 'HEAD:']),
-                      'd2e35c10ac24d6c621e14a1fcadceb533155627d')
+                      'f7c1b0aaff248edf8981c776dcf6014c3eb09936')
     self.assertEquals(scm._Capture(['rev-parse', 'HEAD^1']), rev)
     self.assertEquals(scm._Capture(['rev-parse', 'HEAD^2']),
                       scm._Capture(['rev-parse', 'origin/master']))
@@ -419,12 +449,12 @@ class ManagedGitWrapperTestCase(BaseGitWrapperTestCase):
         '(y)es / (q)uit / (s)kip : ', 'y')
     scm.update(options, (), file_list)
     self.assertEquals(file_list, [join(self.base_path, x)
-                                  for x in ['a', 'b', 'c']])
+                                  for x in "abcd"])
     # The actual commit that is created is unstable, so we verify its tree and
     # parent instead.
     self.assertEquals(scm._Capture(['rev-parse', 'HEAD:']),
-                      'd2e35c10ac24d6c621e14a1fcadceb533155627d')
-    self.assertEquals(scm._Capture(['rev-parse', 'HEAD^']),
+                      'f7c1b0aaff248edf8981c776dcf6014c3eb09936')
+    self.assertEquals(scm._Capture(['rev-parse', 'HEAD^^']),
                       scm._Capture(['rev-parse', 'origin/master']))
     sys.stdout.close()
 
@@ -537,7 +567,7 @@ class ManagedGitWrapperTestCase(BaseGitWrapperTestCase):
                  'Fix the conflict and run gclient again.\n'
                  'See \'man git-rebase\' for details.\n')
     self.assertRaisesError(exception, scm.update, options, (), [])
-    exception = ('\n____ . at refs/remotes/origin/master\n'
+    exception = ('\n____ . at refs/heads/master\n'
                  '\tYou have unstaged changes.\n'
                  '\tPlease commit, stash, or reset.\n')
     self.assertRaisesError(exception, scm.update, options, (), [])
@@ -635,8 +665,9 @@ class ManagedGitWrapperTestCaseMox(BaseTestCase):
                                ).AndReturn(False)
     self.mox.StubOutWithMock(gclient_scm.GitWrapper, '_Clone', True)
     # pylint: disable=no-value-for-parameter
-    gclient_scm.GitWrapper._Clone('refs/remotes/origin/master', self.url,
-                                  options)
+    gclient_scm.GitWrapper._Clone(
+        'refs/heads/master', 'refs/remotes/origin/master', None, self.url,
+        options)
     self.mox.StubOutWithMock(gclient_scm.subprocess2, 'check_output', True)
     gclient_scm.subprocess2.check_output(
         ['git', '-c', 'core.quotePath=false', 'ls-files'], cwd=self.base_path,
@@ -667,13 +698,15 @@ class ManagedGitWrapperTestCaseMox(BaseTestCase):
     self.mox.StubOutWithMock(gclient_scm.GitWrapper, '_Clone', True)
     # pylint: disable=no-value-for-parameter
     gclient_scm.GitWrapper._Clone(
-        'refs/remotes/origin/master', self.url, options
+        'refs/heads/master', 'refs/remotes/origin/master', None, self.url,
+        options
     ).AndRaise(gclient_scm.subprocess2.CalledProcessError(None, None, None,
                                                           None, None))
     self.mox.StubOutWithMock(gclient_scm.GitWrapper, '_DeleteOrMove', True)
     gclient_scm.GitWrapper._DeleteOrMove(False)
-    gclient_scm.GitWrapper._Clone('refs/remotes/origin/master', self.url,
-                                  options)
+    gclient_scm.GitWrapper._Clone(
+        'refs/heads/master', 'refs/remotes/origin/master', None, self.url,
+        options)
     self.mox.StubOutWithMock(gclient_scm.subprocess2, 'check_output', True)
     gclient_scm.subprocess2.check_output(
         ['git', '-c', 'core.quotePath=false', 'ls-files'], cwd=self.base_path,
@@ -735,7 +768,7 @@ class UnmanagedGitWrapperTestCase(BaseGitWrapperTestCase):
 
     self.assertEquals(file_list, expected_file_list)
     self.assertEquals(scm.revinfo(options, (), None),
-                      '069c602044c5388d2d15c3f875b057c852003458')
+                      'a7142dc9f0009350b96a11f372b6ea658592aa95')
     # indicates detached HEAD
     self.assertEquals(self.getCurrentBranch(), None)
     self.checkInStdout(
@@ -743,6 +776,33 @@ class UnmanagedGitWrapperTestCase(BaseGitWrapperTestCase):
 
     rmtree(origin_root_dir)
 
+  def testUpdateCloneRefRevision(self):
+    if not self.enabled:
+      return
+    options = self.Options()
+    options.revision = ('refs/heads/feature'
+                        ':9a51244740b25fa2ded5252ca00a3178d3f665a9')
+
+    origin_root_dir = self.root_dir
+    self.root_dir = tempfile.mkdtemp()
+    self.relpath = '.'
+    self.base_path = join(self.root_dir, self.relpath)
+
+    scm = gclient_scm.GitWrapper(origin_root_dir,
+                                 self.root_dir,
+                                 self.relpath)
+
+    file_list = []
+    scm.update(options, (), file_list)
+    self.assertEquals(scm.revinfo(options, (), None),
+                      '9a51244740b25fa2ded5252ca00a3178d3f665a9')
+    # indicates detached HEAD
+    self.assertEquals(self.getCurrentBranch(), None)
+    self.checkInStdout(
+      'Checked out 9a51244740b25fa2ded5252ca00a3178d3f665a9 to a detached HEAD')
+
+    rmtree(origin_root_dir)
+
   def testUpdateCloneOnCommit(self):
     if not self.enabled:
       return
@@ -792,18 +852,19 @@ class UnmanagedGitWrapperTestCase(BaseGitWrapperTestCase):
 
     expected_file_list = [join(self.base_path, "a"),
                           join(self.base_path, "b"),
-                          join(self.base_path, "c")]
+                          join(self.base_path, "c"),
+                          join(self.base_path, "d")]
     file_list = []
     options.revision = 'unmanaged'
     scm.update(options, (), file_list)
 
     self.assertEquals(file_list, expected_file_list)
     self.assertEquals(scm.revinfo(options, (), None),
-                      '9a51244740b25fa2ded5252ca00a3178d3f665a9')
+                      '8980cdcc0b037755eec87c47f1a9d7e90d580015')
     # indicates detached HEAD
     self.assertEquals(self.getCurrentBranch(), None)
     self.checkInStdout(
-        'Checked out 9a51244740b25fa2ded5252ca00a3178d3f665a9 '
+        'Checked out refs/remotes/origin/feature '
         'to a detached HEAD')
 
     rmtree(origin_root_dir)
@@ -825,14 +886,15 @@ class UnmanagedGitWrapperTestCase(BaseGitWrapperTestCase):
 
     expected_file_list = [join(self.base_path, "a"),
                           join(self.base_path, "b"),
-                          join(self.base_path, "c")]
+                          join(self.base_path, "c"),
+                          join(self.base_path, "d")]
     file_list = []
     options.revision = 'unmanaged'
     scm.update(options, (), file_list)
 
     self.assertEquals(file_list, expected_file_list)
     self.assertEquals(scm.revinfo(options, (), None),
-                      '9a51244740b25fa2ded5252ca00a3178d3f665a9')
+                      '8980cdcc0b037755eec87c47f1a9d7e90d580015')
     # indicates detached HEAD
     self.assertEquals(self.getCurrentBranch(), None)
     self.checkInStdout(
@@ -857,15 +919,16 @@ class UnmanagedGitWrapperTestCase(BaseGitWrapperTestCase):
 
     expected_file_list = [join(self.base_path, "a"),
                           join(self.base_path, "b"),
-                          join(self.base_path, "c")]
+                          join(self.base_path, "c"),
+                          join(self.base_path, "d")]
     file_list = []
     options.revision = 'unmanaged'
     scm.update(options, (), file_list)
 
     self.assertEquals(file_list, expected_file_list)
     self.assertEquals(scm.revinfo(options, (), None),
-                      '9a51244740b25fa2ded5252ca00a3178d3f665a9')
-    # @refs/heads/feature is AKA @refs/remotes/origin/feature in the clone, so
+                      '8980cdcc0b037755eec87c47f1a9d7e90d580015')
+    # @refs/heads/feature is AKA @refs/heads/feature in the clone, so
     # should be treated as such by gclient.
     # 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

tests/gclient_smoketest.py

@@ -39,7 +39,7 @@ class GClientSmokeBase(fake_repos.FakeReposTestBase):
     self.env['DEPOT_TOOLS_UPDATE'] = '0'
     self.env['DEPOT_TOOLS_METRICS'] = '0'
 
-  def gclient(self, cmd, cwd=None):
+  def gclient(self, cmd, cwd=None, ignore_errors=False):
     if not cwd:
       cwd = self.root_dir
     if COVERAGE:
@@ -54,6 +54,8 @@ class GClientSmokeBase(fake_repos.FakeReposTestBase):
     (stdout, stderr) = process.communicate()
     logging.debug("XXX: %s\n%s\nXXX" % (' '.join(cmd), stdout))
     logging.debug("YYY: %s\n%s\nYYY" % (' '.join(cmd), stderr))
+    if process.returncode and not ignore_errors:
+      raise subprocess.CalledProcessError(process.returncode, cmd, stdout)
     return (stdout.replace('\r\n', '\n'), stderr.replace('\r\n', '\n'),
             process.returncode)
 
@@ -158,14 +160,14 @@ class GClientSmoke(GClientSmokeBase):
 
   def testNotConfigured(self):
     res = ('', 'Error: client not configured; see \'gclient config\'\n', 1)
-    self.check(res, self.gclient(['diff']))
-    self.check(res, self.gclient(['pack']))
-    self.check(res, self.gclient(['revert']))
-    self.check(res, self.gclient(['revinfo']))
-    self.check(res, self.gclient(['runhooks']))
-    self.check(res, self.gclient(['status']))
-    self.check(res, self.gclient(['sync']))
-    self.check(res, self.gclient(['update']))
+    self.check(res, self.gclient(['diff'], ignore_errors=True))
+    self.check(res, self.gclient(['pack'], ignore_errors=True))
+    self.check(res, self.gclient(['revert'], ignore_errors=True))
+    self.check(res, self.gclient(['revinfo'], ignore_errors=True))
+    self.check(res, self.gclient(['runhooks'], ignore_errors=True))
+    self.check(res, self.gclient(['status'], ignore_errors=True))
+    self.check(res, self.gclient(['sync'], ignore_errors=True))
+    self.check(res, self.gclient(['update'], ignore_errors=True))
 
   def testConfig(self):
     # Get any bootstrapping out of the way.
@@ -249,7 +251,7 @@ class GClientSmoke(GClientSmokeBase):
     test(['config', '--spec', '["blah blah"]'], '["blah blah"]')
 
     os.remove(p)
-    results = self.gclient(['config', 'foo', 'faa', 'fuu'])
+    results = self.gclient(['config', 'foo', 'faa', 'fuu'], ignore_errors=True)
     err = ('Usage: gclient.py config [options] [url]\n\n'
            'gclient.py: error: Inconsistent arguments. Use either --spec or one'
            ' or 2 args\n')
@@ -381,7 +383,7 @@ class GClientSmokeGIT(GClientSmokeBase):
             'src/repo2/': {
                 'scm': 'git',
                 'url':
-                    self.git_base + 'repo_2@' + self.githash('repo_2', 1)[:7],
+                    self.git_base + 'repo_2@' + self.githash('repo_2', 1),
                 'revision': self.githash('repo_2', 1),
                 'was_processed': True,
             },
@@ -724,7 +726,8 @@ class GClientSmokeGIT(GClientSmokeBase):
                        % (sys.executable, self.root_dir))
     stdout, stderr, retcode = self.gclient(['sync', '--deps', 'mac', '--jobs=1',
                                             '--revision',
-                                            'src@' + self.githash('repo_5', 3)])
+                                            'src@' + self.githash('repo_5', 3)],
+                                           ignore_errors=True)
     self.assertEquals(stderr, expected_stderr)
     self.assertEquals(2, retcode)
     self.checkBlock(stdout, expectated_stdout)
@@ -740,7 +743,7 @@ class GClientSmokeGIT(GClientSmokeBase):
            'src/repo2/repo_renamed: %(base)srepo_3\n' %
           {
             'base': self.git_base,
-            'hash2': self.githash('repo_2', 1)[:7],
+            'hash2': self.githash('repo_2', 1),
           })
     self.check((out, '', 0), results)
 
@@ -783,7 +786,7 @@ class GClientSmokeGIT(GClientSmokeBase):
     out = ('src/repo2: %(base)srepo_2@%(hash2)s\n' %
           {
             'base': self.git_base,
-            'hash2': self.githash('repo_2', 1)[:7],
+            'hash2': self.githash('repo_2', 1),
           })
     self.check((out, '', 0), results)
 
@@ -798,7 +801,7 @@ class GClientSmokeGIT(GClientSmokeBase):
            'src/repo2: %(base)srepo_2@%(hash2)s\n' %
           {
             'base': self.git_base,
-            'hash2': self.githash('repo_2', 1)[:7],
+            'hash2': self.githash('repo_2', 1),
           })
     self.check((out, '', 0), results)
 
@@ -819,7 +822,7 @@ class GClientSmokeGIT(GClientSmokeBase):
         },
         'src/repo2': {
             'url': self.git_base + 'repo_2',
-            'rev': self.githash('repo_2', 1)[:7],
+            'rev': self.githash('repo_2', 1),
         },
        'src/repo2/repo_renamed': {
            'url': self.git_base + 'repo_3',
@@ -987,7 +990,7 @@ class GClientSmokeGIT(GClientSmokeBase):
         '  # src -> src/repo2',
         '  "src/repo2": {',
         '    "url": "' + self.git_base + 'repo_2@%s",' % (
-                 self.githash('repo_2', 1)[:7]),
+                 self.githash('repo_2', 1)),
         '    "condition": \'true_str_var\',',
         '  },',
         '',
@@ -1093,7 +1096,7 @@ class GClientSmokeGIT(GClientSmokeBase):
         '}',
         '',
         '# ' + self.git_base + 'repo_2@%s, DEPS' % (
-                 self.githash('repo_2', 1)[:7]),
+                 self.githash('repo_2', 1)),
         '# ' + self.git_base + 'repo_6, DEPS',
         '# ' + self.git_base + 'repo_8, DEPS',
     ], deps_contents.splitlines())
@@ -1291,7 +1294,7 @@ class GClientSmokeGIT(GClientSmokeBase):
 
     self.maxDiff = None
     self.assertEqual([
-        'gclient_gn_args_file = "src/repo2/gclient.args"',
+        'gclient_gn_args_file = "src/repo9/gclient.args"',
         "gclient_gn_args = ['str_var']",
         'deps = {',
         '  # src',
@@ -1383,7 +1386,8 @@ class GClientSmokeGIT(GClientSmokeBase):
     self.assertFalse(os.path.exists(output_deps))
 
     self.gclient(['config', self.git_base + 'repo_14', '--name', 'src'])
-    self.gclient(['sync'])
+    # We can't sync since we haven't faked a CIPD server to get packages from.
+    self.gclient(['sync'], ignore_errors=True)
     self.gclient(['flatten', '-v', '-v', '-v', '--output-deps', output_deps])
 
     with open(output_deps) as f:
@@ -1452,7 +1456,7 @@ class GClientSmokeGITMutates(GClientSmokeBase):
 
     # Commit new change to repo to make repo_2's hash use a custom_var.
     cur_deps = self.FAKE_REPOS.git_hashes['repo_1'][-1][1]['DEPS']
-    repo_2_hash = self.FAKE_REPOS.git_hashes['repo_2'][1][0][:7]
+    repo_2_hash = self.FAKE_REPOS.git_hashes['repo_2'][1][0]
     new_deps = cur_deps.replace('repo_2@%s\'' % repo_2_hash,
                                 'repo_2@\' + Var(\'r2hash\')')
     new_deps = 'vars = {\'r2hash\': \'%s\'}\n%s' % (repo_2_hash, new_deps)
@@ -1529,7 +1533,7 @@ class GClientSmokeGITMutates(GClientSmokeBase):
       return
     # Create an extra commit in repo_2 and point DEPS to its hash.
     cur_deps = self.FAKE_REPOS.git_hashes['repo_1'][-1][1]['DEPS']
-    repo_2_hash_old = self.FAKE_REPOS.git_hashes['repo_2'][1][0][:7]
+    repo_2_hash_old = self.FAKE_REPOS.git_hashes['repo_2'][1][0]
     self.FAKE_REPOS._commit_git('repo_2', {  # pylint: disable=protected-access
       'last_file': 'file created in last commit',
     })