1 files changed, 678 insertions, 210 deletions

diff --git a/manifest_xml.py b/manifest_xml.py
index 3814a25a..68ead53c 100644
--- a/manifest_xml.py
+++ b/manifest_xml.py
@@ -1,5 +1,3 @@
-# -*- coding:utf-8 -*-
-#
 # Copyright (C) 2008 The Android Open Source Project
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -14,33 +12,34 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from __future__ import print_function
+import collections
 import itertools
 import os
+import platform
 import re
 import sys
 import xml.dom.minidom
-
-from pyversion import is_python3
-if is_python3():
-  import urllib.parse
-else:
-  import imp
-  import urlparse
-  urllib = imp.new_module('urllib')
-  urllib.parse = urlparse
+import urllib.parse
 
 import gitc_utils
-from git_config import GitConfig
+from git_config import GitConfig, IsId
 from git_refs import R_HEADS, HEAD
 import platform_utils
-from project import RemoteSpec, Project, MetaProject
-from error import ManifestParseError, ManifestInvalidRevisionError
+from project import Annotation, RemoteSpec, Project, MetaProject
+from error import (ManifestParseError, ManifestInvalidPathError,
+                   ManifestInvalidRevisionError)
+from wrapper import Wrapper
 
 MANIFEST_FILE_NAME = 'manifest.xml'
 LOCAL_MANIFEST_NAME = 'local_manifest.xml'
 LOCAL_MANIFESTS_DIR_NAME = 'local_manifests'
 
