superproject - More friendly user message when superproject failed.

superproject is going to be default for some users. This change
doesn't fail for repo init or repo sync if source couldn't be synced
because of errors in superproject and superproject=true in the config
file. The commands will fail if --use-superproject is specified on
the command line explicitly.

The error messages are logged with trace2 event logs and will be
monitored.

+ sync - When there are errors with superproject and git_superproject
  says it is fatal failure, sync will exit only when --use-superproject
  option is specified on the command line.

+ init - command doesn't fail *if there are any superproject errors),
  but it will print a warning message and logs message via trace2 event
  logs. For fatal errors, init will exit only when --use-superproject
  option is specified on the command line.

+ All git commands log the command that is being executed so trace2
  event logs will know the manifest, remote url and the branch name.

There is no functional change other than fatal errors are honored with
--use-supeproject option with init/sync commands.

Tested the code with the following commands.

$ ./run_tests -v

Test 1 - sync'ing without errors
--------------------------------
Added the following lines to '~/.repoconfig/config
[repo]
	superproject = true

  $ repo_dev init -u https://android.googlesource.com/platform/manifest -b android-s-beta-2
  NOTICE: --use-superproject is in beta; report any issues to the address described in `repo version`
  repo: error: git fetch call failed, command: git ['fetch', 'https://android.googlesource.com/platform/superproject', '--depth', '1', '--force', '--no-tags', '--filter', 'blob:none', 'android-s-beta-2:android-s-beta-2'], return code: 128, stderr: fatal: couldn't find remote ref android-s-beta-2

  warning: git update of superproject failed, repo sync will not use superproject to fetch source; while this error is not fatal, and you can continue to run repo sync, please run repo init with the --no-use-superproject option to stop seeing this warning

  Your identity is: Raman Tenneti <rtenneti@google.com>
  If you want to change this, please re-run 'repo init' with --config-name

  repo has been initialized in /usr/local/google/home/rtenneti/work/drive2/android/test

$ repo_dev sync
remote: Total 4 (delta 1), reused 4 (delta 1)
NOTICE: --use-superproject is in beta; report any issues to the address described in `repo version`
/usr/local/google/home/rtenneti/work/drive2/android/aosp/.repo/exp-superproject/925043f706ba64db713e9bf3b55987e2-superproject.git: Initial setup for superproject completed.
...

Test 2 - init and sync fail when --use-superproject option is passed
--------------------------------------------------------------------
$ repo_dev init -u https://android.googlesource.com/platform/manifest -b android-s-beta-2 --use-superproject
remote: Total 57 (delta 16), reused 56 (delta 16)
NOTICE: --use-superproject is in beta; report any issues to the address described in `repo version`
repo: error: git fetch call failed, command: git ['fetch', 'https://android.googlesource.com/platform/superproject', '--depth', '1', '--force', '--no-tags', '--filter', 'blob:none', 'android-s-beta-2:android-s-beta-2'], return code: 128, stderr: fatal: couldn't find remote ref android-s-beta-2

warning: git update of superproject failed, repo sync will not use superproject to fetch source; while this error is not fatal, and you can continue to run repo sync, please run repo init with the --no-use-superproject option to stop seeing this warning

rtenneti@rtenneti2:~/work/drive2/android/test$ repo_dev sync --use-superproject
NOTICE: --use-superproject is in beta; report any issues to the address described in `repo version`
repo: error: git fetch call failed, command: git ['fetch', 'https://android.googlesource.com/platform/superproject', '--depth', '1', '--force', '--no-tags', '--filter', 'blob:none', 'android-s-beta-2:android-s-beta-2'], return code: 128, stderr: fatal: couldn't find remote ref android-s-beta-2

warning: Cannot get project commit ids from manifest
warning: Update of revisionId from superproject has failed, repo sync will not use superproject to fetch the source.  Please resync with the --no-use-superproject option to avoid this repo warning.

