sync: fix reporting of failed local checkouts

The refactor to multiprocessing broke status reporting slightly when
checking out projects.  Make sure we mark the step as failed if any
of the projects failed, not just when --fail-fast is set.

Change-Id: I0efb56ce83b068b2c334046df3fef23d797599c9
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/299882
Reviewed-by: Michael Mortensen <mmortensen@google.com>
Tested-by: Mike Frysinger <vapier@google.com>

command.py

@@ -178,9 +178,7 @@ class Command(object):
     mp = manifest.manifestProject
 
     if not groups:
-      groups = mp.config.GetString('manifest.groups')
-    if not groups:
-      groups = 'default,platform-' + platform.system().lower()
+      groups = manifest.GetGroupsStr()
     groups = [x for x in re.split(r'[,\s]+', groups) if x]
 
     if not args:

completion.bash

@@ -0,0 +1,121 @@
+# Copyright 2021 The Android Open Source Project
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# Programmable bash completion.  https://github.com/scop/bash-completion
+
+# Complete the list of repo subcommands.
+__complete_repo_list_commands() {
+  local repo=${COMP_WORDS[0]}
+  (
+  # Handle completions if running outside of a checkout.
+  if ! "${repo}" help --all 2>/dev/null; then
+    repo help 2>/dev/null
+  fi
+  ) | sed -n '/^  /{s/  \([^ ]\+\) .\+/\1/;p}'
+}
+
+# Complete list of all branches available in all projects in the repo client
+# checkout.
+__complete_repo_list_branches() {
+  local repo=${COMP_WORDS[0]}
+  "${repo}" branches 2>/dev/null | \
+    sed -n '/|/{s/[ *][Pp ] *\([^ ]\+\) .*/\1/;p}'
+}
+
+# Complete list of all projects available in the repo client checkout.
+__complete_repo_list_projects() {
+  local repo=${COMP_WORDS[0]}
+  "${repo}" list -n 2>/dev/null
+}
+
+# Complete the repo <command> argument.
+__complete_repo_command() {
+  if [[ ${COMP_CWORD} -ne 1 ]]; then
+    return 1
+  fi
+
+  local command=${COMP_WORDS[1]}
+  COMPREPLY=($(compgen -W "$(__complete_repo_list_commands)" -- "${command}"))
+  return 0
+}
+
+# Complete repo subcommands that take <branch> <projects>.
+__complete_repo_command_branch_projects() {
+  local current=$1
+  if [[ ${COMP_CWORD} -eq 2 ]]; then
+    COMPREPLY=($(compgen -W "$(__complete_repo_list_branches)" -- "${current}"))
+  else
+    COMPREPLY=($(compgen -W "$(__complete_repo_list_projects)" -- "${current}"))
+  fi
+}
+
+# Complete repo subcommands that take only <projects>.
+__complete_repo_command_projects() {
+  local current=$1
+  COMPREPLY=($(compgen -W "$(__complete_repo_list_projects)" -- "${current}"))
+}
+
+# Complete the repo subcommand arguments.
+__complete_repo_arg() {
+  if [[ ${COMP_CWORD} -le 1 ]]; then
+    return 1
+  fi
+
+  local command=${COMP_WORDS[1]}
+  local current=${COMP_WORDS[COMP_CWORD]}
+  case ${command} in
+  abandon|checkout)
+    __complete_repo_command_branch_projects "${current}"
+    return 0
+    ;;
+
+  branch|branches|diff|info|list|overview|prune|rebase|smartsync|stage|status|\
+  sync|upload)
+    __complete_repo_command_projects "${current}"
+    return 0
+    ;;
+
+  help)
+    if [[ ${COMP_CWORD} -eq 2 ]]; then
+      COMPREPLY=(
+        $(compgen -W "$(__complete_repo_list_commands)" -- "${current}")
+      )
+    fi
+    return 0
+    ;;
+
+  start)
+    if [[ ${COMP_CWORD} -gt 2 ]]; then
+      COMPREPLY=(
+        $(compgen -W "$(__complete_repo_list_projects)" -- "${current}")
+      )
+    fi
+    return 0
+    ;;
+
+  *)
+    return 1
+    ;;
+  esac
+}
+
+# Complete the repo arguments.
+__complete_repo() {
+  COMPREPLY=()
+  __complete_repo_command && return 0
+  __complete_repo_arg && return 0
+  return 0
+}
+
+complete -F __complete_repo repo