+# Add all projects from local manifest into a group.
+LOCAL_MANIFEST_GROUP_PREFIX = 'local:'
+
+# ContactInfo has the self-registered bug url, supplied by the manifest authors.
+ContactInfo = collections.namedtuple('ContactInfo', 'bugurl')
+
 # urljoin gets confused if the scheme is not known.
 urllib.parse.uses_relative.extend([
     'ssh',
@@ -55,6 +54,61 @@ urllib.parse.uses_netloc.extend([
     'sso',
     'rpc'])
 
+
+def XmlBool(node, attr, default=None):
+  """Determine boolean value of |node|'s |attr|.
+
+  Invalid values will issue a non-fatal warning.
+
+  Args:
+    node: XML node whose attributes we access.
+    attr: The attribute to access.
+    default: If the attribute is not set (value is empty), then use this.
+
+  Returns:
+    True if the attribute is a valid string representing true.
+    False if the attribute is a valid string representing false.
+    |default| otherwise.
+  """
+  value = node.getAttribute(attr)
+  s = value.lower()
+  if s == '':
+    return default
+  elif s in {'yes', 'true', '1'}:
+    return True
+  elif s in {'no', 'false', '0'}:
+    return False
+  else:
+    print('warning: manifest: %s="%s": ignoring invalid XML boolean' %
+          (attr, value), file=sys.stderr)
+    return default
+
+
+def XmlInt(node, attr, default=None):
+  """Determine integer value of |node|'s |attr|.
+
+  Args:
+    node: XML node whose attributes we access.
+    attr: The attribute to access.
+    default: If the attribute is not set (value is empty), then use this.
+
+  Returns:
+    The number if the attribute is a valid number.
+
+  Raises:
+    ManifestParseError: The number is invalid.
+  """
+  value = node.getAttribute(attr)
+  if not value:
+    return default
+
+  try:
+    return int(value)
+  except ValueError:
+    raise ManifestParseError('manifest: invalid %s="%s" integer' %
+                             (attr, value))
+
+
 class _Default(object):
   """Project defaults within the manifest."""
 
@@ -68,11 +122,16 @@ class _Default(object):
   sync_tags = True
 
   def __eq__(self, other):
+    if not isinstance(other, _Default):
+      return False
     return self.__dict__ == other.__dict__
 
   def __ne__(self, other):
+    if not isinstance(other, _Default):
+      return True
     return self.__dict__ != other.__dict__
 
+
 class _XmlRemote(object):
   def __init__(self,
                name,
@@ -90,14 +149,22 @@ class _XmlRemote(object):
     self.reviewUrl = review
     self.revision = revision
     self.resolvedFetchUrl = self._resolveFetchUrl()
+    self.annotations = []
 
   def __eq__(self, other):
-    return self.__dict__ == other.__dict__
+    if not isinstance(other, _XmlRemote):
+      return False
+    return (sorted(self.annotations) == sorted(other.annotations) and
+      self.name == other.name and self.fetchUrl == other.fetchUrl and
+      self.pushUrl == other.pushUrl and self.remoteAlias == other.remoteAlias
+      and self.reviewUrl == other.reviewUrl and self.revision == other.revision)
 
   def __ne__(self, other):
-    return self.__dict__ != other.__dict__
+    return not self.__eq__(other)
 
   def _resolveFetchUrl(self):
+    if self.fetchUrl is None:
+      return ''
     url = self.fetchUrl.rstrip('/')
     manifestUrl = self.manifestUrl.rstrip('/')
     # urljoin will gets confused over quite a few things.  The ones we care
@@ -126,25 +193,48 @@ class _XmlRemote(object):
                       orig_name=self.name,
                       fetchUrl=self.fetchUrl)
 
+  def AddAnnotation(self, name, value, keep):
+    self.annotations.append(Annotation(name, value, keep))
+
+
 class XmlManifest(object):
   """manages the repo configuration file"""
 
-  def __init__(self, repodir):
+  def __init__(self, repodir, manifest_file, local_manifests=None):
+    """Initialize.
+
+    Args:
+      repodir: Path to the .repo/ dir for holding all internal checkout state.
+          It must be in the top directory of the repo client checkout.
+      manifest_file: Full path to the manifest file to parse.  This will usually
+          be |repodir|/|MANIFEST_FILE_NAME|.
+      local_manifests: Full path to the directory of local override manifests.
+          This will usually be |repodir|/|LOCAL_MANIFESTS_DIR_NAME|.
+    """
+    # TODO(vapier): Move this out of this class.
+    self.globalConfig = GitConfig.ForUser()
+
     self.repodir = os.path.abspath(repodir)
     self.topdir = os.path.dirname(self.repodir)
-    self.manifestFile = os.path.join(self.repodir, MANIFEST_FILE_NAME)
-    self.globalConfig = GitConfig.ForUser()
-    self.localManifestWarning = False
-    self.isGitcClient = False
+    self.manifestFile = manifest_file
+    self.local_manifests = local_manifests
     self._load_local_manifests = True
 
     self.repoProject = MetaProject(self, 'repo',
-      gitdir   = os.path.join(repodir, 'repo/.git'),
-      worktree = os.path.join(repodir, 'repo'))
+                                   gitdir=os.path.join(repodir, 'repo/.git'),
+                                   worktree=os.path.join(repodir, 'repo'))
 
-    self.manifestProject = MetaProject(self, 'manifests',
-      gitdir   = os.path.join(repodir, 'manifests.git'),
-      worktree = os.path.join(repodir, 'manifests'))
+    mp = MetaProject(self, 'manifests',
+                     gitdir=os.path.join(repodir, 'manifests.git'),
+                     worktree=os.path.join(repodir, 'manifests'))
+    self.manifestProject = mp
+
+    # This is a bit hacky, but we're in a chicken & egg situation: all the
+    # normal repo settings live in the manifestProject which we just setup
+    # above, so we couldn't easily query before that.  We assume Project()
+    # init doesn't care if this changes afterwards.
+    if os.path.exists(mp.gitdir) and mp.config.GetBoolean('repo.worktree'):
+      mp.use_git_worktrees = True
 
     self._Unload()
 
@@ -179,12 +269,26 @@ class XmlManifest(object):
     """
     self.Override(name)
 
-    try:
-      if os.path.lexists(self.manifestFile):
-        platform_utils.remove(self.manifestFile)
-      platform_utils.symlink(os.path.join('manifests', name), self.manifestFile)
-    except OSError as e:
-      raise ManifestParseError('cannot link manifest %s: %s' % (name, str(e)))
+    # Old versions of repo would generate symlinks we need to clean up.
+    platform_utils.remove(self.manifestFile, missing_ok=True)
+    # This file is interpreted as if it existed inside the manifest repo.
+    # That allows us to use <include> with the relative file name.
+    with open(self.manifestFile, 'w') as fp:
+      fp.write("""<?xml version="1.0" encoding="UTF-8"?>
+<!--
+DO NOT EDIT THIS FILE!  It is generated by repo and changes will be discarded.
+If you want to use a different manifest, use `repo init -m <file>` instead.
+
+If you want to customize your checkout by overriding manifest settings, use
+the local_manifests/ directory instead.
+
+For more information on repo manifests, check out:
+https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
+-->
+<manifest>
+  <include name="%s" />
+</manifest>
+""" % (name,))
 
   def _RemoteToXml(self, r, doc, root):
     e = doc.createElement('remote')
@@ -200,18 +304,28 @@ class XmlManifest(object):
     if r.revision is not None:
       e.setAttribute('revision', r.revision)
 
-  def _ParseGroups(self, groups):
-    return [x for x in re.split(r'[,\s]+', groups) if x]
+    for a in r.annotations:
+      if a.keep == 'true':
+        ae = doc.createElement('annotation')
+        ae.setAttribute('name', a.name)
+        ae.setAttribute('value', a.value)
+        e.appendChild(ae)
+
+  def _ParseList(self, field):
+    """Parse fields that contain flattened lists.
 
-  def Save(self, fd, peg_rev=False, peg_rev_upstream=True, groups=None):
-    """Write the current manifest out to the given file descriptor.
+    These are whitespace & comma separated.  Empty elements will be discarded.
     """
+    return [x for x in re.split(r'[,\s]+', field) if x]
+
+  def ToXml(self, peg_rev=False, peg_rev_upstream=True, peg_rev_dest_branch=True, groups=None):
+    """Return the current manifest XML."""
     mp = self.manifestProject
 
     if groups is None:
       groups = mp.config.GetString('manifest.groups')
     if groups:
-      groups = self._ParseGroups(groups)
+      groups = self._ParseList(groups)
 
     doc = xml.dom.minidom.Document()
     root = doc.createElement('manifest')
@@ -223,7 +337,7 @@ class XmlManifest(object):
     if self.notice:
       notice_element = root.appendChild(doc.createElement('notice'))
       notice_lines = self.notice.splitlines()
-      indented_notice = ('\n'.join(" "*4 + line for line in notice_lines))[4:]
+      indented_notice = ('\n'.join(" " * 4 + line for line in notice_lines))[4:]
       notice_element.appendChild(doc.createTextNode(indented_notice))
 
     d = self.default
@@ -308,10 +422,19 @@ class XmlManifest(object):
             # Only save the origin if the origin is not a sha1, and the default
             # isn't our value
             e.setAttribute('upstream', p.revisionExpr)
+
+        if peg_rev_dest_branch:
+          if p.dest_branch:
+            e.setAttribute('dest-branch', p.dest_branch)
+          elif value != p.revisionExpr:
+            e.setAttribute('dest-branch', p.revisionExpr)
+
       else:
         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)
@@ -372,11 +495,84 @@ class XmlManifest(object):
                      ' '.join(self._repo_hooks_project.enabled_repo_hooks))
       root.appendChild(e)
 