Test 3 - git fetch command fails and git command is printed
-----------------------------------------------------------
With config change
$ repo_dev init -u https://android.googlesource.com/platform/manifest -b android-s-beta-2
...
NOTICE: --use-superproject is in beta; report any issues to the address described in `repo version`
.../android/test/.repo/exp-superproject/925043f706ba64db713e9bf3b55987e2-superproject.git: Performing initial setup for superproject; this might take several minutes.
repo: error: git fetch call failed,command: git ['fetch', 'https://android.googlesource.com/platform/superproject', '--depth', '1', '--force', '--no-tags', '--filter', 'blob:none', 'android-s-beta-2:android-s-beta-2'], return code: 128, stderr: fatal: couldn't find remote ref android-s-beta-2

warning: git update of superproject failed, repo sync will not use superproject to fetch source; while this error is not fatal and you can continue to run repo sync please run repo init with the --no-use-superproject option to avoid the repo warning

Your identity is: Raman Tenneti <rtenneti@google.com>
If you want to change this, please re-run 'repo init' with --config-name

repo has been initialized in ....

Test 4 - no superproject tag
-----------------------------
$ repo_dev init -u https://android.googlesource.com/platform/manifest -b pie-dev
...
NOTICE: --use-superproject is in beta; report any issues to the address described in `repo version`
repo error: superproject tag is not defined in manifest: .../android/pie_dev/.repo/manifest.xml
warning: git update of superproject failed, repo sync will not use superproject to fetch source; while this error is not fatal and you can continue to run repo sync please run repo init with the --no-use-superproject option to avoid the repo warning

Your identity is: Raman Tenneti <rtenneti@google.com>
If you want to change this, please re-run 'repo init' with --config-name

repo has been initialized in ...

$ repo_dev sync
NOTICE: --use-superproject is in beta; report any issues to the address described in `repo version`
repo error: superproject tag is not defined in manifest: /usr/local/google/home/rtenneti/work/drive2/android/pie_dev/.repo/manifest.xml
warning: Cannot get project commit ids from manifest
warning: Update of revsionId from superproject has failed. Please resync with --no-use-superproject option to avoid the repo warning.

Bug: [google internal] b/192614798
Bug: [google internal] b/Bug: [google internal] b/192614798

Change-Id: I9a97a0e7d9e609fad151bd7dd9cfc523eaa887cd
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/311502
Reviewed-by: Amith Dsouza <amithds@google.com>
Reviewed-by: Xin Li <delphij@google.com>
Tested-by: Raman Tenneti <rtenneti@google.com>

docs/manifest-format.md

@@ -96,6 +96,7 @@ following DTD:
 
   <!ELEMENT remove-project EMPTY>
   <!ATTLIST remove-project name  CDATA #REQUIRED>
+  <!ATTLIST remove-project optional  CDATA #IMPLIED>
 
   <!ELEMENT repo-hooks EMPTY>
   <!ATTLIST repo-hooks in-project CDATA #REQUIRED>
@@ -393,6 +394,9 @@ This element is mostly useful in a local manifest file, where
 the user can remove a project, and possibly replace it with their
 own definition.
 
+Attribute `optional`: Set to true to ignore remove-project elements with no
+matching `project` element.
+
 ### Element repo-hooks
 
 NB: See the [practical documentation](./repo-hooks.md) for using repo hooks.

git_command.py

@@ -75,7 +75,8 @@ def RepoSourceVersion():
     proj = os.path.dirname(os.path.abspath(__file__))
     env[GIT_DIR] = os.path.join(proj, '.git')
     result = subprocess.run([GIT, 'describe', HEAD], stdout=subprocess.PIPE,
-                            encoding='utf-8', env=env, check=False)
+                            stderr=subprocess.DEVNULL, encoding='utf-8',
+                            env=env, check=False)
     if result.returncode == 0:
       ver = result.stdout.strip()
       if ver.startswith('v'):

git_config.py

@@ -65,6 +65,15 @@ class GitConfig(object):
 
   _USER_CONFIG = '~/.gitconfig'
 
