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

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>

git_command.py

@@ -395,7 +395,7 @@ class GitCommand(object):
           s_in.remove(s)
           continue
         if not hasattr(buf, 'encode'):
-          buf = buf.decode()
+          buf = buf.decode('utf-8', 'backslashreplace')
         if s.std_name == 'stdout':
           self.stdout += buf
         else:

git_superproject.py

@@ -25,10 +25,13 @@ Examples:
 import os
 import sys
 
-from error import GitError
+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.
@@ -47,6 +50,10 @@ class Superproject(object):
     self._repodir = os.path.abspath(repodir)
     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_MANIFEST_NAME)
+    self._work_git = os.path.join(self._superproject_path,
+                                  _SUPERPROJECT_GIT_NAME)
 
   @property
   def project_shas(self):
@@ -58,12 +65,14 @@ class Superproject(object):
 
     Args:
       url: superproject's url to be passed to git clone.
-      branch: the branchname to be passed as argument to git clone.
+      branch: The branchname to be passed as argument to git clone.
 
     Returns:
       True if 'git clone <url> <branch>' is successful, or False.
     """
-    cmd = ['clone', url, '--depth', '1']
+    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,
@@ -80,6 +89,29 @@ class Superproject(object):
       return False
     return True
 
+  def _Fetch(self):
+    """Do a 'git fetch' to to fetch the latest content.
+
+    Returns:
+      True if 'git fetch' is successful, or False.
+    """
+    if not os.path.exists(self._work_git):
+      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,
+                   capture_stdout=True,
+                   capture_stderr=True)
+    retval = p.Wait()
+    if retval:
+      print('repo: error: git fetch call failed with return code: %r, stderr: %r' %
+            (retval, p.stderr), file=sys.stderr)
+      return False
+    return True
+
   def _LsTree(self):
     """Returns the data from 'git ls-tree -r HEAD'.
 
@@ -88,14 +120,15 @@ class Superproject(object):
     Returns:
       data: data returned from 'git ls-tree -r HEAD' instead of None.
     """
-    git_dir = os.path.join(self._superproject_path, 'superproject')
-    if not os.path.exists(git_dir):
-      raise GitError('git ls-tree. Missing drectory: %s' % git_dir)
+    if not os.path.exists(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,
                    cmd,
-                   cwd=git_dir,
+                   cwd=self._work_git,
                    capture_stdout=True,
                    capture_stderr=True)
     retval = p.Wait()
@@ -108,25 +141,29 @@ class Superproject(object):
           retval, p.stderr), file=sys.stderr)
     return data
 
-  def GetAllProjectsSHAs(self, url, branch=None):
+  def _GetAllProjectsSHAs(self, url, branch=None):
     """Get SHAs for all projects from superproject and save them in _project_shas.
 
     Args:
-      url: superproject's url to be passed to git clone.
-      branch: the branchname to be passed as argument to git clone.
+      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.')
-    if os.path.exists(self._superproject_path):
-      platform_utils.rmtree(self._superproject_path)
-    os.mkdir(self._superproject_path)
 
-    # TODO(rtenneti): we shouldn't be cloning the repo from scratch every time.
-    if not self._Clone(url, branch):
-      raise GitError('git clone failed for url: %s' % url)
+    do_clone = True
+    if os.path.exists(self._superproject_path):
+      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
+    if do_clone:
+      if not self._Clone(url, branch):
+        raise GitError('git clone failed for url: %s' % url)
 
     data = self._LsTree()
     if not data:
@@ -147,3 +184,67 @@ class Superproject(object):
 
     self._project_shas = shas
     return shas
+
+  def _WriteManfiestFile(self, manifest):
+    """Writes manifest to a file.
+
+    Args:
+      manifest: A Manifest object that is to be written to a file.
+
+    Returns:
+      manifest_path: Path name of the file into which manifest is written instead of None.
+    """
+    if not os.path.exists(self._superproject_path):
+      print('error: missing superproject directory %s' %
+            self._superproject_path,
+            file=sys.stderr)
+      return None
+    manifest_str = manifest.ToXml().toxml()
+    manifest_path = self._manifest_path
+    try:
+      with open(manifest_path, 'w', encoding='utf-8') as fp:
+        fp.write(manifest_str)
+    except IOError as e:
+      print('error: cannot write manifest to %s:\n%s'
+            % (manifest_path, e),
+            file=sys.stderr)
+      return None
+    return manifest_path
+
+  def UpdateProjectsRevisionId(self, manifest, projects, url, branch=None):
+    """Update revisionId of every project in projects with the SHA.
+
+    Args:
+      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 fetch.
+
+    Returns:
+      manifest_path: Path name of the overriding manfiest file instead of None.
+    """
+    try:
+      shas = self._GetAllProjectsSHAs(url=url, branch=branch)
+    except Exception as e:
+      print('error: Cannot get project SHAs for %s: %s: %s' %
+            (url, type(e).__name__, str(e)),
+            file=sys.stderr)
+      return None
+
+    projects_missing_shas = []
+    for project in projects:
+      path = project.relpath
+      if not path:
+        continue
+      sha = shas.get(path)
+      if sha:
+        project.SetRevisionId(sha)
+      else:
+        projects_missing_shas.append(path)
+    if projects_missing_shas:
+      print('error: please file a bug using %s to report missing shas for: %s' %
+            (BUG_REPORT_URL, projects_missing_shas), file=sys.stderr)
+      return None
+
+    manifest_path = self._WriteManfiestFile(manifest)
+    return manifest_path