+    if self._superproject:
+      root.appendChild(doc.createTextNode(''))
+      e = doc.createElement('superproject')
+      e.setAttribute('name', self._superproject['name'])
+      remoteName = None
+      if d.remote:
+        remoteName = d.remote.name
+      remote = self._superproject.get('remote')
+      if not d.remote or remote.orig_name != remoteName:
+        remoteName = remote.orig_name
+        e.setAttribute('remote', remoteName)
+      revision = remote.revision or d.revisionExpr
+      if not revision or revision != self._superproject['revision']:
+        e.setAttribute('revision', self._superproject['revision'])
+      root.appendChild(e)
+
+    if self._contactinfo.bugurl != Wrapper().BUG_URL:
+      root.appendChild(doc.createTextNode(''))
+      e = doc.createElement('contactinfo')
+      e.setAttribute('bugurl', self._contactinfo.bugurl)
+      root.appendChild(e)
+
+    return doc
+
+  def ToDict(self, **kwargs):
+    """Return the current manifest as a dictionary."""
+    # Elements that may only appear once.
+    SINGLE_ELEMENTS = {
+        'notice',
+        'default',
+        'manifest-server',
+        'repo-hooks',
+        'superproject',
+        'contactinfo',
+    }
+    # Elements that may be repeated.
+    MULTI_ELEMENTS = {
+        'remote',
+        'remove-project',
+        'project',
+        'extend-project',
+        'include',
+        # These are children of 'project' nodes.
+        'annotation',
+        'project',
+        'copyfile',
+        'linkfile',
+    }
+
+    doc = self.ToXml(**kwargs)
+    ret = {}
+
+    def append_children(ret, node):
+      for child in node.childNodes:
+        if child.nodeType == xml.dom.Node.ELEMENT_NODE:
+          attrs = child.attributes
+          element = dict((attrs.item(i).localName, attrs.item(i).value)
+                         for i in range(attrs.length))
+          if child.nodeName in SINGLE_ELEMENTS:
+            ret[child.nodeName] = element
+          elif child.nodeName in MULTI_ELEMENTS:
+            ret.setdefault(child.nodeName, []).append(element)
+          else:
+            raise ManifestParseError('Unhandled element "%s"' % (child.nodeName,))
+
+          append_children(element, child)
+
+    append_children(ret, doc.firstChild)
+
+    return ret
+
+  def Save(self, fd, **kwargs):
+    """Write the current manifest out to the given file descriptor."""
+    doc = self.ToXml(**kwargs)
     doc.writexml(fd, '', '  ', '\n', 'UTF-8')
 
   def _output_manifest_project_extras(self, p, e):
     """Manifests can modify e if they support extra project attributes."""
-    pass
 
   @property
   def paths(self):
@@ -404,6 +600,16 @@ class XmlManifest(object):
     return self._repo_hooks_project
 
   @property
+  def superproject(self):
+    self._Load()
+    return self._superproject
+
+  @property
+  def contactinfo(self):
+    self._Load()
+    return self._contactinfo
+
+  @property
   def notice(self):
     self._Load()
     return self._notice
@@ -414,16 +620,45 @@ class XmlManifest(object):
     return self._manifest_server
 
   @property
+  def CloneBundle(self):
+    clone_bundle = self.manifestProject.config.GetBoolean('repo.clonebundle')
+    if clone_bundle is None:
+      return False if self.manifestProject.config.GetBoolean('repo.partialclone') else True
+    else:
+      return clone_bundle
+
+  @property
   def CloneFilter(self):
     if self.manifestProject.config.GetBoolean('repo.partialclone'):
       return self.manifestProject.config.GetString('repo.clonefilter')
     return None
 
   @property
+  def PartialCloneExclude(self):
+    exclude = self.manifest.manifestProject.config.GetString(
+        'repo.partialcloneexclude') or ''
+    return set(x.strip() for x in exclude.split(','))
+
+  @property
+  def UseLocalManifests(self):
+    return self._load_local_manifests
+
+  def SetUseLocalManifests(self, value):
+    self._load_local_manifests = value
+
+  @property
+  def HasLocalManifests(self):
+    return self._load_local_manifests and self.local_manifests
+
+  @property
   def IsMirror(self):
     return self.manifestProject.config.GetBoolean('repo.mirror')
 
   @property
+  def UseGitWorktrees(self):
+    return self.manifestProject.config.GetBoolean('repo.worktree')
+
+  @property
   def IsArchive(self):
     return self.manifestProject.config.GetBoolean('repo.archive')
 
@@ -431,6 +666,17 @@ class XmlManifest(object):
   def HasSubmodules(self):
     return self.manifestProject.config.GetBoolean('repo.submodules')
 
+  def GetDefaultGroupsStr(self):
+    """Returns the default group string for the platform."""
+    return 'default,platform-' + platform.system().lower()
+
+  def GetGroupsStr(self):
+    """Returns the manifest group string that should be synced."""
+    groups = self.manifestProject.config.GetString('manifest.groups')
+    if not groups:
+      groups = self.GetDefaultGroupsStr()
+    return groups
+
   def _Unload(self):
     self._loaded = False
     self._projects = {}
@@ -438,6 +684,8 @@ class XmlManifest(object):
     self._remotes = {}
     self._default = None
     self._repo_hooks_project = None
+    self._superproject = {}
+    self._contactinfo = ContactInfo(Wrapper().BUG_URL)
     self._notice = None
     self.branch = None
     self._manifest_server = None
@@ -450,28 +698,24 @@ class XmlManifest(object):
         b = b[len(R_HEADS):]
       self.branch = b
 
+      # The manifestFile was specified by the user which is why we allow include
+      # paths to point anywhere.
       nodes = []
-      nodes.append(self._ParseManifestXml(self.manifestFile,
-                                          self.manifestProject.worktree))
-
-      if self._load_local_manifests:
-        local = os.path.join(self.repodir, LOCAL_MANIFEST_NAME)
-        if os.path.exists(local):
-          if not self.localManifestWarning:
-            self.localManifestWarning = True
-            print('warning: %s is deprecated; put local manifests '
-                  'in `%s` instead' % (LOCAL_MANIFEST_NAME,
-                  os.path.join(self.repodir, LOCAL_MANIFESTS_DIR_NAME)),
-                  file=sys.stderr)
-          nodes.append(self._ParseManifestXml(local, self.repodir))
-
-        local_dir = os.path.abspath(os.path.join(self.repodir,
-                                    LOCAL_MANIFESTS_DIR_NAME))
+      nodes.append(self._ParseManifestXml(
+          self.manifestFile, self.manifestProject.worktree,
+          restrict_includes=False))
+
+      if self._load_local_manifests and self.local_manifests:
         try:
-          for local_file in sorted(platform_utils.listdir(local_dir)):
+          for local_file in sorted(platform_utils.listdir(self.local_manifests)):
             if local_file.endswith('.xml'):
