Allow gclient clone in non-empty directories

gclient_scm.py

Index: gclient_scm.py
diff --git a/gclient_scm.py b/gclient_scm.py
index 5a5dc075b3c6c4b5ffb81b11c9702a37e1815dc2..8883d3132a73c31bbc8fec2c51dcc059e63c883e 100644
--- a/gclient_scm.py
+++ b/gclient_scm.py
@@ -10,6 +10,7 @@ import os
 import posixpath
 import re
 import sys
+import tempfile
 import threading
 import time
 
@@ -339,21 +340,9 @@ class GitWrapper(SCMWrapper):
       printed_path = True
 
     url = self._CreateOrUpdateCache(url, options)
-
-    if revision.startswith('refs/'):
-      rev_type = "branch"
-    elif revision.startswith('origin/'):
-      # For compatability with old naming, translate 'origin' to 'refs/heads'
-      revision = revision.replace('origin/', 'refs/heads/')
-      rev_type = "branch"
-    else:
-      # hash is also a tag, only make a distinction at checkout
-      rev_type = "hash"
-
-    if not os.path.exists(self.checkout_path) or (
-        os.path.isdir(self.checkout_path) and
-        not os.listdir(self.checkout_path)):
-      gclient_utils.safe_makedirs(os.path.dirname(self.checkout_path))
+    if (not os.path.exists(self.checkout_path) or
+        (os.path.isdir(self.check_path) and

[M-A Ruel, 2013/07/18 14:55:04]

What's check_path? Was this code path ever exercised? If not, look at exercising
it.

[Isaac (away), 2013/07/19 20:13:23]

should be checkout_path, I uploaded w/ bypass-hooks to get a general review. 
This is the new code path -- checking into an existing directory that's not
already a git repo.

+         not os.path.exists(os.path.join(self.checkout_path, '.git')))):
       self._Clone(revision, url, options)
       self.UpdateSubmoduleConfig()
       if file_list is not None:
@@ -400,15 +389,11 @@ class GitWrapper(SCMWrapper):
       self._FetchAndReset(revision, file_list, options)
       return
 
-    if not self._IsValidGitRepo():
-      # .git directory is hosed for some reason, set it back up.
-      print('_____ %s/.git is corrupted, rebuilding' % self.relpath)
-      self._Run(['init'], options)
-      self._Run(['remote', 'set-url', 'origin', url], options)
-
     if not self._HasHead():
       # Previous checkout was aborted before branches could be created in repo,
       # so we need to reconstruct them here.
+      self._Run(['init'], options)
+      self._Run(['remote', 'set-url', 'origin', url], options)
       self._Run(['-c', 'core.deltaBaseCacheLimit=2g', 'pull', 'origin',
                  'master'], options)
       self._FetchAndReset(revision, file_list, options)
@@ -435,6 +420,16 @@ class GitWrapper(SCMWrapper):
     # 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 revision.startswith('refs/'):
+      rev_type = "branch"
+    elif revision.startswith('origin/'):
+      # For compatability with old naming, translate 'origin' to 'refs/heads'
+      revision = revision.replace('origin/', 'refs/heads/')
+      rev_type = "branch"
+    else:
+      # hash is also a tag, only make a distinction at checkout
+      rev_type = "hash"
+
     if cur_branch is None:
       upstream_branch = None
       current_type = "detached"
