Rewrite git gclient sync/update to be simpler and more reliable

gclient_scm.py

Index: gclient_scm.py
diff --git a/gclient_scm.py b/gclient_scm.py
index 4a4e6015461582fbf28a1034f63e66aba1e26770..64dbe3385c94058044d2bb4bca070416ca9a142a 100644
--- a/gclient_scm.py
+++ b/gclient_scm.py
@@ -305,7 +305,7 @@ class GitWrapper(SCMWrapper):
     quiet = []
     if not options.verbose:
       quiet = ['--quiet']
-    self._UpdateBranchHeads(options, fetch=False)
+    self._ConfigBranchHeads(options)
 
     self._Fetch(options, prune=True, quiet=options.verbose)
     self._Run(['reset', '--hard', revision] + quiet, options)
@@ -324,7 +324,7 @@ class GitWrapper(SCMWrapper):
           os.remove(disabled_hook_path)
         os.rename(os.path.join(hook_dir, f), disabled_hook_path)
 
-  def _maybe_break_locks(self, options):
+  def _MaybeBreakLocks(self, options):
     """This removes all .lock files from this repo's .git directory, if the
     user passed the --break_repo_locks command line flag.
 
@@ -353,101 +353,100 @@ class GitWrapper(SCMWrapper):
     Raises:
       Error: if can't get URL for relative path.
     """
+    ### Section 0: Sanity checks.
     if args:
       raise gclient_utils.Error("Unsupported argument(s): %s" % ",".join(args))
 
     self._CheckMinVersion("1.6.6")
 
-    # If a dependency is not pinned, track the default remote branch.
-    default_rev = 'refs/remotes/%s/master' % self.remote
-    url, deps_revision = gclient_utils.SplitUrlRevision(self.url)
-    revision = deps_revision
-    managed = True
+    ### Section 1: Handling basic options and parsing revision.
+    # Command line options trump DEPS-file pins. If neither is provided,
+    # fall back to the default branch (master).
+    # Throughout this function, 'revision' is assumed to be available on the
+    # remote; i.e. it will never be "refs/remotes/foo/bar". When it is necessary
+    # to translate the desired revision into a locally-available refish, that
+    # is handled later.
+    default_ref = 'refs/heads/master'
+    url, revision = gclient_utils.SplitUrlRevision(self.url)
     if options.revision:
-      # Override the revision number.
       revision = str(options.revision)
-    if revision == 'unmanaged':
-      # Check again for a revision in case an initial ref was specified
-      # in the url, for example bla.git@refs/heads/custombranch
-      revision = deps_revision
-      managed = False
     if not revision:
-      revision = default_rev
+      revision = default_ref
 
+    managed = (revision == 'unmanaged')
     if managed:
       self._DisableHooks()
 
     printed_path = False
     verbose = []
     if options.verbose:
-      self.Print('_____ %s at %s' % (self.relpath, revision), timestamp=False)
       verbose = ['--verbose']
+      self.Print('_____ %s at %s' % (self.relpath, revision), timestamp=False)
       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"
-
     mirror = self._GetMirror(url, options)
     if mirror:
       url = mirror.mirror_path
 
-    # If we are going to introduce a new project, there is a possibility that
-    # we are syncing back to a state where the project was originally a
-    # sub-project rolled by DEPS (realistic case: crossing the Blink merge point
-    # syncing backwards, when Blink was a DEPS entry and not part of src.git).
-    # In such case, we might have a backup of the former .git folder, which can
-    # be used to avoid re-fetching the entire repo again (useful for bisects).
-    backup_dir = self.GetGitBackupDirPath()
-    target_dir = os.path.join(self.checkout_path, '.git')
-    if os.path.exists(backup_dir) and not os.path.exists(target_dir):
-      gclient_utils.safe_makedirs(self.checkout_path)
-      os.rename(backup_dir, target_dir)
+    ### Section 2: Ensuring a checkout exists at all.
+    # If our checkout path isn't a git repository, or if the repository which
+    # is there is corrupt, we need to bootstrap it.
+    needs_bootstrap = False
+    if not os.path.exists(self.checkout_path):
+      needs_boostrap = True
+    elif not os.path.exists(os.path.join(self.checkout_path, '.git')):
+      needs_bootstrap = True
+    else:
+      try:
+        self._Capture(['rev-list', '-n', '1', 'HEAD'])
+      except subprocess2.CalledProcessError as e:
+        if 'fatal: bad object HEAD' in e.stderr:
+          needs_bootstrap = True
+          # If a cache_dir is configured, that's likely why the repo is
+          # corrupt, so provide a helpful message.
+          if self.cache_dir and self.cache_dir in url:
+            self.Print(
+                'Likely due to DEPS change with git cache_dir, '
+                'the current commit points to no longer existing object.\n'
+                '%s' % e)
+
+    if needs_bootstrap:
+      # Get anything that might be there out of the way, just in case.
+      self._DeleteOrMove(options.force)
+
+      # If we are going to introduce a new project, there is a possibility that
+      # we are syncing back to a state where the project was originally
+      # a sub-project rolled by DEPS (realistic case: crossing the Blink merge
+      # point syncing backwards, when Blink was a DEPS entry and not part of
+      # src.git). In such case, we might have a backup of the former .git
+      # folder, which can be used to avoid re-fetching the entire repo again.
+      backup_dir = self.GetGitBackupDirPath()
+      if os.path.exists(backup_dir):
+        gclient_utils.safe_makedirs(self.checkout_path)
+        gclient_utils.safe_rename(backup_dir, target_dir)
+      else:
+        if mirror:
+          self._UpdateMirror(mirror, options)
+        self._Clone(revision, url, options)
+
       # Reset to a clean state
       self._Run(['reset', '--hard', 'HEAD'], options)
 