-              local = os.path.join(local_dir, local_file)
-              nodes.append(self._ParseManifestXml(local, self.repodir))
+              local = os.path.join(self.local_manifests, local_file)
+              # Since local manifests are entirely managed by the user, allow
+              # them to point anywhere the user wants.
+              nodes.append(self._ParseManifestXml(
+                  local, self.repodir,
+                  parent_groups=f'{LOCAL_MANIFEST_GROUP_PREFIX}:{local_file[:-4]}',
+                  restrict_includes=False))
         except OSError:
           pass
 
@@ -489,7 +733,19 @@ class XmlManifest(object):
 
       self._loaded = True
 
-  def _ParseManifestXml(self, path, include_root):
+  def _ParseManifestXml(self, path, include_root, parent_groups='',
+                        restrict_includes=True):
+    """Parse a manifest XML and return the computed nodes.
+
+    Args:
+      path: The XML file to read & parse.
+      include_root: The path to interpret include "name"s relative to.
+      parent_groups: The groups to apply to this projects.
+      restrict_includes: Whether to constrain the "name" attribute of includes.
+
+    Returns:
+      List of XML nodes.
+    """
     try:
       root = xml.dom.minidom.parse(path)
     except (OSError, xml.parsers.expat.ExpatError) as e:
@@ -508,20 +764,35 @@ class XmlManifest(object):
     for node in manifest.childNodes:
       if node.nodeName == 'include':
         name = self._reqatt(node, 'name')
+        if restrict_includes:
+          msg = self._CheckLocalPath(name)
+          if msg:
+            raise ManifestInvalidPathError(
+                '<include> invalid "name": %s: %s' % (name, msg))
+        include_groups = ''
+        if parent_groups:
+          include_groups = parent_groups
+        if node.hasAttribute('groups'):
+          include_groups = node.getAttribute('groups') + ',' + include_groups
         fp = os.path.join(include_root, name)
         if not os.path.isfile(fp):
-          raise ManifestParseError("include %s doesn't exist or isn't a file"
-              % (name,))
+          raise ManifestParseError("include [%s/]%s doesn't exist or isn't a file"
+                                   % (include_root, name))
         try:
-          nodes.extend(self._ParseManifestXml(fp, include_root))
+          nodes.extend(self._ParseManifestXml(fp, include_root, include_groups))
         # should isolate this to the exact exception, but that's
         # tricky.  actual parsing implementation may vary.
-        except (KeyboardInterrupt, RuntimeError, SystemExit):
+        except (KeyboardInterrupt, RuntimeError, SystemExit, ManifestParseError):
           raise
         except Exception as e:
           raise ManifestParseError(
               "failed parsing included manifest %s: %s" % (name, e))
       else:
+        if parent_groups and node.nodeName == 'project':
+          nodeGroups = parent_groups
+          if node.hasAttribute('groups'):
+            nodeGroups = node.getAttribute('groups') + ',' + nodeGroups
+          node.setAttribute('groups', nodeGroups)
         nodes.append(node)
     return nodes
 
@@ -541,9 +812,10 @@ class XmlManifest(object):
     for node in itertools.chain(*node_list):
       if node.nodeName == 'default':
         new_default = self._ParseDefault(node)
+        emptyDefault = not node.hasAttributes() and not node.hasChildNodes()
         if self._default is None:
           self._default = new_default
-        elif new_default != self._default:
+        elif not emptyDefault and new_default != self._default:
           raise ManifestParseError('duplicate default in %s' %
                                    (self.manifestFile))
 
@@ -582,6 +854,8 @@ class XmlManifest(object):
       for subproject in project.subprojects:
         recursively_add_projects(subproject)
 
+    repo_hooks_project = None
+    enabled_repo_hooks = None
     for node in itertools.chain(*node_list):
       if node.nodeName == 'project':
         project = self._ParseProject(node)
@@ -594,61 +868,108 @@ class XmlManifest(object):
                                    'project: %s' % name)
 
         path = node.getAttribute('path')
+        dest_path = node.getAttribute('dest-path')
         groups = node.getAttribute('groups')
         if groups:
-          groups = self._ParseGroups(groups)
+          groups = self._ParseList(groups)
         revision = node.getAttribute('revision')
+        remote = node.getAttribute('remote')
+        if remote:
+          remote = self._get_remote(node)
 
+        named_projects = self._projects[name]
+        if dest_path and not path and len(named_projects) > 1:
+          raise ManifestParseError('extend-project cannot use dest-path when '
+                                   'matching multiple projects: %s' % name)
         for p in self._projects[name]:
           if path and p.relpath != path:
             continue
           if groups:
             p.groups.extend(groups)
           if revision:
-            p.revisionExpr = revision
-      if node.nodeName == 'repo-hooks':
-        # Get the name of the project and the (space-separated) list of enabled.
-        repo_hooks_project = self._reqatt(node, 'in-project')
-        enabled_repo_hooks = self._reqatt(node, 'enabled-list').split()
+            p.SetRevision(revision)
+
+          if remote:
+            p.remote = remote.ToRemoteSpec(name)
+
+          if dest_path:
+            del self._paths[p.relpath]
+            relpath, worktree, gitdir, objdir, _ = self.GetProjectPaths(name, dest_path)
+            p.UpdatePaths(relpath, worktree, gitdir, objdir)
+            self._paths[p.relpath] = p
 
+      if node.nodeName == 'repo-hooks':
         # Only one project can be the hooks project
-        if self._repo_hooks_project is not None:
+        if repo_hooks_project is not None:
           raise ManifestParseError(
               'duplicate repo-hooks in %s' %
               (self.manifestFile))
 
-        # Store a reference to the Project.
-        try:
-          repo_hooks_projects = self._projects[repo_hooks_project]
-        except KeyError:
-          raise ManifestParseError(
-              'project %s not found for repo-hooks' %
-              (repo_hooks_project))
-
-        if len(repo_hooks_projects) != 1:
+        # Get the name of the project and the (space-separated) list of enabled.
+        repo_hooks_project = self._reqatt(node, 'in-project')
+        enabled_repo_hooks = self._ParseList(self._reqatt(node, 'enabled-list'))
+      if node.nodeName == 'superproject':
+        name = self._reqatt(node, 'name')
+        # There can only be one superproject.
+        if self._superproject.get('name'):
           raise ManifestParseError(
-              'internal error parsing repo-hooks in %s' %
+              'duplicate superproject in %s' %
               (self.manifestFile))