docs/manifest-format.md

@@ -252,12 +252,25 @@ name will be prefixed by the parent's.
 The project name must match the name Gerrit knows, if Gerrit is
 being used for code reviews.
 
+"name" must not be empty, and may not be an absolute path or use "." or ".."
+path components.  It is always interpreted relative to the remote's fetch
+settings, so if a different base path is needed, declare a different remote
+with the new settings needed.
+These restrictions are not enforced for [Local Manifests].
+
 Attribute `path`: An optional path relative to the top directory
 of the repo client where the Git working directory for this project
-should be placed.  If not supplied the project name is used.
+should be placed.  If not supplied the project "name" is used.
 If the project has a parent element, its path will be prefixed
 by the parent's.
 
+"path" may not be an absolute path or use "." or ".." path components.
+These restrictions are not enforced for [Local Manifests].
+
+If you want to place files into the root of the checkout (e.g. a README or
+Makefile or another build script), use the [copyfile] or [linkfile] elements
+instead.
+
 Attribute `remote`: Name of a previously defined remote element.
 If not supplied the remote given by the default element is used.
 
@@ -419,12 +432,15 @@ target manifest to include - it must be a usable manifest on its own.
 Attribute `name`: the manifest to include, specified relative to
 the manifest repository's root.
 
+"name" may not be an absolute path or use "." or ".." path components.
+These restrictions are not enforced for [Local Manifests].
+
 Attribute `groups`: List of additional groups to which all projects
 in the included manifest belong. This appends and recurses, meaning
 all projects in sub-manifests carry all parent include groups.
 Same syntax as the corresponding element of `project`.
 
-## Local Manifests
+## Local Manifests {#local-manifests}
 
 Additional remotes and projects may be added through local manifest
 files stored in `$TOP_DIR/.repo/local_manifests/*.xml`.
@@ -452,3 +468,8 @@ Manifest files stored in `$TOP_DIR/.repo/local_manifests/*.xml` will
 be loaded in alphabetical order.
 
 The legacy `$TOP_DIR/.repo/local_manifest.xml` path is no longer supported.
+
+
+[copyfile]: #Element-copyfile
+[linkfile]: #Element-linkfile
+[Local Manifests]: #local-manifests

error.py

@@ -22,12 +22,12 @@ class ManifestParseError(Exception):
   """
 
 
-class ManifestInvalidRevisionError(Exception):
+class ManifestInvalidRevisionError(ManifestParseError):
   """The revision value in a project is incorrect.
   """
 
 
-class ManifestInvalidPathError(Exception):
+class ManifestInvalidPathError(ManifestParseError):
   """A path used in <copyfile> or <linkfile> is incorrect.
   """
 

git_config.py

@@ -145,6 +145,21 @@ class GitConfig(object):
     except ValueError:
       return None
 
+  def DumpConfigDict(self):
+    """Returns the current configuration dict.
+
+    Configuration data is information only (e.g. logging) and
+    should not be considered a stable data-source.
+
+    Returns:
+      dict of {<key>, <value>} for git configuration cache.
+      <value> are strings converted by GetString.
+    """
+    config_dict = {}
+    for key in self._cache:
+      config_dict[key] = self.GetString(key)
+    return config_dict
+
   def GetBoolean(self, name):
     """Returns a boolean from the configuration file.
        None : The value was not defined, or is not a boolean.

git_superproject.py

@@ -41,7 +41,8 @@ class Superproject(object):
   lookup of commit ids for all projects. It contains _project_commit_ids which
   is a dictionary with project/commit id entries.
   """
-  def __init__(self, manifest, repodir, superproject_dir='exp-superproject'):
+  def __init__(self, manifest, repodir, superproject_dir='exp-superproject',
+               quiet=False):
     """Initializes superproject.
 
     Args:
@@ -49,9 +50,11 @@ class Superproject(object):
       repodir: Path to the .repo/ dir for holding all internal checkout state.
           It must be in the top directory of the repo client checkout.
       superproject_dir: Relative path under |repodir| to checkout superproject.