-    if (not os.path.exists(self.checkout_path) or
-        (os.path.isdir(self.checkout_path) and
-         not os.path.exists(os.path.join(self.checkout_path, '.git')))):
-      if mirror:
-        self._UpdateMirror(mirror, options)
-      try:
-        self._Clone(revision, url, options)
-      except subprocess2.CalledProcessError:
-        self._DeleteOrMove(options.force)
-        self._Clone(revision, url, options)
       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])
-      if not verbose:
-        # Make the output a little prettier. It's nice to have some whitespace
-        # between projects when cloning.
-        self.Print('')
-      return self._Capture(['rev-parse', '--verify', 'HEAD'])
 
+    ### Section 3: Getting the checkout ready to be updated.
     if not managed:
-      self._UpdateBranchHeads(options, 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._UpdateMirror(mirror, options)
+    self._MaybeBreakLocks(options)
 
+    # TODO(agable): The logic here is pretty crazy. Clean it up.
     # See if the url has changed (the unittests use git://foo for the url, let
     # that through).
     current_url = self._Capture(['config', 'remote.%s.url' % self.remote])
-    return_early = False
     # TODO(maruel): Delete url != 'git://foo' since it's just to make the
     # unit test pass. (and update the comment above)
     # Skip url auto-correction if remote.origin.gclient-auto-fix-url is set.
@@ -469,18 +468,72 @@ 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)
 
-      return_early = True
-    else:
-      self._EnsureValidHeadObjectOrCheckout(revision, options, url)
+    # If the user wants to set up refs/branch-heads/* and refs/tags/*, oblige.
+    self._ConfigBranchHeads(options)
 
-    if return_early:
-      return self._Capture(['rev-parse', '--verify', 'HEAD'])
+    try:
+      self._CheckClean(revision)
+    except gclient_utils.Error:
+      if options.reset:
+        self._Run(['reset', '--hard', 'HEAD'], options)
+      else:
+        raise
+
+    ### Section 4: Ensuring the desired revision is available.
+    # If the desired revision is already checked out, we have no work to do.
+    try:
+      revision_hash = self._Capture(['rev-parse', revision])
+      current_hash = self._Capture(['rev-parse', 'HEAD'])
+      if revision_hash == current_hash:
+        return self._Capture(['rev-parse', '--verify', 'HEAD'])
+    except subprocess2.CalledProcessError:
+      # We know HEAD is reachable, because if it wasn't we did the bootstrap.
+      # But if the desired revision isn't reachable, it's clearly not checked
+      # out, so continue to the rest of the routine.
+      pass
+
+    # Perform a basic fetch to see if the desired revision shows up.
+    if mirror:
+      self._UpdateMirror(mirror, options)
+    self._Fetch(options)
+    revision_hash = self._Capture(['rev-parse', revision])
+    if not gclient_utils.IsGitSha(revision_hash):
+      # If it didn't exist, fetch just that ref and save the FETCH_HEAD.
+      try:
+        revision_hash = self._Fetch(options, refspec=[revision])
+      except subprocess2.CalledProcessError as e:
+        raise gclient_utils.Error(
+            '\n____ %s at %s\n\n'
+            'The given revision is neither reachable from a normal clone,\n'
+            'nor directly fetchable from the remote. Aborting.\n'
+            % (self.relpath, revision))
 