@@ -775,66 +770,62 @@ class GitWrapper(SCMWrapper):
     return folder
 
   def _Clone(self, 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
-    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."""
+    """Clone a git repository from the given URL."""
     if not options.verbose:
       # git clone doesn't seem to insert a newline properly before printing
       # to stdout
       print('')
     template_path = os.path.join(
         os.path.dirname(THIS_FILE_PATH), 'git-templates')
-    clone_cmd = ['-c', 'core.deltaBaseCacheLimit=2g', 'clone', '--progress',
-                 '--template=%s' % template_path]
+    clone_cmd = ['-c', 'core.deltaBaseCacheLimit=2g', 'clone', '--no-checkout',
+                 '--progress', '--template=%s' % template_path]
     if self.cache_dir:
       clone_cmd.append('--shared')
-    if revision.startswith('refs/heads/'):
-      clone_cmd.extend(['-b', revision.replace('refs/heads/', '')])
-      detach_head = False
-    else:
-      detach_head = True
     if options.verbose:
       clone_cmd.append('--verbose')
-    clone_cmd.extend([url, self.checkout_path])
-
+    clone_cmd.append(url)
     # If the parent directory does not exist, Git clone on Windows will not
     # create it, so we need to do it manually.
     parent_dir = os.path.dirname(self.checkout_path)
     if not os.path.exists(parent_dir):
       gclient_utils.safe_makedirs(parent_dir)
-
-    for _ in range(3):
-      try:
-        self._Run(clone_cmd, options, cwd=self._root_dir, git_filter=True)
-        break
-      except subprocess2.CalledProcessError, e:
-        # Too bad we don't have access to the actual output yet.
-        # We should check for "transfer closed with NNN bytes remaining to
-        # read". In the meantime, just make sure .git exists.
-        if (e.returncode == 128 and
-            os.path.exists(os.path.join(self.checkout_path, '.git'))):
-          print(str(e))
-          print('Retrying...')
-          continue
-        raise e
-
-    # Update the "branch-heads" remote-tracking branches, since we might need it
-    # to checkout a specific revision below.
-    self._UpdateBranchHeads(options, fetch=True)
-
-    if detach_head:
+    try:
+      tmp_dir = tempfile.mkdtemp(

[M-A Ruel, 2013/07/18 14:55:04]

In practice, you want to have this just before the try, so that if this call
fails, like /tmp is full, then the finally has less chances of throwing itself.

[Isaac (away), 2013/07/19 20:13:23]

Done.

+          suffix='_%s' % os.path.basename(self.checkout_path),

[M-A Ruel, 2013/07/18 14:55:04]

I'd also prefer a prefix, so it's easier to remove, e.g. "rm -rf gclient_*"

[Isaac (away), 2013/07/19 20:13:23]

Done.

+          dir=parent_dir)
+      clone_cmd.append(tmp_dir)
+      for _ in range(3):
+        try:
+          self._Run(clone_cmd, options, cwd=None, git_filter=True)
+          break
+        except subprocess2.CalledProcessError, e:
+          # Too bad we don't have access to the actual output yet.
+          # We should check for "transfer closed with NNN bytes remaining to
+          # read". In the meantime, just make sure .git exists.
+          if (e.returncode == 128 and
+              os.path.exists(os.path.join(tmp_dir, '.git'))):
+            print(str(e))
+            print('Retrying...')
+            continue
+          raise e
+      os.rename(os.path.join(tmp_dir, '.git'),
+                os.path.join(self.checkout_path, '.git'))
+    finally:
+      if os.listdir(tmp_dir):
+        print('\n_____ removing non-empty tmp dir %s' % tmp_dir)
+      gclient_utils.rmtree(tmp_dir)
+    if revision.startswith('refs/heads/'):
+      self._Run(['checkout', revision.replace('refs/heads/', '')])
+    else:
       # Squelch git's very verbose detached HEAD warning and use our own
-      self._Capture(['checkout', '--quiet', '%s' % revision])
+      self._Run(['checkout', '--quiet', revision])
       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 origin -b <branch>\' to\n'
          'create a new branch for your work.') % revision)
 
+

[M-A Ruel, 2013/07/18 14:55:04]

Remove, it's not between file level symbols.

   def _AttemptRebase(self, upstream, files, options, newbase=None,
                      branch=None, printed_path=False):
     """Attempt to rebase onto either upstream or, if specified, newbase."""
@@ -906,17 +897,6 @@ class GitWrapper(SCMWrapper):
       # whitespace between projects when syncing.
       print('')
 
-  def _IsValidGitRepo(self):
-    """Returns if the directory is a valid git repository.
-
-    Checks if git status works.
-    """
-    try:
-      self._Capture(['status'])
-      return True
-    except subprocess2.CalledProcessError:
-      return False
-
   def _HasHead(self):
     """Returns True if any commit is checked out.