-        self._repo_hooks_project = repo_hooks_projects[0]
+        self._superproject['name'] = name
+        remote_name = node.getAttribute('remote')
+        if not remote_name:
+          remote = self._default.remote
+        else:
+          remote = self._get_remote(node)
+        if remote is None:
+          raise ManifestParseError("no remote for superproject %s within %s" %
+                                   (name, self.manifestFile))
+        self._superproject['remote'] = remote.ToRemoteSpec(name)
+        revision = node.getAttribute('revision') or remote.revision
+        if not revision:
+          revision = self._default.revisionExpr
+        if not revision:
+          raise ManifestParseError('no revision for superproject %s within %s' %
+                                   (name, self.manifestFile))
+        self._superproject['revision'] = revision
+      if node.nodeName == 'contactinfo':
+        bugurl = self._reqatt(node, 'bugurl')
+        # This element can be repeated, later entries will clobber earlier ones.
+        self._contactinfo = ContactInfo(bugurl)
 
-        # Store the enabled hooks in the Project object.
-        self._repo_hooks_project.enabled_repo_hooks = enabled_repo_hooks
       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 repo_hooks_project == name:
+            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
+    # Store repo hooks project information.
+    if repo_hooks_project:
+      # Store a reference to the Project.
+      try:
+        repo_hooks_projects = self._projects[repo_hooks_project]
+      except KeyError:
+        raise ManifestParseError(
+            'project %s not found for repo-hooks' %
+            (repo_hooks_project))
 
+      if len(repo_hooks_projects) != 1:
+        raise ManifestParseError(
+            'internal error parsing repo-hooks in %s' %
+            (self.manifestFile))
+      self._repo_hooks_project = repo_hooks_projects[0]
+      # Store the enabled hooks in the Project object.
+      self._repo_hooks_project.enabled_repo_hooks = enabled_repo_hooks
 
   def _AddMetaProjectMirror(self, m):
     name = None
@@ -676,15 +997,15 @@ class XmlManifest(object):
     if name not in self._projects:
       m.PreSync()
       gitdir = os.path.join(self.topdir, '%s.git' % name)
-      project = Project(manifest = self,
-                        name = name,
-                        remote = remote.ToRemoteSpec(name),
-                        gitdir = gitdir,
-                        objdir = gitdir,
-                        worktree = None,
-                        relpath = name or None,
-                        revisionExpr = m.revisionExpr,
-                        revisionId = None)
+      project = Project(manifest=self,
+                        name=name,
+                        remote=remote.ToRemoteSpec(name),
+                        gitdir=gitdir,
+                        objdir=gitdir,
+                        worktree=None,
+                        relpath=name or None,
+                        revisionExpr=m.revisionExpr,
+                        revisionId=None)
       self._projects[project.name] = [project]
       self._paths[project.relpath] = project
 
@@ -707,7 +1028,14 @@ class XmlManifest(object):
     if revision == '':
       revision = None
     manifestUrl = self.manifestProject.config.GetString('remote.origin.url')