+  _ForSystem = None
+  _SYSTEM_CONFIG = '/etc/gitconfig'
+
+  @classmethod
+  def ForSystem(cls):
+    if cls._ForSystem is None:
+      cls._ForSystem = cls(configfile=cls._SYSTEM_CONFIG)
+    return cls._ForSystem
+
   @classmethod
   def ForUser(cls):
     if cls._ForUser is None:
@@ -356,7 +365,10 @@ class GitConfig(object):
     return c
 
   def _do(self, *args):
-    command = ['config', '--file', self.file, '--includes']
+    if self.file == self._SYSTEM_CONFIG:
+      command = ['config', '--system', '--includes']
+    else:
+      command = ['config', '--file', self.file, '--includes']
     command.extend(args)
 
     p = GitCommand(None,

git_superproject.py

@@ -23,11 +23,14 @@ Examples:
 """
 
 import hashlib
+import functools
 import os
 import sys
+import time
 from typing import NamedTuple
 
 from git_command import git_require, GitCommand
+from git_config import RepoConfig
 from git_refs import R_HEADS
 from manifest_xml import LOCAL_MANIFEST_GROUP_PREFIX
 
@@ -114,6 +117,11 @@ class Superproject(object):
       branch = branch[len(R_HEADS):]
     return branch
 
+  def _LogError(self, message):
+    """Logs message to stderr and _git_event_log."""
+    print(message, file=sys.stderr)
+    self._git_event_log.ErrorEvent(message, '')
+
   def _Init(self):
     """Sets up a local Git repository to get a copy of a superproject.
 
@@ -133,8 +141,8 @@ class Superproject(object):
                    capture_stderr=True)
     retval = p.Wait()
     if retval:
-      print('repo: error: git init call failed with return code: %r, stderr: %r' %
+      self._LogError(f'repo: error: git init call failed, command: git {cmd}, '
-            (retval, p.stderr), file=sys.stderr)
+                     f'return code: {retval}, stderr: {p.stderr}')
       return False
     return True
 
@@ -148,8 +156,7 @@ class Superproject(object):
       True if fetch is successful, or False.
     """
     if not os.path.exists(self._work_git):
-      print('git fetch missing drectory: %s' % self._work_git,
+      self._LogError(f'git fetch missing directory: {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)
@@ -164,8 +171,8 @@ class Superproject(object):
                    capture_stderr=True)
     retval = p.Wait()
     if retval:
-      print('repo: error: git fetch call failed with return code: %r, stderr: %r' %
+      self._LogError(f'repo: error: git fetch call failed, command: git {cmd}, '
-            (retval, p.stderr), file=sys.stderr)
+                     f'return code: {retval}, stderr: {p.stderr}')
       return False
     return True
 
@@ -178,8 +185,7 @@ class Superproject(object):
       data: data returned from 'git ls-tree ...' instead of None.
     """
     if not os.path.exists(self._work_git):
-      print('git ls-tree missing drectory: %s' % self._work_git,
+      self._LogError(f'git ls-tree missing directory: {self._work_git}')
-            file=sys.stderr)
       return None
     data = None
     branch = 'HEAD' if not self._branch else self._branch
@@ -194,8 +200,8 @@ class Superproject(object):
     if retval == 0:
       data = p.stdout
     else:
-      print('repo: error: git ls-tree call failed with return code: %r, stderr: %r' % (
+      self._LogError(f'repo: error: git ls-tree call failed, command: git {cmd}, '
-          retval, p.stderr), file=sys.stderr)
+                     f'return code: {retval}, stderr: {p.stderr}')
     return data
 
   def Sync(self):
@@ -208,17 +214,15 @@ class Superproject(object):
           'address described in `repo version`', file=sys.stderr)
 
     if not self._manifest.superproject:
-      msg = (f'repo error: superproject tag is not defined in manifest: '
+      self._LogError(f'repo error: superproject tag is not defined in manifest: '
-             f'{self._manifest.manifestFile}')
+                     f'{self._manifest.manifestFile}')
-      print(msg, file=sys.stderr)
-      self._git_event_log.ErrorEvent(msg, '')
       return SyncResult(False, False)
 
     should_exit = True
     url = self._manifest.superproject['remote'].url
     if not url:
-      print('error: superproject URL is not defined in manifest',
+      self._LogError(f'repo error: superproject URL is not defined in manifest: '
-            file=sys.stderr)
+                     f'{self._manifest.manifestFile}')
       return SyncResult(False, should_exit)
 
     if not self._Init():
@@ -241,7 +245,7 @@ class Superproject(object):
 
     data = self._LsTree()
     if not data:
-      print('error: git ls-tree failed to return data for superproject',
+      print('warning: git ls-tree failed to return data for superproject',
             file=sys.stderr)
       return CommitIdsResult(None, True)
 
@@ -268,9 +272,7 @@ class Superproject(object):
       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._LogError(f'error: missing superproject directory: {self._superproject_path}')
-            self._superproject_path,
-            file=sys.stderr)
       return None
     manifest_str = self._manifest.ToXml(groups=self._manifest.GetGroupsStr()).toxml()
     manifest_path = self._manifest_path
@@ -278,9 +280,7 @@ class Superproject(object):
       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'
+      self._LogError(f'error: cannot write manifest to : {manifest_path} {e}')
-            % (manifest_path, e),
-            file=sys.stderr)
       return None
     return manifest_path
 
@@ -298,6 +298,9 @@ class Superproject(object):
     path = project.relpath
     if not path:
       return True
+    # Skip the project with revisionId.
+    if project.revisionId:
+      return True
     # Skip the project if it comes from the local manifest.
     return any(s.startswith(LOCAL_MANIFEST_GROUP_PREFIX) for s in project.groups)
 
@@ -313,7 +316,7 @@ class Superproject(object):
     commit_ids_result = self._GetAllProjectsCommitIds()
     commit_ids = commit_ids_result.commit_ids
     if not commit_ids:
-      print('error: Cannot get project commit ids from manifest', file=sys.stderr)
+      print('warning: Cannot get project commit ids from manifest', file=sys.stderr)
       return UpdateProjectsResult(None, commit_ids_result.fatal)
 
     projects_missing_commit_ids = []
@@ -328,10 +331,8 @@ class Superproject(object):
     # If superproject doesn't have a commit id for a project, then report an
     # error event and continue as if do not use superproject is specified.
     if projects_missing_commit_ids:
-      msg = (f'error: please file a bug using {self._manifest.contactinfo.bugurl} '
+      self._LogError(f'error: please file a bug using {self._manifest.contactinfo.bugurl} '
-             f'to report missing commit_ids for: {projects_missing_commit_ids}')
+                     f'to report missing commit_ids for: {projects_missing_commit_ids}')
-      print(msg, file=sys.stderr)
-      self._git_event_log.ErrorEvent(msg, '')
       return UpdateProjectsResult(None, False)
 
     for project in projects:
@@ -340,3 +341,81 @@ class Superproject(object):
 
     manifest_path = self._WriteManfiestFile()
     return UpdateProjectsResult(manifest_path, False)