+      quiet:  If True then only print the progress messages.
     """
     self._project_commit_ids = None
     self._manifest = manifest
+    self._quiet = quiet
     self._branch = self._GetBranch()
     self._repodir = os.path.abspath(repodir)
     self._superproject_dir = superproject_dir
@@ -89,6 +92,9 @@ class Superproject(object):
     """
     if not os.path.exists(self._superproject_path):
       os.mkdir(self._superproject_path)
+    if not self._quiet and not os.path.exists(self._work_git):
+      print('%s: Performing initial setup for superproject; this might take '
+            'several minutes.' % self._work_git)
     cmd = ['init', '--bare', self._work_git_name]
     p = GitCommand(None,
                    cmd,
@@ -183,6 +189,8 @@ class Superproject(object):
       return False
     if not self._Fetch(url):
       return False
+    if not self._quiet:
+      print('%s: Initial setup for superproject completed.' % self._work_git)
     return True
 
   def _GetAllProjectsCommitIds(self):
@@ -227,7 +235,7 @@ class Superproject(object):
             self._superproject_path,
             file=sys.stderr)
       return None
-    manifest_str = self._manifest.ToXml().toxml()
+    manifest_str = self._manifest.ToXml(groups=self._manifest.GetGroupsStr()).toxml()
     manifest_path = self._manifest_path
     try:
       with open(manifest_path, 'w', encoding='utf-8') as fp:

git_trace2_event_log.py

@@ -132,6 +132,21 @@ class EventLog(object):
     exit_event['code'] = result
     self._log.append(exit_event)
 
+  def DefParamRepoEvents(self, config):
+    """Append a 'def_param' event for each repo.* config key to the current log.
+
+    Args:
+      config: Repo configuration dictionary
+    """
+    # Only output the repo.* config parameters.
+    repo_config = {k: v for k, v in config.items() if k.startswith('repo.')}
+
+    for param, value in repo_config.items():
+      def_param_event = self._CreateEventDict('def_param')
+      def_param_event['param'] = param
+      def_param_event['value'] = value
+      self._log.append(def_param_event)
+
   def _GetEventTargetPath(self):
     """Get the 'trace2.eventtarget' path from git configuration.
 

gitc_utils.py

@@ -77,22 +77,6 @@ def _set_project_revisions(projects):
       project.revisionExpr = revisionExpr
 
 
-def _manifest_groups(manifest):
-  """Returns the manifest group string that should be synced
-
-  This is the same logic used by Command.GetProjects(), which is used during
-  repo sync
-
-  Args:
-    manifest: The XmlManifest object
-  """
-  mp = manifest.manifestProject
-  groups = mp.config.GetString('manifest.groups')
-  if not groups:
-    groups = 'default,platform-' + platform.system().lower()
-  return groups
-
-
 def generate_gitc_manifest(gitc_manifest, manifest, paths=None):
   """Generate a manifest for shafsd to use for this GITC client.
 
@@ -107,7 +91,7 @@ def generate_gitc_manifest(gitc_manifest, manifest, paths=None):
   if paths is None:
     paths = list(manifest.paths.keys())
 
-  groups = [x for x in re.split(r'[,\s]+', _manifest_groups(manifest)) if x]
+  groups = [x for x in re.split(r'[,\s]+', manifest.GetGroupsStr()) if x]
 
   # Convert the paths to projects, and filter them to the matched groups.
   projects = [manifest.paths[p] for p in paths]
@@ -166,7 +150,7 @@ def save_manifest(manifest, client_dir=None):
   else:
     manifest_file = os.path.join(client_dir, '.manifest')
   with open(manifest_file, 'w') as f:
-    manifest.Save(f, groups=_manifest_groups(manifest))
+    manifest.Save(f, groups=manifest.GetGroupsStr())
   # TODO(sbasi/jorg): Come up with a solution to remove the sleep below.
   # Give the GITC filesystem time to register the manifest changes.
   time.sleep(3)

main.py

@@ -297,6 +297,8 @@ class _Repo(object):
 
       cmd.event_log.FinishEvent(cmd_event, finish,
                                 result is None or result == 0)