-    return _XmlRemote(name, alias, fetch, pushUrl, manifestUrl, review, revision)
+
+    remote = _XmlRemote(name, alias, fetch, pushUrl, manifestUrl, review, revision)
+
+    for n in node.childNodes:
+      if n.nodeName == 'annotation':
+        self._ParseAnnotation(remote, n)
+
+    return remote
 
   def _ParseDefault(self, node):
     """
@@ -722,29 +1050,14 @@ class XmlManifest(object):
     d.destBranchExpr = node.getAttribute('dest-branch') or None
     d.upstreamExpr = node.getAttribute('upstream') or None
 
-    sync_j = node.getAttribute('sync-j')
-    if sync_j == '' or sync_j is None:
-      d.sync_j = 1
-    else:
-      d.sync_j = int(sync_j)
-
-    sync_c = node.getAttribute('sync-c')
-    if not sync_c:
-      d.sync_c = False
-    else:
-      d.sync_c = sync_c.lower() in ("yes", "true", "1")
+    d.sync_j = XmlInt(node, 'sync-j', 1)
+    if d.sync_j <= 0:
+      raise ManifestParseError('%s: sync-j must be greater than 0, not "%s"' %
+                               (self.manifestFile, d.sync_j))
 
-    sync_s = node.getAttribute('sync-s')
-    if not sync_s:
-      d.sync_s = False
-    else:
-      d.sync_s = sync_s.lower() in ("yes", "true", "1")
-
-    sync_tags = node.getAttribute('sync-tags')
-    if not sync_tags:
-      d.sync_tags = True
-    else:
-      d.sync_tags = sync_tags.lower() in ("yes", "true", "1")
+    d.sync_c = XmlBool(node, 'sync-c', False)
+    d.sync_s = XmlBool(node, 'sync-s', False)
+    d.sync_tags = XmlBool(node, 'sync-tags', True)
     return d
 
   def _ParseNotice(self, node):
@@ -792,11 +1105,15 @@ class XmlManifest(object):
   def _UnjoinName(self, parent_name, name):
     return os.path.relpath(name, parent_name)
 
-  def _ParseProject(self, node, parent = None, **extra_proj_attrs):
+  def _ParseProject(self, node, parent=None, **extra_proj_attrs):
     """
     reads a <project> element from the manifest file
     """
     name = self._reqatt(node, 'name')
+    msg = self._CheckLocalPath(name, dir_ok=True)
+    if msg:
+      raise ManifestInvalidPathError(
+          '<project> invalid "name": %s: %s' % (name, msg))
     if parent:
       name = self._JoinName(parent.name, name)
 
@@ -805,55 +1122,34 @@ class XmlManifest(object):
       remote = self._default.remote
     if remote is None:
       raise ManifestParseError("no remote for project %s within %s" %
-            (name, self.manifestFile))
+                               (name, self.manifestFile))
 
     revisionExpr = node.getAttribute('revision') or remote.revision
     if not revisionExpr:
       revisionExpr = self._default.revisionExpr
     if not revisionExpr:
       raise ManifestParseError("no revision for project %s within %s" %
-            (name, self.manifestFile))
+                               (name, self.manifestFile))
 
     path = node.getAttribute('path')
     if not path:
       path = name
-    if path.startswith('/'):
-      raise ManifestParseError("project %s path cannot be absolute in %s" %
-            (name, self.manifestFile))
-
-    rebase = node.getAttribute('rebase')
-    if not rebase:
-      rebase = True
     else:
-      rebase = rebase.lower() in ("yes", "true", "1")
-
-    sync_c = node.getAttribute('sync-c')
-    if not sync_c:
-      sync_c = False
-    else:
-      sync_c = sync_c.lower() in ("yes", "true", "1")
-
-    sync_s = node.getAttribute('sync-s')
-    if not sync_s:
-      sync_s = self._default.sync_s
-    else:
-      sync_s = sync_s.lower() in ("yes", "true", "1")
-
-    sync_tags = node.getAttribute('sync-tags')
-    if not sync_tags:
-      sync_tags = self._default.sync_tags
-    else:
-      sync_tags = sync_tags.lower() in ("yes", "true", "1")
-
-    clone_depth = node.getAttribute('clone-depth')
-    if clone_depth:
-      try:
-        clone_depth = int(clone_depth)
-        if  clone_depth <= 0:
-          raise ValueError()
-      except ValueError:
-        raise ManifestParseError('invalid clone-depth %s in %s' %
-                                 (clone_depth, self.manifestFile))
+      # NB: The "." project is handled specially in Project.Sync_LocalHalf.
+      msg = self._CheckLocalPath(path, dir_ok=True, cwd_dot_ok=True)
+      if msg:
+        raise ManifestInvalidPathError(
+            '<project> invalid "path": %s: %s' % (path, msg))
+
+    rebase = XmlBool(node, 'rebase', True)
+    sync_c = XmlBool(node, 'sync-c', False)
+    sync_s = XmlBool(node, 'sync-s', self._default.sync_s)
+    sync_tags = XmlBool(node, 'sync-tags', self._default.sync_tags)
+
+    clone_depth = XmlInt(node, 'clone-depth')
+    if clone_depth is not None and clone_depth <= 0:
+      raise ManifestParseError('%s: clone-depth must be greater than 0, not "%s"' %
+                               (self.manifestFile, clone_depth))
 
     dest_branch = node.getAttribute('dest-branch') or self._default.destBranchExpr
 
@@ -862,11 +1158,13 @@ class XmlManifest(object):
     groups = ''
     if node.hasAttribute('groups'):
       groups = node.getAttribute('groups')
-    groups = self._ParseGroups(groups)
+    groups = self._ParseList(groups)
 
     if parent is None:
-      relpath, worktree, gitdir, objdir = self.GetProjectPaths(name, path)
+      relpath, worktree, gitdir, objdir, use_git_worktrees = \
+          self.GetProjectPaths(name, path)
     else:
+      use_git_worktrees = False
       relpath, worktree, gitdir, objdir = \
           self.GetSubprojectPaths(parent, name, path)
 
@@ -874,27 +1172,28 @@ class XmlManifest(object):
     groups.extend(set(default_groups).difference(groups))
 
     if self.IsMirror and node.hasAttribute('force-path'):
-      if node.getAttribute('force-path').lower() in ("yes", "true", "1"):
+      if XmlBool(node, 'force-path', False):
         gitdir = os.path.join(self.topdir, '%s.git' % path)
 
-    project = Project(manifest = self,
-                      name = name,
-                      remote = remote.ToRemoteSpec(name),
-                      gitdir = gitdir,
-                      objdir = objdir,
-                      worktree = worktree,
-                      relpath = relpath,
-                      revisionExpr = revisionExpr,
-                      revisionId = None,
-                      rebase = rebase,
-                      groups = groups,
-                      sync_c = sync_c,
-                      sync_s = sync_s,
-                      sync_tags = sync_tags,
-                      clone_depth = clone_depth,
-                      upstream = upstream,
-                      parent = parent,
-                      dest_branch = dest_branch,
+    project = Project(manifest=self,
+                      name=name,
+                      remote=remote.ToRemoteSpec(name),
+                      gitdir=gitdir,
+                      objdir=objdir,
+                      worktree=worktree,
+                      relpath=relpath,
+                      revisionExpr=revisionExpr,
+                      revisionId=None,
+                      rebase=rebase,
+                      groups=groups,
+                      sync_c=sync_c,
+                      sync_s=sync_s,
+                      sync_tags=sync_tags,
+                      clone_depth=clone_depth,
+                      upstream=upstream,
+                      parent=parent,
+                      dest_branch=dest_branch,
+                      use_git_worktrees=use_git_worktrees,
                       **extra_proj_attrs)
 
     for n in node.childNodes:
@@ -905,11 +1204,16 @@ class XmlManifest(object):
       if n.nodeName == 'annotation':
         self._ParseAnnotation(project, n)
       if n.nodeName == 'project':
-        project.subprojects.append(self._ParseProject(n, parent = project))
+        project.subprojects.append(self._ParseProject(n, parent=project))
 
     return project
 
   def GetProjectPaths(self, name, path):
+    # The manifest entries might have trailing slashes.  Normalize them to avoid
+    # unexpected filesystem behavior since we do string concatenation below.
+    path = path.rstrip('/')
+    name = name.rstrip('/')
+    use_git_worktrees = False
     relpath = path
     if self.IsMirror:
       worktree = None
@@ -918,8 +1222,15 @@ class XmlManifest(object):
     else:
       worktree = os.path.join(self.topdir, path).replace('\\', '/')
       gitdir = os.path.join(self.repodir, 'projects', '%s.git' % path)
-      objdir = os.path.join(self.repodir, 'project-objects', '%s.git' % name)
-    return relpath, worktree, gitdir, objdir
+      # We allow people to mix git worktrees & non-git worktrees for now.
+      # This allows for in situ migration of repo clients.
+      if os.path.exists(gitdir) or not self.UseGitWorktrees:
+        objdir = os.path.join(self.repodir, 'project-objects', '%s.git' % name)
+      else:
+        use_git_worktrees = True
+        gitdir = os.path.join(self.repodir, 'worktrees', '%s.git' % name)
+        objdir = gitdir
+    return relpath, worktree, gitdir, objdir, use_git_worktrees
 
   def GetProjectsWithName(self, name):
     return self._projects.get(name, [])
@@ -934,6 +1245,10 @@ class XmlManifest(object):
     return os.path.relpath(relpath, parent_relpath)
 
   def GetSubprojectPaths(self, parent, name, path):
+    # The manifest entries might have trailing slashes.  Normalize them to avoid
+    # unexpected filesystem behavior since we do string concatenation below.
+    path = path.rstrip('/')
+    name = name.rstrip('/')
     relpath = self._JoinRelpath(parent.relpath, path)
     gitdir = os.path.join(parent.gitdir, 'subprojects', '%s.git' % path)
     objdir = os.path.join(parent.gitdir, 'subproject-objects', '%s.git' % name)
@@ -943,23 +1258,151 @@ class XmlManifest(object):
       worktree = os.path.join(parent.worktree, path).replace('\\', '/')
     return relpath, worktree, gitdir, objdir
 
+  @staticmethod
+  def _CheckLocalPath(path, dir_ok=False, cwd_dot_ok=False):
+    """Verify |path| is reasonable for use in filesystem paths.
+
+    Used with <copyfile> & <linkfile> & <project> elements.
+
+    This only validates the |path| in isolation: it does not check against the
+    current filesystem state.  Thus it is suitable as a first-past in a parser.
+
+    It enforces a number of constraints:
+    * No empty paths.
+    * No "~" in paths.
+    * No Unicode codepoints that filesystems might elide when normalizing.
+    * No relative path components like "." or "..".
+    * No absolute paths.
+    * No ".git" or ".repo*" path components.
+
+    Args:
+      path: The path name to validate.
+      dir_ok: Whether |path| may force a directory (e.g. end in a /).
+      cwd_dot_ok: Whether |path| may be just ".".
+
+    Returns:
+      None if |path| is OK, a failure message otherwise.
+    """
+    if not path:
+      return 'empty paths not allowed'
+
+    if '~' in path:
+      return '~ not allowed (due to 8.3 filenames on Windows filesystems)'
+
+    path_codepoints = set(path)
+
+    # Some filesystems (like Apple's HFS+) try to normalize Unicode codepoints
+    # which means there are alternative names for ".git".  Reject paths with
+    # these in it as there shouldn't be any reasonable need for them here.
+    # The set of codepoints here was cribbed from jgit's implementation:
+    # https://eclipse.googlesource.com/jgit/jgit/+/9110037e3e9461ff4dac22fee84ef3694ed57648/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java#884
+    BAD_CODEPOINTS = {
+        u'\u200C',  # ZERO WIDTH NON-JOINER
+        u'\u200D',  # ZERO WIDTH JOINER
+        u'\u200E',  # LEFT-TO-RIGHT MARK
+        u'\u200F',  # RIGHT-TO-LEFT MARK
+        u'\u202A',  # LEFT-TO-RIGHT EMBEDDING
+        u'\u202B',  # RIGHT-TO-LEFT EMBEDDING
+        u'\u202C',  # POP DIRECTIONAL FORMATTING
+        u'\u202D',  # LEFT-TO-RIGHT OVERRIDE
+        u'\u202E',  # RIGHT-TO-LEFT OVERRIDE
+        u'\u206A',  # INHIBIT SYMMETRIC SWAPPING
+        u'\u206B',  # ACTIVATE SYMMETRIC SWAPPING
+        u'\u206C',  # INHIBIT ARABIC FORM SHAPING
+        u'\u206D',  # ACTIVATE ARABIC FORM SHAPING
+        u'\u206E',  # NATIONAL DIGIT SHAPES
+        u'\u206F',  # NOMINAL DIGIT SHAPES
+        u'\uFEFF',  # ZERO WIDTH NO-BREAK SPACE
+    }
+    if BAD_CODEPOINTS & path_codepoints:
+      # This message is more expansive than reality, but should be fine.
+      return 'Unicode combining characters not allowed'
+
+    # Reject newlines as there shouldn't be any legitmate use for them, they'll
+    # be confusing to users, and they can easily break tools that expect to be
+    # able to iterate over newline delimited lists.  This even applies to our
+    # own code like .repo/project.list.
+    if {'\r', '\n'} & path_codepoints:
+      return 'Newlines not allowed'
+
+    # Assume paths might be used on case-insensitive filesystems.
+    path = path.lower()
+
+    # Split up the path by its components.  We can't use os.path.sep exclusively
+    # as some platforms (like Windows) will convert / to \ and that bypasses all
+    # our constructed logic here.  Especially since manifest authors only use
+    # / in their paths.
+    resep = re.compile(r'[/%s]' % re.escape(os.path.sep))
+    # Strip off trailing slashes as those only produce '' elements, and we use
+    # parts to look for individual bad components.
+    parts = resep.split(path.rstrip('/'))
+
+    # Some people use src="." to create stable links to projects.  Lets allow
+    # that but reject all other uses of "." to keep things simple.
+    if not cwd_dot_ok or parts != ['.']:
+      for part in set(parts):
+        if part in {'.', '..', '.git'} or part.startswith('.repo'):
+          return 'bad component: %s' % (part,)
+
+    if not dir_ok and resep.match(path[-1]):
+      return 'dirs not allowed'
+
+    # NB: The two abspath checks here are to handle platforms with multiple
+    # filesystem path styles (e.g. Windows).
+    norm = os.path.normpath(path)
+    if (norm == '..' or
+        (len(norm) >= 3 and norm.startswith('..') and resep.match(norm[0])) or
+        os.path.isabs(norm) or
+        norm.startswith('/')):
+      return 'path cannot be outside'
+
+  @classmethod
+  def _ValidateFilePaths(cls, element, src, dest):
+    """Verify |src| & |dest| are reasonable for <copyfile> & <linkfile>.
+
+    We verify the path independent of any filesystem state as we won't have a
+    checkout available to compare to.  i.e. This is for parsing validation
+    purposes only.
+
+    We'll do full/live sanity checking before we do the actual filesystem
+    modifications in _CopyFile/_LinkFile/etc...
+    """
+    # |dest| is the file we write to or symlink we create.
+    # It is relative to the top of the repo client checkout.
+    msg = cls._CheckLocalPath(dest)
+    if msg:
+      raise ManifestInvalidPathError(
+          '<%s> invalid "dest": %s: %s' % (element, dest, msg))
+
+    # |src| is the file we read from or path we point to for symlinks.
+    # It is relative to the top of the git project checkout.
+    is_linkfile = element == 'linkfile'
+    msg = cls._CheckLocalPath(src, dir_ok=is_linkfile, cwd_dot_ok=is_linkfile)
+    if msg:
+      raise ManifestInvalidPathError(
+          '<%s> invalid "src": %s: %s' % (element, src, msg))
+
   def _ParseCopyFile(self, project, node):
     src = self._reqatt(node, 'src')
     dest = self._reqatt(node, 'dest')
     if not self.IsMirror:
       # src is project relative;
-      # dest is relative to the top of the tree
-      project.AddCopyFile(src, dest, os.path.join(self.topdir, dest))
+      # dest is relative to the top of the tree.
+      # We only validate paths if we actually plan to process them.
+      self._ValidateFilePaths('copyfile', src, dest)
+      project.AddCopyFile(src, dest, self.topdir)
 
   def _ParseLinkFile(self, project, node):
     src = self._reqatt(node, 'src')
     dest = self._reqatt(node, 'dest')
     if not self.IsMirror:
       # src is project relative;
-      # dest is relative to the top of the tree
-      project.AddLinkFile(src, dest, os.path.join(self.topdir, dest))
+      # dest is relative to the top of the tree.
+      # We only validate paths if we actually plan to process them.
+      self._ValidateFilePaths('linkfile', src, dest)
+      project.AddLinkFile(src, dest, self.topdir)
 
-  def _ParseAnnotation(self, project, node):
+  def _ParseAnnotation(self, element, node):
     name = self._reqatt(node, 'name')
     value = self._reqatt(node, 'value')
     try:
@@ -968,8 +1411,8 @@ class XmlManifest(object):
       keep = "true"
     if keep != "true" and keep != "false":
       raise ManifestParseError('optional "keep" attribute must be '
-            '"true" or "false"')
-    project.AddAnnotation(name, value, keep)
+                               '"true" or "false"')
+    element.AddAnnotation(name, value, keep)
 
   def _get_remote(self, node):
     name = node.getAttribute('remote')
@@ -979,7 +1422,7 @@ class XmlManifest(object):
     v = self._remotes.get(name)
     if not v:
       raise ManifestParseError("remote %s not defined in %s" %
-            (name, self.manifestFile))
+                               (name, self.manifestFile))
     return v
 
   def _reqatt(self, node, attname):
@@ -989,7 +1432,7 @@ class XmlManifest(object):
     v = node.getAttribute(attname)
     if not v:
       raise ManifestParseError("no %s in <%s> within %s" %
-            (attname, node.nodeName, self.manifestFile))
+                               (attname, node.nodeName, self.manifestFile))
     return v
 
   def projectsDiff(self, manifest):
@@ -1007,7 +1450,7 @@ class XmlManifest(object):
     diff = {'added': [], 'removed': [], 'changed': [], 'unreachable': []}
 
     for proj in fromKeys:
-      if not proj in toKeys:
+      if proj not in toKeys:
         diff['removed'].append(fromProjects[proj])
       else:
         fromProj = fromProjects[proj]
@@ -1029,19 +1472,11 @@ class XmlManifest(object):
 
 
 class GitcManifest(XmlManifest):
+  """Parser for GitC (git-in-the-cloud) manifests."""
 
-  def __init__(self, repodir, gitc_client_name):
-    """Initialize the GitcManifest object."""
-    super(GitcManifest, self).__init__(repodir)
-    self.isGitcClient = True
-    self.gitc_client_name = gitc_client_name
-    self.gitc_client_dir = os.path.join(gitc_utils.get_gitc_manifest_dir(),
-                                        gitc_client_name)
-    self.manifestFile = os.path.join(self.gitc_client_dir, '.manifest')
-
-  def _ParseProject(self, node, parent = None):
+  def _ParseProject(self, node, parent=None):
     """Override _ParseProject and add support for GITC specific attributes."""
-    return super(GitcManifest, self)._ParseProject(
+    return super()._ParseProject(
         node, parent=parent, old_revision=node.getAttribute('old-revision'))
 
   def _output_manifest_project_extras(self, p, e):
@@ -1049,3 +1484,36 @@ class GitcManifest(XmlManifest):
     if p.old_revision:
       e.setAttribute('old-revision', str(p.old_revision))
 
+
+class RepoClient(XmlManifest):
+  """Manages a repo client checkout."""
+
+  def __init__(self, repodir, manifest_file=None):
+    self.isGitcClient = False
+
+    if os.path.exists(os.path.join(repodir, LOCAL_MANIFEST_NAME)):
+      print('error: %s is not supported; put local manifests in `%s` instead' %
+            (LOCAL_MANIFEST_NAME, os.path.join(repodir, LOCAL_MANIFESTS_DIR_NAME)),
+            file=sys.stderr)
+      sys.exit(1)
+
+    if manifest_file is None:
+      manifest_file = os.path.join(repodir, MANIFEST_FILE_NAME)
+    local_manifests = os.path.abspath(os.path.join(repodir, LOCAL_MANIFESTS_DIR_NAME))
+    super().__init__(repodir, manifest_file, local_manifests)
+
+    # TODO: Completely separate manifest logic out of the client.
+    self.manifest = self
+
+
+class GitcClient(RepoClient, GitcManifest):
+  """Manages a GitC client checkout."""
+
+  def __init__(self, repodir, gitc_client_name):
+    """Initialize the GitcManifest object."""
+    self.gitc_client_name = gitc_client_name
+    self.gitc_client_dir = os.path.join(gitc_utils.get_gitc_manifest_dir(),
+                                        gitc_client_name)
+
+    super().__init__(repodir, os.path.join(self.gitc_client_dir, '.manifest'))
+    self.isGitcClient = True