+
+
+@functools.lru_cache(maxsize=None)
+def _UseSuperprojectFromConfiguration():
+  """Returns the user choice of whether to use superproject."""
+  user_cfg = RepoConfig.ForUser()
+  system_cfg = RepoConfig.ForSystem()
+  time_now = int(time.time())
+
+  user_value = user_cfg.GetBoolean('repo.superprojectChoice')
+  if user_value is not None:
+    user_expiration = user_cfg.GetInt('repo.superprojectChoiceExpire')
+    if user_expiration is not None and (user_expiration <= 0 or user_expiration >= time_now):
+      # TODO(b/190688390) - Remove prompt when we are comfortable with the new
+      # default value.
+      print(('You are currently enrolled in Git submodules experiment '
+             '(go/android-submodules-quickstart).  Use --no-use-superproject '
+             'to override.\n'), file=sys.stderr)
+    return user_value
+
+  # We don't have an unexpired choice, ask for one.
+  system_value = system_cfg.GetBoolean('repo.superprojectChoice')
+  if system_value:
+    # The system configuration is proposing that we should enable the
+    # use of superproject. Present this to user for confirmation if we
+    # are on a TTY, or, when we are not on a TTY, accept the system
+    # default for this time only.
+    #
+    # TODO(b/190688390) - Remove prompt when we are comfortable with the new
+    # default value.
+    prompt = ('Repo can now use Git submodules (go/android-submodules-quickstart) '
+              'instead of manifests to represent the state of the Android '
+              'superproject, which results in faster syncs and better atomicity.\n\n')
+    if sys.stdout.isatty():
+      prompt += 'Would you like to opt in for two weeks (y/N)? '
+      response = input(prompt).lower()
+      time_choiceexpire = time_now + (86400 * 14)
+      if response in ('y', 'yes'):
+        userchoice = True
+      elif response in ('a', 'always'):
+        userchoice = True
+        time_choiceexpire = 0
+      elif response == 'never':
+        userchoice = False
+        time_choiceexpire = 0
+      elif response in ('n', 'no'):
+        userchoice = False
+      else:
+        # Unrecognized user response, assume the intention was no, but
+        # only for 2 hours instead of 2 weeks to balance between not
+        # being overly pushy while still retain the opportunity to
+        # enroll.
+        userchoice = False
+        time_choiceexpire = time_now + 7200
+
+      user_cfg.SetString('repo.superprojectChoiceExpire', str(time_choiceexpire))
+      user_cfg.SetBoolean('repo.superprojectChoice', userchoice)
+
+      return userchoice
+    else:
+      print('Accepting once since we are not on a TTY', file=sys.stderr)
+      return True
+
+  # For all other cases, we would not use superproject by default.
+  return False
+
+
+def UseSuperproject(opt, manifest):
+  """Returns a boolean if use-superproject option is enabled."""
+
+  if opt.use_superproject is not None:
+    return opt.use_superproject
+  else:
+    client_value = manifest.manifestProject.config.GetBoolean('repo.superproject')
+    if client_value is not None:
+      return client_value
+    else:
+      return _UseSuperprojectFromConfiguration()

manifest_xml.py

@@ -918,19 +918,19 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
       if node.nodeName == 'remove-project':
         name = self._reqatt(node, 'name')
 
-        if name not in self._projects:
+        if name in self._projects:
+          for p in self._projects[name]:
+            del self._paths[p.relpath]
+          del self._projects[name]
+
+          # If the manifest removes the hooks project, treat it as if it deleted
+          # the repo-hooks element too.
+          if self._repo_hooks_project and (self._repo_hooks_project.name == name):
+            self._repo_hooks_project = None
+        elif not XmlBool(node, 'optional', False):
           raise ManifestParseError('remove-project element specifies non-existent '
                                    'project: %s' % name)
 
-        for p in self._projects[name]:
-          del self._paths[p.relpath]
-        del self._projects[name]
-
-        # If the manifest removes the hooks project, treat it as if it deleted
-        # the repo-hooks element too.
-        if self._repo_hooks_project and (self._repo_hooks_project.name == name):
-          self._repo_hooks_project = None
-
   def _AddMetaProjectMirror(self, m):
     name = None
     m_url = m.GetRemote(m.remote.name).url

project.py

@@ -1216,7 +1216,7 @@ class Project(object):
                                          (self.revisionExpr, self.name))
 
   def SetRevisionId(self, revisionId):
-    if self.clone_depth or self.manifest.manifestProject.config.GetString('repo.depth'):
+    if self.revisionExpr:
       self.upstream = self.revisionExpr
 
     self.revisionId = revisionId
@@ -1967,6 +1967,10 @@ class Project(object):
       # throws an error.
       self.bare_git.rev_list('-1', '--missing=allow-any',
                              '%s^0' % self.revisionExpr, '--')