+      git_trace2_event_log.DefParamRepoEvents(
+          cmd.manifest.manifestProject.config.DumpConfigDict())
       git_trace2_event_log.ExitEvent(result)
 
       if gopts.event_log:

manifest_xml.py

@@ -14,6 +14,7 @@
 
 import itertools
 import os
+import platform
 import re
 import sys
 import xml.dom.minidom
@@ -604,6 +605,17 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
   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 = {}
@@ -624,16 +636,22 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
         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))
+      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(self.local_manifests)):
             if local_file.endswith('.xml'):
               local = os.path.join(self.local_manifests, local_file)
-              nodes.append(self._ParseManifestXml(local, self.repodir))
+              # Since local manifests are entirely managed by the user, allow
+              # them to point anywhere the user wants.
+              nodes.append(self._ParseManifestXml(
+                  local, self.repodir, restrict_includes=False))
         except OSError:
           pass
 
@@ -651,7 +669,19 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
 
       self._loaded = True
 
-  def _ParseManifestXml(self, path, include_root, parent_groups=''):
+  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:
@@ -670,10 +700,11 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
     for node in manifest.childNodes:
       if node.nodeName == 'include':
         name = self._reqatt(node, 'name')
-        msg = self._CheckLocalPath(name)
-        if msg:
-          raise ManifestInvalidPathError(
-              '<include> invalid "name": %s: %s' % (name, msg))
+        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
@@ -681,13 +712,13 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
           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, 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(
@@ -1008,7 +1039,8 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
     if not path:
       path = name
     else:
-      msg = self._CheckLocalPath(path, dir_ok=True)
+      # 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))
@@ -1196,7 +1228,9 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
     # our constructed logic here.  Especially since manifest authors only use
     # / in their paths.
     resep = re.compile(r'[/%s]' % re.escape(os.path.sep))
-    parts = resep.split(path)
+    # 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.

project.py

@@ -863,7 +863,7 @@ class Project(object):
       out.nl()
       out.project('project %s/' % self.relpath)
       out.nl()
-      out.write(p.stdout)
+      out.write('%s', p.stdout)
     return p.Wait() == 0
 
 # Publish / Upload ##
@@ -1150,7 +1150,7 @@ class Project(object):
         p = GitCommand(self, cmd, bare=True, capture_stdout=bool(output_redir),
                        merge_output=bool(output_redir))
         if p.stdout and output_redir:
-          buf.write(p.stdout)
+          output_redir.write(p.stdout)
         if p.Wait() != 0:
           return False
         platform_utils.remove(alternates_file)
@@ -1227,6 +1227,18 @@ class Project(object):
     self.CleanPublishedCache(all_refs)
     revid = self.GetRevisionId(all_refs)
 
+    # Special case the root of the repo client checkout.  Make sure it doesn't
+    # contain files being checked out to dirs we don't allow.
+    if self.relpath == '.':
+      PROTECTED_PATHS = {'.repo'}
+      paths = set(self.work_git.ls_tree('-z', '--name-only', '--', revid).split('\0'))
+      bad_paths = paths & PROTECTED_PATHS
+      if bad_paths:
+        syncbuf.fail(self,
+                     'Refusing to checkout project that writes to protected '
+                     'paths: %s' % (', '.join(bad_paths),))
+        return
+
     def _doff():
       self._FastForward(revid)
       self._CopyAndLinkFiles()
@@ -1698,6 +1710,11 @@ class Project(object):
         if cb is None or name != cb:
           kill.append(name)
 
+    # Minor optimization: If there's nothing to prune, then don't try to read
+    # any project state.
+    if not kill and not cb:
+      return []
+
     rev = self.GetRevisionId(left)
     if cb is not None \
        and not self._revlist(HEAD + '...' + rev) \

repo

@@ -318,7 +318,7 @@ def GetParser(gitc_init=False):
                    help='filter for use with --partial-clone '
                         '[default: %default]')
   group.add_option('--worktree', action='store_true',
-                   help=optparse.SUPPRESS_HELP)
+                   help='use git-worktree to manage projects')
   group.add_option('--archive', action='store_true',
                    help='checkout an archive instead of a git repository for '
                         'each project. See git archive.')

subcmds/init.py