manifest_xml.py

@@ -403,6 +403,8 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
         revision = self.remotes[p.remote.orig_name].revision or d.revisionExpr
         if not revision or revision != p.revisionExpr:
           e.setAttribute('revision', p.revisionExpr)
+        elif p.revisionId:
+          e.setAttribute('revision', p.revisionId)
         if (p.upstream and (p.upstream != p.revisionExpr or
                             p.upstream != d.upstreamExpr)):
           e.setAttribute('upstream', p.upstream)

project.py

@@ -438,6 +438,7 @@ class RemoteSpec(object):
     self.orig_name = orig_name
     self.fetchUrl = fetchUrl
 
+
 class Project(object):
   # These objects can be shared between several working trees.
   shareable_files = ['description', 'info']
@@ -1927,7 +1928,8 @@ class Project(object):
     try:
       # if revision (sha or tag) is not present then following function
       # throws an error.
-      self.bare_git.rev_parse('--verify', '%s^0' % self.revisionExpr)
+      self.bare_git.rev_list('-1', '--missing=allow-any',
+                             '%s^0' % self.revisionExpr, '--')
       return True
     except GitError:
       # There is no such persistent revision. We have to fetch it.

subcmds/sync.py

@@ -56,7 +56,7 @@ import gitc_utils
 from project import Project
 from project import RemoteSpec
 from command import Command, MirrorSafeCommand
-from error import BUG_REPORT_URL, RepoChangedException, GitError, ManifestParseError
+from error import RepoChangedException, GitError, ManifestParseError
 import platform_utils
 from project import SyncBuffer
 from progress import Progress
@@ -271,6 +271,58 @@ 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.
+
+    This function updates each project's revisionId with SHA from superproject.
+    It writes the updated manifest into a file and reloads the manifest from it.
+
+    Args:
+      opt: Program options returned from optparse.  See _Options().
+      args: Arguments to pass to GetProjects. See the GetProjects
+          docstring for details.
+
+    Returns:
+      Returns path to the overriding manifest file.
+    """
+    if not self.manifest.superproject:
+      print('error: superproject tag is not defined in manifest.xml',
+            file=sys.stderr)
+      sys.exit(1)
+    print('WARNING: --use-superproject is experimental and not '
+          'for general use', file=sys.stderr)
+
+    superproject_url = self.manifest.superproject['remote'].url
+    if not superproject_url:
+      print('error: superproject URL is not defined in manifest.xml',
+            file=sys.stderr)
+      sys.exit(1)
+
+    superproject = git_superproject.Superproject(self.manifest.repodir)
+    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,
+                                                          branch=branch)
+    if not manifest_path:
+      print('error: Update of revsionId from superproject has failed',
+            file=sys.stderr)
+      sys.exit(1)
+    self._ReloadManifest(manifest_path)
+    return manifest_path
+
   def _FetchProjectList(self, opt, projects, sem, *args, **kwargs):
     """Main function of the fetch threads.
 
@@ -561,8 +613,9 @@ later is required to fix a server side protocol bug.
       # Make sure pruning never kicks in with shared projects.
       if (not project.use_git_worktrees and
               len(project.manifest.GetProjectsWithName(project.name)) > 1):
-        print('%s: Shared project %s found, disabling pruning.' %
-              (project.relpath, project.name))
+        if not opt.quiet:
+          print('%s: Shared project %s found, disabling pruning.' %
+                (project.relpath, project.name))
         if git_require((2, 7, 0)):
           project.EnableRepositoryExtension('preciousObjects')
         else:
@@ -711,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']
@@ -858,6 +907,9 @@ later is required to fix a server side protocol bug.
     else:
       self._UpdateManifestProject(opt, mp, manifest_name)
 
