git_utils.py: allow checkouts of local repositories at any commithash along master branch

py/utils/git_utils.py

Index: py/utils/git_utils.py
diff --git a/py/utils/git_utils.py b/py/utils/git_utils.py
index 52ad83a6409fd4ff17853e966ba69b68775cdc28..c2a67bce38a029d7ae6f692f7eac13dc64311c54 100644
--- a/py/utils/git_utils.py
+++ b/py/utils/git_utils.py
@@ -152,25 +152,25 @@ class GitBranch(object):
 class NewGitCheckout(object):
   """Creates a new local checkout of a Git repository."""
 
-  def __init__(self, repository, refspec=None, subdir=None,
-               containing_dir=None):
-    """Check out a new local copy of the repository.
+  def __init__(self, repository, commithash='HEAD',

[borenet, 2014/08/19 15:24:28]

Should the default for commithash be None, since you use "if self._commithash" ?
 "git checkout HEAD" will be a no-op.

[epoger, 2014/08/19 15:42:05]

I'd rather make the default behavior explicit, and minimize the number of
different code paths.  So I kept the default of 'HEAD' here and got rid of the
branch within __enter__().

+               subdir=None, containing_dir=None):
+    """Set parameters for this local copy of a Git repository.
 
     Because this is a new checkout, rather than a reference to an existing
     checkout on disk, it is safe to assume that the calling thread is the
     only thread manipulating the checkout.
 
-    You can use the 'with' statement to create this object in such a way that
-    it cleans up after itself:
+    You must use the 'with' statement to create this object:
 
     with NewGitCheckout(*args) as checkout:
       # use checkout instance
     # the checkout is automatically cleaned up here
 
     Args:
-      repository: name of the remote repository
-      refspec: an arbitrary remote ref (e.g., the name of a branch);
-          if None, allow the git command to pick a default
+      repository: URL of the remote repository (e.g.,
+          'https://skia.googlesource.com/common') or path to a local repository
+          (e.g., '/path/to/repo/.git') to check out a copy of
+      commithash: rewind the local checkout to a particular commithash
       subdir: if specified, the caller only wants access to files within this
           subdir in the repository.
           For now, we check out the entire repository regardless of this param,
@@ -180,19 +180,13 @@ class NewGitCheckout(object):
           within this directory; otherwise, a system-dependent default location
           will be used, as determined by tempfile.mkdtemp()
     """
-    # _git_root points to the tree holding the git checkout in its entirety;
-    # _file_root points to the files the caller wants to look at
-    self._git_root = tempfile.mkdtemp(dir=containing_dir)
-    if subdir:
-      self._file_root = os.path.join(self._git_root, subdir)
-    else:
-      self._file_root = self._git_root
+    self._repository = repository
+    self._commithash = commithash
+    self._subdir = subdir
+    self._containing_dir = containing_dir
+    self._git_root = None
+    self._file_root = None
 
-    pull_cmd = [GIT, 'pull', repository]
-    if refspec:
-      pull_cmd.append(refspec)
-    self._run_in_git_root(args=[GIT, 'init'])
-    self._run_in_git_root(args=pull_cmd)
 
   @property
   def root(self):
@@ -209,6 +203,22 @@ class NewGitCheckout(object):
         args=[GIT, 'rev-parse', 'HEAD']).strip()
 
   def __enter__(self):
+    """Check out a new local copy of the repository.
+
+    Uses the parameters that were passed into the constructor.
+    """
+    # _git_root points to the tree holding the git checkout in its entirety;
+    # _file_root points to the files the caller wants to look at
+    self._git_root = tempfile.mkdtemp(dir=self._containing_dir)
+    if self._subdir:
+      self._file_root = os.path.join(self._git_root, self._subdir)
+    else:
+      self._file_root = self._git_root
+
+    self._run_in_git_root(args=[GIT, 'clone', self._repository, '.'])
+    if self._commithash:
+      self._run_in_git_root(args=[GIT, 'checkout', self._commithash])

[borenet, 2014/08/19 15:24:28]

This doesn't create a new branch like your old code did.  Is that what you want?
 Also, in this case self._commithash can actually be a branch name, tag, or
commit hash.  It may be worth naming it that way.

[epoger, 2014/08/19 15:42:06]

We don't really need the new branch, so this is fine.  (The self-test exercises
all required functionality, and it still passes.)
I have followed the lead of "git help checkout" and named this parameter
"commit".

+
     return self
 
   # pylint: disable=W0622