+      if self.upstream:
+        rev = self.GetRemote(self.remote.name).ToLocal(self.upstream)
+        self.bare_git.rev_list('-1', '--missing=allow-any',
+                               '%s^0' % rev, '--')
       return True
     except GitError:
       # There is no such persistent revision. We have to fetch it.

subcmds/init.py

@@ -100,8 +100,12 @@ to update the working directory files.
                                                  quiet=opt.quiet)
     sync_result = superproject.Sync()
     if not sync_result.success:
-      print('error: git update of superproject failed', file=sys.stderr)
+      print('warning: git update of superproject failed, repo sync will not '
-      if sync_result.fatal:
+            'use superproject to fetch source; while this error is not fatal, '
+            'and you can continue to run repo sync, please run repo init with '
+            'the --no-use-superproject option to stop seeing this warning',
+            file=sys.stderr)
+      if sync_result.fatal and opt.use_superproject is not None:
         sys.exit(1)
 
   def _SyncManifest(self, opt):

subcmds/sync.py

@@ -278,16 +278,9 @@ later is required to fix a server side protocol bug.
       branch = branch[len(R_HEADS):]
     return branch
 
-  def _UseSuperproject(self, opt):
-    """Returns True if use-superproject option is enabled"""
-    if opt.use_superproject is not None:
-      return opt.use_superproject
-    else:
-      return self.manifest.manifestProject.config.GetBoolean('repo.superproject')
-
   def _GetCurrentBranchOnly(self, opt):
     """Returns True if current-branch or use-superproject options are enabled."""
