sync: pass --bare option when doing git clone of superproject.v2.12.1

Changed "git pull" to "git fetch" as we are using --bare option. Used the
following command to fetch:
  git fetch origin +refs/heads/*:refs/heads/* --prune

Pass --branch argument to Superproject's UpdateProjectsRevisionId function.

Returned False/None when directories don't exist instead of raise
GitError exception from _Fetch and _LsTree functions. The caller of Fetch
does Clone if Fetch fails.

Tested the code with the following commands.

$ ./run_tests -v

Tested the init and sync code by copying all the repo changes into my Android
AOSP checkout and running repo sync with --use-superproject option.

Bug: https://crbug.com/gerrit/13709
Bug: https://crbug.com/gerrit/13707
Tested-by: Raman Tenneti <rtenneti@google.com>
Change-Id: I3e441ecdfc87c735f46eff0eb98efa63cc2eb22a
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/296222
Reviewed-by: Mike Frysinger <vapier@google.com>

3 files changed, 43 insertions, 26 deletions

diff --git a/git_superproject.py b/git_superproject.py
index 465d1f87..57a3a533 100644
--- a/git_superproject.py
+++ b/git_superproject.py
@@ -29,6 +29,9 @@ from error import BUG_REPORT_URL, GitError
 from git_command import GitCommand
 import platform_utils
 
+_SUPERPROJECT_GIT_NAME = 'superproject.git'
+_SUPERPROJECT_MANIFEST_NAME = 'superproject_override.xml'
+
 
 class Superproject(object):
   """Get SHAs from superproject.
@@ -48,8 +51,9 @@ class Superproject(object):
     self._superproject_dir = superproject_dir
     self._superproject_path = os.path.join(self._repodir, superproject_dir)
     self._manifest_path = os.path.join(self._superproject_path,
-                                       'superproject_override.xml')
-    self._work_git = os.path.join(self._superproject_path, 'superproject')
+                                       _SUPERPROJECT_MANIFEST_NAME)
+    self._work_git = os.path.join(self._superproject_path,
+                                  _SUPERPROJECT_GIT_NAME)
 
   @property
   def project_shas(self):
@@ -66,8 +70,9 @@ class Superproject(object):
     Returns:
       True if 'git clone <url> <branch>' is successful, or False.
     """
-    os.mkdir(self._superproject_path)
-    cmd = ['clone', url, '--filter', 'blob:none']
+    if not os.path.exists(self._superproject_path):
+      os.mkdir(self._superproject_path)
+    cmd = ['clone', url, '--filter', 'blob:none', '--bare']
     if branch:
       cmd += ['--branch', branch]
     p = GitCommand(None,
@@ -84,15 +89,17 @@ class Superproject(object):
       return False
     return True
 
-  def _Pull(self):
-    """Do a 'git pull' to to fetch the latest content.
+  def _Fetch(self):
+    """Do a 'git fetch' to to fetch the latest content.
 
     Returns:
-      True if 'git pull <branch>' is successful, or False.
+      True if 'git fetch' is successful, or False.
     """
     if not os.path.exists(self._work_git):
-      raise GitError('git pull missing drectory: %s' % self._work_git)
-    cmd = ['pull']
+      print('git fetch missing drectory: %s' % self._work_git,
+            file=sys.stderr)
+      return False
+    cmd = ['fetch', 'origin', '+refs/heads/*:refs/heads/*', '--prune']
     p = GitCommand(None,
                    cmd,
                    cwd=self._work_git,
@@ -100,7 +107,7 @@ class Superproject(object):
                    capture_stderr=True)
     retval = p.Wait()
     if retval:
-      print('repo: error: git pull call failed with return code: %r, stderr: %r' %
+      print('repo: error: git fetch call failed with return code: %r, stderr: %r' %
             (retval, p.stderr), file=sys.stderr)
       return False
     return True
@@ -114,7 +121,9 @@ class Superproject(object):
       data: data returned from 'git ls-tree -r HEAD' instead of None.
     """
     if not os.path.exists(self._work_git):
-      raise GitError('git ls-tree. Missing drectory: %s' % self._work_git)
+      print('git ls-tree missing drectory: %s' % self._work_git,
+            file=sys.stderr)
+      return None
     data = None
     cmd = ['ls-tree', '-z', '-r', 'HEAD']
     p = GitCommand(None,
@@ -136,18 +145,19 @@ class Superproject(object):
     """Get SHAs for all projects from superproject and save them in _project_shas.
 
     Args:
-      url: superproject's url to be passed to git clone or pull.
-      branch: The branchname to be passed as argument to git clone or pull.
+      url: superproject's url to be passed to git clone or fetch.
+      branch: The branchname to be passed as argument to git clone or fetch.
 
     Returns:
       A dictionary with the projects/SHAs instead of None.
     """
     if not url:
       raise ValueError('url argument is not supplied.')
+
     do_clone = True
     if os.path.exists(self._superproject_path):
-      if not self._Pull():
-        # If pull fails due to a corrupted git directory, then do a git clone.
+      if not self._Fetch():
+        # If fetch fails due to a corrupted git directory, then do a git clone.
         platform_utils.rmtree(self._superproject_path)
       else:
         do_clone = False
@@ -208,7 +218,7 @@ class Superproject(object):
       manifest: A Manifest object that is to be written to a file.
       projects: List of projects whose revisionId needs to be updated.
       url: superproject's url to be passed to git clone or fetch.
-      branch: The branchname to be passed as argument to git clone or pull.
+      branch: The branchname to be passed as argument to git clone or fetch.
 
     Returns:
       manifest_path: Path name of the overriding manfiest file instead of None.
diff --git a/subcmds/sync.py b/subcmds/sync.py
index c0f605a8..5855af55 100644
--- a/subcmds/sync.py
+++ b/subcmds/sync.py
@@ -271,6 +271,15 @@ later is required to fix a server side protocol bug.
                  dest='repo_upgraded', action='store_true',
                  help=SUPPRESS_HELP)
 
+  def _GetBranch(self):
+    """Returns the branch name for getting the approved manifest."""
+    p = self.manifest.manifestProject
+    b = p.GetBranch(p.CurrentBranch)
+    branch = b.merge
+    if branch.startswith(R_HEADS):
+      branch = branch[len(R_HEADS):]
+    return branch
+
   def _UpdateProjectsRevisionId(self, opt, args):
     """Update revisionId of every project with the SHA from superproject.
 
@@ -302,9 +311,11 @@ later is required to fix a server side protocol bug.
     all_projects = self.GetProjects(args,
                                     missing_ok=True,
                                     submodules_ok=opt.fetch_submodules)
+    branch = self._GetBranch()
     manifest_path = superproject.UpdateProjectsRevisionId(self.manifest,
                                                           all_projects,
-                                                          url=superproject_url)
+                                                          url=superproject_url,
+                                                          branch=branch)
     if not manifest_path:
       print('error: Update of revsionId from superproject has failed',
             file=sys.stderr)
@@ -753,11 +764,7 @@ later is required to fix a server side protocol bug.
     try:
       server = xmlrpc.client.Server(manifest_server, transport=transport)
       if opt.smart_sync:
-        p = self.manifest.manifestProject
-        b = p.GetBranch(p.CurrentBranch)
-        branch = b.merge
-        if branch.startswith(R_HEADS):
-          branch = branch[len(R_HEADS):]
+        branch = self._GetBranch()
 
         if 'SYNC_TARGET' in os.environ:
           target = os.environ['SYNC_TARGET']
diff --git a/tests/test_git_superproject.py b/tests/test_git_superproject.py
index d2c2f501..fc9101dd 100644
--- a/tests/test_git_superproject.py
+++ b/tests/test_git_superproject.py
@@ -78,11 +78,11 @@ class SuperprojectTestCase(unittest.TestCase):
       with mock.patch.object(self._superproject, '_Clone', return_value=False):
         self._superproject._GetAllProjectsSHAs(url='localhost')
 
-  def test_superproject_get_project_shas_mock_pull(self):
-    """Test with _Pull failing."""
+  def test_superproject_get_project_shas_mock_fetch(self):
+    """Test with _Fetch failing."""
     with self.assertRaises(GitError):
       with mock.patch.object(self._superproject, '_Clone', return_value=True):
-        with mock.patch.object(self._superproject, '_Pull', return_value=False):
+        with mock.patch.object(self._superproject, '_Fetch', return_value=False):
           self._superproject._GetAllProjectsSHAs(url='localhost')
 
   def test_superproject_get_project_shas_mock_ls_tree(self):
@@ -141,7 +141,7 @@ class SuperprojectTestCase(unittest.TestCase):
     data = ('160000 commit 2c2724cb36cd5a9cec6c852c681efc3b7c6b86ea\tart\x00'
             '160000 commit e9d25da64d8d365dbba7c8ee00fe8c4473fe9a06\tbootable/recovery\x00')
     with mock.patch.object(self._superproject, '_Clone', return_value=True):
-      with mock.patch.object(self._superproject, '_Pull', return_value=True):
+      with mock.patch.object(self._superproject, '_Fetch', return_value=True):
         with mock.patch.object(self._superproject, '_LsTree', return_value=data):
           # Create temporary directory so that it can write the file.
           os.mkdir(self._superproject._superproject_path)