+    if opt.use_superproject:
+      manifest_name = self._UpdateProjectsRevisionId(opt, args)
+
     if self.gitc_manifest:
       gitc_manifest_projects = self.GetProjects(args,
                                                 missing_ok=True)
@@ -897,41 +949,6 @@ later is required to fix a server side protocol bug.
                                     missing_ok=True,
                                     submodules_ok=opt.fetch_submodules)
 
-    if opt.use_superproject:
-      if not self.manifest.superproject:
-        print('error: superproject tag is not defined in manifest.xml',
-              file=sys.stderr)
-        sys.exit(1)
-      print('WARNING: --use-superproject is experimental and not '
-            'for general use', file=sys.stderr)
-      superproject_url = self.manifest.superproject['remote'].url
-      if not superproject_url:
-        print('error: superproject URL is not defined in manifest.xml',
-              file=sys.stderr)
-        sys.exit(1)
-      superproject = git_superproject.Superproject(self.manifest.repodir)
-      try:
-        superproject_shas = superproject.GetAllProjectsSHAs(url=superproject_url)
-      except Exception as e:
-        print('error: Cannot get project SHAs for %s: %s: %s' %
-              (superproject_url, type(e).__name__, str(e)),
-              file=sys.stderr)
-        sys.exit(1)
-      projects_missing_shas = []
-      for project in all_projects:
-        path = project.relpath
-        if not path:
-          continue
-        sha = superproject_shas.get(path)
-        if sha:
-          project.SetRevisionId(sha)
-        else:
-          projects_missing_shas.append(path)
-      if projects_missing_shas:
-        print('error: please file a bug using %s to report missing shas for: %s' %
-              (BUG_REPORT_URL, projects_missing_shas), file=sys.stderr)
-        sys.exit(1)
-
     err_network_sync = False
     err_update_projects = False
     err_checkout = False

tests/test_git_superproject.py

@@ -21,6 +21,7 @@ from unittest import mock
 
 from error import GitError
 import git_superproject
+import manifest_xml
 import platform_utils
 
 
@@ -31,27 +32,43 @@ class SuperprojectTestCase(unittest.TestCase):
     """Set up superproject every time."""
     self.tempdir = tempfile.mkdtemp(prefix='repo_tests')
     self.repodir = os.path.join(self.tempdir, '.repo')
-    os.mkdir(self.repodir)
     self._superproject = git_superproject.Superproject(self.repodir)
+    self.manifest_file = os.path.join(
+        self.repodir, manifest_xml.MANIFEST_FILE_NAME)
+    os.mkdir(self.repodir)
+
+    # The manifest parsing really wants a git repo currently.
+    gitdir = os.path.join(self.repodir, 'manifests.git')
+    os.mkdir(gitdir)
+    with open(os.path.join(gitdir, 'config'), 'w') as fp:
+      fp.write("""[remote "origin"]
+        url = https://localhost:0/manifest
+""")
 
   def tearDown(self):
     """Tear down superproject every time."""
     platform_utils.rmtree(self.tempdir)
 
+  def getXmlManifest(self, data):
+    """Helper to initialize a manifest for testing."""
+    with open(self.manifest_file, 'w') as fp:
+      fp.write(data)
+    return manifest_xml.XmlManifest(self.repodir, self.manifest_file)
+
   def test_superproject_get_project_shas_no_url(self):
     """Test with no url."""
     with self.assertRaises(ValueError):
-      self._superproject.GetAllProjectsSHAs(url=None)
+      self._superproject._GetAllProjectsSHAs(url=None)
 
   def test_superproject_get_project_shas_invalid_url(self):
     """Test with an invalid url."""
     with self.assertRaises(GitError):
-      self._superproject.GetAllProjectsSHAs(url='localhost')
+      self._superproject._GetAllProjectsSHAs(url='localhost')
 
   def test_superproject_get_project_shas_invalid_branch(self):
     """Test with an invalid branch."""
     with self.assertRaises(GitError):
-      self._superproject.GetAllProjectsSHAs(
+      self._superproject._GetAllProjectsSHAs(
           url='sso://android/platform/superproject',
           branch='junk')
 
@@ -59,7 +76,14 @@ class SuperprojectTestCase(unittest.TestCase):
     """Test with _Clone failing."""
     with self.assertRaises(GitError):
       with mock.patch.object(self._superproject, '_Clone', return_value=False):
-        self._superproject.GetAllProjectsSHAs(url='localhost')
+        self._superproject._GetAllProjectsSHAs(url='localhost')
+
+  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, '_Fetch', return_value=False):
+          self._superproject._GetAllProjectsSHAs(url='localhost')
 
   def test_superproject_get_project_shas_mock_ls_tree(self):
     """Test with LsTree being a mock."""
