superproject: require git version 2.28.0 or higher.

Tested the code with the following commands.

$ repo init --use-superproject -u https://android.googlesource.com/platform/manifest

+ Tested for the wrong git version.
  $ repo_dev init --use-superproject -u https://android.googlesource.com/platform/manifest
  WARNING: --use-superproject is experimental and not for general use
  superproject requires a git version 2.28 or later
  error: git update of superproject failed

$ ./run_tests -v

Bug: https://crbug.com/gerrit/14617
Bug: [google internal] b/189846687
Change-Id: I5cd4158ea29b3b3c8c81234f4e818165de346e63
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/308442
Tested-by: Raman Tenneti <rtenneti@google.com>
Reviewed-by: Mike Frysinger <vapier@google.com>

git_superproject.py

@@ -26,7 +26,7 @@ import hashlib
 import os
 import sys
 
-from git_command import GitCommand
+from git_command import git_require, GitCommand
 from git_refs import R_HEADS
 
 _SUPERPROJECT_GIT_NAME = 'superproject.git'
@@ -120,6 +120,9 @@ class Superproject(object):
       print('git fetch missing drectory: %s' % self._work_git,
             file=sys.stderr)
       return False
+    if not git_require((2, 28, 0)):
+      print('superproject requires a git version 2.28 or later', file=sys.stderr)
+      return False
     cmd = ['fetch', url, '--depth', '1', '--force', '--no-tags', '--filter', 'blob:none']
     if self._branch:
       cmd += [self._branch + ':' + self._branch]

project.py

@@ -2197,7 +2197,7 @@ class Project(object):
         ret = prunecmd.Wait()
         if ret:
           break
-        output_redir.write('retrying fetch after pruning remote branches')
+        print('retrying fetch after pruning remote branches', file=output_redir)
         # Continue right away so we don't sleep as we shouldn't need to.
         continue
       elif current_branch_only and is_sha1 and ret == 128:
@@ -2210,10 +2210,11 @@ class Project(object):
         break
 
       # Figure out how long to sleep before the next attempt, if there is one.
-      if not verbose:
-        output_redir.write('\n%s:\n%s' % (self.name, gitcmd.stdout))
+      if not verbose and gitcmd.stdout:
+        print('\n%s:\n%s' % (self.name, gitcmd.stdout), end='', file=output_redir)
       if try_n < retry_fetches - 1:
-        output_redir.write('sleeping %s seconds before retrying' % retry_cur_sleep)
+        print('%s: sleeping %s seconds before retrying' % (self.name, retry_cur_sleep),
+              file=output_redir)
         time.sleep(retry_cur_sleep)
         retry_cur_sleep = min(retry_exp_factor * retry_cur_sleep,
                               MAXIMUM_RETRY_SLEEP_SEC)

subcmds/sync.py

@@ -469,11 +469,14 @@ later is required to fix a server side protocol bug.
     Args:
       opt: Program options returned from optparse.  See _Options().
       args: Command line args used to filter out projects.
-      all_projects: List of all projects that should be checked out.
+      all_projects: List of all projects that should be fetched.
       err_event: Whether an error was hit while processing.
       manifest_name: Manifest file to be reloaded.
       load_local_manifests: Whether to load local manifests.
       ssh_proxy: SSH manager for clients & masters.
+
+    Returns:
+      List of all projects that should be checked out.
     """
     rp = self.manifest.repoProject
 
@@ -520,6 +523,8 @@ later is required to fix a server side protocol bug.
         err_event.set()
       fetched.update(new_fetched)
 
+    return all_projects
+
   def _CheckoutOne(self, detach_head, force_sync, project):
     """Checkout work tree for one project
 
@@ -1006,8 +1011,9 @@ later is required to fix a server side protocol bug.
         with ssh.ProxyManager(manager) as ssh_proxy:
           # Initialize the socket dir once in the parent.
           ssh_proxy.sock()
-          self._FetchMain(opt, args, all_projects, err_event, manifest_name,
-                          load_local_manifests, ssh_proxy)
+          all_projects = self._FetchMain(opt, args, all_projects, err_event,
+                                         manifest_name, load_local_manifests,
+                                         ssh_proxy)
 
       if opt.network_only:
         return

tests/test_git_superproject.py

@@ -23,6 +23,7 @@ from unittest import mock
 import git_superproject
 import manifest_xml
 import platform_utils
+from test_manifest_xml import sort_attributes
 
 
 class SuperprojectTestCase(unittest.TestCase):
@@ -140,12 +141,12 @@ class SuperprojectTestCase(unittest.TestCase):
     with open(manifest_path, 'r') as fp:
       manifest_xml = fp.read()
     self.assertEqual(
-        manifest_xml,
+        sort_attributes(manifest_xml),
         '<?xml version="1.0" ?><manifest>'
-        '<remote name="default-remote" fetch="http://localhost"/>'
+        '<remote fetch="http://localhost" name="default-remote"/>'
         '<default remote="default-remote" revision="refs/heads/main"/>'
-        '<project name="platform/art" path="art" revision="ABCDEF" '
-        'groups="notdefault,platform-' + self.platform + '"/>'
+        '<project groups="notdefault,platform-' + self.platform + '" '
+        'name="platform/art" path="art" revision="ABCDEF"/>'
         '<superproject name="superproject"/>'
         '</manifest>')
 
@@ -167,13 +168,13 @@ class SuperprojectTestCase(unittest.TestCase):
           with open(manifest_path, 'r') as fp:
             manifest_xml = fp.read()
           self.assertEqual(
-              manifest_xml,
+              sort_attributes(manifest_xml),
               '<?xml version="1.0" ?><manifest>'
-              '<remote name="default-remote" fetch="http://localhost"/>'
+              '<remote fetch="http://localhost" name="default-remote"/>'
               '<default remote="default-remote" revision="refs/heads/main"/>'
-              '<project name="platform/art" path="art" '
-              'revision="2c2724cb36cd5a9cec6c852c681efc3b7c6b86ea" '
-              'groups="notdefault,platform-' + self.platform + '"/>'
+              '<project groups="notdefault,platform-' + self.platform + '" '
+              'name="platform/art" path="art" '
+              'revision="2c2724cb36cd5a9cec6c852c681efc3b7c6b86ea"/>'
               '<superproject name="superproject"/>'
               '</manifest>')
 
@@ -208,16 +209,17 @@ class SuperprojectTestCase(unittest.TestCase):
           with open(manifest_path, 'r') as fp:
             manifest_xml = fp.read()
           self.assertEqual(
-              manifest_xml,
+              sort_attributes(manifest_xml),
               '<?xml version="1.0" ?><manifest>'
-              '<remote name="default-remote" fetch="http://localhost"/>'
-              '<remote name="goog" fetch="http://localhost2"/>'
+              '<remote fetch="http://localhost" name="default-remote"/>'
+              '<remote fetch="http://localhost2" name="goog"/>'
               '<default remote="default-remote" revision="refs/heads/main"/>'
-              '<project name="platform/art" path="art" '
-              'revision="2c2724cb36cd5a9cec6c852c681efc3b7c6b86ea" '
-              'groups="notdefault,platform-' + self.platform + '"/>'
-              '<project name="platform/vendor/x" path="vendor/x" remote="goog" '
-              'revision="master-with-vendor" groups="vendor" clone-depth="1"/>'
+              '<project groups="notdefault,platform-' + self.platform + '" '
+              'name="platform/art" path="art" '
+              'revision="2c2724cb36cd5a9cec6c852c681efc3b7c6b86ea"/>'
+              '<project clone-depth="1" groups="vendor" '
+              'name="platform/vendor/x" path="vendor/x" remote="goog" '
+              'revision="master-with-vendor"/>'
               '<superproject name="superproject"/>'
               '</manifest>')
 

tests/test_manifest_xml.py

@@ -16,6 +16,7 @@
 
 import os
 import platform
+import re
 import shutil
 import tempfile
 import unittest
@@ -63,6 +64,30 @@ if os.path.sep != '/':
   INVALID_FS_PATHS += tuple(x.replace('/', os.path.sep) for x in INVALID_FS_PATHS)
 
 
+def sort_attributes(manifest):
+  """Sort the attributes of all elements alphabetically.
+
+  This is needed because different versions of the toxml() function from
+  xml.dom.minidom outputs the attributes of elements in different orders.
+  Before Python 3.8 they were output alphabetically, later versions preserve
+  the order specified by the user.
+
+  Args:
+    manifest: String containing an XML manifest.
+
+  Returns:
+    The XML manifest with the attributes of all elements sorted alphabetically.
+  """
+  new_manifest = ''
+  # This will find every element in the XML manifest, whether they have
+  # attributes or not. This simplifies recreating the manifest below.
+  matches = re.findall(r'(<[/?]?[a-z-]+\s*)((?:\S+?="[^"]+"\s*?)*)(\s*[/?]?>)', manifest)
+  for head, attrs, tail in matches:
+    m = re.findall(r'\S+?="[^"]+"', attrs)
+    new_manifest += head + ' '.join(sorted(m)) + tail
+  return new_manifest
+
+
 class ManifestParseTestCase(unittest.TestCase):
   """TestCase for parsing manifests."""
 
@@ -254,9 +279,9 @@ class XmlManifestTests(ManifestParseTestCase):
     self.assertEqual(manifest.superproject['name'], 'superproject')
     self.assertEqual(manifest.superproject['remote'].name, 'test-remote')
     self.assertEqual(
-        manifest.ToXml().toxml(),
+        sort_attributes(manifest.ToXml().toxml()),
         '<?xml version="1.0" ?><manifest>'
-        '<remote name="test-remote" fetch="http://localhost"/>'
+        '<remote fetch="http://localhost" name="test-remote"/>'
         '<default remote="test-remote" revision="refs/heads/main"/>'
         '<superproject name="superproject"/>'
         '</manifest>')
@@ -408,9 +433,9 @@ class ProjectElementTests(ManifestParseTestCase):
     project = manifest.projects[0]
     project.SetRevisionId('ABCDEF')
     self.assertEqual(
-        manifest.ToXml().toxml(),
+        sort_attributes(manifest.ToXml().toxml()),
         '<?xml version="1.0" ?><manifest>'
-        '<remote name="default-remote" fetch="http://localhost"/>'
+        '<remote fetch="http://localhost" name="default-remote"/>'
         '<default remote="default-remote" revision="refs/heads/main"/>'
         '<project name="test-name" revision="ABCDEF"/>'
         '</manifest>')
@@ -516,9 +541,9 @@ class SuperProjectElementTests(ManifestParseTestCase):
     self.assertEqual(manifest.superproject['remote'].name, 'test-remote')
     self.assertEqual(manifest.superproject['remote'].url, 'http://localhost/superproject')
     self.assertEqual(
-        manifest.ToXml().toxml(),
+        sort_attributes(manifest.ToXml().toxml()),
         '<?xml version="1.0" ?><manifest>'
-        '<remote name="test-remote" fetch="http://localhost"/>'
+        '<remote fetch="http://localhost" name="test-remote"/>'
         '<default remote="test-remote" revision="refs/heads/main"/>'
         '<superproject name="superproject"/>'
         '</manifest>')
@@ -537,10 +562,10 @@ class SuperProjectElementTests(ManifestParseTestCase):
     self.assertEqual(manifest.superproject['remote'].name, 'superproject-remote')
     self.assertEqual(manifest.superproject['remote'].url, 'http://localhost/platform/superproject')
     self.assertEqual(
-        manifest.ToXml().toxml(),
+        sort_attributes(manifest.ToXml().toxml()),
         '<?xml version="1.0" ?><manifest>'
-        '<remote name="default-remote" fetch="http://localhost"/>'
-        '<remote name="superproject-remote" fetch="http://localhost"/>'
+        '<remote fetch="http://localhost" name="default-remote"/>'
+        '<remote fetch="http://localhost" name="superproject-remote"/>'
         '<default remote="default-remote" revision="refs/heads/main"/>'
         '<superproject name="platform/superproject" remote="superproject-remote"/>'
         '</manifest>')
@@ -557,9 +582,9 @@ class SuperProjectElementTests(ManifestParseTestCase):
     self.assertEqual(manifest.superproject['name'], 'superproject')
     self.assertEqual(manifest.superproject['remote'].name, 'default-remote')
     self.assertEqual(
-        manifest.ToXml().toxml(),
+        sort_attributes(manifest.ToXml().toxml()),
         '<?xml version="1.0" ?><manifest>'
-        '<remote name="default-remote" fetch="http://localhost"/>'
+        '<remote fetch="http://localhost" name="default-remote"/>'
         '<default remote="default-remote" revision="refs/heads/main"/>'
         '<superproject name="superproject"/>'
         '</manifest>')