+    ### Section 5: Performing the checkout.
     cur_branch = self._GetCurrentBranch()
 
+    if cur_branch is None:
+      # We're in a detached HEAD state. Ensure the current commit is reachable
+      # (by creating a new branch if necessary), and then check out the new one.
+      self._CheckDetachedHead(revision, options)
+      # 'git checkout' may need to overwrite existing untracked files. Allow
+      # it only when nuclear options are enabled.
+      self._Checkout(
+          options,
+          revision,
+          force=(options.force and options.delete_unversioned_trees),
+          quiet=True,
+      )
+      if not printed_path:
+        self.Print('_____ %s at %s' % (self.relpath, revision), timestamp=False)
+      return self._Capture(['rev-parse', '--verify', 'HEAD'])
+
+    # TODO(agable): Drastically simplify this. I think it can be reduced to
+    # * Only rebase if --auto_rebase is passed
+    # * If rebase fails, and --force is passed, just switch
+    # * If rebase fails, and --force is not passed, abort
+    # * If not told to rebase, just switch
+    # But not reducing it yet, because I'm scared of breaking too much at once.
     # Cases:
     # 0) HEAD is detached. Probably from our initial clone.
     #   - make sure HEAD is contained by a named ref, then update.
@@ -501,33 +554,19 @@ class GitWrapper(SCMWrapper):
     #      - checkout new branch
     #   c) otherwise exit
 
-    # GetUpstreamBranch returns something like 'refs/remotes/origin/master' for
-    # a tracking branch
+    # GetUpstreamBranch returns something like 'refs/remotes/origin/master'
+    # for a tracking branch
     # or 'master' if not a tracking branch (it's based on a specific rev/hash)
     # or it returns None if it couldn't find an upstream
-    if cur_branch is None:
-      upstream_branch = None
-      current_type = "detached"
-      logging.debug("Detached HEAD")
+    upstream_branch = scm.GIT.GetUpstreamBranch(self.checkout_path)
+    if not upstream_branch or not upstream_branch.startswith('refs/remotes'):
+      current_type = "hash"
+      logging.debug("Current branch is not tracking an upstream (remote)"
+                    " branch.")
+    elif upstream_branch.startswith('refs/remotes'):
+      current_type = "branch"
     else:
-      upstream_branch = scm.GIT.GetUpstreamBranch(self.checkout_path)
-      if not upstream_branch or not upstream_branch.startswith('refs/remotes'):
-        current_type = "hash"
-        logging.debug("Current branch is not tracking an upstream (remote)"
-                      " branch.")
-      elif upstream_branch.startswith('refs/remotes'):
-        current_type = "branch"
-      else:
-        raise gclient_utils.Error('Invalid Upstream: %s' % upstream_branch)
-
-    if 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)
+      raise gclient_utils.Error('Invalid Upstream: %s' % upstream_branch)
 
     # This is a big hammer, debatable if it should even be here...
     if options.force or options.reset:
@@ -678,7 +717,8 @@ class GitWrapper(SCMWrapper):
         file_list.extend(
             [os.path.join(self.checkout_path, f) for f in rebase_files])
 
-    # If the rebase generated a conflict, abort and ask user to fix
+    ### Section 6: Final cleanup.
+    # If the rebase generated a conflict, abort and ask user to fix.
     if self._IsRebasing():
       raise gclient_utils.Error('\n____ %s at %s\n'
                                 '\nConflict while rebasing this branch.\n'
@@ -847,18 +887,17 @@ class GitWrapper(SCMWrapper):
                     lock_timeout=getattr(options, 'lock_timeout', 0))
     mirror.unlock()
 
-  def _Clone(self, revision, url, options):
-    """Clone a git repository from the given URL.
+  def _Clone(self, url, options):
+    """Perform a bare clone a git repository from the given URL.
 
-    Once we've cloned the repo, we checkout a working branch if the specified
-    revision is a branch head. If it is a tag or a specific commit, then we
-    leave HEAD detached as it makes future updates simpler -- in this case the
-    user should first create a new branch or switch to an existing branch before
-    making changes in the repo."""
+    Does not perform any checkout. Only fetches the default set of refs that
+    a normal clone would get, and does not set up any custom fetch specs.
+    """
     if not options.verbose:
       # git clone doesn't seem to insert a newline properly before printing
       # to stdout
       self.Print('')