@@ -127,10 +127,8 @@ to update the working directory files.
     g.add_option('--clone-filter', action='store', default='blob:none',
                  dest='clone_filter',
                  help='filter for use with --partial-clone [default: %default]')
-    # TODO(vapier): Expose option with real help text once this has been in the
-    # wild for a while w/out significant bug reports.  Goal is by ~Sep 2020.
     g.add_option('--worktree', action='store_true',
-                 help=optparse.SUPPRESS_HELP)
+                 help='use git-worktree to manage projects')
     g.add_option('--archive',
                  dest='archive', action='store_true',
                  help='checkout an archive instead of a git repository for '
@@ -185,10 +183,15 @@ to update the working directory files.
     return {'REPO_MANIFEST_URL': 'manifest_url',
             'REPO_MIRROR_LOCATION': 'reference'}
 
-  def _CloneSuperproject(self):
-    """Clone the superproject based on the superproject's url and branch."""
+  def _CloneSuperproject(self, opt):
+    """Clone the superproject based on the superproject's url and branch.
+
+    Args:
+      opt: Program options returned from optparse.  See _Options().
+    """
     superproject = git_superproject.Superproject(self.manifest,
-                                                 self.repodir)
+                                                 self.repodir,
+                                                 quiet=opt.quiet)
     if not superproject.Sync():
       print('error: git update of superproject failed', file=sys.stderr)
       sys.exit(1)
@@ -264,7 +267,7 @@ to update the working directory files.
 
     groups = [x for x in groups if x]
     groupstr = ','.join(groups)
-    if opt.platform == 'auto' and groupstr == 'default,platform-' + platform.system().lower():
+    if opt.platform == 'auto' and groupstr == self.manifest.GetDefaultGroupsStr():
       groupstr = None
     m.config.SetString('manifest.groups', groupstr)
 
@@ -553,7 +556,7 @@ to update the working directory files.
     self._LinkManifest(opt.manifest_name)
 
     if self.manifest.manifestProject.config.GetBoolean('repo.superproject'):
-      self._CloneSuperproject()
+      self._CloneSuperproject(opt)
 
     if os.isatty(0) and os.isatty(1) and not self.manifest.IsMirror:
       if opt.config_name or self._ShouldConfigureUser(opt):

subcmds/sync.py

@@ -292,7 +292,8 @@ later is required to fix a server side protocol bug.
       Returns path to the overriding manifest file.
     """
     superproject = git_superproject.Superproject(self.manifest,
-                                                 self.repodir)
+                                                 self.repodir,
+                                                 quiet=opt.quiet)
     all_projects = self.GetProjects(args,
                                     missing_ok=True,
                                     submodules_ok=opt.fetch_submodules)
@@ -527,7 +528,7 @@ later is required to fix a server side protocol bug.
 
     pm.end()
 
-    return ret
+    return ret and not err_results
 
   def _GCProjects(self, projects, opt, err_event):
     gc_gitdirs = {}

tests/test_git_superproject.py

@@ -15,6 +15,7 @@
 """Unittests for the git_superproject.py module."""
 
 import os
+import platform
 import tempfile
 import unittest
 from unittest import mock
@@ -34,6 +35,7 @@ class SuperprojectTestCase(unittest.TestCase):
     self.manifest_file = os.path.join(
         self.repodir, manifest_xml.MANIFEST_FILE_NAME)
     os.mkdir(self.repodir)
+    self.platform = platform.system().lower()
 
     # The manifest parsing really wants a git repo currently.
     gitdir = os.path.join(self.repodir, 'manifests.git')
@@ -48,8 +50,8 @@ class SuperprojectTestCase(unittest.TestCase):
   <remote name="default-remote" fetch="http://localhost" />
   <default remote="default-remote" revision="refs/heads/main" />
   <superproject name="superproject"/>
-  <project path="art" name="platform/art" />
-</manifest>
+  <project path="art" name="platform/art" groups="notdefault,platform-""" + self.platform + """
+  " /></manifest>
 """)
     self._superproject = git_superproject.Superproject(manifest, self.repodir)
 
@@ -142,7 +144,8 @@ class SuperprojectTestCase(unittest.TestCase):
         '<?xml version="1.0" ?><manifest>' +
         '<remote name="default-remote" fetch="http://localhost"/>' +
         '<default remote="default-remote" revision="refs/heads/main"/>' +