-    return opt.current_branch_only or self._UseSuperproject(opt)
+    return opt.current_branch_only or git_superproject.UseSuperproject(opt, self.manifest)
 
   def _UpdateProjectsRevisionId(self, opt, args, load_local_manifests):
     """Update revisionId of every project with the SHA from superproject.
@@ -316,10 +309,11 @@ later is required to fix a server side protocol bug.
     if manifest_path:
       self._ReloadManifest(manifest_path, load_local_manifests)
     else:
-      print('error: Update of revsionId from superproject has failed. '
+      print('warning: Update of revisionId from superproject has failed, '
-            'Please resync with --no-use-superproject option',
+            'repo sync will not use superproject to fetch the source. ',
+            'Please resync with the --no-use-superproject option to avoid this repo warning.',
             file=sys.stderr)
-      if update_result.fatal:
+      if update_result.fatal and opt.use_superproject is not None:
         sys.exit(1)
     return manifest_path
 
@@ -367,7 +361,7 @@ later is required to fix a server side protocol bug.
           partial_clone_exclude=self.manifest.PartialCloneExclude)
 
       output = buf.getvalue()
-      if opt.verbose and output:
+      if (opt.verbose or not success) and output:
         print('\n' + output.rstrip())
 
       if not success:
@@ -964,10 +958,10 @@ later is required to fix a server side protocol bug.
       self._UpdateManifestProject(opt, mp, manifest_name)
 
     load_local_manifests = not self.manifest.HasLocalManifests
-    if self._UseSuperproject(opt):
+    if git_superproject.UseSuperproject(opt, self.manifest):
       new_manifest_name = self._UpdateProjectsRevisionId(opt, args, load_local_manifests)
       if not new_manifest_name:
-        manifest_name = new_manifest_name
+        manifest_name = opt.manifest_name
 
     if self.gitc_manifest:
       gitc_manifest_projects = self.GetProjects(args,

tests/test_git_superproject.py

@@ -213,7 +213,7 @@ class SuperprojectTestCase(unittest.TestCase):
         '<remote fetch="http://localhost" name="default-remote"/>'
         '<default remote="default-remote" revision="refs/heads/main"/>'
         '<project groups="notdefault,platform-' + self.platform + '" '
-        'name="platform/art" path="art" revision="ABCDEF"/>'
+        'name="platform/art" path="art" revision="ABCDEF" upstream="refs/heads/main"/>'
         '<superproject name="superproject"/>'
         '</manifest>')
 
@@ -242,7 +242,7 @@ class SuperprojectTestCase(unittest.TestCase):
               '<default remote="default-remote" revision="refs/heads/main"/>'
               '<project groups="notdefault,platform-' + self.platform + '" '
               'name="platform/art" path="art" '
-              'revision="2c2724cb36cd5a9cec6c852c681efc3b7c6b86ea"/>'
+              'revision="2c2724cb36cd5a9cec6c852c681efc3b7c6b86ea" upstream="refs/heads/main"/>'
               '<superproject name="superproject"/>'
               '</manifest>')
 
@@ -271,7 +271,7 @@ class SuperprojectTestCase(unittest.TestCase):
         '<?xml version="1.0" ?><manifest>'
         '<remote fetch="http://localhost" name="default-remote"/>'
         '<default remote="default-remote" revision="refs/heads/main"/>'
-        '<project name="test-name" revision="ABCDEF"/>'
+        '<project name="test-name" revision="ABCDEF" upstream="refs/heads/main"/>'
         '</manifest>')
 
   def test_superproject_update_project_revision_id_from_local_manifest_group(self):
@@ -294,8 +294,7 @@ class SuperprojectTestCase(unittest.TestCase):
                                                        self.git_event_log)
     self.assertEqual(len(self._superproject._manifest.projects), 2)
     projects = self._superproject._manifest.projects
-    data = ('160000 commit 2c2724cb36cd5a9cec6c852c681efc3b7c6b86ea\tart\x00'
+    data = ('160000 commit 2c2724cb36cd5a9cec6c852c681efc3b7c6b86ea\tart\x00')
-            '160000 commit e9d25da64d8d365dbba7c8ee00fe8c4473fe9a06\tbootable/recovery\x00')
     with mock.patch.object(self._superproject, '_Init', return_value=True):
       with mock.patch.object(self._superproject, '_Fetch', return_value=True):
         with mock.patch.object(self._superproject,
@@ -317,13 +316,61 @@ class SuperprojectTestCase(unittest.TestCase):
               '<default remote="default-remote" revision="refs/heads/main"/>'
               '<project groups="notdefault,platform-' + self.platform + '" '
               'name="platform/art" path="art" '
-              'revision="2c2724cb36cd5a9cec6c852c681efc3b7c6b86ea"/>'
+              'revision="2c2724cb36cd5a9cec6c852c681efc3b7c6b86ea" upstream="refs/heads/main"/>'
               '<project clone-depth="1" groups="' + local_group + '" '
               'name="platform/vendor/x" path="vendor/x" remote="goog" '
               'revision="master-with-vendor"/>'
               '<superproject name="superproject"/>'
               '</manifest>')
 
+  def test_superproject_update_project_revision_id_with_pinned_manifest(self):
+    """Test update of commit ids of a pinned manifest."""
+    manifest = self.getXmlManifest("""
+<manifest>
+  <remote name="default-remote" fetch="http://localhost" />
+  <default remote="default-remote" revision="refs/heads/main" />
+  <superproject name="superproject"/>
+  <project path="vendor/x" name="platform/vendor/x" revision="" />
+  <project path="vendor/y" name="platform/vendor/y"
+           revision="52d3c9f7c107839ece2319d077de0cd922aa9d8f" />
+  <project path="art" name="platform/art" groups="notdefault,platform-""" + self.platform + """
+  " /></manifest>
+""")
+    self.maxDiff = None
+    self._superproject = git_superproject.Superproject(manifest, self.repodir,
+                                                       self.git_event_log)
+    self.assertEqual(len(self._superproject._manifest.projects), 3)
+    projects = self._superproject._manifest.projects
+    data = ('160000 commit 2c2724cb36cd5a9cec6c852c681efc3b7c6b86ea\tart\x00'
+            '160000 commit e9d25da64d8d365dbba7c8ee00fe8c4473fe9a06\tvendor/x\x00')
+    with mock.patch.object(self._superproject, '_Init', 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)
+          update_result = self._superproject.UpdateProjectsRevisionId(projects)
+          self.assertIsNotNone(update_result.manifest_path)
+          self.assertFalse(update_result.fatal)
+          with open(update_result.manifest_path, 'r') as fp:
+            manifest_xml_data = fp.read()
+          # Verify platform/vendor/x's project revision hasn't changed.
+          self.assertEqual(
+              sort_attributes(manifest_xml_data),
+              '<?xml version="1.0" ?><manifest>'
+              '<remote fetch="http://localhost" name="default-remote"/>'
+              '<default remote="default-remote" revision="refs/heads/main"/>'
+              '<project groups="notdefault,platform-' + self.platform + '" '
+              'name="platform/art" path="art" '
+              'revision="2c2724cb36cd5a9cec6c852c681efc3b7c6b86ea" upstream="refs/heads/main"/>'
+              '<project name="platform/vendor/x" path="vendor/x" '
+              'revision="e9d25da64d8d365dbba7c8ee00fe8c4473fe9a06" upstream="refs/heads/main"/>'
+              '<project name="platform/vendor/y" path="vendor/y" '
+              'revision="52d3c9f7c107839ece2319d077de0cd922aa9d8f"/>'
+              '<superproject name="superproject"/>'
+              '</manifest>')
+
 
 if __name__ == '__main__':
   unittest.main()