+
     cfg = gclient_utils.DefaultIndexPackConfig(url)
     clone_cmd = cfg + ['clone', '--no-checkout', '--progress']
     if self.cache_dir:
@@ -892,6 +931,7 @@ class GitWrapper(SCMWrapper):
     tmp_dir = tempfile.mkdtemp(
         prefix='_gclient_%s_' % os.path.basename(self.checkout_path),
         dir=parent_dir)
+
     try:
       clone_cmd.append(tmp_dir)
       self._Run(clone_cmd, options, cwd=self._root_dir, retry=True)
@@ -907,16 +947,11 @@ class GitWrapper(SCMWrapper):
       gclient_utils.rmtree(tmp_dir)
       if template_dir:
         gclient_utils.rmtree(template_dir)
-    self._UpdateBranchHeads(options, fetch=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(
-        ('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))
+
+    if not verbose:
+      # Make the output a little prettier. It's nice to have some whitespace
+      # between projects when cloning.
+      self.Print('')
 
   def _AskForData(self, prompt, options):
     if options.jobs > 1:
@@ -930,7 +965,6 @@ class GitWrapper(SCMWrapper):
       # Hide the exception.
       sys.exit(1)
 
-
   def _AttemptRebase(self, upstream, files, options, newbase=None,
                      branch=None, printed_path=False, merge=False):
     """Attempt to rebase onto either upstream or, if specified, newbase."""
@@ -1016,29 +1050,6 @@ class GitWrapper(SCMWrapper):
       raise gclient_utils.Error('git version %s < minimum required %s' %
                                 (current_version, min_version))
 
-  def _EnsureValidHeadObjectOrCheckout(self, 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.
-    #   * git cache is used
-    # This manifests itself in current checkout having invalid HEAD commit on
-    # most git operations. Since git cache is used, just deleted the .git
-    # folder, and re-create it by cloning.
-    try:
-      self._Capture(['rev-list', '-n', '1', 'HEAD'])
-    except subprocess2.CalledProcessError as e:
-      if ('fatal: bad object HEAD' in e.stderr
-          and self.cache_dir and self.cache_dir in url):
-        self.Print((
-          'Likely due to DEPS change with git cache_dir, '
-          'the current commit points to no longer existing object.\n'
-          '%s' % e)
-        )
-        self._DeleteOrMove(options.force)
-        self._Clone(revision, url, options)
-      else:
-        raise
-
   def _IsRebasing(self):
     # Check for any of REBASE-i/REBASE-m/REBASE/AM. Unfortunately git doesn't
     # have a plumbing command to determine whether a rebase is in progress, so
@@ -1133,12 +1144,15 @@ class GitWrapper(SCMWrapper):
     checkout_args.append(ref)
     return self._Capture(checkout_args)
 
-  def _Fetch(self, options, remote=None, prune=False, quiet=False):
+  def _Fetch(
+      self, options, remote=None, refspec=None, prune=False, quiet=False):
     cfg = gclient_utils.DefaultIndexPackConfig(self.url)
+    remote = remote or self.remote
+    refspec = refspec or []
     fetch_cmd =  cfg + [
         'fetch',
-        remote or self.remote,
-    ]
+        remote,
+    ] + refspec
 
     if prune:
       fetch_cmd.append('--prune')
@@ -1151,24 +1165,18 @@ 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):
-    """Adds, and optionally fetches, "branch-heads" and "tags" refspecs
-    if requested."""
-    need_fetch = fetch
+  def _ConfigBranchHeads(self, options):
+    """Adds "branch-heads" and "tags" refspecs if requested."""
     if hasattr(options, 'with_branch_heads') and options.with_branch_heads:
       config_cmd = ['config', 'remote.%s.fetch' % self.remote,
                     '+refs/branch-heads/*:refs/remotes/branch-heads/*',
                     '^\\+refs/branch-heads/\\*:.*$']
       self._Run(config_cmd, options)
-      need_fetch = True
     if hasattr(options, 'with_tags') and options.with_tags:
       config_cmd = ['config', 'remote.%s.fetch' % self.remote,
                     '+refs/tags/*:refs/tags/*',
                     '^\\+refs/tags/\\*:.*$']
       self._Run(config_cmd, options)
-      need_fetch = True
-    if fetch and need_fetch:
-      self._Fetch(options, prune=options.force)
 
   def _Run(self, args, options, show_header=True, **kwargs):
     # Disable 'unused options' warning | pylint: disable=W0613