-        '<project name="platform/art" path="art" revision="ABCDEF"/>' +
+        '<project name="platform/art" path="art" revision="ABCDEF" ' +
+        'groups="notdefault,platform-' + self.platform + '"/>' +
         '<superproject name="superproject"/>' +
         '</manifest>')
 
@@ -169,7 +172,8 @@ class SuperprojectTestCase(unittest.TestCase):
               '<remote name="default-remote" fetch="http://localhost"/>' +
               '<default remote="default-remote" revision="refs/heads/main"/>' +
               '<project name="platform/art" path="art" ' +
-              'revision="2c2724cb36cd5a9cec6c852c681efc3b7c6b86ea"/>' +
+              'revision="2c2724cb36cd5a9cec6c852c681efc3b7c6b86ea" ' +
+              'groups="notdefault,platform-' + self.platform + '"/>' +
               '<superproject name="superproject"/>' +
               '</manifest>')
 

tests/test_git_trace2_event_log.py

@@ -161,6 +161,55 @@ class EventLogTestCase(unittest.TestCase):
     self.assertIn('code', exit_event)
     self.assertEqual(exit_event['code'], 2)
 
+  def test_def_params_event_repo_config(self):
+    """Test 'def_params' event data outputs only repo config keys.
+
+    Expected event log:
+    <version event>
+    <def_param event>
+    <def_param event>
+    """
+    config = {
+        'git.foo': 'bar',
+        'repo.partialclone': 'true',
+        'repo.partialclonefilter': 'blob:none',
+    }
+    self._event_log_module.DefParamRepoEvents(config)
+
+    with tempfile.TemporaryDirectory(prefix='event_log_tests') as tempdir:
+      log_path = self._event_log_module.Write(path=tempdir)
+      self._log_data = self.readLog(log_path)
+
+    self.assertEqual(len(self._log_data), 3)
+    def_param_events = self._log_data[1:]
+    self.verifyCommonKeys(self._log_data[0], expected_event_name='version')
+
+    for event in def_param_events:
+      self.verifyCommonKeys(event, expected_event_name='def_param')
+      # Check for 'def_param' event specific fields.
+      self.assertIn('param', event)
+      self.assertIn('value', event)
+      self.assertTrue(event['param'].startswith('repo.'))
+
+  def test_def_params_event_no_repo_config(self):
+    """Test 'def_params' event data won't output non-repo config keys.
+
+    Expected event log:
+    <version event>
+    """
+    config = {
+        'git.foo': 'bar',
+        'git.core.foo2': 'baz',
+    }
+    self._event_log_module.DefParamRepoEvents(config)
+
+    with tempfile.TemporaryDirectory(prefix='event_log_tests') as tempdir:
+      log_path = self._event_log_module.Write(path=tempdir)
+      self._log_data = self.readLog(log_path)
+
+    self.assertEqual(len(self._log_data), 1)
+    self.verifyCommonKeys(self._log_data[0], expected_event_name='version')
+
   def test_write_with_filename(self):
     """Test Write() with a path to a file exits with None."""
     self.assertIsNone(self._event_log_module.Write(path='path/to/file'))

tests/test_manifest_xml.py

@@ -15,6 +15,7 @@
 """Unittests for the manifest_xml.py module."""
 
 import os
+import platform
 import shutil
 import tempfile
 import unittest