@@ -70,13 +94,71 @@ class SuperprojectTestCase(unittest.TestCase):
             '160000 commit ade9b7a0d874e25fff4bf2552488825c6f111928\tbuild/bazel\x00')
     with mock.patch.object(self._superproject, '_Clone', return_value=True):
       with mock.patch.object(self._superproject, '_LsTree', return_value=data):
-        shas = self._superproject.GetAllProjectsSHAs(url='localhost', branch='junk')
+        shas = self._superproject._GetAllProjectsSHAs(url='localhost', branch='junk')
         self.assertEqual(shas, {
             'art': '2c2724cb36cd5a9cec6c852c681efc3b7c6b86ea',
             'bootable/recovery': 'e9d25da64d8d365dbba7c8ee00fe8c4473fe9a06',
             'build/bazel': 'ade9b7a0d874e25fff4bf2552488825c6f111928'
         })
 
+  def test_superproject_write_manifest_file(self):
+    """Test with writing manifest to a file after setting revisionId."""
+    manifest = self.getXmlManifest("""
+<manifest>
+  <remote name="default-remote" fetch="http://localhost" />
+  <default remote="default-remote" revision="refs/heads/main" />
+  <project name="test-name"/>
+</manifest>
+""")
+    self.assertEqual(len(manifest.projects), 1)
+    project = manifest.projects[0]
+    project.SetRevisionId('ABCDEF')
+    # Create temporary directory so that it can write the file.
+    os.mkdir(self._superproject._superproject_path)
+    manifest_path = self._superproject._WriteManfiestFile(manifest)
+    self.assertIsNotNone(manifest_path)
+    with open(manifest_path, 'r') as fp:
+      manifest_xml = fp.read()
+    self.assertEqual(
+        manifest_xml,
+        '<?xml version="1.0" ?><manifest>' +
+        '<remote name="default-remote" fetch="http://localhost"/>' +
+        '<default remote="default-remote" revision="refs/heads/main"/>' +
+        '<project name="test-name" revision="ABCDEF"/>' +
+        '</manifest>')
+
+  def test_superproject_update_project_revision_id(self):
+    """Test with LsTree being a mock."""
+    manifest = self.getXmlManifest("""
+<manifest>
+  <remote name="default-remote" fetch="http://localhost" />
+  <default remote="default-remote" revision="refs/heads/main" />
+  <project path="art" name="platform/art" />
+</manifest>
+""")
+    self.assertEqual(len(manifest.projects), 1)
+    projects = manifest.projects
+    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, '_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)
+          manifest_path = self._superproject.UpdateProjectsRevisionId(
+              manifest, projects, url='localhost')
+          self.assertIsNotNone(manifest_path)
+          with open(manifest_path, 'r') as fp:
+            manifest_xml = fp.read()
+          self.assertEqual(
+              manifest_xml,
+              '<?xml version="1.0" ?><manifest>' +
+              '<remote name="default-remote" fetch="http://localhost"/>' +
+              '<default remote="default-remote" revision="refs/heads/main"/>' +
+              '<project name="platform/art" path="art" ' +
+              'revision="2c2724cb36cd5a9cec6c852c681efc3b7c6b86ea"/>' +
+              '</manifest>')
+
 
 if __name__ == '__main__':
   unittest.main()

tests/test_manifest_xml.py

@@ -327,6 +327,26 @@ class XmlManifestTests(unittest.TestCase):
         result['extras'],
         ['g1', 'g2', 'g1', 'name:extras', 'all', 'path:path'])
 
+  def test_project_set_revision_id(self):
+    """Check setting of project's revisionId."""
+    manifest = self.getXmlManifest("""
+<manifest>
+  <remote name="default-remote" fetch="http://localhost" />
+  <default remote="default-remote" revision="refs/heads/main" />
+  <project name="test-name"/>
+</manifest>
+""")
+    self.assertEqual(len(manifest.projects), 1)
+    project = manifest.projects[0]
+    project.SetRevisionId('ABCDEF')
+    self.assertEqual(
+        manifest.ToXml().toxml(),
+        '<?xml version="1.0" ?><manifest>' +
+        '<remote name="default-remote" fetch="http://localhost"/>' +
+        '<default remote="default-remote" revision="refs/heads/main"/>' +
+        '<project name="test-name" revision="ABCDEF"/>' +
+        '</manifest>')
+
   def test_include_levels(self):
     root_m = os.path.join(self.manifest_dir, 'root.xml')
     with open(root_m, 'w') as fp: