forall: handle missing project refs better

If the project exists, but the ref the manifest wants doesn't exist,
don't throw an error (and abort the process in general).  This can
come up with a partially synced tree: the manifest is up-to-date,
but not all the projects have yet been synced.

Bug: https://crbug.com/gerrit/14289
Change-Id: Iba97413c476544223ffe518198c900c2193a00ed
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/301262
Reviewed-by: LaMont Jones <lamontjones@google.com>
Reviewed-by: Mike Frysinger <vapier@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,
@@ -115,7 +121,7 @@ class Superproject(object):
       print('git fetch missing drectory: %s' % self._work_git,
             file=sys.stderr)
       return False
-    cmd = ['fetch', url, '--force', '--no-tags', '--filter', 'blob:none']
+    cmd = ['fetch', url, '--depth', '1', '--force', '--no-tags', '--filter', 'blob:none']
     if self._branch:
       cmd += [self._branch + ':' + self._branch]
     p = GitCommand(None,
@@ -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,33 @@ class EventLog(object):
     exit_event['code'] = result
     self._log.append(exit_event)
 
+  def CommandEvent(self, name, subcommands):
+    """Append a 'command' event to the current log.
+
+    Args:
+      name: Name of the primary command (ex: repo, git)
+      subcommands: List of the sub-commands (ex: version, init, sync)
+    """
+    command_event = self._CreateEventDict('command')
+    command_event['name'] = name
+    command_event['subcommands'] = subcommands
+    self._log.append(command_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

@@ -254,6 +254,7 @@ class _Repo(object):
     cmd_event = cmd.event_log.Add(name, event_log.TASK_COMMAND, start)
     cmd.event_log.SetParent(cmd_event)
     git_trace2_event_log.StartEvent()
+    git_trace2_event_log.CommandEvent(name='repo', subcommands=[name])
 
     try:
       cmd.ValidateOptions(copts, cargs)
@@ -297,6 +298,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/forall.py

@@ -24,6 +24,7 @@ import subprocess
 
 from color import Coloring
 from command import DEFAULT_LOCAL_JOBS, Command, MirrorSafeCommand, WORKER_BATCH_SIZE
+from error import ManifestInvalidRevisionError
 
 _CAN_COLOR = [
     'branch',
@@ -44,7 +45,7 @@ class Forall(Command, MirrorSafeCommand):
   helpSummary = "Run a shell command in each project"
   helpUsage = """
 %prog [<project>...] -c <command> [<arg>...]
-%prog -r str1 [str2] ... -c <command> [<arg>...]"
+%prog -r str1 [str2] ... -c <command> [<arg>...]
 """
   helpDescription = """
 Executes the same shell command in each project.
@@ -52,6 +53,11 @@ Executes the same shell command in each project.
 The -r option allows running the command only on projects matching
 regex or wildcard expression.
 
+By default, projects are processed non-interactively in parallel.  If you want
+to run interactive commands, make sure to pass --interactive to force --jobs 1.
+While the processing order of projects is not guaranteed, the order of project
+output is stable.
+
 # Output Formatting
 
 The -p option causes '%prog' to bind pipes to the command's stdin,
@@ -154,6 +160,9 @@ without iterating through the remaining projects.
     g.add_option('-v', '--verbose',
                  dest='verbose', action='store_true',
                  help='Show command error messages')
+    p.add_option('--interactive',
+                 action='store_true',
+                 help='force interactive usage')
 
   def WantPager(self, opt):
     return opt.project_header and opt.jobs == 1
@@ -173,6 +182,11 @@ without iterating through the remaining projects.
       cmd.append(cmd[0])
     cmd.extend(opt.command[1:])
 
+    # Historically, forall operated interactively, and in serial.  If the user
+    # has selected 1 job, then default to interacive mode.
+    if opt.jobs == 1:
+      opt.interactive = True
+
     if opt.project_header \
             and not shell \
             and cmd[0] == 'git':
@@ -239,7 +253,7 @@ without iterating through the remaining projects.
       rc = rc or errno.EINTR
     except Exception as e:
       # Catch any other exceptions raised
-      print('Got an error, terminating the pool: %s: %s' %
+      print('forall: unhandled error, terminating the pool: %s: %s' %
             (type(e).__name__, e),
             file=sys.stderr)
       rc = rc or getattr(e, 'errno', 1)
@@ -282,7 +296,13 @@ def DoWork(project, mirror, opt, cmd, shell, cnt, config):
   setenv('REPO_PROJECT', project.name)
   setenv('REPO_PATH', project.relpath)
   setenv('REPO_REMOTE', project.remote.name)
-  setenv('REPO_LREV', '' if mirror else project.GetRevisionId())
+  try:
+    # If we aren't in a fully synced state and we don't have the ref the manifest
+    # wants, then this will fail.  Ignore it for the purposes of this code.
+    lrev = '' if mirror else project.GetRevisionId()
+  except ManifestInvalidRevisionError:
+    lrev = ''
+  setenv('REPO_LREV', lrev)
   setenv('REPO_RREV', project.revisionExpr)
   setenv('REPO_UPSTREAM', project.upstream)
   setenv('REPO_DEST_BRANCH', project.dest_branch)
@@ -313,10 +333,12 @@ def DoWork(project, mirror, opt, cmd, shell, cnt, config):
   else:
     stderr = subprocess.DEVNULL
 
+  stdin = None if opt.interactive else subprocess.DEVNULL
+
   result = subprocess.run(
       cmd, cwd=cwd, shell=shell, env=env, check=False,
       encoding='utf-8', errors='replace',
-      stdin=subprocess.DEVNULL, stdout=subprocess.PIPE, stderr=stderr)
+      stdin=stdin, stdout=subprocess.PIPE, stderr=stderr)
 
   output = result.stdout
   if opt.project_header:

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/list.py

@@ -20,7 +20,7 @@ class List(Command, MirrorSafeCommand):
   helpSummary = "List projects and their associated directories"
   helpUsage = """
 %prog [-f] [<project>...]
-%prog [-f] -r str1 [str2]..."
+%prog [-f] -r str1 [str2]...
 """
   helpDescription = """
 List all projects; pass '.' to list the project for the cwd.

subcmds/start.py

@@ -44,7 +44,8 @@ revision specified in the manifest.
                  help='begin branch in all projects')
     p.add_option('-r', '--rev', '--revision', dest='revision',
                  help='point branch at this revision instead of upstream')
-    p.add_option('--head', dest='revision', action='store_const', const='HEAD',
+    p.add_option('--head', '--HEAD',
+                 dest='revision', action='store_const', const='HEAD',
                  help='abbreviation for --rev HEAD')
 
   def ValidateOptions(self, opt, args):

subcmds/sync.py

@@ -277,6 +277,16 @@ 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"""
+    return (opt.use_superproject or
+            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)
+
   def _UpdateProjectsRevisionId(self, opt, args):
     """Update revisionId of every project with the SHA from superproject.
 
@@ -292,7 +302,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)
@@ -362,7 +373,7 @@ later is required to fix a server side protocol bug.
             quiet=opt.quiet,
             verbose=opt.verbose,
             output_redir=buf,
-            current_branch_only=opt.current_branch_only,
+            current_branch_only=self._GetCurrentBranchOnly(opt),
             force_sync=opt.force_sync,
             clone_bundle=opt.clone_bundle,
             tags=opt.tags, archive=self.manifest.IsArchive,
@@ -527,7 +538,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 = {}
@@ -734,7 +745,7 @@ later is required to fix a server side protocol bug.
     if not opt.local_only:
       start = time.time()
       success = mp.Sync_NetworkHalf(quiet=opt.quiet, verbose=opt.verbose,
-                                    current_branch_only=opt.current_branch_only,
+                                    current_branch_only=self._GetCurrentBranchOnly(opt),
                                     force_sync=opt.force_sync,
                                     tags=opt.tags,
                                     optimized_fetch=opt.optimized_fetch,
@@ -829,9 +840,7 @@ later is required to fix a server side protocol bug.
     else:
       self._UpdateManifestProject(opt, mp, manifest_name)
 
-    if (opt.use_superproject or
-        self.manifest.manifestProject.config.GetBoolean(
-            'repo.superproject')):
+    if self._UseSuperproject(opt):
       manifest_name = self._UpdateProjectsRevisionId(opt, args)
 
     if self.gitc_manifest:

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,79 @@ class EventLogTestCase(unittest.TestCase):
     self.assertIn('code', exit_event)
     self.assertEqual(exit_event['code'], 2)
 
+  def test_command_event(self):
+    """Test and validate 'command' event data is valid.
+
+    Expected event log:
+    <version event>
+    <command event>
+    """
+    name = 'repo'
+    subcommands = ['init' 'this']
+    self._event_log_module.CommandEvent(name='repo', subcommands=subcommands)
+    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), 2)
+    command_event = self._log_data[1]
+    self.verifyCommonKeys(self._log_data[0], expected_event_name='version')
+    self.verifyCommonKeys(command_event, expected_event_name='command')
+    # Check for 'command' event specific fields.
+    self.assertIn('name', command_event)
+    self.assertIn('subcommands', command_event)
+    self.assertEqual(command_event['name'], name)
+    self.assertEqual(command_event['subcommands'], subcommands)
+
+  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()