tests/test_manifest_xml.py

@@ -437,7 +437,7 @@ class ProjectElementTests(ManifestParseTestCase):
         '<?xml version="1.0" ?><manifest>'
         '<remote fetch="http://localhost" name="default-remote"/>'
         '<default remote="default-remote" revision="refs/heads/main"/>'
-        '<project name="test-name" revision="ABCDEF"/>'
+        '<project name="test-name" revision="ABCDEF" upstream="refs/heads/main"/>'
         '</manifest>')
 
   def test_trailing_slash(self):
@@ -638,3 +638,53 @@ class RemoteElementTests(ManifestParseTestCase):
     self.assertNotEqual(a, manifest_xml._Default())
     self.assertNotEqual(a, 123)
     self.assertNotEqual(a, None)
+
+
+class RemoveProjectElementTests(ManifestParseTestCase):
+  """Tests for <remove-project>."""
+
+  def test_remove_one_project(self):
+    manifest = self.getXmlManifest("""
+<manifest>
+  <remote name="default-remote" fetch="http://localhost" />
+  <default remote="default-remote" revision="refs/heads/main" />
+  <project name="myproject" />
+  <remove-project name="myproject" />
+</manifest>
+""")
+    self.assertEqual(manifest.projects, [])
+
+  def test_remove_one_project_one_remains(self):
+    manifest = self.getXmlManifest("""
+<manifest>
+  <remote name="default-remote" fetch="http://localhost" />
+  <default remote="default-remote" revision="refs/heads/main" />
+  <project name="myproject" />
+  <project name="yourproject" />
+  <remove-project name="myproject" />
+</manifest>
+""")
+
+    self.assertEqual(len(manifest.projects), 1)
+    self.assertEqual(manifest.projects[0].name, 'yourproject')
+
+  def test_remove_one_project_doesnt_exist(self):
+    with self.assertRaises(manifest_xml.ManifestParseError):
+      manifest = self.getXmlManifest("""
+<manifest>
+  <remote name="default-remote" fetch="http://localhost" />
+  <default remote="default-remote" revision="refs/heads/main" />
+  <remove-project name="myproject" />
+</manifest>
+""")
+      manifest.projects
+
+  def test_remove_one_optional_project_doesnt_exist(self):
+    manifest = self.getXmlManifest("""
+<manifest>
+  <remote name="default-remote" fetch="http://localhost" />
+  <default remote="default-remote" revision="refs/heads/main" />
+  <remove-project name="myproject" optional="true" />
+</manifest>
+""")
+    self.assertEqual(manifest.projects, [])