@@ -31,6 +32,7 @@ INVALID_FS_PATHS = (
     '..',
     '../',
     './',
+    './/',
     'foo/',
     './foo',
     '../foo',
@@ -298,8 +300,8 @@ class IncludeElementTests(ManifestParseTestCase):
         # Check level2 proj group not removed.
         self.assertIn('l2g1', proj.groups)
 
-  def test_bad_name_checks(self):
-    """Check handling of bad name attribute."""
+  def test_allow_bad_name_from_user(self):
+    """Check handling of bad name attribute from the user's input."""
     def parse(name):
       manifest = self.getXmlManifest(f"""
 <manifest>
@@ -307,6 +309,34 @@ class IncludeElementTests(ManifestParseTestCase):
   <default remote="default-remote" revision="refs/heads/main" />
   <include name="{name}" />
 </manifest>
+""")
+      # Force the manifest to be parsed.
+      manifest.ToXml()
+
+    # Setup target of the include.
+    target = os.path.join(self.tempdir, 'target.xml')
+    with open(target, 'w') as fp:
+      fp.write('<manifest></manifest>')
+
+    # Include with absolute path.
+    parse(os.path.abspath(target))
+
+    # Include with relative path.
+    parse(os.path.relpath(target, self.manifest_dir))
+
+  def test_bad_name_checks(self):
+    """Check handling of bad name attribute."""
+    def parse(name):
+      # Setup target of the include.
+      with open(os.path.join(self.manifest_dir, 'target.xml'), 'w') as fp:
+        fp.write(f'<manifest><include name="{name}"/></manifest>')
+
+      manifest = self.getXmlManifest("""
+<manifest>
+  <remote name="default-remote" fetch="http://localhost" />
+  <default remote="default-remote" revision="refs/heads/main" />
+  <include name="target.xml" />
+</manifest>
 """)
       # Force the manifest to be parsed.
       manifest.ToXml()
@@ -349,6 +379,11 @@ class ProjectElementTests(ManifestParseTestCase):
     self.assertCountEqual(
         result['extras'],
         ['g1', 'g2', 'g1', 'name:extras', 'all', 'path:path'])
+    groupstr = 'default,platform-' + platform.system().lower()
+    self.assertEqual(groupstr, manifest.GetGroupsStr())
+    groupstr = 'g1,g2,g1'
+    manifest.manifestProject.config.SetString('manifest.groups', groupstr)
+    self.assertEqual(groupstr, manifest.GetGroupsStr())
 
   def test_set_revision_id(self):
     """Check setting of project's revisionId."""
@@ -393,6 +428,28 @@ class ProjectElementTests(ManifestParseTestCase):
     self.assertEqual(manifest.projects[0].objdir,
                      os.path.join(self.tempdir, '.repo/project-objects/a/path.git'))
 
+    manifest = parse('a/path', 'foo//////')
+    self.assertEqual(manifest.projects[0].gitdir,
+                     os.path.join(self.tempdir, '.repo/projects/foo.git'))
+    self.assertEqual(manifest.projects[0].objdir,
+                     os.path.join(self.tempdir, '.repo/project-objects/a/path.git'))
+
+  def test_toplevel_path(self):
+    """Check handling of path=. specially."""
+    def parse(name, path):
+      return self.getXmlManifest(f"""
+<manifest>
+  <remote name="default-remote" fetch="http://localhost" />
+  <default remote="default-remote" revision="refs/heads/main" />
+  <project name="{name}" path="{path}" />
+</manifest>
+""")
+
+    for path in ('.', './', './/', './//'):
+      manifest = parse('server/path', path)
+      self.assertEqual(manifest.projects[0].gitdir,
+                       os.path.join(self.tempdir, '.repo/projects/..git'))
+
   def test_bad_path_name_checks(self):
     """Check handling of bad path & name attributes."""
     def parse(name, path):
@@ -420,8 +477,11 @@ class ProjectElementTests(ManifestParseTestCase):
 
       with self.assertRaises(error.ManifestInvalidPathError):
         parse(path, 'ok')
-      with self.assertRaises(error.ManifestInvalidPathError):
-        parse('ok', path)
+
+      # We have a dedicated test for path=".".
+      if path not in {'.'}:
+        with self.assertRaises(error.ManifestInvalidPathError):
+          parse('ok', path)
 
 
 class SuperProjectElementTests(ManifestParseTestCase):

tests/test_wrapper.py

@@ -305,8 +305,8 @@ class Requirements(RepoWrapperTestCase):
     reqs = self.wrapper.Requirements({'python': {'hard': sys.version_info}})
     reqs.assert_all()
 
-  def test_assert_all_old_repo(self):
-    """Check assert_all rejects old repo."""
+  def test_assert_all_old_python(self):
+    """Check assert_all rejects old python."""
     reqs = self.wrapper.Requirements({'python': {'hard': [99999, 0]}})
     with self.assertRaises(SystemExit):
       reqs.assert_all()