sync: Fix exception in an exsiting clone (without partial-clone).

Default the partial_clone_exclude argument to an empty set.

Fixes the following report by Emil Medve.

With this change (up to v2.14.1), on an existing "normal" clone (without partial-clone options) I'm seeing this traceback during `repo selfupdate`:

Traceback (most recent call last):

  File ".../.repo/repo/main.py", line 630, in <module>
    _Main(sys.argv[1:])
  File ".../.repo/repo/main.py", line 604, in _Main
    result = run()
  File ".../.repo/repo/main.py", line 597, in <lambda>
    run = lambda: repo._Run(name, gopts, argv) or 0
  File ".../.repo/repo/main.py", line 261, in _Run
    result = cmd.Execute(copts, cargs)
  File ".../.repo/repo/subcmds/selfupdate.py", line 54, in Execute
    if not rp.Sync_NetworkHalf():
  File ".../.repo/repo/project.py", line 1091, in Sync_NetworkHalf
    if self.name in partial_clone_exclude:
TypeError: argument of type 'NoneType' is not iterable

$ ./run_tests -v

Change-Id: I71e744e4ef2a37b13aa9ba42eba3935e78c4e40a
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/304082
Reviewed-by: Mike Frysinger <vapier@google.com>
Tested-by: Raman Tenneti <rtenneti@google.com>

.github/workflows/test-ci.yml

@@ -14,7 +14,7 @@ jobs:
       fail-fast: false
       matrix:
         os: [ubuntu-latest, macos-latest, windows-latest]
-        python-version: [3.6, 3.7, 3.8]
+        python-version: [3.5, 3.6, 3.7, 3.8, 3.9]
     runs-on: ${{ matrix.os }}
 
     steps:

command.py

@@ -12,6 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import multiprocessing
 import os
 import optparse
 import platform
@@ -21,6 +22,21 @@ import sys
 from event_log import EventLog
 from error import NoSuchProjectError
 from error import InvalidProjectGroupsError
+import progress
+
+
+# Number of projects to submit to a single worker process at a time.
+# This number represents a tradeoff between the overhead of IPC and finer
+# grained opportunity for parallelism. This particular value was chosen by
+# iterating through powers of two until the overall performance no longer
+# improved. The performance of this batch size is not a function of the
+# number of cores on the system.
+WORKER_BATCH_SIZE = 32
+
+
+# How many jobs to run in parallel by default?  This assumes the jobs are
+# largely I/O bound and do not hit the network.
+DEFAULT_LOCAL_JOBS = min(os.cpu_count(), 8)
 
 
 class Command(object):
@@ -32,6 +48,10 @@ class Command(object):
   manifest = None
   _optparse = None
 
+  # Whether this command supports running in parallel.  If greater than 0,
+  # it is the number of parallel jobs to default to.
+  PARALLEL_JOBS = None
+
   def WantPager(self, _opt):
     return False
 
@@ -66,12 +86,33 @@ class Command(object):
         usage = 'repo %s' % self.NAME
       epilog = 'Run `repo help %s` to view the detailed manual.' % self.NAME
       self._optparse = optparse.OptionParser(usage=usage, epilog=epilog)
+      self._CommonOptions(self._optparse)
       self._Options(self._optparse)
     return self._optparse
 
-  def _Options(self, p):
-    """Initialize the option parser.
+  def _CommonOptions(self, p, opt_v=True):
+    """Initialize the option parser with common options.
+
+    These will show up for *all* subcommands, so use sparingly.
+    NB: Keep in sync with repo:InitParser().
     """
+    g = p.add_option_group('Logging options')
+    opts = ['-v'] if opt_v else []
+    g.add_option(*opts, '--verbose',
+                 dest='output_mode', action='store_true',
+                 help='show all output')
+    g.add_option('-q', '--quiet',
+                 dest='output_mode', action='store_false',
+                 help='only show errors')
+
+    if self.PARALLEL_JOBS is not None:
+      p.add_option(
+          '-j', '--jobs',
+          type=int, default=self.PARALLEL_JOBS,
+          help='number of jobs to run in parallel (default: %s)' % self.PARALLEL_JOBS)
+
+  def _Options(self, p):
+    """Initialize the option parser with subcommand-specific options."""
 
   def _RegisteredEnvironmentOptions(self):
     """Get options that can be set from environment variables.
@@ -97,6 +138,11 @@ class Command(object):
     self.OptionParser.print_usage()
     sys.exit(1)
 
+  def CommonValidateOptions(self, opt, args):
+    """Validate common options."""
+    opt.quiet = opt.output_mode is False
+    opt.verbose = opt.output_mode is True
+
   def ValidateOptions(self, opt, args):
     """Validate the user options & arguments before executing.
 
@@ -112,6 +158,44 @@ class Command(object):
     """
     raise NotImplementedError
 
+  @staticmethod
+  def ExecuteInParallel(jobs, func, inputs, callback, output=None, ordered=False):
+    """Helper for managing parallel execution boiler plate.
+
+    For subcommands that can easily split their work up.
+
+    Args:
+      jobs: How many parallel processes to use.
+      func: The function to apply to each of the |inputs|.  Usually a
+          functools.partial for wrapping additional arguments.  It will be run
+          in a separate process, so it must be pickalable, so nested functions
+          won't work.  Methods on the subcommand Command class should work.
+      inputs: The list of items to process.  Must be a list.
+      callback: The function to pass the results to for processing.  It will be
+          executed in the main thread and process the results of |func| as they
+          become available.  Thus it may be a local nested function.  Its return
+          value is passed back directly.  It takes three arguments:
+          - The processing pool (or None with one job).
+          - The |output| argument.
+          - An iterator for the results.
+      output: An output manager.  May be progress.Progess or color.Coloring.
+      ordered: Whether the jobs should be processed in order.
+
+    Returns:
+      The |callback| function's results are returned.
+    """
+    try:
+      # NB: Multiprocessing is heavy, so don't spin it up for one job.
+      if len(inputs) == 1 or jobs == 1:
+        return callback(None, output, (func(x) for x in inputs))
+      else:
+        with multiprocessing.Pool(jobs) as pool:
+          submit = pool.imap if ordered else pool.imap_unordered
+          return callback(pool, output, submit(func, inputs, chunksize=WORKER_BATCH_SIZE))
+    finally:
+      if isinstance(output, progress.Progress):
+        output.end()
+
   def _ResetPathToProjectMap(self, projects):
     self._by_path = dict((p.worktree, p) for p in projects)
 
@@ -155,9 +239,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/internal-fs-layout.md

@@ -93,6 +93,23 @@ support, see the [manifest-format.md] file.
 
 ### Project objects
 
+*** note
+**Warning**: Please do not use repo's approach to projects/ & project-objects/
+layouts as a model for other tools to implement similar approaches.
+It has a number of known downsides like:
+*   [Symlinks do not work well under Windows](./windows.md).
+*   Git sometimes replaces symlinks under .git/ with real files (under unknown
+    circumstances), and then the internal state gets out of sync, and data loss
+    may ensue.
+*   When sharing project-objects between multiple project checkouts, Git might
+    automatically run `gc` or `prune` which may lead to data loss or corruption
+    (since those operate on leaf projects and miss refs in other leaves).  See
+    https://gerrit-review.googlesource.com/c/git-repo/+/254392 for more details.
+
+Instead, you should use standard Git workflows like [git worktree] or
+[gitsubmodules] with [superprojects].
+***
+
 *   `project.list`: Tracking file used by `repo sync` to determine when projects
     are added or removed and need corresponding updates in the checkout.
 *   `projects/`: Bare checkouts of every project synced by the manifest.  The
@@ -121,7 +138,7 @@ support, see the [manifest-format.md] file.
     (i.e. the path on the remote server) with a `.git` suffix.  This has the
     same advantages as the `project-objects/` layout above.
 
-    This is used when git worktrees are enabled.
+    This is used when [git worktree]'s are enabled.
 
 ### Global settings
 
@@ -130,23 +147,26 @@ repo client checkout.
 Most settings use the `[repo]` section to avoid conflicts with git.
 User controlled settings are initialized when running `repo init`.
 
-| Setting           | `repo init` Option        | Use/Meaning |
-|-------------------|---------------------------|-------------|
-| manifest.groups   | `--groups` & `--platform` | The manifest groups to sync |
-| repo.archive      | `--archive`               | Use `git archive` for checkouts |
-| repo.clonebundle  | `--clone-bundle`          | Whether the initial sync used clone.bundle explicitly |
-| repo.clonefilter  | `--clone-filter`          | Filter setting when using [partial git clones] |
-| repo.depth        | `--depth`                 | Create shallow checkouts when cloning |
-| repo.dissociate   | `--dissociate`            | Dissociate from any reference/mirrors after initial clone |
-| repo.mirror       | `--mirror`                | Checkout is a repo mirror |
-| repo.partialclone | `--partial-clone`         | Create [partial git clones] |
-| repo.reference    | `--reference`             | Reference repo client checkout |
-| repo.submodules   | `--submodules`            | Sync git submodules |
-| repo.worktree     | `--worktree`              | Use `git worktree` for checkouts |
-| user.email        | `--config-name`           | User's e-mail address; Copied into `.git/config` when checking out a new project |
-| user.name         | `--config-name`           | User's name; Copied into `.git/config` when checking out a new project |
+| Setting                  | `repo init` Option        | Use/Meaning |
+|-------------------       |---------------------------|-------------|
+| manifest.groups          | `--groups` & `--platform` | The manifest groups to sync |
+| repo.archive             | `--archive`               | Use `git archive` for checkouts |
+| repo.clonebundle         | `--clone-bundle`          | Whether the initial sync used clone.bundle explicitly |
+| repo.clonefilter         | `--clone-filter`          | Filter setting when using [partial git clones] |
+| repo.depth               | `--depth`                 | Create shallow checkouts when cloning |
+| repo.dissociate          | `--dissociate`            | Dissociate from any reference/mirrors after initial clone |
+| repo.mirror              | `--mirror`                | Checkout is a repo mirror |
+| repo.partialclone        | `--partial-clone`         | Create [partial git clones] |
+| repo.partialcloneexclude | `--partial-clone-exclude` | Comma-delimited list of project names (not paths) to exclude while using [partial git clones] |
+| repo.reference           | `--reference`             | Reference repo client checkout |
+| repo.submodules          | `--submodules`            | Sync git submodules |
+| repo.superproject        | `--use-superproject`      | Sync [superproject] |
+| repo.worktree            | `--worktree`              | Use [git worktree] for checkouts |
+| user.email               | `--config-name`           | User's e-mail address; Copied into `.git/config` when checking out a new project |
+| user.name                | `--config-name`           | User's name; Copied into `.git/config` when checking out a new project |
 
 [partial git clones]: https://git-scm.com/docs/gitrepository-layout#_code_partialclone_code
+[superproject]: https://en.wikibooks.org/wiki/Git/Submodules_and_Superprojects
 
 ### Repo hooks settings
 
@@ -226,7 +246,10 @@ Repo will create & maintain a few files in the user's home directory.
 
 
 [git-config]: https://git-scm.com/docs/git-config
+[git worktree]: https://git-scm.com/docs/git-worktree
+[gitsubmodules]: https://git-scm.com/docs/gitsubmodules
 [manifest-format.md]: ./manifest-format.md
 [local manifests]: ./manifest-format.md#Local-Manifests
+[superprojects]: https://en.wikibooks.org/wiki/Git/Submodules_and_Superprojects
 [topic]: https://gerrit-review.googlesource.com/Documentation/intro-user.html#topics
 [upload-notify]: https://gerrit-review.googlesource.com/Documentation/user-upload.html#notify

docs/manifest-format.md

@@ -21,6 +21,7 @@ following DTD:
 
 ```xml
 <!DOCTYPE manifest [
+
   <!ELEMENT manifest (notice?,
                       remote*,
                       default?,
@@ -252,12 +253,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 +433,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 +469,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.
   """
 
@@ -37,7 +37,7 @@ class NoManifestException(Exception):
   """
 
   def __init__(self, path, reason):
-    super(NoManifestException, self).__init__()
+    super().__init__(path, reason)
     self.path = path
     self.reason = reason
 
@@ -50,7 +50,7 @@ class EditorError(Exception):
   """
 
   def __init__(self, reason):
-    super(EditorError, self).__init__()
+    super().__init__(reason)
     self.reason = reason
 
   def __str__(self):
@@ -62,7 +62,7 @@ class GitError(Exception):
   """
 
   def __init__(self, command):
-    super(GitError, self).__init__()
+    super().__init__(command)
     self.command = command
 
   def __str__(self):
@@ -74,7 +74,7 @@ class UploadError(Exception):
   """
 
   def __init__(self, reason):
-    super(UploadError, self).__init__()
+    super().__init__(reason)
     self.reason = reason
 
   def __str__(self):
@@ -86,7 +86,7 @@ class DownloadError(Exception):
   """
 
   def __init__(self, reason):
-    super(DownloadError, self).__init__()
+    super().__init__(reason)
     self.reason = reason
 
   def __str__(self):
@@ -98,7 +98,7 @@ class NoSuchProjectError(Exception):
   """
 
   def __init__(self, name=None):
-    super(NoSuchProjectError, self).__init__()
+    super().__init__(name)
     self.name = name
 
   def __str__(self):
@@ -112,7 +112,7 @@ class InvalidProjectGroupsError(Exception):
   """
 
   def __init__(self, name=None):
-    super(InvalidProjectGroupsError, self).__init__()
+    super().__init__(name)
     self.name = name
 
   def __str__(self):
@@ -128,7 +128,7 @@ class RepoChangedException(Exception):
   """
 
   def __init__(self, extra_args=None):
-    super(RepoChangedException, self).__init__()
+    super().__init__(extra_args)
     self.extra_args = extra_args or []
 
 

git_command.py

@@ -162,11 +162,10 @@ def RepoSourceVersion():
 
     proj = os.path.dirname(os.path.abspath(__file__))
     env[GIT_DIR] = os.path.join(proj, '.git')
-
-    p = subprocess.Popen([GIT, 'describe', HEAD], stdout=subprocess.PIPE,
-                         env=env)
-    if p.wait() == 0:
-      ver = p.stdout.read().strip().decode('utf-8')
+    result = subprocess.run([GIT, 'describe', HEAD], stdout=subprocess.PIPE,
+                            encoding='utf-8', env=env, check=False)
+    if result.returncode == 0:
+      ver = result.stdout.strip()
       if ver.startswith('v'):
         ver = ver[1:]
     else:
@@ -250,7 +249,7 @@ class GitCommand(object):
                project,
                cmdv,
                bare=False,
-               provide_stdin=False,
+               input=None,
                capture_stdout=False,
                capture_stderr=False,
                merge_output=False,
@@ -260,9 +259,6 @@ class GitCommand(object):
                gitdir=None):
     env = self._GetBasicEnv()
 
-    # If we are not capturing std* then need to print it.
-    self.tee = {'stdout': not capture_stdout, 'stderr': not capture_stderr}
-
     if disable_editor:
       env['GIT_EDITOR'] = ':'
     if ssh_proxy:
@@ -289,6 +285,9 @@ class GitCommand(object):
     command = [GIT]
     if bare:
       if gitdir:
+        # Git on Windows wants its paths only using / for reliability.
+        if platform_utils.isWindows():
+          gitdir = gitdir.replace('\\', '/')
         env[GIT_DIR] = gitdir
       cwd = None
     command.append(cmdv[0])
@@ -299,13 +298,10 @@ class GitCommand(object):
         command.append('--progress')
     command.extend(cmdv[1:])
 
-    if provide_stdin:
-      stdin = subprocess.PIPE
-    else:
-      stdin = None
-
-    stdout = subprocess.PIPE
-    stderr = subprocess.STDOUT if merge_output else subprocess.PIPE
+    stdin = subprocess.PIPE if input else None
+    stdout = subprocess.PIPE if capture_stdout else None
+    stderr = (subprocess.STDOUT if merge_output else
+              (subprocess.PIPE if capture_stderr else None))
 
     if IsTrace():
       global LAST_CWD
@@ -341,6 +337,8 @@ class GitCommand(object):
       p = subprocess.Popen(command,
                            cwd=cwd,
                            env=env,
+                           encoding='utf-8',
+                           errors='backslashreplace',
                            stdin=stdin,
                            stdout=stdout,
                            stderr=stderr)
@@ -351,7 +349,17 @@ class GitCommand(object):
       _add_ssh_client(p)
 
     self.process = p
-    self.stdin = p.stdin
+    if input:
+      if isinstance(input, str):
+        input = input.encode('utf-8')
+      p.stdin.write(input)
+      p.stdin.close()
+
+    try:
+      self.stdout, self.stderr = p.communicate()
+    finally:
+      _remove_ssh_client(p)
+    self.rc = p.wait()
 
   @staticmethod
   def _GetBasicEnv():
@@ -371,36 +379,4 @@ class GitCommand(object):
     return env
 
   def Wait(self):
-    try:
-      p = self.process
-      rc = self._CaptureOutput()
-    finally:
-      _remove_ssh_client(p)
-    return rc
-
-  def _CaptureOutput(self):
-    p = self.process
-    s_in = platform_utils.FileDescriptorStreams.create()
-    s_in.add(p.stdout, sys.stdout, 'stdout')
-    if p.stderr is not None:
-      s_in.add(p.stderr, sys.stderr, 'stderr')
-    self.stdout = ''
-    self.stderr = ''
-
-    while not s_in.is_done:
-      in_ready = s_in.select()
-      for s in in_ready:
-        buf = s.read()
-        if not buf:
-          s_in.remove(s)
-          continue
-        if not hasattr(buf, 'encode'):
-          buf = buf.decode('utf-8', 'backslashreplace')
-        if s.std_name == 'stdout':
-          self.stdout += buf
-        else:
-          self.stderr += buf
-        if self.tee[s.std_name]:
-          s.dest.write(buf)
-          s.dest.flush()
-    return p.wait()
+    return self.rc

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.
@@ -161,6 +176,12 @@ class GitConfig(object):
       return False
     return None
 
+  def SetBoolean(self, name, value):
+    """Set the truthy value for a key."""
+    if value is not None:
+      value = 'true' if value else 'false'
+    self.SetString(name, value)
+
   def GetString(self, name, all_keys=False):
     """Get the first value for a key, or None if it is not defined.
 

git_refs.py

@@ -131,11 +131,14 @@ class GitRefs(object):
     base = os.path.join(self._gitdir, prefix)
     for name in platform_utils.listdir(base):
       p = os.path.join(base, name)
-      if platform_utils.isdir(p):
+      # We don't implement the full ref validation algorithm, just the simple
+      # rules that would show up in local filesystems.
+      # https://git-scm.com/docs/git-check-ref-format
+      if name.startswith('.') or name.endswith('.lock'):
+        pass
+      elif platform_utils.isdir(p):
         self._mtime[prefix] = os.path.getmtime(base)
         self._ReadLoose(prefix + name + '/')
-      elif name.endswith('.lock'):
-        pass
       else:
         self._ReadLoose1(p, prefix + name)
 
@@ -144,7 +147,7 @@ class GitRefs(object):
       with open(path) as fd:
         mtime = os.path.getmtime(path)
         ref_id = fd.readline()
-    except (IOError, OSError):
+    except (OSError, UnicodeError):
       return
 
     try:

git_superproject.py

@@ -12,69 +12,90 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-"""Provide functionality to get all projects and their SHAs from Superproject.
+"""Provide functionality to get all projects and their commit ids from Superproject.
 
 For more information on superproject, check out:
 https://en.wikibooks.org/wiki/Git/Submodules_and_Superprojects
 
 Examples:
   superproject = Superproject()
-  project_shas = superproject.GetAllProjectsSHAs()
+  project_commit_ids = superproject.UpdateProjectsRevisionId(projects)
 """
 
+import hashlib
 import os
 import sys
 
-from error import BUG_REPORT_URL, GitError
+from error import BUG_REPORT_URL
 from git_command import GitCommand
-import platform_utils
+from git_refs import R_HEADS
 
 _SUPERPROJECT_GIT_NAME = 'superproject.git'
 _SUPERPROJECT_MANIFEST_NAME = 'superproject_override.xml'
 
 
 class Superproject(object):
-  """Get SHAs from superproject.
+  """Get commit ids from superproject.
 
-  It does a 'git clone' of superproject and 'git ls-tree' to get list of SHAs for all projects.
-  It contains project_shas which is a dictionary with project/sha entries.
+  Initializes a local copy of a superproject for the manifest. This allows
+  lookup of commit ids for all projects. It contains _project_commit_ids which
+  is a dictionary with project/commit id entries.
   """
-  def __init__(self, repodir, superproject_dir='exp-superproject'):
+  def __init__(self, manifest, repodir, superproject_dir='exp-superproject',
+               quiet=False):
     """Initializes superproject.
 
     Args:
+      manifest: A Manifest object that is to be written to a file.
       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_shas = None
+    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
     self._superproject_path = os.path.join(self._repodir, superproject_dir)
     self._manifest_path = os.path.join(self._superproject_path,
                                        _SUPERPROJECT_MANIFEST_NAME)
-    self._work_git = os.path.join(self._superproject_path,
-                                  _SUPERPROJECT_GIT_NAME)
+    git_name = ''
+    if self._manifest.superproject:
+      remote_name = self._manifest.superproject['remote'].name
+      git_name = hashlib.md5(remote_name.encode('utf8')).hexdigest() + '-'
+    self._work_git_name = git_name + _SUPERPROJECT_GIT_NAME
+    self._work_git = os.path.join(self._superproject_path, self._work_git_name)
 
   @property
-  def project_shas(self):
-    """Returns a dictionary of projects and their SHAs."""
-    return self._project_shas
+  def project_commit_ids(self):
+    """Returns a dictionary of projects and their commit ids."""
+    return self._project_commit_ids
 
-  def _Clone(self, url, branch=None):
-    """Do a 'git clone' for the given url and branch.
+  def _GetBranch(self):
+    """Returns the branch name for getting the approved manifest."""
+    p = self._manifest.manifestProject
+    b = p.GetBranch(p.CurrentBranch)
+    if not b:
+      return None
+    branch = b.merge
+    if branch and branch.startswith(R_HEADS):
+      branch = branch[len(R_HEADS):]
+    return branch
 
-    Args:
-      url: superproject's url to be passed to git clone.
-      branch: The branchname to be passed as argument to git clone.
+  def _Init(self):
+    """Sets up a local Git repository to get a copy of a superproject.
 
     Returns:
-      True if 'git clone <url> <branch>' is successful, or False.
+      True if initialization is successful, or False.
     """
     if not os.path.exists(self._superproject_path):
       os.mkdir(self._superproject_path)
-    cmd = ['clone', url, '--filter', 'blob:none', '--bare']
-    if branch:
-      cmd += ['--branch', branch]
+    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,
                    cwd=self._superproject_path,
@@ -82,24 +103,27 @@ class Superproject(object):
                    capture_stderr=True)
     retval = p.Wait()
     if retval:
-      # `git clone` is documented to produce an exit status of `128` if
-      # the requested url or branch are not present in the configuration.
-      print('repo: error: git clone call failed with return code: %r, stderr: %r' %
+      print('repo: error: git init call failed with return code: %r, stderr: %r' %
             (retval, p.stderr), file=sys.stderr)
       return False
     return True
 
-  def _Fetch(self):
-    """Do a 'git fetch' to to fetch the latest content.
+  def _Fetch(self, url):
+    """Fetches a local copy of a superproject for the manifest based on url.
+
+    Args:
+      url: superproject's url.
 
     Returns:
-      True if 'git fetch' is successful, or False.
+      True if fetch is successful, or False.
     """
     if not os.path.exists(self._work_git):
       print('git fetch missing drectory: %s' % self._work_git,
             file=sys.stderr)
       return False
-    cmd = ['fetch', 'origin', '+refs/heads/*:refs/heads/*', '--prune']
+    cmd = ['fetch', url, '--depth', '1', '--force', '--no-tags', '--filter', 'blob:none']
+    if self._branch:
+      cmd += [self._branch + ':' + self._branch]
     p = GitCommand(None,
                    cmd,
                    cwd=self._work_git,
@@ -113,19 +137,21 @@ class Superproject(object):
     return True
 
   def _LsTree(self):
-    """Returns the data from 'git ls-tree -r HEAD'.
+    """Gets the commit ids for all projects.
 
     Works only in git repositories.
 
     Returns:
-      data: data returned from 'git ls-tree -r HEAD' instead of None.
+      data: data returned from 'git ls-tree ...' instead of None.
     """
     if not os.path.exists(self._work_git):
       print('git ls-tree missing drectory: %s' % self._work_git,
             file=sys.stderr)
       return None
     data = None
-    cmd = ['ls-tree', '-z', '-r', 'HEAD']
+    branch = 'HEAD' if not self._branch else self._branch
+    cmd = ['ls-tree', '-z', '-r', branch]
+
     p = GitCommand(None,
                    cmd,
                    cwd=self._work_git,
@@ -135,62 +161,72 @@ class Superproject(object):
     if retval == 0:
       data = p.stdout
     else:
-      # `git clone` is documented to produce an exit status of `128` if
-      # the requested url or branch are not present in the configuration.
       print('repo: error: git ls-tree call failed with return code: %r, stderr: %r' % (
           retval, p.stderr), file=sys.stderr)
     return data
 
-  def _GetAllProjectsSHAs(self, url, branch=None):
-    """Get SHAs for all projects from superproject and save them in _project_shas.
-
-    Args:
-      url: superproject's url to be passed to git clone or fetch.
-      branch: The branchname to be passed as argument to git clone or fetch.
+  def Sync(self):
+    """Gets a local copy of a superproject for the manifest.
 
     Returns:
-      A dictionary with the projects/SHAs instead of None.
+      True if sync of superproject is successful, or False.
     """
-    if not url:
-      raise ValueError('url argument is not supplied.')
+    print('WARNING: --use-superproject is experimental and not '
+          'for general use', file=sys.stderr)
 
-    do_clone = True
-    if os.path.exists(self._superproject_path):
-      if not self._Fetch():
-        # If fetch fails due to a corrupted git directory, then do a git clone.
-        platform_utils.rmtree(self._superproject_path)
-      else:
-        do_clone = False
-    if do_clone:
-      if not self._Clone(url, branch):
-        raise GitError('git clone failed for url: %s' % url)
+    if not self._manifest.superproject:
+      print('error: superproject tag is not defined in manifest',
+            file=sys.stderr)
+      return False
+
+    url = self._manifest.superproject['remote'].url
+    if not url:
+      print('error: superproject URL is not defined in manifest',
+            file=sys.stderr)
+      return False
+
+    if not self._Init():
+      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):
+    """Get commit ids for all projects from superproject and save them in _project_commit_ids.
+
+    Returns:
+      A dictionary with the projects/commit ids on success, otherwise None.
+    """
+    if not self.Sync():
+      return None
 
     data = self._LsTree()
     if not data:
-      raise GitError('git ls-tree failed for url: %s' % url)
+      print('error: git ls-tree failed to return data for superproject',
+            file=sys.stderr)
+      return None
 
     # Parse lines like the following to select lines starting with '160000' and
-    # build a dictionary with project path (last element) and its SHA (3rd element).
+    # build a dictionary with project path (last element) and its commit id (3rd element).
     #
     # 160000 commit 2c2724cb36cd5a9cec6c852c681efc3b7c6b86ea\tart\x00
     # 120000 blob acc2cbdf438f9d2141f0ae424cec1d8fc4b5d97f\tbootstrap.bash\x00
-    shas = {}
+    commit_ids = {}
     for line in data.split('\x00'):
       ls_data = line.split(None, 3)
       if not ls_data:
         break
       if ls_data[0] == '160000':
-        shas[ls_data[3]] = ls_data[2]
+        commit_ids[ls_data[3]] = ls_data[2]
 
-    self._project_shas = shas
-    return shas
+    self._project_commit_ids = commit_ids
+    return commit_ids
 
-  def _WriteManfiestFile(self, manifest):
+  def _WriteManfiestFile(self):
     """Writes manifest to a file.
 
-    Args:
-      manifest: A Manifest object that is to be written to a file.
-
     Returns:
       manifest_path: Path name of the file into which manifest is written instead of None.
     """
@@ -199,7 +235,7 @@ class Superproject(object):
             self._superproject_path,
             file=sys.stderr)
       return None
-    manifest_str = 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:
@@ -211,40 +247,34 @@ class Superproject(object):
       return None
     return manifest_path
 
-  def UpdateProjectsRevisionId(self, manifest, projects, url, branch=None):
-    """Update revisionId of every project in projects with the SHA.
+  def UpdateProjectsRevisionId(self, projects):
+    """Update revisionId of every project in projects with the commit id.
 
     Args:
-      manifest: A Manifest object that is to be written to a file.
       projects: List of projects whose revisionId needs to be updated.
-      url: superproject's url to be passed to git clone or fetch.
-      branch: The branchname to be passed as argument to git clone or fetch.
 
     Returns:
       manifest_path: Path name of the overriding manfiest file instead of None.
     """
-    try:
-      shas = self._GetAllProjectsSHAs(url=url, branch=branch)
-    except Exception as e:
-      print('error: Cannot get project SHAs for %s: %s: %s' %
-            (url, type(e).__name__, str(e)),
-            file=sys.stderr)
+    commit_ids = self._GetAllProjectsCommitIds()
+    if not commit_ids:
+      print('error: Cannot get project commit ids from manifest', file=sys.stderr)
       return None
 
-    projects_missing_shas = []
+    projects_missing_commit_ids = []
     for project in projects:
       path = project.relpath
       if not path:
         continue
-      sha = shas.get(path)
-      if sha:
-        project.SetRevisionId(sha)
+      commit_id = commit_ids.get(path)
+      if commit_id:
+        project.SetRevisionId(commit_id)
       else:
-        projects_missing_shas.append(path)
-    if projects_missing_shas:
-      print('error: please file a bug using %s to report missing shas for: %s' %
-            (BUG_REPORT_URL, projects_missing_shas), file=sys.stderr)
+        projects_missing_commit_ids.append(path)
+    if projects_missing_commit_ids:
+      print('error: please file a bug using %s to report missing commit_ids for: %s' %
+            (BUG_REPORT_URL, projects_missing_commit_ids), file=sys.stderr)
       return None
 
-    manifest_path = self._WriteManfiestFile(manifest)
+    manifest_path = self._WriteManfiestFile()
     return manifest_path

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

@@ -13,6 +13,7 @@
 # limitations under the License.
 
 import os
+import multiprocessing
 import platform
 import re
 import sys
@@ -35,6 +36,15 @@ def parse_clientdir(gitc_fs_path):
   return wrapper.Wrapper().gitc_parse_clientdir(gitc_fs_path)
 
 
+def _get_project_revision(args):
+  """Worker for _set_project_revisions to lookup one project remote."""
+  (i, url, expr) = args
+  gitcmd = git_command.GitCommand(
+      None, ['ls-remote', url, expr], capture_stdout=True, cwd='/tmp')
+  rc = gitcmd.Wait()
+  return (i, rc, gitcmd.stdout.split('\t', 1)[0])
+
+
 def _set_project_revisions(projects):
   """Sets the revisionExpr for a list of projects.
 
@@ -47,38 +57,24 @@ def _set_project_revisions(projects):
   """
   # Retrieve the commit id for each project based off of it's current
   # revisionExpr and it is not already a commit id.
-  project_gitcmds = [(
-      project, git_command.GitCommand(None,
-                                      ['ls-remote',
-                                       project.remote.url,
-                                       project.revisionExpr],
-                                      capture_stdout=True, cwd='/tmp'))
-                     for project in projects if not git_config.IsId(project.revisionExpr)]
-  for proj, gitcmd in project_gitcmds:
-    if gitcmd.Wait():
-      print('FATAL: Failed to retrieve revisionExpr for %s' % proj)
-      sys.exit(1)
-    revisionExpr = gitcmd.stdout.split('\t')[0]
-    if not revisionExpr:
-      raise ManifestParseError('Invalid SHA-1 revision project %s (%s)' %
-                               (proj.remote.url, proj.revisionExpr))
-    proj.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
+  with multiprocessing.Pool(NUM_BATCH_RETRIEVE_REVISIONID) as pool:
+    results_iter = pool.imap_unordered(
+        _get_project_revision,
+        ((i, project.remote.url, project.revisionExpr)
+         for i, project in enumerate(projects)
+         if not git_config.IsId(project.revisionExpr)),
+        chunksize=8)
+    for (i, rc, revisionExpr) in results_iter:
+      project = projects[i]
+      if rc:
+        print('FATAL: Failed to retrieve revisionExpr for %s' % project.name)
+        pool.terminate()
+        sys.exit(1)
+      if not revisionExpr:
+        pool.terminate()
+        raise ManifestParseError('Invalid SHA-1 revision project %s (%s)' %
+                                 (project.remote.url, project.revisionExpr))
+      project.revisionExpr = revisionExpr
 
 
 def generate_gitc_manifest(gitc_manifest, manifest, paths=None):
@@ -95,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]
@@ -123,11 +119,7 @@ def generate_gitc_manifest(gitc_manifest, manifest, paths=None):
         else:
           proj.revisionExpr = gitc_proj.revisionExpr
 
-  index = 0
-  while index < len(projects):
-    _set_project_revisions(
-        projects[index:(index + NUM_BATCH_RETRIEVE_REVISIONID)])
-    index += NUM_BATCH_RETRIEVE_REVISIONID
+  _set_project_revisions(projects)
 
   if gitc_manifest is not None:
     for path, proj in gitc_manifest.paths.items():
@@ -158,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,8 +254,10 @@ 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.CommonValidateOptions(copts, cargs)
       cmd.ValidateOptions(copts, cargs)
       result = cmd.Execute(copts, cargs)
     except (DownloadError, ManifestInvalidRevisionError,
@@ -297,6 +299,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
@@ -533,7 +534,6 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
 
   def _output_manifest_project_extras(self, p, e):
     """Manifests can modify e if they support extra project attributes."""
-    pass
 
   @property
   def paths(self):
@@ -589,6 +589,12 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
       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 IsMirror(self):
     return self.manifestProject.config.GetBoolean('repo.mirror')
@@ -605,6 +611,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 = {}
@@ -625,16 +642,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
 
@@ -652,7 +675,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:
@@ -671,6 +706,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')
+        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
@@ -678,13 +718,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(
@@ -980,6 +1020,10 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
     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)
 
@@ -1000,9 +1044,12 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
     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))
+    else:
+      # 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)
@@ -1122,8 +1169,33 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
     return relpath, worktree, gitdir, objdir
 
   @staticmethod
-  def _CheckLocalPath(path, symlink=False):
-    """Verify |path| is reasonable for use in <copyfile> & <linkfile>."""
+  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)'
 
@@ -1162,16 +1234,18 @@ 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.
-    if parts != ['.']:
+    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 symlink and resep.match(path[-1]):
+    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
@@ -1203,7 +1277,8 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
 
     # |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.
-    msg = cls._CheckLocalPath(src, symlink=element == 'linkfile')
+    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))
@@ -1302,7 +1377,7 @@ class GitcManifest(XmlManifest):
 
   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):
@@ -1326,7 +1401,7 @@ class RepoClient(XmlManifest):
     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(RepoClient, self).__init__(repodir, manifest_file, local_manifests)
+    super().__init__(repodir, manifest_file, local_manifests)
 
     # TODO: Completely separate manifest logic out of the client.
     self.manifest = self
@@ -1341,6 +1416,5 @@ class GitcClient(RepoClient, GitcManifest):
     self.gitc_client_dir = os.path.join(gitc_utils.get_gitc_manifest_dir(),
                                         gitc_client_name)
 
-    super(GitcManifest, self).__init__(
-        repodir, os.path.join(self.gitc_client_dir, '.manifest'))
+    super().__init__(repodir, os.path.join(self.gitc_client_dir, '.manifest'))
     self.isGitcClient = True

platform_utils.py

@@ -15,11 +15,8 @@
 import errno
 import os
 import platform
-from queue import Queue
-import select
 import shutil
 import stat
-from threading import Thread
 
 
 def isWindows():
@@ -31,161 +28,6 @@ def isWindows():
   return platform.system() == "Windows"
 
 
-class FileDescriptorStreams(object):
-  """ Platform agnostic abstraction enabling non-blocking I/O over a
-  collection of file descriptors. This abstraction is required because
-  fctnl(os.O_NONBLOCK) is not supported on Windows.
-  """
-  @classmethod
-  def create(cls):
-    """ Factory method: instantiates the concrete class according to the
-    current platform.
-    """
-    if isWindows():
-      return _FileDescriptorStreamsThreads()
-    else:
-      return _FileDescriptorStreamsNonBlocking()
-
-  def __init__(self):
-    self.streams = []
-
-  def add(self, fd, dest, std_name):
-    """ Wraps an existing file descriptor as a stream.
-    """
-    self.streams.append(self._create_stream(fd, dest, std_name))
-
-  def remove(self, stream):
-    """ Removes a stream, when done with it.
-    """
-    self.streams.remove(stream)
-
-  @property
-  def is_done(self):
-    """ Returns True when all streams have been processed.
-    """
-    return len(self.streams) == 0
-
-  def select(self):
-    """ Returns the set of streams that have data available to read.
-    The returned streams each expose a read() and a close() method.
-    When done with a stream, call the remove(stream) method.
-    """
-    raise NotImplementedError
-
-  def _create_stream(self, fd, dest, std_name):
-    """ Creates a new stream wrapping an existing file descriptor.
-    """
-    raise NotImplementedError
-
-
-class _FileDescriptorStreamsNonBlocking(FileDescriptorStreams):
-  """ Implementation of FileDescriptorStreams for platforms that support
-  non blocking I/O.
-  """
-  def __init__(self):
-    super(_FileDescriptorStreamsNonBlocking, self).__init__()
-    self._poll = select.poll()
-    self._fd_to_stream = {}
-
-  class Stream(object):
-    """ Encapsulates a file descriptor """
-
-    def __init__(self, fd, dest, std_name):
-      self.fd = fd
-      self.dest = dest
-      self.std_name = std_name
-      self.set_non_blocking()
-
-    def set_non_blocking(self):
-      import fcntl
-      flags = fcntl.fcntl(self.fd, fcntl.F_GETFL)
-      fcntl.fcntl(self.fd, fcntl.F_SETFL, flags | os.O_NONBLOCK)
-
-    def fileno(self):
-      return self.fd.fileno()
-
-    def read(self):
-      return self.fd.read(4096)
-
-    def close(self):
-      self.fd.close()
-
-  def _create_stream(self, fd, dest, std_name):
-    stream = self.Stream(fd, dest, std_name)
-    self._fd_to_stream[stream.fileno()] = stream
-    self._poll.register(stream, select.POLLIN)
-    return stream
-
-  def remove(self, stream):
-    self._poll.unregister(stream)
-    del self._fd_to_stream[stream.fileno()]
-    super(_FileDescriptorStreamsNonBlocking, self).remove(stream)
-
-  def select(self):
-    return [self._fd_to_stream[fd] for fd, _ in self._poll.poll()]
-
-
-class _FileDescriptorStreamsThreads(FileDescriptorStreams):
-  """ Implementation of FileDescriptorStreams for platforms that don't support
-  non blocking I/O. This implementation requires creating threads issuing
-  blocking read operations on file descriptors.
-  """
-
-  def __init__(self):
-    super(_FileDescriptorStreamsThreads, self).__init__()
-    # The queue is shared accross all threads so we can simulate the
-    # behavior of the select() function
-    self.queue = Queue(10)  # Limit incoming data from streams
-
-  def _create_stream(self, fd, dest, std_name):
-    return self.Stream(fd, dest, std_name, self.queue)
-
-  def select(self):
-    # Return only one stream at a time, as it is the most straighforward
-    # thing to do and it is compatible with the select() function.
-    item = self.queue.get()
-    stream = item.stream
-    stream.data = item.data
-    return [stream]
-
-  class QueueItem(object):
-    """ Item put in the shared queue """
-
-    def __init__(self, stream, data):
-      self.stream = stream
-      self.data = data
-
-  class Stream(object):
-    """ Encapsulates a file descriptor """
-
-    def __init__(self, fd, dest, std_name, queue):
-      self.fd = fd
-      self.dest = dest
-      self.std_name = std_name
-      self.queue = queue
-      self.data = None
-      self.thread = Thread(target=self.read_to_queue)
-      self.thread.daemon = True
-      self.thread.start()
-
-    def close(self):
-      self.fd.close()
-
-    def read(self):
-      data = self.data
-      self.data = None
-      return data
-
-    def read_to_queue(self):
-      """ The thread function: reads everything from the file descriptor into
-      the shared queue and terminates when reaching EOF.
-      """
-      for line in iter(self.fd.readline, b''):
-        self.queue.put(_FileDescriptorStreamsThreads.QueueItem(self, line))
-      self.fd.close()
-      self.queue.put(_FileDescriptorStreamsThreads.QueueItem(self, b''))
-
-
 def symlink(source, link_name):
   """Creates a symbolic link pointing to source named link_name.
   Note: On Windows, source must exist on disk, as the implementation needs

progress.py

@@ -25,18 +25,52 @@ _NOT_TTY = not os.isatty(2)
 CSI_ERASE_LINE = '\x1b[2K'
 
 
+def duration_str(total):
+  """A less noisy timedelta.__str__.
+
+  The default timedelta stringification contains a lot of leading zeros and
+  uses microsecond resolution.  This makes for noisy output.
+  """
+  hours, rem = divmod(total, 3600)
+  mins, secs = divmod(rem, 60)
+  ret = '%.3fs' % (secs,)
+  if mins:
+    ret = '%im%s' % (mins, ret)
+  if hours:
+    ret = '%ih%s' % (hours, ret)
+  return ret
+
+
 class Progress(object):
-  def __init__(self, title, total=0, units='', print_newline=False,
-               always_print_percentage=False):
+  def __init__(self, title, total=0, units='', print_newline=False, delay=True,
+               quiet=False):
     self._title = title
     self._total = total
     self._done = 0
-    self._lastp = -1
     self._start = time()
-    self._show = False
+    self._show = not delay
     self._units = units
     self._print_newline = print_newline
-    self._always_print_percentage = always_print_percentage
+    # Only show the active jobs section if we run more than one in parallel.
+    self._show_jobs = False
+    self._active = 0
+
+    # When quiet, never show any output.  It's a bit hacky, but reusing the
+    # existing logic that delays initial output keeps the rest of the class
+    # clean.  Basically we set the start time to years in the future.
+    if quiet:
+      self._show = False
+      self._start += 2**32
+
+  def start(self, name):
+    self._active += 1
+    if not self._show_jobs:
+      self._show_jobs = self._active > 1
+    self.update(inc=0, msg='started ' + name)
+
+  def finish(self, name):
+    self.update(msg='finished ' + name)
+    self._active -= 1
 
   def update(self, inc=1, msg=''):
     self._done += inc
@@ -58,35 +92,40 @@ class Progress(object):
       sys.stderr.flush()
     else:
       p = (100 * self._done) / self._total
-
-      if self._lastp != p or self._always_print_percentage:
-        self._lastp = p
-        sys.stderr.write('%s\r%s: %3d%% (%d%s/%d%s)%s%s%s' % (
-            CSI_ERASE_LINE,
-            self._title,
-            p,
-            self._done, self._units,
-            self._total, self._units,
-            ' ' if msg else '', msg,
-            "\n" if self._print_newline else ""))
-        sys.stderr.flush()
+      if self._show_jobs:
+        jobs = '[%d job%s] ' % (self._active, 's' if self._active > 1 else '')
+      else:
+        jobs = ''
+      sys.stderr.write('%s\r%s: %2d%% %s(%d%s/%d%s)%s%s%s' % (
+          CSI_ERASE_LINE,
+          self._title,
+          p,
+          jobs,
+          self._done, self._units,
+          self._total, self._units,
+          ' ' if msg else '', msg,
+          '\n' if self._print_newline else ''))
+      sys.stderr.flush()
 
   def end(self):
     if _NOT_TTY or IsTrace() or not self._show:
       return
 
+    duration = duration_str(time() - self._start)
     if self._total <= 0:
-      sys.stderr.write('%s\r%s: %d, done.\n' % (
+      sys.stderr.write('%s\r%s: %d, done in %s\n' % (
           CSI_ERASE_LINE,
           self._title,
-          self._done))
+          self._done,
+          duration))
       sys.stderr.flush()
     else:
       p = (100 * self._done) / self._total
-      sys.stderr.write('%s\r%s: %3d%% (%d%s/%d%s), done.\n' % (
+      sys.stderr.write('%s\r%s: %3d%% (%d%s/%d%s), done in %s\n' % (
           CSI_ERASE_LINE,
           self._title,
           p,
           self._done, self._units,
-          self._total, self._units))
+          self._total, self._units,
+          duration))
       sys.stderr.flush()

project.py

@@ -232,7 +232,7 @@ class ReviewableBranch(object):
 class StatusColoring(Coloring):
 
   def __init__(self, config):
-    Coloring.__init__(self, config, 'status')
+    super().__init__(config, 'status')
     self.project = self.printer('header', attr='bold')
     self.branch = self.printer('header', attr='bold')
     self.nobranch = self.printer('nobranch', fg='red')
@@ -246,7 +246,7 @@ class StatusColoring(Coloring):
 class DiffColoring(Coloring):
 
   def __init__(self, config):
-    Coloring.__init__(self, config, 'diff')
+    super().__init__(config, 'diff')
     self.project = self.printer('header', attr='bold')
     self.fail = self.printer('fail', fg='red')
 
@@ -832,10 +832,12 @@ class Project(object):
 
     return 'DIRTY'
 
-  def PrintWorkTreeDiff(self, absolute_paths=False):
+  def PrintWorkTreeDiff(self, absolute_paths=False, output_redir=None):
     """Prints the status of the repository to stdout.
     """
     out = DiffColoring(self.config)
+    if output_redir:
+      out.redirect(output_redir)
     cmd = ['diff']
     if out.is_on:
       cmd.append('--color')
@@ -849,6 +851,7 @@ class Project(object):
                      cmd,
                      capture_stdout=True,
                      capture_stderr=True)
+      p.Wait()
     except GitError as e:
       out.nl()
       out.project('project %s/' % self.relpath)
@@ -856,16 +859,11 @@ class Project(object):
       out.fail('%s', str(e))
       out.nl()
       return False
-    has_diff = False
-    for line in p.process.stdout:
-      if not hasattr(line, 'encode'):
-        line = line.decode()
-      if not has_diff:
-        out.nl()
-        out.project('project %s/' % self.relpath)
-        out.nl()
-        has_diff = True
-      print(line[:-1])
+    if p.stdout:
+      out.nl()
+      out.project('project %s/' % self.relpath)
+      out.nl()
+      out.write('%s', p.stdout)
     return p.Wait() == 0
 
 # Publish / Upload ##
@@ -1041,6 +1039,7 @@ class Project(object):
   def Sync_NetworkHalf(self,
                        quiet=False,
                        verbose=False,
+                       output_redir=None,
                        is_new=None,
                        current_branch_only=False,
                        force_sync=False,
@@ -1051,7 +1050,8 @@ class Project(object):
                        retry_fetches=0,
                        prune=False,
                        submodules=False,
-                       clone_filter=None):
+                       clone_filter=None,
+                       partial_clone_exclude=set()):
     """Perform only the network IO portion of the sync process.
        Local working directory/branch state is not affected.
     """
@@ -1082,6 +1082,16 @@ class Project(object):
         _warn("Cannot remove archive %s: %s", tarpath, str(e))
       self._CopyAndLinkFiles()
       return True
+
+    # If the shared object dir already exists, don't try to rebootstrap with a
+    # clone bundle download.  We should have the majority of objects already.
+    if clone_bundle and os.path.exists(self.objdir):
+      clone_bundle = False
+
+    if self.name in partial_clone_exclude:
+      clone_bundle = True
+      clone_filter = None
+
     if is_new is None:
       is_new = not self.Exists
     if is_new:
@@ -1128,8 +1138,9 @@ class Project(object):
             (ID_RE.match(self.revisionExpr) and
              self._CheckForImmutableRevision())):
       if not self._RemoteFetch(
-              initial=is_new, quiet=quiet, verbose=verbose, alt_dir=alt_dir,
-              current_branch_only=current_branch_only,
+              initial=is_new,
+              quiet=quiet, verbose=verbose, output_redir=output_redir,
+              alt_dir=alt_dir, current_branch_only=current_branch_only,
               tags=tags, prune=prune, depth=depth,
               submodules=submodules, force_sync=force_sync,
               clone_filter=clone_filter, retry_fetches=retry_fetches):
@@ -1141,7 +1152,11 @@ class Project(object):
       alternates_file = os.path.join(self.gitdir, 'objects/info/alternates')
       if os.path.exists(alternates_file):
         cmd = ['repack', '-a', '-d']
-        if GitCommand(self, cmd, bare=True).Wait() != 0:
+        p = GitCommand(self, cmd, bare=True, capture_stdout=bool(output_redir),
+                       merge_output=bool(output_redir))
+        if p.stdout and output_redir:
+          output_redir.write(p.stdout)
+        if p.Wait() != 0:
           return False
         platform_utils.remove(alternates_file)
 
@@ -1217,6 +1232,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()
@@ -1688,6 +1715,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) \
@@ -1953,6 +1985,7 @@ class Project(object):
                    initial=False,
                    quiet=False,
                    verbose=False,
+                   output_redir=None,
                    alt_dir=None,
                    tags=True,
                    prune=False,
@@ -2130,29 +2163,27 @@ class Project(object):
     ok = prune_tried = False
     for try_n in range(retry_fetches):
       gitcmd = GitCommand(self, cmd, bare=True, ssh_proxy=ssh_proxy,
-                          merge_output=True, capture_stdout=quiet)
+                          merge_output=True, capture_stdout=quiet or bool(output_redir))
+      if gitcmd.stdout and not quiet and output_redir:
+        output_redir.write(gitcmd.stdout)
       ret = gitcmd.Wait()
       if ret == 0:
         ok = True
         break
 
       # Retry later due to HTTP 429 Too Many Requests.
-      elif ('error:' in gitcmd.stderr and
-            'HTTP 429' in gitcmd.stderr):
-        if not quiet:
-          print('429 received, sleeping: %s sec' % retry_cur_sleep,
-                file=sys.stderr)
-        time.sleep(retry_cur_sleep)
-        retry_cur_sleep = min(retry_exp_factor * retry_cur_sleep,
-                              MAXIMUM_RETRY_SLEEP_SEC)
-        retry_cur_sleep *= (1 - random.uniform(-RETRY_JITTER_PERCENT,
-                                               RETRY_JITTER_PERCENT))
-        continue
+      elif (gitcmd.stdout and
+            'error:' in gitcmd.stdout and
+            'HTTP 429' in gitcmd.stdout):
+        # Fallthru to sleep+retry logic at the bottom.
+        pass
 
-      # If this is not last attempt, try 'git remote prune'.
-      elif (try_n < retry_fetches - 1 and
-            'error:' in gitcmd.stderr and
-            'git remote prune' in gitcmd.stderr and
+      # Try to prune remote branches once in case there are conflicts.
+      # For example, if the remote had refs/heads/upstream, but deleted that and
+      # now has refs/heads/upstream/foo.
+      elif (gitcmd.stdout and
+            'error:' in gitcmd.stdout and
+            'git remote prune' in gitcmd.stdout and
             not prune_tried):
         prune_tried = True
         prunecmd = GitCommand(self, ['remote', 'prune', name], bare=True,
@@ -2160,6 +2191,8 @@ class Project(object):
         ret = prunecmd.Wait()
         if ret:
           break
+        output_redir.write('retrying fetch after pruning remote branches')
+        # Continue right away so we don't sleep as we shouldn't need to.
         continue
       elif current_branch_only and is_sha1 and ret == 128:
         # Exit code 128 means "couldn't find the ref you asked for"; if we're
@@ -2169,9 +2202,17 @@ class Project(object):
       elif ret < 0:
         # Git died with a signal, exit immediately
         break
+
+      # Figure out how long to sleep before the next attempt, if there is one.
       if not verbose:
-        print('%s:\n%s' % (self.name, gitcmd.stdout), file=sys.stderr)
-      time.sleep(random.randint(30, 45))
+        output_redir.write('\n%s:\n%s' % (self.name, gitcmd.stdout), file=sys.stderr)
+      if try_n < retry_fetches - 1:
+        output_redir.write('sleeping %s seconds before retrying' % retry_cur_sleep)
+        time.sleep(retry_cur_sleep)
+        retry_cur_sleep = min(retry_exp_factor * retry_cur_sleep,
+                              MAXIMUM_RETRY_SLEEP_SEC)
+        retry_cur_sleep *= (1 - random.uniform(-RETRY_JITTER_PERCENT,
+                                               RETRY_JITTER_PERCENT))
 
     if initial:
       if alt_dir:
@@ -2189,7 +2230,7 @@ class Project(object):
         # Sync the current branch only with depth set to None.
         # We always pass depth=None down to avoid infinite recursion.
         return self._RemoteFetch(
-            name=name, quiet=quiet, verbose=verbose,
+            name=name, quiet=quiet, verbose=verbose, output_redir=output_redir,
             current_branch_only=current_branch_only and depth,
             initial=False, alt_dir=alt_dir,
             depth=None, clone_filter=clone_filter)
@@ -2472,10 +2513,7 @@ class Project(object):
             self.config.SetString(key, m.GetString(key))
         self.config.SetString('filter.lfs.smudge', 'git-lfs smudge --skip -- %f')
         self.config.SetString('filter.lfs.process', 'git-lfs filter-process --skip')
-        if self.manifest.IsMirror:
-          self.config.SetString('core.bare', 'true')
-        else:
-          self.config.SetString('core.bare', None)
+        self.config.SetBoolean('core.bare', True if self.manifest.IsMirror else None)
     except Exception:
       if init_obj_dir and os.path.exists(self.objdir):
         platform_utils.rmtree(self.objdir)
@@ -2864,48 +2902,44 @@ class Project(object):
                      bare=False,
                      capture_stdout=True,
                      capture_stderr=True)
-      try:
-        out = p.process.stdout.read()
-        if not hasattr(out, 'encode'):
-          out = out.decode()
-        r = {}
-        if out:
-          out = iter(out[:-1].split('\0'))
-          while out:
-            try:
-              info = next(out)
-              path = next(out)
-            except StopIteration:
-              break
+      p.Wait()
+      r = {}
+      out = p.stdout
+      if out:
+        out = iter(out[:-1].split('\0'))
+        while out:
+          try:
+            info = next(out)
+            path = next(out)
+          except StopIteration:
+            break
 
-            class _Info(object):
+          class _Info(object):
 
-              def __init__(self, path, omode, nmode, oid, nid, state):
-                self.path = path
-                self.src_path = None
-                self.old_mode = omode
-                self.new_mode = nmode
-                self.old_id = oid
-                self.new_id = nid
+            def __init__(self, path, omode, nmode, oid, nid, state):
+              self.path = path
+              self.src_path = None
+              self.old_mode = omode
+              self.new_mode = nmode
+              self.old_id = oid
+              self.new_id = nid
 
-                if len(state) == 1:
-                  self.status = state
-                  self.level = None
-                else:
-                  self.status = state[:1]
-                  self.level = state[1:]
-                  while self.level.startswith('0'):
-                    self.level = self.level[1:]
+              if len(state) == 1:
+                self.status = state
+                self.level = None
+              else:
+                self.status = state[:1]
+                self.level = state[1:]
+                while self.level.startswith('0'):
+                  self.level = self.level[1:]
 
-            info = info[1:].split(' ')
-            info = _Info(path, *info)
-            if info.status in ('R', 'C'):
-              info.src_path = info.path
-              info.path = next(out)
-            r[info.path] = info
-        return r
-      finally:
-        p.Wait()
+          info = info[1:].split(' ')
+          info = _Info(path, *info)
+          if info.status in ('R', 'C'):
+            info.src_path = info.path
+            info.path = next(out)
+          r[info.path] = info
+      return r
 
     def GetDotgitPath(self, subpath=None):
       """Return the full path to the .git dir.
@@ -3102,7 +3136,7 @@ class _Later(object):
 class _SyncColoring(Coloring):
 
   def __init__(self, config):
-    Coloring.__init__(self, config, 'reposync')
+    super().__init__(config, 'reposync')
     self.project = self.printer('header', attr='bold')
     self.info = self.printer('info')
     self.fail = self.printer('fail', fg='red')

repo

@@ -147,7 +147,7 @@ if not REPO_REV:
   REPO_REV = 'stable'
 
 # increment this whenever we make important changes to this script
-VERSION = (2, 12)
+VERSION = (2, 14)
 
 # increment this if the MAINTAINER_KEYS block is modified
 KEYRING_VERSION = (2, 3)
@@ -270,11 +270,18 @@ gpg_dir = os.path.join(home_dot_repo, 'gnupg')
 def GetParser(gitc_init=False):
   """Setup the CLI parser."""
   if gitc_init:
-    usage = 'repo gitc-init -u url -c client [options]'
+    usage = 'repo gitc-init -c client [options] [-u] url'
   else:
-    usage = 'repo init -u url [options]'
+    usage = 'repo init [options] [-u] url'
 
   parser = optparse.OptionParser(usage=usage)
+  InitParser(parser, gitc_init=gitc_init)
+  return parser
+
+
+def InitParser(parser, gitc_init=False):
+  """Setup the CLI parser."""
+  # NB: Keep in sync with command.py:_CommonOptions().
 
   # Logging.
   group = parser.add_option_group('Logging options')
@@ -289,10 +296,24 @@ def GetParser(gitc_init=False):
   group = parser.add_option_group('Manifest options')
   group.add_option('-u', '--manifest-url',
                    help='manifest repository location', metavar='URL')
-  group.add_option('-b', '--manifest-branch',
-                   help='manifest branch or revision', metavar='REVISION')
-  group.add_option('-m', '--manifest-name',
+  group.add_option('-b', '--manifest-branch', metavar='REVISION',
+                   help='manifest branch or revision (use HEAD for default)')
+  group.add_option('-m', '--manifest-name', default='default.xml',
                    help='initial manifest file', metavar='NAME.xml')
+  group.add_option('-g', '--groups', default='default',
+                   help='restrict manifest projects to ones with specified '
+                        'group(s) [default|all|G1,G2,G3|G4,-G5,-G6]',
+                   metavar='GROUP')
+  group.add_option('-p', '--platform', default='auto',
+                   help='restrict manifest projects to ones with a specified '
+                        'platform group [auto|all|none|linux|darwin|...]',
+                   metavar='PLATFORM')
+  group.add_option('--submodules', action='store_true',
+                   help='sync any submodules associated with the manifest repo')
+
+  # Options that only affect manifest project, and not any of the projects
+  # specified in the manifest itself.
+  group = parser.add_option_group('Manifest (only) checkout options')
   cbr_opts = ['--current-branch']
   # The gitc-init subcommand allocates -c itself, but a lot of init users
   # want -c, so try to satisfy both as best we can.
@@ -301,9 +322,23 @@ def GetParser(gitc_init=False):
   group.add_option(*cbr_opts,
                    dest='current_branch_only', action='store_true',
                    help='fetch only current manifest branch from server')
+  group.add_option('--no-tags',
+                   dest='tags', default=True, action='store_false',
+                   help="don't fetch tags in the manifest")
+
+  # These are fundamentally different ways of structuring the checkout.
+  group = parser.add_option_group('Checkout modes')
   group.add_option('--mirror', action='store_true',
                    help='create a replica of the remote repositories '
                         'rather than a client working directory')
+  group.add_option('--archive', action='store_true',
+                   help='checkout an archive instead of a git repository for '
+                        'each project. See git archive.')
+  group.add_option('--worktree', action='store_true',
+                   help='use git-worktree to manage projects')
+
+  # These are fundamentally different ways of structuring the checkout.
+  group = parser.add_option_group('Project checkout optimizations')
   group.add_option('--reference',
                    help='location of mirror directory', metavar='DIR')
   group.add_option('--dissociate', action='store_true',
@@ -314,32 +349,27 @@ def GetParser(gitc_init=False):
   group.add_option('--partial-clone', action='store_true',
                    help='perform partial clone (https://git-scm.com/'
                         'docs/gitrepository-layout#_code_partialclone_code)')
+  group.add_option('--no-partial-clone', action='store_false',
+                   help='disable use of partial clone (https://git-scm.com/'
+                        'docs/gitrepository-layout#_code_partialclone_code)')
+  group.add_option('--partial-clone-exclude', action='store',
+                   help='exclude the specified projects (a comma-delimited '
+                        'project names) from partial clone (https://git-scm.com'
+                        '/docs/gitrepository-layout#_code_partialclone_code)')
   group.add_option('--clone-filter', action='store', default='blob:none',
                    help='filter for use with --partial-clone '
                         '[default: %default]')
-  group.add_option('--worktree', action='store_true',
-                   help=optparse.SUPPRESS_HELP)
-  group.add_option('--archive', action='store_true',
-                   help='checkout an archive instead of a git repository for '
-                        'each project. See git archive.')
-  group.add_option('--submodules', action='store_true',
-                   help='sync any submodules associated with the manifest repo')
-  group.add_option('-g', '--groups', default='default',
-                   help='restrict manifest projects to ones with specified '
-                        'group(s) [default|all|G1,G2,G3|G4,-G5,-G6]',
-                   metavar='GROUP')
-  group.add_option('-p', '--platform', default='auto',
-                   help='restrict manifest projects to ones with a specified '
-                        'platform group [auto|all|none|linux|darwin|...]',
-                   metavar='PLATFORM')
+  group.add_option('--use-superproject', action='store_true', default=None,
+                   help='use the manifest superproject to sync projects')
+  group.add_option('--no-use-superproject', action='store_false',
+                   dest='use_superproject',
+                   help='disable use of manifest superprojects')
   group.add_option('--clone-bundle', action='store_true',
-                   help='enable use of /clone.bundle on HTTP/HTTPS (default if not --partial-clone)')
+                   help='enable use of /clone.bundle on HTTP/HTTPS '
+                        '(default if not --partial-clone)')
   group.add_option('--no-clone-bundle',
                    dest='clone_bundle', action='store_false',
                    help='disable use of /clone.bundle on HTTP/HTTPS (default if --partial-clone)')
-  group.add_option('--no-tags',
-                   dest='tags', default=True, action='store_false',
-                   help="don't fetch tags in the manifest")
 
   # Tool.
   group = parser.add_option_group('repo Version options')
@@ -516,8 +546,11 @@ def _Init(args, gitc_init=False):
   parser = GetParser(gitc_init=gitc_init)
   opt, args = parser.parse_args(args)
   if args:
-    parser.print_usage()
-    sys.exit(1)
+    if not opt.manifest_url:
+      opt.manifest_url = args.pop(0)
+    if args:
+      parser.print_usage()
+      sys.exit(1)
   opt.quiet = opt.output_mode is False
   opt.verbose = opt.output_mode is True
 

run_tests

@@ -34,8 +34,8 @@ def find_pytest():
   if ret:
     return ret
 
-  print(f'{__file__}: unable to find pytest.', file=sys.stderr)
-  print(f'{__file__}: Try installing: sudo apt-get install python-pytest',
+  print('%s: unable to find pytest.' % (__file__,), file=sys.stderr)
+  print('%s: Try installing: sudo apt-get install python-pytest' % (__file__,),
         file=sys.stderr)
 
 
@@ -50,7 +50,7 @@ def main(argv):
   os.environ['PYTHONPATH'] = pythonpath
 
   pytest = find_pytest()
-  return subprocess.run([pytest] + argv, check=True)
+  return subprocess.run([pytest] + argv, check=False).returncode
 
 
 if __name__ == '__main__':

setup.py

@@ -32,7 +32,7 @@ with open(os.path.join(TOPDIR, 'README.md')) as fp:
 # https://packaging.python.org/tutorials/packaging-projects/
 setuptools.setup(
     name='repo',
-    version='1.13.8',
+    version='2',
     maintainer='Various',
     maintainer_email='repo-discuss@googlegroups.com',
     description='Repo helps manage many Git repositories',
@@ -56,6 +56,6 @@ setuptools.setup(
         'Programming Language :: Python :: 3 :: Only',
         'Topic :: Software Development :: Version Control :: Git',
     ],
-    python_requires='>=3.6',
+    python_requires='>=3.5',
     packages=['subcmds'],
 )

subcmds/abandon.py

@@ -13,9 +13,11 @@
 # limitations under the License.
 
 from collections import defaultdict
+import functools
+import itertools
 import sys
 
-from command import Command
+from command import Command, DEFAULT_LOCAL_JOBS
 from git_command import git
 from progress import Progress
 
@@ -31,11 +33,9 @@ deleting it (and all its history) from your local repository.
 
 It is equivalent to "git branch -D <branchname>".
 """
+  PARALLEL_JOBS = DEFAULT_LOCAL_JOBS
 
   def _Options(self, p):
-    p.add_option('-q', '--quiet',
-                 action='store_true', default=False,
-                 help='be quiet')
     p.add_option('--all',
                  dest='all', action='store_true',
                  help='delete all branches in all projects')
@@ -51,35 +51,44 @@ It is equivalent to "git branch -D <branchname>".
     else:
       args.insert(0, "'All local branches'")
 
+  def _ExecuteOne(self, all_branches, nb, project):
+    """Abandon one project."""
+    if all_branches:
+      branches = project.GetBranches()
+    else:
+      branches = [nb]
+
+    ret = {}
+    for name in branches:
+      status = project.AbandonBranch(name)
+      if status is not None:
+        ret[name] = status
+    return (ret, project)
+
   def Execute(self, opt, args):
     nb = args[0]
     err = defaultdict(list)
     success = defaultdict(list)
     all_projects = self.GetProjects(args[1:])
 
-    pm = Progress('Abandon %s' % nb, len(all_projects))
-    for project in all_projects:
-      pm.update()
-
-      if opt.all:
-        branches = list(project.GetBranches().keys())
-      else:
-        branches = [nb]
-
-      for name in branches:
-        status = project.AbandonBranch(name)
-        if status is not None:
+    def _ProcessResults(_pool, pm, states):
+      for (results, project) in states:
+        for branch, status in results.items():
           if status:
-            success[name].append(project)
+            success[branch].append(project)
           else:
-            err[name].append(project)
-    pm.end()
+            err[branch].append(project)
+        pm.update()
 
-    width = 25
-    for name in branches:
-      if width < len(name):
-        width = len(name)
+    self.ExecuteInParallel(
+        opt.jobs,
+        functools.partial(self._ExecuteOne, opt.all, nb),
+        all_projects,
+        callback=_ProcessResults,
+        output=Progress('Abandon %s' % (nb,), len(all_projects), quiet=opt.quiet))
 
+    width = max(itertools.chain(
+        [25], (len(x) for x in itertools.chain(success, err))))
     if err:
       for br in err.keys():
         err_msg = "error: cannot abandon %s" % br

subcmds/branches.py

@@ -13,18 +13,10 @@
 # limitations under the License.
 
 import itertools
-import multiprocessing
 import sys
-from color import Coloring
-from command import Command
 
-# Number of projects to submit to a single worker process at a time.
-# This number represents a tradeoff between the overhead of IPC and finer
-# grained opportunity for parallelism. This particular value was chosen by
-# iterating through powers of two until the overall performance no longer
-# improved. The performance of this batch size is not a function of the
-# number of cores on the system.
-WORKER_BATCH_SIZE = 32
+from color import Coloring
+from command import Command, DEFAULT_LOCAL_JOBS
 
 
 class BranchColoring(Coloring):
@@ -103,32 +95,26 @@ the branch appears in, or does not appear in.  If no project list
 is shown, then the branch appears in all projects.
 
 """
-
-  def _Options(self, p):
-    """Add flags to CLI parser for this subcommand."""
-    default_jobs = min(multiprocessing.cpu_count(), 8)
-    p.add_option(
-        '-j',
-        '--jobs',
-        type=int,
-        default=default_jobs,
-        help='Number of worker processes to spawn '
-        '(default: %s)' % default_jobs)
+  PARALLEL_JOBS = DEFAULT_LOCAL_JOBS
 
   def Execute(self, opt, args):
     projects = self.GetProjects(args)
     out = BranchColoring(self.manifest.manifestProject.config)
     all_branches = {}
     project_cnt = len(projects)
-    with multiprocessing.Pool(processes=opt.jobs) as pool:
-      project_branches = pool.imap_unordered(
-          expand_project_to_branches, projects, chunksize=WORKER_BATCH_SIZE)
 
-      for name, b in itertools.chain.from_iterable(project_branches):
+    def _ProcessResults(_pool, _output, results):
+      for name, b in itertools.chain.from_iterable(results):
         if name not in all_branches:
           all_branches[name] = BranchInfo(name)
         all_branches[name].add(b)
 
+    self.ExecuteInParallel(
+        opt.jobs,
+        expand_project_to_branches,
+        projects,
+        callback=_ProcessResults)
+
     names = sorted(all_branches)
 
     if not names:

subcmds/checkout.py

@@ -12,8 +12,10 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import functools
 import sys
-from command import Command
+
+from command import Command, DEFAULT_LOCAL_JOBS
 from progress import Progress
 
 
@@ -31,28 +33,37 @@ The command is equivalent to:
 
   repo forall [<project>...] -c git checkout <branchname>
 """
+  PARALLEL_JOBS = DEFAULT_LOCAL_JOBS
 
   def ValidateOptions(self, opt, args):
     if not args:
       self.Usage()
 
+  def _ExecuteOne(self, nb, project):
+    """Checkout one project."""
+    return (project.CheckoutBranch(nb), project)
+
   def Execute(self, opt, args):
     nb = args[0]
     err = []
     success = []
     all_projects = self.GetProjects(args[1:])
 
-    pm = Progress('Checkout %s' % nb, len(all_projects))
-    for project in all_projects:
-      pm.update()
+    def _ProcessResults(_pool, pm, results):
+      for status, project in results:
+        if status is not None:
+          if status:
+            success.append(project)
+          else:
+            err.append(project)
+        pm.update()
 
-      status = project.CheckoutBranch(nb)
-      if status is not None:
-        if status:
-          success.append(project)
-        else:
-          err.append(project)
-    pm.end()
+    self.ExecuteInParallel(
+        opt.jobs,
+        functools.partial(self._ExecuteOne, nb),
+        all_projects,
+        callback=_ProcessResults,
+        output=Progress('Checkout %s' % (nb,), len(all_projects), quiet=opt.quiet))
 
     if err:
       for p in err:

subcmds/cherry_pick.py

@@ -32,9 +32,6 @@ The change id will be updated, and a reference to the old
 change id will be added.
 """
 
-  def _Options(self, p):
-    pass
-
   def ValidateOptions(self, opt, args):
     if len(args) != 1:
       self.Usage()
@@ -72,11 +69,9 @@ change id will be added.
       new_msg = self._Reformat(old_msg, sha1)
 
       p = GitCommand(None, ['commit', '--amend', '-F', '-'],
-                     provide_stdin=True,
+                     input=new_msg,
                      capture_stdout=True,
                      capture_stderr=True)
-      p.stdin.write(new_msg)
-      p.stdin.close()
       if p.Wait() != 0:
         print("error: Failed to update commit message", file=sys.stderr)
         sys.exit(1)

subcmds/diff.py

@@ -12,7 +12,10 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from command import PagedCommand
+import functools
+import io
+
+from command import DEFAULT_LOCAL_JOBS, PagedCommand
 
 
 class Diff(PagedCommand):
@@ -25,15 +28,42 @@ The -u option causes '%prog' to generate diff output with file paths
 relative to the repository root, so the output can be applied
 to the Unix 'patch' command.
 """
+  PARALLEL_JOBS = DEFAULT_LOCAL_JOBS
 
   def _Options(self, p):
     p.add_option('-u', '--absolute',
                  dest='absolute', action='store_true',
                  help='Paths are relative to the repository root')
 
+  def _ExecuteOne(self, absolute, project):
+    """Obtains the diff for a specific project.
+
+    Args:
+      absolute: Paths are relative to the root.
+      project: Project to get status of.
+
+    Returns:
+      The status of the project.
+    """
+    buf = io.StringIO()
+    ret = project.PrintWorkTreeDiff(absolute, output_redir=buf)
+    return (ret, buf.getvalue())
+
   def Execute(self, opt, args):
-    ret = 0
-    for project in self.GetProjects(args):
-      if not project.PrintWorkTreeDiff(opt.absolute):
-        ret = 1
-    return ret
+    all_projects = self.GetProjects(args)
+
+    def _ProcessResults(_pool, _output, results):
+      ret = 0
+      for (state, output) in results:
+        if output:
+          print(output, end='')
+        if not state:
+          ret = 1
+      return ret
+
+    return self.ExecuteInParallel(
+        opt.jobs,
+        functools.partial(self._ExecuteOne, opt.absolute),
+        all_projects,
+        callback=_ProcessResults,
+        ordered=True)

subcmds/diffmanifests.py

@@ -191,12 +191,12 @@ synced and their revisions won't be found.
     else:
       self.printProject = self.printAdded = self.printRemoved = self.printRevision = self.printText
 
-    manifest1 = RepoClient(self.manifest.repodir)
+    manifest1 = RepoClient(self.repodir)
     manifest1.Override(args[0], load_local_manifests=False)
     if len(args) == 1:
       manifest2 = self.manifest
     else:
-      manifest2 = RepoClient(self.manifest.repodir)
+      manifest2 = RepoClient(self.repodir)
       manifest2.Override(args[1], load_local_manifests=False)
 
     diff = manifest1.projectsDiff(manifest2)

subcmds/download.py

@@ -16,7 +16,7 @@ import re
 import sys
 
 from command import Command
-from error import GitError
+from error import GitError, NoSuchProjectError
 
 CHANGE_RE = re.compile(r'^([1-9][0-9]*)(?:[/\.-]([1-9][0-9]*))?$')
 
@@ -60,6 +60,7 @@ If no project is specified try to use current directory as a project.
       if m:
         if not project:
           project = self.GetProjects(".")[0]
+          print('Defaulting to cwd project', project.name)
         chg_id = int(m.group(1))
         if m.group(2):
           ps_id = int(m.group(2))
@@ -76,7 +77,23 @@ If no project is specified try to use current directory as a project.
                 ps_id = max(int(match.group(1)), ps_id)
         to_get.append((project, chg_id, ps_id))
       else:
-        project = self.GetProjects([a])[0]
+        projects = self.GetProjects([a])
+        if len(projects) > 1:
+          # If the cwd is one of the projects, assume they want that.
+          try:
+            project = self.GetProjects('.')[0]
+          except NoSuchProjectError:
+            project = None
+          if project not in projects:
+            print('error: %s matches too many projects; please re-run inside '
+                  'the project checkout.' % (a,), file=sys.stderr)
+            for project in projects:
+              print('  %s/ @ %s' % (project.relpath, project.revisionExpr),
+                    file=sys.stderr)
+            sys.exit(1)
+        else:
+          project = projects[0]
+          print('Defaulting to cwd project', project.name)
     return to_get
 
   def ValidateOptions(self, opt, args):

subcmds/forall.py

@@ -13,6 +13,8 @@
 # limitations under the License.
 
 import errno
+import functools
+import io
 import multiprocessing
 import re
 import os
@@ -21,8 +23,8 @@ import sys
 import subprocess
 
 from color import Coloring
-from command import Command, MirrorSafeCommand
-import platform_utils
+from command import DEFAULT_LOCAL_JOBS, Command, MirrorSafeCommand, WORKER_BATCH_SIZE
+from error import ManifestInvalidRevisionError
 
 _CAN_COLOR = [
     'branch',
@@ -43,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.
@@ -51,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,
@@ -113,12 +120,15 @@ terminal and are not redirected.
 If -e is used, when a command exits unsuccessfully, '%prog' will abort
 without iterating through the remaining projects.
 """
+  PARALLEL_JOBS = DEFAULT_LOCAL_JOBS
+
+  @staticmethod
+  def _cmd_option(option, _opt_str, _value, parser):
+    setattr(parser.values, option.dest, list(parser.rargs))
+    while parser.rargs:
+      del parser.rargs[0]
 
   def _Options(self, p):
-    def cmd(option, opt_str, value, parser):
-      setattr(parser.values, option.dest, list(parser.rargs))
-      while parser.rargs:
-        del parser.rargs[0]
     p.add_option('-r', '--regex',
                  dest='regex', action='store_true',
                  help="Execute the command only on projects matching regex or wildcard expression")
@@ -133,7 +143,7 @@ without iterating through the remaining projects.
                  help='Command (and arguments) to execute',
                  dest='command',
                  action='callback',
-                 callback=cmd)
+                 callback=self._cmd_option)
     p.add_option('-e', '--abort-on-errors',
                  dest='abort_on_errors', action='store_true',
                  help='Abort if a command exits unsuccessfully')
@@ -141,45 +151,17 @@ without iterating through the remaining projects.
                  help='Silently skip & do not exit non-zero due missing '
                       'checkouts')
 
-    g = p.add_option_group('Output')
+    g = p.get_option_group('--quiet')
     g.add_option('-p',
                  dest='project_header', action='store_true',
                  help='Show project headers before output')
-    g.add_option('-v', '--verbose',
-                 dest='verbose', action='store_true',
-                 help='Show command error messages')
-    g.add_option('-j', '--jobs',
-                 dest='jobs', action='store', type='int', default=1,
-                 help='number of commands to execute simultaneously')
+    p.add_option('--interactive',
+                 action='store_true',
+                 help='force interactive usage')
 
   def WantPager(self, opt):
     return opt.project_header and opt.jobs == 1
 
-  def _SerializeProject(self, project):
-    """ Serialize a project._GitGetByExec instance.
-
-    project._GitGetByExec is not pickle-able. Instead of trying to pass it
-    around between processes, make a dict ourselves containing only the
-    attributes that we need.
-
-    """
-    if not self.manifest.IsMirror:
-      lrev = project.GetRevisionId()
-    else:
-      lrev = None
-    return {
-        'name': project.name,
-        'relpath': project.relpath,
-        'remote_name': project.remote.name,
-        'lrev': lrev,
-        'rrev': project.revisionExpr,
-        'annotations': dict((a.name, a.value) for a in project.annotations),
-        'gitdir': project.gitdir,
-        'worktree': project.worktree,
-        'upstream': project.upstream,
-        'dest_branch': project.dest_branch,
-    }
-
   def ValidateOptions(self, opt, args):
     if not opt.command:
       self.Usage()
@@ -195,6 +177,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':
@@ -234,60 +221,50 @@ without iterating through the remaining projects.
 
     os.environ['REPO_COUNT'] = str(len(projects))
 
-    pool = multiprocessing.Pool(opt.jobs, InitWorker)
     try:
       config = self.manifest.manifestProject.config
-      results_it = pool.imap(
-          DoWorkWrapper,
-          self.ProjectArgs(projects, mirror, opt, cmd, shell, config))
-      pool.close()
-      for r in results_it:
-        rc = rc or r
-        if r != 0 and opt.abort_on_errors:
-          raise Exception('Aborting due to previous error')
+      with multiprocessing.Pool(opt.jobs, InitWorker) as pool:
+        results_it = pool.imap(
+            functools.partial(DoWorkWrapper, mirror, opt, cmd, shell, config),
+            enumerate(projects),
+            chunksize=WORKER_BATCH_SIZE)
+        first = True
+        for (r, output) in results_it:
+          if output:
+            if first:
+              first = False
+            elif opt.project_header:
+              print()
+            # To simplify the DoWorkWrapper, take care of automatic newlines.
+            end = '\n'
+            if output[-1] == '\n':
+              end = ''
+            print(output, end=end)
+          rc = rc or r
+          if r != 0 and opt.abort_on_errors:
+            raise Exception('Aborting due to previous error')
     except (KeyboardInterrupt, WorkerKeyboardInterrupt):
       # Catch KeyboardInterrupt raised inside and outside of workers
-      print('Interrupted - terminating the pool')
-      pool.terminate()
       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)
-      pool.terminate()
       rc = rc or getattr(e, 'errno', 1)
-    finally:
-      pool.join()
     if rc != 0:
       sys.exit(rc)
 
-  def ProjectArgs(self, projects, mirror, opt, cmd, shell, config):
-    for cnt, p in enumerate(projects):
-      try:
-        project = self._SerializeProject(p)
-      except Exception as e:
-        print('Project list error on project %s: %s: %s' %
-              (p.name, type(e).__name__, e),
-              file=sys.stderr)
-        return
-      except KeyboardInterrupt:
-        print('Project list interrupted',
-              file=sys.stderr)
-        return
-      yield [mirror, opt, cmd, shell, cnt, config, project]
-
 
 class WorkerKeyboardInterrupt(Exception):
   """ Keyboard interrupt exception for worker processes. """
-  pass
 
 
 def InitWorker():
   signal.signal(signal.SIGINT, signal.SIG_IGN)
 
 
-def DoWorkWrapper(args):
+def DoWorkWrapper(mirror, opt, cmd, shell, config, args):
   """ A wrapper around the DoWork() method.
 
   Catch the KeyboardInterrupt exceptions here and re-raise them as a different,
@@ -295,11 +272,11 @@ def DoWorkWrapper(args):
   and making the parent hang indefinitely.
 
   """
-  project = args.pop()
+  cnt, project = args
   try:
-    return DoWork(project, *args)
+    return DoWork(project, mirror, opt, cmd, shell, cnt, config)
   except KeyboardInterrupt:
-    print('%s: Worker interrupted' % project['name'])
+    print('%s: Worker interrupted' % project.name)
     raise WorkerKeyboardInterrupt()
 
 
@@ -311,94 +288,65 @@ def DoWork(project, mirror, opt, cmd, shell, cnt, config):
       val = ''
     env[name] = val
 
-  setenv('REPO_PROJECT', project['name'])
-  setenv('REPO_PATH', project['relpath'])
-  setenv('REPO_REMOTE', project['remote_name'])
-  setenv('REPO_LREV', project['lrev'])
-  setenv('REPO_RREV', project['rrev'])
-  setenv('REPO_UPSTREAM', project['upstream'])
-  setenv('REPO_DEST_BRANCH', project['dest_branch'])
+  setenv('REPO_PROJECT', project.name)
+  setenv('REPO_PATH', project.relpath)
+  setenv('REPO_REMOTE', project.remote.name)
+  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)
   setenv('REPO_I', str(cnt + 1))
-  for name in project['annotations']:
-    setenv("REPO__%s" % (name), project['annotations'][name])
+  for annotation in project.annotations:
+    setenv("REPO__%s" % (annotation.name), annotation.value)
 
   if mirror:
-    setenv('GIT_DIR', project['gitdir'])
-    cwd = project['gitdir']
+    setenv('GIT_DIR', project.gitdir)
+    cwd = project.gitdir
   else:
-    cwd = project['worktree']
+    cwd = project.worktree
 
   if not os.path.exists(cwd):
     # Allow the user to silently ignore missing checkouts so they can run on
     # partial checkouts (good for infra recovery tools).
     if opt.ignore_missing:
-      return 0
+      return (0, '')
+
+    output = ''
     if ((opt.project_header and opt.verbose)
             or not opt.project_header):
-      print('skipping %s/' % project['relpath'], file=sys.stderr)
-    return 1
+      output = 'skipping %s/' % project.relpath
+    return (1, output)
 
-  if opt.project_header:
-    stdin = subprocess.PIPE
-    stdout = subprocess.PIPE
-    stderr = subprocess.PIPE
+  if opt.verbose:
+    stderr = subprocess.STDOUT
   else:
-    stdin = None
-    stdout = None
-    stderr = None
+    stderr = subprocess.DEVNULL
 
-  p = subprocess.Popen(cmd,
-                       cwd=cwd,
-                       shell=shell,
-                       env=env,
-                       stdin=stdin,
-                       stdout=stdout,
-                       stderr=stderr)
+  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=stdin, stdout=subprocess.PIPE, stderr=stderr)
+
+  output = result.stdout
   if opt.project_header:
-    out = ForallColoring(config)
-    out.redirect(sys.stdout)
-    empty = True
-    errbuf = ''
-
-    p.stdin.close()
-    s_in = platform_utils.FileDescriptorStreams.create()
-    s_in.add(p.stdout, sys.stdout, 'stdout')
-    s_in.add(p.stderr, sys.stderr, 'stderr')
-
-    while not s_in.is_done:
-      in_ready = s_in.select()
-      for s in in_ready:
-        buf = s.read().decode()
-        if not buf:
-          s_in.remove(s)
-          s.close()
-          continue
-
-        if not opt.verbose:
-          if s.std_name == 'stderr':
-            errbuf += buf
-            continue
-
-        if empty and out:
-          if not cnt == 0:
-            out.nl()
-
-          if mirror:
-            project_header_path = project['name']
-          else:
-            project_header_path = project['relpath']
-          out.project('project %s/', project_header_path)
-          out.nl()
-          out.flush()
-          if errbuf:
-            sys.stderr.write(errbuf)
-            sys.stderr.flush()
-            errbuf = ''
-          empty = False
-
-        s.dest.write(buf)
-        s.dest.flush()
-
-  r = p.wait()
-  return r
+    if output:
+      buf = io.StringIO()
+      out = ForallColoring(config)
+      out.redirect(buf)
+      if mirror:
+        project_header_path = project.name
+      else:
+        project_header_path = project.relpath
+      out.project('project %s/' % project_header_path)
+      out.nl()
+      buf.write(output)
+      output = buf.getvalue()
+  return (result.returncode, output)

subcmds/gitc_init.py

@@ -47,14 +47,7 @@ use for this GITC client.
 """
 
   def _Options(self, p):
-    super(GitcInit, self)._Options(p, gitc_init=True)
-    g = p.add_option_group('GITC options')
-    g.add_option('-f', '--manifest-file',
-                 dest='manifest_file',
-                 help='Optional manifest file to use for this GITC client.')
-    g.add_option('-c', '--gitc-client',
-                 dest='gitc_client',
-                 help='The name of the gitc_client instance to create or modify.')
+    super()._Options(p, gitc_init=True)
 
   def Execute(self, opt, args):
     gitc_client = gitc_utils.parse_clientdir(os.getcwd())
@@ -64,7 +57,7 @@ use for this GITC client.
       sys.exit(1)
     self.client_dir = os.path.join(gitc_utils.get_gitc_manifest_dir(),
                                    gitc_client)
-    super(GitcInit, self).Execute(opt, args)
+    super().Execute(opt, args)
 
     manifest_file = self.manifest.manifestFile
     if opt.manifest_file:

subcmds/grep.py

@@ -12,10 +12,11 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import functools
 import sys
 
 from color import Coloring
-from command import PagedCommand
+from command import DEFAULT_LOCAL_JOBS, PagedCommand
 from error import GitError
 from git_command import GitCommand
 
@@ -61,30 +62,33 @@ contain a line that matches both expressions:
   repo grep --all-match -e NODE -e Unexpected
 
 """
+  PARALLEL_JOBS = DEFAULT_LOCAL_JOBS
+
+  @staticmethod
+  def _carry_option(_option, opt_str, value, parser):
+    pt = getattr(parser.values, 'cmd_argv', None)
+    if pt is None:
+      pt = []
+      setattr(parser.values, 'cmd_argv', pt)
+
+    if opt_str == '-(':
+      pt.append('(')
+    elif opt_str == '-)':
+      pt.append(')')
+    else:
+      pt.append(opt_str)
+
+    if value is not None:
+      pt.append(value)
+
+  def _CommonOptions(self, p):
+    """Override common options slightly."""
+    super()._CommonOptions(p, opt_v=False)
 
   def _Options(self, p):
-    def carry(option,
-              opt_str,
-              value,
-              parser):
-      pt = getattr(parser.values, 'cmd_argv', None)
-      if pt is None:
-        pt = []
-        setattr(parser.values, 'cmd_argv', pt)
-
-      if opt_str == '-(':
-        pt.append('(')
-      elif opt_str == '-)':
-        pt.append(')')
-      else:
-        pt.append(opt_str)
-
-      if value is not None:
-        pt.append(value)
-
     g = p.add_option_group('Sources')
     g.add_option('--cached',
-                 action='callback', callback=carry,
+                 action='callback', callback=self._carry_option,
                  help='Search the index, instead of the work tree')
     g.add_option('-r', '--revision',
                  dest='revision', action='append', metavar='TREEish',
@@ -92,68 +96,134 @@ contain a line that matches both expressions:
 
     g = p.add_option_group('Pattern')
     g.add_option('-e',
-                 action='callback', callback=carry,
+                 action='callback', callback=self._carry_option,
                  metavar='PATTERN', type='str',
                  help='Pattern to search for')
     g.add_option('-i', '--ignore-case',
-                 action='callback', callback=carry,
+                 action='callback', callback=self._carry_option,
                  help='Ignore case differences')
     g.add_option('-a', '--text',
-                 action='callback', callback=carry,
+                 action='callback', callback=self._carry_option,
                  help="Process binary files as if they were text")
     g.add_option('-I',
-                 action='callback', callback=carry,
+                 action='callback', callback=self._carry_option,
                  help="Don't match the pattern in binary files")
     g.add_option('-w', '--word-regexp',
-                 action='callback', callback=carry,
+                 action='callback', callback=self._carry_option,
                  help='Match the pattern only at word boundaries')
     g.add_option('-v', '--invert-match',
-                 action='callback', callback=carry,
+                 action='callback', callback=self._carry_option,
                  help='Select non-matching lines')
     g.add_option('-G', '--basic-regexp',
-                 action='callback', callback=carry,
+                 action='callback', callback=self._carry_option,
                  help='Use POSIX basic regexp for patterns (default)')
     g.add_option('-E', '--extended-regexp',
-                 action='callback', callback=carry,
+                 action='callback', callback=self._carry_option,
                  help='Use POSIX extended regexp for patterns')
     g.add_option('-F', '--fixed-strings',
-                 action='callback', callback=carry,
+                 action='callback', callback=self._carry_option,
                  help='Use fixed strings (not regexp) for pattern')
 
     g = p.add_option_group('Pattern Grouping')
     g.add_option('--all-match',
-                 action='callback', callback=carry,
+                 action='callback', callback=self._carry_option,
                  help='Limit match to lines that have all patterns')
     g.add_option('--and', '--or', '--not',
-                 action='callback', callback=carry,
+                 action='callback', callback=self._carry_option,
                  help='Boolean operators to combine patterns')
     g.add_option('-(', '-)',
-                 action='callback', callback=carry,
+                 action='callback', callback=self._carry_option,
                  help='Boolean operator grouping')
 
     g = p.add_option_group('Output')
     g.add_option('-n',
-                 action='callback', callback=carry,
+                 action='callback', callback=self._carry_option,
                  help='Prefix the line number to matching lines')
     g.add_option('-C',
-                 action='callback', callback=carry,
+                 action='callback', callback=self._carry_option,
                  metavar='CONTEXT', type='str',
                  help='Show CONTEXT lines around match')
     g.add_option('-B',
-                 action='callback', callback=carry,
+                 action='callback', callback=self._carry_option,
                  metavar='CONTEXT', type='str',
                  help='Show CONTEXT lines before match')
     g.add_option('-A',
-                 action='callback', callback=carry,
+                 action='callback', callback=self._carry_option,
                  metavar='CONTEXT', type='str',
                  help='Show CONTEXT lines after match')
     g.add_option('-l', '--name-only', '--files-with-matches',
-                 action='callback', callback=carry,
+                 action='callback', callback=self._carry_option,
                  help='Show only file names containing matching lines')
     g.add_option('-L', '--files-without-match',
-                 action='callback', callback=carry,
+                 action='callback', callback=self._carry_option,
                  help='Show only file names not containing matching lines')
 
+  def _ExecuteOne(self, cmd_argv, project):
+    """Process one project."""
+    try:
+      p = GitCommand(project,
+                     cmd_argv,
+                     bare=False,
+                     capture_stdout=True,
+                     capture_stderr=True)
+    except GitError as e:
+      return (project, -1, None, str(e))
+
+    return (project, p.Wait(), p.stdout, p.stderr)
+
+  @staticmethod
+  def _ProcessResults(full_name, have_rev, _pool, out, results):
+    git_failed = False
+    bad_rev = False
+    have_match = False
+
+    for project, rc, stdout, stderr in results:
+      if rc < 0:
+        git_failed = True
+        out.project('--- project %s ---' % project.relpath)
+        out.nl()
+        out.fail('%s', stderr)
+        out.nl()
+        continue
+
+      if rc:
+        # no results
+        if stderr:
+          if have_rev and 'fatal: ambiguous argument' in stderr:
+            bad_rev = True
+          else:
+            out.project('--- project %s ---' % project.relpath)
+            out.nl()
+            out.fail('%s', stderr.strip())
+            out.nl()
+        continue
+      have_match = True
+
+      # We cut the last element, to avoid a blank line.
+      r = stdout.split('\n')
+      r = r[0:-1]
+
+      if have_rev and full_name:
+        for line in r:
+          rev, line = line.split(':', 1)
+          out.write("%s", rev)
+          out.write(':')
+          out.project(project.relpath)
+          out.write('/')
+          out.write("%s", line)
+          out.nl()
+      elif full_name:
+        for line in r:
+          out.project(project.relpath)
+          out.write('/')
+          out.write("%s", line)
+          out.nl()
+      else:
+        for line in r:
+          print(line)
+
+    return (git_failed, bad_rev, have_match)
+
   def Execute(self, opt, args):
     out = GrepColoring(self.manifest.manifestProject.config)
 
@@ -185,62 +255,13 @@ contain a line that matches both expressions:
       cmd_argv.extend(opt.revision)
     cmd_argv.append('--')
 
-    git_failed = False
-    bad_rev = False
-    have_match = False
-
-    for project in projects:
-      try:
-        p = GitCommand(project,
-                       cmd_argv,
-                       bare=False,
-                       capture_stdout=True,
-                       capture_stderr=True)
-      except GitError as e:
-        git_failed = True
-        out.project('--- project %s ---' % project.relpath)
-        out.nl()
-        out.fail('%s', str(e))
-        out.nl()
-        continue
-
-      if p.Wait() != 0:
-        # no results
-        #
-        if p.stderr:
-          if have_rev and 'fatal: ambiguous argument' in p.stderr:
-            bad_rev = True
-          else:
-            out.project('--- project %s ---' % project.relpath)
-            out.nl()
-            out.fail('%s', p.stderr.strip())
-            out.nl()
-        continue
-      have_match = True
-
-      # We cut the last element, to avoid a blank line.
-      #
-      r = p.stdout.split('\n')
-      r = r[0:-1]
-
-      if have_rev and full_name:
-        for line in r:
-          rev, line = line.split(':', 1)
-          out.write("%s", rev)
-          out.write(':')
-          out.project(project.relpath)
-          out.write('/')
-          out.write("%s", line)
-          out.nl()
-      elif full_name:
-        for line in r:
-          out.project(project.relpath)
-          out.write('/')
-          out.write("%s", line)
-          out.nl()
-      else:
-        for line in r:
-          print(line)
+    git_failed, bad_rev, have_match = self.ExecuteInParallel(
+        opt.jobs,
+        functools.partial(self._ExecuteOne, cmd_argv),
+        projects,
+        callback=functools.partial(self._ProcessResults, full_name, have_rev),
+        output=out,
+        ordered=True)
 
     if git_failed:
       sys.exit(1)

subcmds/help.py

@@ -14,7 +14,7 @@
 
 import re
 import sys
-from formatter import AbstractFormatter, DumbWriter
+import textwrap
 
 from subcmds import all_commands
 from color import Coloring
@@ -84,8 +84,7 @@ Displays detailed usage information about a command.
       def __init__(self, gc):
         Coloring.__init__(self, gc, 'help')
         self.heading = self.printer('heading', attr='bold')
-
-        self.wrap = AbstractFormatter(DumbWriter())
+        self._first = True
 
       def _PrintSection(self, heading, bodyAttr):
         try:
@@ -95,7 +94,9 @@ Displays detailed usage information about a command.
         if body == '' or body is None:
           return
 
-        self.nl()
+        if not self._first:
+          self.nl()
+        self._first = False
 
         self.heading('%s%s', header_prefix, heading)
         self.nl()
@@ -105,7 +106,8 @@ Displays detailed usage information about a command.
         body = body.strip()
         body = body.replace('%prog', me)
 
-        asciidoc_hdr = re.compile(r'^\n?#+ (.+)$')
+        # Extract the title, but skip any trailing {#anchors}.
+        asciidoc_hdr = re.compile(r'^\n?#+ ([^{]+)(\{#.+\})?$')
         for para in body.split("\n\n"):
           if para.startswith(' '):
             self.write('%s', para)
@@ -120,9 +122,12 @@ Displays detailed usage information about a command.
             self.nl()
             continue
 
-          self.wrap.add_flowing_data(para)
-          self.wrap.end_paragraph(1)
-        self.wrap.end_paragraph(0)
+          lines = textwrap.wrap(para.replace('  ', ' '), width=80,
+                                break_long_words=False, break_on_hyphens=False)
+          for line in lines:
+            self.write('%s', line)
+            self.nl()
+          self.nl()
 
     out = _Out(self.client.globalConfig)
     out._PrintSection('Summary', 'helpSummary')

subcmds/init.py

@@ -25,15 +25,16 @@ from error import ManifestParseError
 from project import SyncBuffer
 from git_config import GitConfig
 from git_command import git_require, MIN_GIT_VERSION_SOFT, MIN_GIT_VERSION_HARD
+import git_superproject
 import platform_utils
 from wrapper import Wrapper
 
 
 class Init(InteractiveCommand, MirrorSafeCommand):
   common = True
-  helpSummary = "Initialize repo in the current directory"
+  helpSummary = "Initialize a repo client checkout in the current directory"
   helpUsage = """
-%prog [options]
+%prog [options] [manifest url]
 """
   helpDescription = """
 The '%prog' command is run once to install and initialize repo.
@@ -41,9 +42,13 @@ The latest repo source code and manifest collection is downloaded
 from the server and is installed in the .repo/ directory in the
 current working directory.
 
+When creating a new checkout, the manifest URL is the only required setting.
+It may be specified using the --manifest-url option, or as the first optional
+argument.
+
 The optional -b argument can be used to select the manifest branch
 to checkout and use.  If no branch is specified, the remote's default
-branch is used.
+branch is used.  This is equivalent to using -b HEAD.
 
 The optional -m argument can be used to specify an alternate manifest
 to be used. If no manifest is specified, the manifest default.xml
@@ -74,115 +79,36 @@ manifest, a subsequent `repo sync` (or `repo sync -d`) is necessary
 to update the working directory files.
 """
 
+  def _CommonOptions(self, p):
+    """Disable due to re-use of Wrapper()."""
+
   def _Options(self, p, gitc_init=False):
-    # Logging
-    g = p.add_option_group('Logging options')
-    g.add_option('-v', '--verbose',
-                 dest='output_mode', action='store_true',
-                 help='show all output')
-    g.add_option('-q', '--quiet',
-                 dest='output_mode', action='store_false',
-                 help='only show errors')
-
-    # Manifest
-    g = p.add_option_group('Manifest options')
-    g.add_option('-u', '--manifest-url',
-                 dest='manifest_url',
-                 help='manifest repository location', metavar='URL')
-    g.add_option('-b', '--manifest-branch',
-                 dest='manifest_branch',
-                 help='manifest branch or revision', metavar='REVISION')
-    cbr_opts = ['--current-branch']
-    # The gitc-init subcommand allocates -c itself, but a lot of init users
-    # want -c, so try to satisfy both as best we can.
-    if not gitc_init:
-      cbr_opts += ['-c']
-    g.add_option(*cbr_opts,
-                 dest='current_branch_only', action='store_true',
-                 help='fetch only current manifest branch from server')
-    g.add_option('-m', '--manifest-name',
-                 dest='manifest_name', default='default.xml',
-                 help='initial manifest file', metavar='NAME.xml')
-    g.add_option('--mirror',
-                 dest='mirror', action='store_true',
-                 help='create a replica of the remote repositories '
-                      'rather than a client working directory')
-    g.add_option('--reference',
-                 dest='reference',
-                 help='location of mirror directory', metavar='DIR')
-    g.add_option('--dissociate',
-                 dest='dissociate', action='store_true',
-                 help='dissociate from reference mirrors after clone')
-    g.add_option('--depth', type='int', default=None,
-                 dest='depth',
-                 help='create a shallow clone with given depth; see git clone')
-    g.add_option('--partial-clone', action='store_true',
-                 dest='partial_clone',
-                 help='perform partial clone (https://git-scm.com/'
-                 'docs/gitrepository-layout#_code_partialclone_code)')
-    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)
-    g.add_option('--archive',
-                 dest='archive', action='store_true',
-                 help='checkout an archive instead of a git repository for '
-                      'each project. See git archive.')
-    g.add_option('--submodules',
-                 dest='submodules', action='store_true',
-                 help='sync any submodules associated with the manifest repo')
-    g.add_option('-g', '--groups',
-                 dest='groups', default='default',
-                 help='restrict manifest projects to ones with specified '
-                      'group(s) [default|all|G1,G2,G3|G4,-G5,-G6]',
-                 metavar='GROUP')
-    g.add_option('-p', '--platform',
-                 dest='platform', default='auto',
-                 help='restrict manifest projects to ones with a specified '
-                      'platform group [auto|all|none|linux|darwin|...]',
-                 metavar='PLATFORM')
-    g.add_option('--clone-bundle', action='store_true',
-                 help='force use of /clone.bundle on HTTP/HTTPS (default if not --partial-clone)')
-    g.add_option('--no-clone-bundle',
-                 dest='clone_bundle', action='store_false',
-                 help='disable use of /clone.bundle on HTTP/HTTPS (default if --partial-clone)')
-    g.add_option('--no-tags',
-                 dest='tags', default=True, action='store_false',
-                 help="don't fetch tags in the manifest")
-
-    # Tool
-    g = p.add_option_group('repo Version options')
-    g.add_option('--repo-url',
-                 dest='repo_url',
-                 help='repo repository location', metavar='URL')
-    g.add_option('--repo-rev', metavar='REV',
-                 help='repo branch or revision')
-    g.add_option('--repo-branch', dest='repo_rev',
-                 help=optparse.SUPPRESS_HELP)
-    g.add_option('--no-repo-verify',
-                 dest='repo_verify', default=True, action='store_false',
-                 help='do not verify repo source code')
-
-    # Other
-    g = p.add_option_group('Other options')
-    g.add_option('--config-name',
-                 dest='config_name', action="store_true", default=False,
-                 help='Always prompt for name/e-mail')
+    Wrapper().InitParser(p, gitc_init=gitc_init)
 
   def _RegisteredEnvironmentOptions(self):
     return {'REPO_MANIFEST_URL': 'manifest_url',
             'REPO_MIRROR_LOCATION': 'reference'}
 
+  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,
+                                                 quiet=opt.quiet)
+    if not superproject.Sync():
+      print('error: git update of superproject failed', file=sys.stderr)
+      sys.exit(1)
+
   def _SyncManifest(self, opt):
     m = self.manifest.manifestProject
     is_new = not m.Exists
 
     if is_new:
       if not opt.manifest_url:
-        print('fatal: manifest url (-u) is required.', file=sys.stderr)
+        print('fatal: manifest url is required.', file=sys.stderr)
         sys.exit(1)
 
       if not opt.quiet:
@@ -214,6 +140,11 @@ to update the working directory files.
       r.Save()
 
     if opt.manifest_branch:
+      if opt.manifest_branch == 'HEAD':
+        opt.manifest_branch = m.ResolveRemoteHead()
+        if opt.manifest_branch is None:
+          print('fatal: unable to resolve HEAD', file=sys.stderr)
+          sys.exit(1)
       m.revisionExpr = opt.manifest_branch
     else:
       if is_new:
@@ -242,7 +173,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)
 
@@ -250,7 +181,7 @@ to update the working directory files.
       m.config.SetString('repo.reference', opt.reference)
 
     if opt.dissociate:
-      m.config.SetString('repo.dissociate', 'true')
+      m.config.SetBoolean('repo.dissociate', opt.dissociate)
 
     if opt.worktree:
       if opt.mirror:
@@ -261,14 +192,14 @@ to update the working directory files.
         print('fatal: --submodules and --worktree are incompatible',
               file=sys.stderr)
         sys.exit(1)
-      m.config.SetString('repo.worktree', 'true')
+      m.config.SetBoolean('repo.worktree', opt.worktree)
       if is_new:
         m.use_git_worktrees = True
       print('warning: --worktree is experimental!', file=sys.stderr)
 
     if opt.archive:
       if is_new:
-        m.config.SetString('repo.archive', 'true')
+        m.config.SetBoolean('repo.archive', opt.archive)
       else:
         print('fatal: --archive is only supported when initializing a new '
               'workspace.', file=sys.stderr)
@@ -278,7 +209,7 @@ to update the working directory files.
 
     if opt.mirror:
       if is_new:
-        m.config.SetString('repo.mirror', 'true')
+        m.config.SetBoolean('repo.mirror', opt.mirror)
       else:
         print('fatal: --mirror is only supported when initializing a new '
               'workspace.', file=sys.stderr)
@@ -286,30 +217,39 @@ to update the working directory files.
               'in another location.', file=sys.stderr)
         sys.exit(1)
 
-    if opt.partial_clone:
+    if opt.partial_clone is not None:
       if opt.mirror:
         print('fatal: --mirror and --partial-clone are mutually exclusive',
               file=sys.stderr)
         sys.exit(1)
-      m.config.SetString('repo.partialclone', 'true')
+      m.config.SetBoolean('repo.partialclone', opt.partial_clone)
       if opt.clone_filter:
         m.config.SetString('repo.clonefilter', opt.clone_filter)
+    elif m.config.GetBoolean('repo.partialclone'):
+      opt.clone_filter = m.config.GetString('repo.clonefilter')
     else:
       opt.clone_filter = None
 
+    if opt.partial_clone_exclude is not None:
+      m.config.SetString('repo.partialcloneexclude', opt.partial_clone_exclude)
+
     if opt.clone_bundle is None:
       opt.clone_bundle = False if opt.partial_clone else True
     else:
-      m.config.SetString('repo.clonebundle', 'true' if opt.clone_bundle else 'false')
+      m.config.SetBoolean('repo.clonebundle', opt.clone_bundle)
 
     if opt.submodules:
-      m.config.SetString('repo.submodules', 'true')
+      m.config.SetBoolean('repo.submodules', opt.submodules)
+
+    if opt.use_superproject is not None:
+      m.config.SetBoolean('repo.superproject', opt.use_superproject)
 
     if not m.Sync_NetworkHalf(is_new=is_new, quiet=opt.quiet, verbose=opt.verbose,
                               clone_bundle=opt.clone_bundle,
                               current_branch_only=opt.current_branch_only,
                               tags=opt.tags, submodules=opt.submodules,
-                              clone_filter=opt.clone_filter):
+                              clone_filter=opt.clone_filter,
+                              partial_clone_exclude=self.manifest.PartialCloneExclude):
       r = m.GetRemote(m.remote.name)
       print('fatal: cannot obtain manifest %s' % r.url, file=sys.stderr)
 
@@ -481,7 +421,15 @@ to update the working directory files.
       self.OptionParser.error('--mirror and --archive cannot be used together.')
 
     if args:
-      self.OptionParser.error('init takes no arguments')
+      if opt.manifest_url:
+        self.OptionParser.error(
+            '--manifest-url option and URL argument both specified: only use '
+            'one to select the manifest URL.')
+
+      opt.manifest_url = args.pop(0)
+
+      if args:
+        self.OptionParser.error('too many arguments to init')
 
   def Execute(self, opt, args):
     git_require(MIN_GIT_VERSION_HARD, fail=True)
@@ -491,9 +439,6 @@ to update the working directory files.
             % ('.'.join(str(x) for x in MIN_GIT_VERSION_SOFT),),
             file=sys.stderr)
 
-    opt.quiet = opt.output_mode is False
-    opt.verbose = opt.output_mode is True
-
     rp = self.manifest.repoProject
 
     # Handle new --repo-url requests.
@@ -519,6 +464,9 @@ to update the working directory files.
     self._SyncManifest(opt)
     self._LinkManifest(opt.manifest_name)
 
+    if self.manifest.manifestProject.config.GetBoolean('repo.superproject'):
+      self._CloneSuperproject(opt)
+
     if os.isatty(0) and os.isatty(1) and not self.manifest.IsMirror:
       if opt.config_name or self._ShouldConfigureUser(opt):
         self._ConfigureUser(opt)

subcmds/list.py

@@ -20,11 +20,16 @@ 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.
 
+By default, only projects that currently exist in the checkout are shown.  If
+you want to list all projects (using the specified filter settings), use the
+--all option.  If you want to show all projects regardless of the manifest
+groups, then also pass --groups all.
+
 This is similar to running: repo forall -c 'echo "$REPO_PATH : $REPO_PROJECT"'.
 """
 
@@ -35,6 +40,9 @@ This is similar to running: repo forall -c 'echo "$REPO_PATH : $REPO_PROJECT"'.
     p.add_option('-g', '--groups',
                  dest='groups',
                  help="Filter the project list based on the groups the project is in")
+    p.add_option('-a', '--all',
+                 action='store_true',
+                 help='Show projects regardless of checkout state')
     p.add_option('-f', '--fullpath',
                  dest='fullpath', action='store_true',
                  help="Display the full work tree path instead of the relative path")
@@ -61,7 +69,7 @@ This is similar to running: repo forall -c 'echo "$REPO_PATH : $REPO_PROJECT"'.
       args: Positional args.  Can be a list of projects to list, or empty.
     """
     if not opt.regex:
-      projects = self.GetProjects(args, groups=opt.groups)
+      projects = self.GetProjects(args, groups=opt.groups, missing_ok=opt.all)
     else:
       projects = self.FindProjects(args)
 
@@ -79,5 +87,6 @@ This is similar to running: repo forall -c 'echo "$REPO_PATH : $REPO_PROJECT"'.
       else:
         lines.append("%s : %s" % (_getpath(project), project.name))
 
-    lines.sort()
-    print('\n'.join(lines))
+    if lines:
+      lines.sort()
+      print('\n'.join(lines))

subcmds/prune.py

@@ -12,8 +12,10 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import itertools
+
 from color import Coloring
-from command import PagedCommand
+from command import DEFAULT_LOCAL_JOBS, PagedCommand
 
 
 class Prune(PagedCommand):
@@ -22,11 +24,26 @@ class Prune(PagedCommand):
   helpUsage = """
 %prog [<project>...]
 """
+  PARALLEL_JOBS = DEFAULT_LOCAL_JOBS
+
+  def _ExecuteOne(self, project):
+    """Process one project."""
+    return project.PruneHeads()
 
   def Execute(self, opt, args):
-    all_branches = []
-    for project in self.GetProjects(args):
-      all_branches.extend(project.PruneHeads())
+    projects = self.GetProjects(args)
+
+    # NB: Should be able to refactor this module to display summary as results
+    # come back from children.
+    def _ProcessResults(_pool, _output, results):
+      return list(itertools.chain.from_iterable(results))
+
+    all_branches = self.ExecuteInParallel(
+        opt.jobs,
+        self._ExecuteOne,
+        projects,
+        callback=_ProcessResults,
+        ordered=True)
 
     if not all_branches:
       return

subcmds/rebase.py

@@ -39,7 +39,8 @@ branch but need to incorporate new upstream changes "underneath" them.
 """
 
   def _Options(self, p):
-    p.add_option('-i', '--interactive',
+    g = p.get_option_group('--quiet')
+    g.add_option('-i', '--interactive',
                  dest="interactive", action="store_true",
                  help="interactive rebase (single project only)")
 
@@ -52,9 +53,6 @@ branch but need to incorporate new upstream changes "underneath" them.
     p.add_option('--no-ff',
                  dest='ff', default=True, action='store_false',
                  help='Pass --no-ff to git rebase')
-    p.add_option('-q', '--quiet',
-                 dest='quiet', action='store_true',
-                 help='Pass --quiet to git rebase')
     p.add_option('--autosquash',
                  dest='autosquash', action='store_true',
                  help='Pass --autosquash to git rebase')

subcmds/stage.py

@@ -38,7 +38,8 @@ The '%prog' command stages files to prepare the next commit.
 """
 
   def _Options(self, p):
-    p.add_option('-i', '--interactive',
+    g = p.get_option_group('--quiet')
+    g.add_option('-i', '--interactive',
                  dest='interactive', action='store_true',
                  help='use interactive staging')
 

subcmds/start.py

@@ -12,10 +12,11 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import functools
 import os
 import sys
 
-from command import Command
+from command import Command, DEFAULT_LOCAL_JOBS
 from git_config import IsImmutable
 from git_command import git
 import gitc_utils
@@ -33,6 +34,7 @@ class Start(Command):
 '%prog' begins a new branch of development, starting from the
 revision specified in the manifest.
 """
+  PARALLEL_JOBS = DEFAULT_LOCAL_JOBS
 
   def _Options(self, p):
     p.add_option('--all',
@@ -40,7 +42,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):
@@ -51,6 +54,26 @@ revision specified in the manifest.
     if not git.check_ref_format('heads/%s' % nb):
       self.OptionParser.error("'%s' is not a valid name" % nb)
 
+  def _ExecuteOne(self, revision, nb, project):
+    """Start one project."""
+    # If the current revision is immutable, such as a SHA1, a tag or
+    # a change, then we can't push back to it. Substitute with
+    # dest_branch, if defined; or with manifest default revision instead.
+    branch_merge = ''
+    if IsImmutable(project.revisionExpr):
+      if project.dest_branch:
+        branch_merge = project.dest_branch
+      else:
+        branch_merge = self.manifest.default.revisionExpr
+
+    try:
+      ret = project.StartBranch(
+          nb, branch_merge=branch_merge, revision=revision)
+    except Exception as e:
+      print('error: unable to checkout %s: %s' % (project.name, e), file=sys.stderr)
+      ret = False
+    return (ret, project)
+
   def Execute(self, opt, args):
     nb = args[0]
     err = []
@@ -82,11 +105,8 @@ revision specified in the manifest.
       if not os.path.exists(os.getcwd()):
         os.chdir(self.manifest.topdir)
 
-    pm = Progress('Starting %s' % nb, len(all_projects))
-    for project in all_projects:
-      pm.update()
-
-      if self.gitc_manifest:
+      pm = Progress('Syncing %s' % nb, len(all_projects), quiet=opt.quiet)
+      for project in all_projects:
         gitc_project = self.gitc_manifest.paths[project.relpath]
         # Sync projects that have not been opened.
         if not gitc_project.already_synced:
@@ -99,21 +119,21 @@ revision specified in the manifest.
           sync_buf = SyncBuffer(self.manifest.manifestProject.config)
           project.Sync_LocalHalf(sync_buf)
           project.revisionId = gitc_project.old_revision
+        pm.update()
+      pm.end()
 
-      # If the current revision is immutable, such as a SHA1, a tag or
-      # a change, then we can't push back to it. Substitute with
-      # dest_branch, if defined; or with manifest default revision instead.
-      branch_merge = ''
-      if IsImmutable(project.revisionExpr):
-        if project.dest_branch:
-          branch_merge = project.dest_branch
-        else:
-          branch_merge = self.manifest.default.revisionExpr
+    def _ProcessResults(_pool, pm, results):
+      for (result, project) in results:
+        if not result:
+          err.append(project)
+        pm.update()
 
-      if not project.StartBranch(
-              nb, branch_merge=branch_merge, revision=opt.revision):
-        err.append(project)
-    pm.end()
+    self.ExecuteInParallel(
+        opt.jobs,
+        functools.partial(self._ExecuteOne, opt.revision, nb),
+        all_projects,
+        callback=_ProcessResults,
+        output=Progress('Starting %s' % (nb,), len(all_projects), quiet=opt.quiet))
 
     if err:
       for p in err:

subcmds/status.py

@@ -14,10 +14,10 @@
 
 import functools
 import glob
-import multiprocessing
+import io
 import os
 
-from command import PagedCommand
+from command import DEFAULT_LOCAL_JOBS, PagedCommand
 
 from color import Coloring
 import platform_utils
@@ -76,16 +76,12 @@ the following meanings:
  d:  deleted       (    in index, not in work tree                )
 
 """
+  PARALLEL_JOBS = DEFAULT_LOCAL_JOBS
 
   def _Options(self, p):
-    p.add_option('-j', '--jobs',
-                 dest='jobs', action='store', type='int', default=2,
-                 help="number of projects to check simultaneously")
     p.add_option('-o', '--orphans',
                  dest='orphans', action='store_true',
                  help="include objects in working directory outside of repo projects")
-    p.add_option('-q', '--quiet', action='store_true',
-                 help="only print the name of modified projects")
 
   def _StatusHelper(self, quiet, project):
     """Obtains the status for a specific project.
@@ -100,7 +96,9 @@ the following meanings:
     Returns:
       The status of the project.
     """
-    return project.PrintWorkTreeStatus(quiet=quiet)
+    buf = io.StringIO()
+    ret = project.PrintWorkTreeStatus(quiet=quiet, output_redir=buf)
+    return (ret, buf.getvalue())
 
   def _FindOrphans(self, dirs, proj_dirs, proj_dirs_parents, outstring):
     """find 'dirs' that are present in 'proj_dirs_parents' but not in 'proj_dirs'"""
@@ -120,17 +118,23 @@ the following meanings:
 
   def Execute(self, opt, args):
     all_projects = self.GetProjects(args)
-    counter = 0
 
-    if opt.jobs == 1:
-      for project in all_projects:
-        state = project.PrintWorkTreeStatus(quiet=opt.quiet)
+    def _ProcessResults(_pool, _output, results):
+      ret = 0
+      for (state, output) in results:
+        if output:
+          print(output, end='')
         if state == 'CLEAN':
-          counter += 1
-    else:
-      with multiprocessing.Pool(opt.jobs) as pool:
-        states = pool.map(functools.partial(self._StatusHelper, opt.quiet), all_projects)
-        counter += states.count('CLEAN')
+          ret += 1
+      return ret
+
+    counter = self.ExecuteInParallel(
+        opt.jobs,
+        functools.partial(self._StatusHelper, opt.quiet),
+        all_projects,
+        callback=_ProcessResults,
+        ordered=True)
+
     if not opt.quiet and len(all_projects) == counter:
       print('nothing to commit (working directory clean)')
 

subcmds/sync.py

@@ -12,14 +12,15 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import functools
 import http.cookiejar as cookielib
+import io
 import json
+import multiprocessing
 import netrc
 from optparse import SUPPRESS_HELP
 import os
-import re
 import socket
-import subprocess
 import sys
 import tempfile
 import time
@@ -42,20 +43,15 @@ except ImportError:
   def _rlimit_nofile():
     return (256, 256)
 
-try:
-  import multiprocessing
-except ImportError:
-  multiprocessing = None
-
 import event_log
-from git_command import GIT, git_require
+from git_command import git_require
 from git_config import GetUrlCookieFile
 from git_refs import R_HEADS, HEAD
 import git_superproject
 import gitc_utils
 from project import Project
 from project import RemoteSpec
-from command import Command, MirrorSafeCommand
+from command import Command, MirrorSafeCommand, WORKER_BATCH_SIZE
 from error import RepoChangedException, GitError, ManifestParseError
 import platform_utils
 from project import SyncBuffer
@@ -66,15 +62,6 @@ from manifest_xml import GitcManifest
 _ONE_DAY_S = 24 * 60 * 60
 
 
-class _FetchError(Exception):
-  """Internal error thrown in _FetchHelper() when we don't want stack trace."""
-  pass
-
-
-class _CheckoutError(Exception):
-  """Internal error thrown in _CheckoutOne() when we don't want stack trace."""
-
-
 class Sync(Command, MirrorSafeCommand):
   jobs = 1
   common = True
@@ -178,12 +165,20 @@ If the remote SSH daemon is Gerrit Code Review, version 2.0.10 or
 later is required to fix a server side protocol bug.
 
 """
+  PARALLEL_JOBS = 1
+
+  def _CommonOptions(self, p):
+    try:
+      self.PARALLEL_JOBS = self.manifest.default.sync_j
+    except ManifestParseError:
+      pass
+    super()._CommonOptions(p)
 
   def _Options(self, p, show_smart=True):
-    try:
-      self.jobs = self.manifest.default.sync_j
-    except ManifestParseError:
-      self.jobs = 1
+    p.add_option('--jobs-network', default=None, type=int, metavar='JOBS',
+                 help='number of network jobs to run in parallel (defaults to --jobs)')
+    p.add_option('--jobs-checkout', default=None, type=int, metavar='JOBS',
+                 help='number of local checkout jobs to run in parallel (defaults to --jobs)')
 
     p.add_option('-f', '--force-broken',
                  dest='force_broken', action='store_true',
@@ -217,15 +212,6 @@ later is required to fix a server side protocol bug.
     p.add_option('-c', '--current-branch',
                  dest='current_branch_only', action='store_true',
                  help='fetch only current branch from server')
-    p.add_option('-v', '--verbose',
-                 dest='output_mode', action='store_true',
-                 help='show all sync output')
-    p.add_option('-q', '--quiet',
-                 dest='output_mode', action='store_false',
-                 help='only show errors')
-    p.add_option('-j', '--jobs',
-                 dest='jobs', action='store', type='int',
-                 help="projects to fetch simultaneously (default %d)" % self.jobs)
     p.add_option('-m', '--manifest-name',
                  dest='manifest_name',
                  help='temporary manifest to use for this sync', metavar='NAME.xml')
@@ -280,6 +266,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.
 
@@ -294,28 +290,13 @@ later is required to fix a server side protocol bug.
     Returns:
       Returns path to the overriding manifest file.
     """
-    if not self.manifest.superproject:
-      print('error: superproject tag is not defined in manifest.xml',
-            file=sys.stderr)
-      sys.exit(1)
-    print('WARNING: --use-superproject is experimental and not '
-          'for general use', file=sys.stderr)
-
-    superproject_url = self.manifest.superproject['remote'].url
-    if not superproject_url:
-      print('error: superproject URL is not defined in manifest.xml',
-            file=sys.stderr)
-      sys.exit(1)
-
-    superproject = git_superproject.Superproject(self.manifest.repodir)
+    superproject = git_superproject.Superproject(self.manifest,
+                                                 self.repodir,
+                                                 quiet=opt.quiet)
     all_projects = self.GetProjects(args,
                                     missing_ok=True,
                                     submodules_ok=opt.fetch_submodules)
-    branch = self._GetBranch()
-    manifest_path = superproject.UpdateProjectsRevisionId(self.manifest,
-                                                          all_projects,
-                                                          url=superproject_url,
-                                                          branch=branch)
+    manifest_path = superproject.UpdateProjectsRevisionId(all_projects)
     if not manifest_path:
       print('error: Update of revsionId from superproject has failed',
             file=sys.stderr)
@@ -323,140 +304,123 @@ later is required to fix a server side protocol bug.
     self._ReloadManifest(manifest_path)
     return manifest_path
 
-  def _FetchProjectList(self, opt, projects, sem, *args, **kwargs):
-    """Main function of the fetch threads.
+  def _FetchProjectList(self, opt, projects):
+    """Main function of the fetch worker.
+
+    The projects we're given share the same underlying git object store, so we
+    have to fetch them in serial.
 
     Delegates most of the work to _FetchHelper.
 
     Args:
       opt: Program options returned from optparse.  See _Options().
       projects: Projects to fetch.
-      sem: We'll release() this semaphore when we exit so that another thread
-          can be started up.
-      *args, **kwargs: Remaining arguments to pass to _FetchHelper. See the
-          _FetchHelper docstring for details.
     """
-    try:
-        for project in projects:
-          success = self._FetchHelper(opt, project, *args, **kwargs)
-          if not success and opt.fail_fast:
-            break
-    finally:
-        sem.release()
+    return [self._FetchOne(opt, x) for x in projects]
 
-  def _FetchHelper(self, opt, project, lock, fetched, pm, err_event,
-                   clone_filter):
+  def _FetchOne(self, opt, project):
     """Fetch git objects for a single project.
 
     Args:
       opt: Program options returned from optparse.  See _Options().
       project: Project object for the project to fetch.
-      lock: Lock for accessing objects that are shared amongst multiple
-          _FetchHelper() threads.
-      fetched: set object that we will add project.gitdir to when we're done
-          (with our lock held).
-      pm: Instance of a Project object.  We will call pm.update() (with our
-          lock held).
-      err_event: We'll set this event in the case of an error (after printing
-          out info about the error).
-      clone_filter: Filter for use in a partial clone.
 
     Returns:
       Whether the fetch was successful.
     """
-    # We'll set to true once we've locked the lock.
-    did_lock = False
-
-    # Encapsulate everything in a try/except/finally so that:
-    # - We always set err_event in the case of an exception.
-    # - We always make sure we unlock the lock if we locked it.
     start = time.time()
     success = False
+    buf = io.StringIO()
     try:
-      try:
-        success = project.Sync_NetworkHalf(
-            quiet=opt.quiet,
-            verbose=opt.verbose,
-            current_branch_only=opt.current_branch_only,
-            force_sync=opt.force_sync,
-            clone_bundle=opt.clone_bundle,
-            tags=opt.tags, archive=self.manifest.IsArchive,
-            optimized_fetch=opt.optimized_fetch,
-            retry_fetches=opt.retry_fetches,
-            prune=opt.prune,
-            clone_filter=clone_filter)
-        self._fetch_times.Set(project, time.time() - start)
+      success = project.Sync_NetworkHalf(
+          quiet=opt.quiet,
+          verbose=opt.verbose,
+          output_redir=buf,
+          current_branch_only=self._GetCurrentBranchOnly(opt),
+          force_sync=opt.force_sync,
+          clone_bundle=opt.clone_bundle,
+          tags=opt.tags, archive=self.manifest.IsArchive,
+          optimized_fetch=opt.optimized_fetch,
+          retry_fetches=opt.retry_fetches,
+          prune=opt.prune,
+          clone_filter=self.manifest.CloneFilter,
+          partial_clone_exclude=self.manifest.PartialCloneExclude)
 
-        # Lock around all the rest of the code, since printing, updating a set
-        # and Progress.update() are not thread safe.
-        lock.acquire()
-        did_lock = True
+      output = buf.getvalue()
+      if opt.verbose and output:
+        print('\n' + output.rstrip())
 
-        if not success:
-          err_event.set()
-          print('error: Cannot fetch %s from %s'
-                % (project.name, project.remote.url),
-                file=sys.stderr)
-          if opt.fail_fast:
-            raise _FetchError()
+      if not success:
+        print('error: Cannot fetch %s from %s'
+              % (project.name, project.remote.url),
+              file=sys.stderr)
+    except GitError as e:
+      print('error.GitError: Cannot fetch %s' % str(e), file=sys.stderr)
+    except Exception as e:
+      print('error: Cannot fetch %s (%s: %s)'
+            % (project.name, type(e).__name__, str(e)), file=sys.stderr)
+      raise
 
-        fetched.add(project.gitdir)
-        pm.update(msg=project.name)
-      except _FetchError:
-        pass
-      except Exception as e:
-        print('error: Cannot fetch %s (%s: %s)'
-              % (project.name, type(e).__name__, str(e)), file=sys.stderr)
-        err_event.set()
-        raise
-    finally:
-      if did_lock:
-        lock.release()
-      finish = time.time()
-      self.event_log.AddSync(project, event_log.TASK_SYNC_NETWORK,
-                             start, finish, success)
-
-    return success
+    finish = time.time()
+    return (success, project, start, finish)
 
   def _Fetch(self, projects, opt, err_event):
+    ret = True
+
+    jobs = opt.jobs_network if opt.jobs_network else self.jobs
     fetched = set()
-    lock = _threading.Lock()
-    pm = Progress('Fetching projects', len(projects),
-                  always_print_percentage=opt.quiet)
+    pm = Progress('Fetching', len(projects), delay=False, quiet=opt.quiet)
 
     objdir_project_map = dict()
     for project in projects:
       objdir_project_map.setdefault(project.objdir, []).append(project)
+    projects_list = list(objdir_project_map.values())
 
-    threads = set()
-    sem = _threading.Semaphore(self.jobs)
-    for project_list in objdir_project_map.values():
-      # Check for any errors before running any more tasks.
-      # ...we'll let existing threads finish, though.
-      if err_event.isSet() and opt.fail_fast:
-        break
+    def _ProcessResults(results_sets):
+      ret = True
+      for results in results_sets:
+        for (success, project, start, finish) in results:
+          self._fetch_times.Set(project, finish - start)
+          self.event_log.AddSync(project, event_log.TASK_SYNC_NETWORK,
+                                 start, finish, success)
+          # Check for any errors before running any more tasks.
+          # ...we'll let existing jobs finish, though.
+          if not success:
+            ret = False
+          else:
+            fetched.add(project.gitdir)
+          pm.update(msg=project.name)
+        if not ret and opt.fail_fast:
+          break
+      return ret
 
-      sem.acquire()
-      kwargs = dict(opt=opt,
-                    projects=project_list,
-                    sem=sem,
-                    lock=lock,
-                    fetched=fetched,
-                    pm=pm,
-                    err_event=err_event,
-                    clone_filter=self.manifest.CloneFilter)
-      if self.jobs > 1:
-        t = _threading.Thread(target=self._FetchProjectList,
-                              kwargs=kwargs)
-        # Ensure that Ctrl-C will not freeze the repo process.
-        t.daemon = True
-        threads.add(t)
-        t.start()
+    # NB: Multiprocessing is heavy, so don't spin it up for one job.
+    if len(projects_list) == 1 or jobs == 1:
+      if not _ProcessResults(self._FetchProjectList(opt, x) for x in projects_list):
+        ret = False
+    else:
+      # Favor throughput over responsiveness when quiet.  It seems that imap()
+      # will yield results in batches relative to chunksize, so even as the
+      # children finish a sync, we won't see the result until one child finishes
+      # ~chunksize jobs.  When using a large --jobs with large chunksize, this
+      # can be jarring as there will be a large initial delay where repo looks
+      # like it isn't doing anything and sits at 0%, but then suddenly completes
+      # a lot of jobs all at once.  Since this code is more network bound, we
+      # can accept a bit more CPU overhead with a smaller chunksize so that the
+      # user sees more immediate & continuous feedback.
+      if opt.quiet:
+        chunksize = WORKER_BATCH_SIZE
       else:
-        self._FetchProjectList(**kwargs)
-
-    for t in threads:
-      t.join()
+        pm.update(inc=0, msg='warming up')
+        chunksize = 4
+      with multiprocessing.Pool(jobs) as pool:
+        results = pool.imap_unordered(
+            functools.partial(self._FetchProjectList, opt),
+            projects_list,
+            chunksize=chunksize)
+        if not _ProcessResults(results):
+          ret = False
+          pool.close()
 
     pm.end()
     self._fetch_times.Save()
@@ -464,178 +428,108 @@ later is required to fix a server side protocol bug.
     if not self.manifest.IsArchive:
       self._GCProjects(projects, opt, err_event)
 
-    return fetched
+    return (ret, fetched)
 
-  def _CheckoutWorker(self, opt, sem, project, *args, **kwargs):
-    """Main function of the fetch threads.
-
-    Delegates most of the work to _CheckoutOne.
-
-    Args:
-      opt: Program options returned from optparse.  See _Options().
-      projects: Projects to fetch.
-      sem: We'll release() this semaphore when we exit so that another thread
-          can be started up.
-      *args, **kwargs: Remaining arguments to pass to _CheckoutOne. See the
-          _CheckoutOne docstring for details.
-    """
-    try:
-      return self._CheckoutOne(opt, project, *args, **kwargs)
-    finally:
-      sem.release()
-
-  def _CheckoutOne(self, opt, project, lock, pm, err_event, err_results):
+  def _CheckoutOne(self, detach_head, force_sync, project):
     """Checkout work tree for one project
 
     Args:
-      opt: Program options returned from optparse.  See _Options().
+      detach_head: Whether to leave a detached HEAD.
+      force_sync: Force checking out of the repo.
       project: Project object for the project to checkout.
-      lock: Lock for accessing objects that are shared amongst multiple
-          _CheckoutWorker() threads.
-      pm: Instance of a Project object.  We will call pm.update() (with our
-          lock held).
-      err_event: We'll set this event in the case of an error (after printing
-          out info about the error).
-      err_results: A list of strings, paths to git repos where checkout
-          failed.
 
     Returns:
       Whether the fetch was successful.
     """
-    # We'll set to true once we've locked the lock.
-    did_lock = False
-
-    # Encapsulate everything in a try/except/finally so that:
-    # - We always set err_event in the case of an exception.
-    # - We always make sure we unlock the lock if we locked it.
     start = time.time()
     syncbuf = SyncBuffer(self.manifest.manifestProject.config,
-                         detach_head=opt.detach_head)
+                         detach_head=detach_head)
     success = False
     try:
-      try:
-        project.Sync_LocalHalf(syncbuf, force_sync=opt.force_sync)
+      project.Sync_LocalHalf(syncbuf, force_sync=force_sync)
+      success = syncbuf.Finish()
+    except GitError as e:
+      print('error.GitError: Cannot checkout %s: %s' %
+            (project.name, str(e)), file=sys.stderr)
+    except Exception as e:
+      print('error: Cannot checkout %s: %s: %s' %
+            (project.name, type(e).__name__, str(e)),
+            file=sys.stderr)
+      raise
 
-        # Lock around all the rest of the code, since printing, updating a set
-        # and Progress.update() are not thread safe.
-        lock.acquire()
-        success = syncbuf.Finish()
-        did_lock = True
+    if not success:
+      print('error: Cannot checkout %s' % (project.name), file=sys.stderr)
+    finish = time.time()
+    return (success, project, start, finish)
 
-        if not success:
-          err_event.set()
-          print('error: Cannot checkout %s' % (project.name),
-                file=sys.stderr)
-          raise _CheckoutError()
-
-        pm.update(msg=project.name)
-      except _CheckoutError:
-        pass
-      except Exception as e:
-        print('error: Cannot checkout %s: %s: %s' %
-              (project.name, type(e).__name__, str(e)),
-              file=sys.stderr)
-        err_event.set()
-        raise
-    finally:
-      if did_lock:
-        if not success:
-          err_results.append(project.relpath)
-        lock.release()
-      finish = time.time()
-      self.event_log.AddSync(project, event_log.TASK_SYNC_LOCAL,
-                             start, finish, success)
-
-    return success
-
-  def _Checkout(self, all_projects, opt, err_event, err_results):
+  def _Checkout(self, all_projects, opt, err_results):
     """Checkout projects listed in all_projects
 
     Args:
       all_projects: List of all projects that should be checked out.
       opt: Program options returned from optparse.  See _Options().
-      err_event: We'll set this event in the case of an error (after printing
-          out info about the error).
-      err_results: A list of strings, paths to git repos where checkout
-          failed.
+      err_results: A list of strings, paths to git repos where checkout failed.
     """
+    # Only checkout projects with worktrees.
+    all_projects = [x for x in all_projects if x.worktree]
 
-    # Perform checkouts in multiple threads when we are using partial clone.
-    # Without partial clone, all needed git objects are already downloaded,
-    # in this situation it's better to use only one process because the checkout
-    # would be mostly disk I/O; with partial clone, the objects are only
-    # downloaded when demanded (at checkout time), which is similar to the
-    # Sync_NetworkHalf case and parallelism would be helpful.
-    if self.manifest.CloneFilter:
-      syncjobs = self.jobs
-    else:
-      syncjobs = 1
+    def _ProcessResults(pool, pm, results):
+      ret = True
+      for (success, project, start, finish) in results:
+        self.event_log.AddSync(project, event_log.TASK_SYNC_LOCAL,
+                               start, finish, success)
+        # Check for any errors before running any more tasks.
+        # ...we'll let existing jobs finish, though.
+        if not success:
+          ret = False
+          err_results.append(project.relpath)
+          if opt.fail_fast:
+            if pool:
+              pool.close()
+            return ret
+        pm.update(msg=project.name)
+      return ret
 
-    lock = _threading.Lock()
-    pm = Progress('Checking out projects', len(all_projects))
-
-    threads = set()
-    sem = _threading.Semaphore(syncjobs)
-
-    for project in all_projects:
-      # Check for any errors before running any more tasks.
-      # ...we'll let existing threads finish, though.
-      if err_event.isSet() and opt.fail_fast:
-        break
-
-      sem.acquire()
-      if project.worktree:
-        kwargs = dict(opt=opt,
-                      sem=sem,
-                      project=project,
-                      lock=lock,
-                      pm=pm,
-                      err_event=err_event,
-                      err_results=err_results)
-        if syncjobs > 1:
-          t = _threading.Thread(target=self._CheckoutWorker,
-                                kwargs=kwargs)
-          # Ensure that Ctrl-C will not freeze the repo process.
-          t.daemon = True
-          threads.add(t)
-          t.start()
-        else:
-          self._CheckoutWorker(**kwargs)
-
-    for t in threads:
-      t.join()
-
-    pm.end()
+    return self.ExecuteInParallel(
+        opt.jobs_checkout if opt.jobs_checkout else self.jobs,
+        functools.partial(self._CheckoutOne, opt.detach_head, opt.force_sync),
+        all_projects,
+        callback=_ProcessResults,
+        output=Progress('Checking out', len(all_projects), quiet=opt.quiet)) and not err_results
 
   def _GCProjects(self, projects, opt, err_event):
+    pm = Progress('Garbage collecting', len(projects), delay=False, quiet=opt.quiet)
+    pm.update(inc=0, msg='prescan')
+
     gc_gitdirs = {}
     for project in projects:
       # Make sure pruning never kicks in with shared projects.
       if (not project.use_git_worktrees and
               len(project.manifest.GetProjectsWithName(project.name)) > 1):
         if not opt.quiet:
-          print('%s: Shared project %s found, disabling pruning.' %
+          print('\r%s: Shared project %s found, disabling pruning.' %
                 (project.relpath, project.name))
         if git_require((2, 7, 0)):
           project.EnableRepositoryExtension('preciousObjects')
         else:
           # This isn't perfect, but it's the best we can do with old git.
-          print('%s: WARNING: shared projects are unreliable when using old '
+          print('\r%s: WARNING: shared projects are unreliable when using old '
                 'versions of git; please upgrade to git-2.7.0+.'
                 % (project.relpath,),
                 file=sys.stderr)
           project.config.SetString('gc.pruneExpire', 'never')
       gc_gitdirs[project.gitdir] = project.bare_git
 
-    if multiprocessing:
-      cpu_count = multiprocessing.cpu_count()
-    else:
-      cpu_count = 1
+    pm.update(inc=len(projects) - len(gc_gitdirs), msg='warming up')
+
+    cpu_count = os.cpu_count()
     jobs = min(self.jobs, cpu_count)
 
     if jobs < 2:
       for bare_git in gc_gitdirs.values():
+        pm.update(msg=bare_git._project.name)
         bare_git.gc('--auto')
+      pm.end()
       return
 
     config = {'pack.threads': cpu_count // jobs if cpu_count > jobs else 1}
@@ -644,6 +538,7 @@ later is required to fix a server side protocol bug.
     sem = _threading.Semaphore(jobs)
 
     def GC(bare_git):
+      pm.start(bare_git._project.name)
       try:
         try:
           bare_git.gc('--auto', config=config)
@@ -653,10 +548,11 @@ later is required to fix a server side protocol bug.
           err_event.set()
           raise
       finally:
+        pm.finish(bare_git._project.name)
         sem.release()
 
     for bare_git in gc_gitdirs.values():
-      if err_event.isSet() and opt.fail_fast:
+      if err_event.is_set() and opt.fail_fast:
         break
       sem.acquire()
       t = _threading.Thread(target=GC, args=(bare_git,))
@@ -666,6 +562,7 @@ later is required to fix a server side protocol bug.
 
     for t in threads:
       t.join()
+    pm.end()
 
   def _ReloadManifest(self, manifest_name=None):
     if manifest_name:
@@ -680,7 +577,7 @@ later is required to fix a server side protocol bug.
       if project.relpath:
         new_project_paths.append(project.relpath)
     file_name = 'project.list'
-    file_path = os.path.join(self.manifest.repodir, file_name)
+    file_path = os.path.join(self.repodir, file_name)
     old_project_paths = []
 
     if os.path.exists(file_path):
@@ -812,13 +709,14 @@ 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,
                                     retry_fetches=opt.retry_fetches,
                                     submodules=self.manifest.HasSubmodules,
-                                    clone_filter=self.manifest.CloneFilter)
+                                    clone_filter=self.manifest.CloneFilter,
+                                    partial_clone_exclude=self.manifest.PartialCloneExclude)
       finish = time.time()
       self.event_log.AddSync(mp, event_log.TASK_SYNC_NETWORK,
                              start, finish, success)
@@ -861,9 +759,6 @@ later is required to fix a server side protocol bug.
       soft_limit, _ = _rlimit_nofile()
       self.jobs = min(self.jobs, (soft_limit - 5) // 3)
 
-    opt.quiet = opt.output_mode is False
-    opt.verbose = opt.output_mode is True
-
     if opt.manifest_name:
       self.manifest.Override(opt.manifest_name)
 
@@ -907,7 +802,7 @@ later is required to fix a server side protocol bug.
     else:
       self._UpdateManifestProject(opt, mp, manifest_name)
 
-    if opt.use_superproject:
+    if self._UseSuperproject(opt):
       manifest_name = self._UpdateProjectsRevisionId(opt, args)
 
     if self.gitc_manifest:
@@ -951,7 +846,6 @@ later is required to fix a server side protocol bug.
 
     err_network_sync = False
     err_update_projects = False
-    err_checkout = False
 
     self._fetch_times = _FetchTimes(self.manifest)
     if not opt.local_only:
@@ -962,12 +856,14 @@ later is required to fix a server side protocol bug.
       to_fetch.extend(all_projects)
       to_fetch.sort(key=self._fetch_times.Get, reverse=True)
 
-      fetched = self._Fetch(to_fetch, opt, err_event)
+      success, fetched = self._Fetch(to_fetch, opt, err_event)
+      if not success:
+        err_event.set()
 
       _PostRepoFetch(rp, opt.repo_verify)
       if opt.network_only:
         # bail out now; the rest touches the working tree
-        if err_event.isSet():
+        if err_event.is_set():
           print('\nerror: Exited sync due to fetch errors.\n', file=sys.stderr)
           sys.exit(1)
         return
@@ -991,10 +887,13 @@ later is required to fix a server side protocol bug.
         if previously_missing_set == missing_set:
           break
         previously_missing_set = missing_set
-        fetched.update(self._Fetch(missing, opt, err_event))
+        success, new_fetched = self._Fetch(to_fetch, opt, err_event)
+        if not success:
+          err_event.set()
+        fetched.update(new_fetched)
 
       # If we saw an error, exit with code 1 so that other scripts can check.
-      if err_event.isSet():
+      if err_event.is_set():
         err_network_sync = True
         if opt.fail_fast:
           print('\nerror: Exited sync due to fetch errors.\n'
@@ -1016,10 +915,10 @@ later is required to fix a server side protocol bug.
         sys.exit(1)
 
     err_results = []
-    self._Checkout(all_projects, opt, err_event, err_results)
-    if err_event.isSet():
-      err_checkout = True
-      # NB: We don't exit here because this is the last step.
+    # NB: We don't exit here because this is the last step.
+    err_checkout = not self._Checkout(all_projects, opt, err_results)
+    if err_checkout:
+      err_event.set()
 
     # If there's a notice that's supposed to print at the end of the sync, print
     # it now...
@@ -1027,7 +926,7 @@ later is required to fix a server side protocol bug.
       print(self.manifest.notice)
 
     # If we saw an error, exit with code 1 so that other scripts can check.
-    if err_event.isSet():
+    if err_event.is_set():
       print('\nerror: Unable to fully sync the tree.', file=sys.stderr)
       if err_network_sync:
         print('error: Downloading network changes failed.', file=sys.stderr)
@@ -1057,12 +956,25 @@ def _PostRepoUpgrade(manifest, quiet=False):
 def _PostRepoFetch(rp, repo_verify=True, verbose=False):
   if rp.HasChanges:
     print('info: A new version of repo is available', file=sys.stderr)
-    print(file=sys.stderr)
-    if not repo_verify or _VerifyTag(rp):
-      syncbuf = SyncBuffer(rp.config)
-      rp.Sync_LocalHalf(syncbuf)
-      if not syncbuf.Finish():
-        sys.exit(1)
+    wrapper = Wrapper()
+    try:
+      rev = rp.bare_git.describe(rp.GetRevisionId())
+    except GitError:
+      rev = None
+    _, new_rev = wrapper.check_repo_rev(rp.gitdir, rev, repo_verify=repo_verify)
+    # See if we're held back due to missing signed tag.
+    current_revid = rp.bare_git.rev_parse('HEAD')
+    new_revid = rp.bare_git.rev_parse('--verify', new_rev)
+    if current_revid != new_revid:
+      # We want to switch to the new rev, but also not trash any uncommitted
+      # changes.  This helps with local testing/hacking.
+      # If a local change has been made, we will throw that away.
+      # We also have to make sure this will switch to an older commit if that's
+      # the latest tag in order to support release rollback.
+      try:
+        rp.work_git.reset('--keep', new_rev)
+      except GitError as e:
+        sys.exit(str(e))
       print('info: Restarting repo with latest version', file=sys.stderr)
       raise RepoChangedException(['--repo-upgraded'])
     else:
@@ -1073,54 +985,6 @@ def _PostRepoFetch(rp, repo_verify=True, verbose=False):
             file=sys.stderr)
 
 
-def _VerifyTag(project):
-  gpg_dir = os.path.expanduser('~/.repoconfig/gnupg')
-  if not os.path.exists(gpg_dir):
-    print('warning: GnuPG was not available during last "repo init"\n'
-          'warning: Cannot automatically authenticate repo."""',
-          file=sys.stderr)
-    return True
-
-  try:
-    cur = project.bare_git.describe(project.GetRevisionId())
-  except GitError:
-    cur = None
-
-  if not cur \
-     or re.compile(r'^.*-[0-9]{1,}-g[0-9a-f]{1,}$').match(cur):
-    rev = project.revisionExpr
-    if rev.startswith(R_HEADS):
-      rev = rev[len(R_HEADS):]
-
-    print(file=sys.stderr)
-    print("warning: project '%s' branch '%s' is not signed"
-          % (project.name, rev), file=sys.stderr)
-    return False
-
-  env = os.environ.copy()
-  env['GIT_DIR'] = project.gitdir
-  env['GNUPGHOME'] = gpg_dir
-
-  cmd = [GIT, 'tag', '-v', cur]
-  proc = subprocess.Popen(cmd,
-                          stdout=subprocess.PIPE,
-                          stderr=subprocess.PIPE,
-                          env=env)
-  out = proc.stdout.read()
-  proc.stdout.close()
-
-  err = proc.stderr.read()
-  proc.stderr.close()
-
-  if proc.wait() != 0:
-    print(file=sys.stderr)
-    print(out, file=sys.stderr)
-    print(err, file=sys.stderr)
-    print(file=sys.stderr)
-    return False
-  return True
-
-
 class _FetchTimes(object):
   _ALPHA = 0.5
 

tests/test_error.py

@@ -0,0 +1,53 @@
+# 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.
+
+"""Unittests for the error.py module."""
+
+import inspect
+import pickle
+import unittest
+
+import error
+
+
+class PickleTests(unittest.TestCase):
+  """Make sure all our custom exceptions can be pickled."""
+
+  def getExceptions(self):
+    """Return all our custom exceptions."""
+    for name in dir(error):
+      cls = getattr(error, name)
+      if isinstance(cls, type) and issubclass(cls, Exception):
+        yield cls
+
+  def testExceptionLookup(self):
+    """Make sure our introspection logic works."""
+    classes = list(self.getExceptions())
+    self.assertIn(error.HookError, classes)
+    # Don't assert the exact number to avoid being a change-detector test.
+    self.assertGreater(len(classes), 10)
+
+  def testPickle(self):
+    """Try to pickle all the exceptions."""
+    for cls in self.getExceptions():
+      args = inspect.getfullargspec(cls.__init__).args[1:]
+      obj = cls(*args)
+      p = pickle.dumps(obj)
+      try:
+        newobj = pickle.loads(p)
+      except Exception as e:  # pylint: disable=broad-except
+        self.fail('Class %s is unable to be pickled: %s\n'
+                  'Incomplete super().__init__(...) call?' % (cls, e))
+      self.assertIsInstance(newobj, cls)
+      self.assertEqual(str(obj), str(newobj))

tests/test_git_config.py

@@ -15,6 +15,7 @@
 """Unittests for the git_config.py module."""
 
 import os
+import tempfile
 import unittest
 
 import git_config
@@ -26,9 +27,8 @@ def fixture(*paths):
   return os.path.join(os.path.dirname(__file__), 'fixtures', *paths)
 
 
-class GitConfigUnitTest(unittest.TestCase):
-  """Tests the GitConfig class.
-  """
+class GitConfigReadOnlyTests(unittest.TestCase):
+  """Read-only tests of the GitConfig class."""
 
   def setUp(self):
     """Create a GitConfig object using the test.gitconfig fixture.
@@ -105,5 +105,69 @@ class GitConfigUnitTest(unittest.TestCase):
       self.assertEqual(value, self.config.GetInt('section.%s' % (key,)))
 
 
+class GitConfigReadWriteTests(unittest.TestCase):
+  """Read/write tests of the GitConfig class."""
+
+  def setUp(self):
+    self.tmpfile = tempfile.NamedTemporaryFile()
+    self.config = self.get_config()
+
+  def get_config(self):
+    """Get a new GitConfig instance."""
+    return git_config.GitConfig(self.tmpfile.name)
+
+  def test_SetString(self):
+    """Test SetString behavior."""
+    # Set a value.
+    self.assertIsNone(self.config.GetString('foo.bar'))
+    self.config.SetString('foo.bar', 'val')
+    self.assertEqual('val', self.config.GetString('foo.bar'))
+
+    # Make sure the value was actually written out.
+    config = self.get_config()
+    self.assertEqual('val', config.GetString('foo.bar'))
+
+    # Update the value.
+    self.config.SetString('foo.bar', 'valll')
+    self.assertEqual('valll', self.config.GetString('foo.bar'))
+    config = self.get_config()
+    self.assertEqual('valll', config.GetString('foo.bar'))
+
+    # Delete the value.
+    self.config.SetString('foo.bar', None)
+    self.assertIsNone(self.config.GetString('foo.bar'))
+    config = self.get_config()
+    self.assertIsNone(config.GetString('foo.bar'))
+
+  def test_SetBoolean(self):
+    """Test SetBoolean behavior."""
+    # Set a true value.
+    self.assertIsNone(self.config.GetBoolean('foo.bar'))
+    for val in (True, 1):
+      self.config.SetBoolean('foo.bar', val)
+      self.assertTrue(self.config.GetBoolean('foo.bar'))
+
+    # Make sure the value was actually written out.
+    config = self.get_config()
+    self.assertTrue(config.GetBoolean('foo.bar'))
+    self.assertEqual('true', config.GetString('foo.bar'))
+
+    # Set a false value.
+    for val in (False, 0):
+      self.config.SetBoolean('foo.bar', val)
+      self.assertFalse(self.config.GetBoolean('foo.bar'))
+
+    # Make sure the value was actually written out.
+    config = self.get_config()
+    self.assertFalse(config.GetBoolean('foo.bar'))
+    self.assertEqual('false', config.GetString('foo.bar'))
+
+    # Delete the value.
+    self.config.SetBoolean('foo.bar', None)
+    self.assertIsNone(self.config.GetBoolean('foo.bar'))
+    config = self.get_config()
+    self.assertIsNone(config.GetBoolean('foo.bar'))
+
+
 if __name__ == '__main__':
   unittest.main()

tests/test_git_superproject.py

@@ -15,11 +15,11 @@
 """Unittests for the git_superproject.py module."""
 
 import os
+import platform
 import tempfile
 import unittest
 from unittest import mock
 
-from error import GitError
 import git_superproject
 import manifest_xml
 import platform_utils
@@ -32,10 +32,10 @@ class SuperprojectTestCase(unittest.TestCase):
     """Set up superproject every time."""
     self.tempdir = tempfile.mkdtemp(prefix='repo_tests')
     self.repodir = os.path.join(self.tempdir, '.repo')
-    self._superproject = git_superproject.Superproject(self.repodir)
     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')
@@ -45,6 +45,16 @@ class SuperprojectTestCase(unittest.TestCase):
         url = https://localhost:0/manifest
 """)
 
+    manifest = self.getXmlManifest("""
+<manifest>
+  <remote name="default-remote" fetch="http://localhost" />
+  <default remote="default-remote" revision="refs/heads/main" />
+  <superproject name="superproject"/>
+  <project path="art" name="platform/art" groups="notdefault,platform-""" + self.platform + """
+  " /></manifest>
+""")
+    self._superproject = git_superproject.Superproject(manifest, self.repodir)
+
   def tearDown(self):
     """Tear down superproject every time."""
     platform_utils.rmtree(self.tempdir)
@@ -55,67 +65,77 @@ class SuperprojectTestCase(unittest.TestCase):
       fp.write(data)
     return manifest_xml.XmlManifest(self.repodir, self.manifest_file)
 
-  def test_superproject_get_project_shas_no_url(self):
+  def test_superproject_get_superproject_no_superproject(self):
     """Test with no url."""
-    with self.assertRaises(ValueError):
-      self._superproject._GetAllProjectsSHAs(url=None)
+    manifest = self.getXmlManifest("""
+<manifest>
+</manifest>
+""")
+    superproject = git_superproject.Superproject(manifest, self.repodir)
+    self.assertFalse(superproject.Sync())
 
-  def test_superproject_get_project_shas_invalid_url(self):
+  def test_superproject_get_superproject_invalid_url(self):
     """Test with an invalid url."""
-    with self.assertRaises(GitError):
-      self._superproject._GetAllProjectsSHAs(url='localhost')
+    manifest = self.getXmlManifest("""
+<manifest>
+  <remote name="test-remote" fetch="localhost" />
+  <default remote="test-remote" revision="refs/heads/main" />
+  <superproject name="superproject"/>
+</manifest>
+""")
+    superproject = git_superproject.Superproject(manifest, self.repodir)
+    self.assertFalse(superproject.Sync())
 
-  def test_superproject_get_project_shas_invalid_branch(self):
+  def test_superproject_get_superproject_invalid_branch(self):
     """Test with an invalid branch."""
-    with self.assertRaises(GitError):
-      self._superproject._GetAllProjectsSHAs(
-          url='sso://android/platform/superproject',
-          branch='junk')
+    manifest = self.getXmlManifest("""
+<manifest>
+  <remote name="test-remote" fetch="localhost" />
+  <default remote="test-remote" revision="refs/heads/main" />
+  <superproject name="superproject"/>
+</manifest>
+""")
+    superproject = git_superproject.Superproject(manifest, self.repodir)
+    with mock.patch.object(self._superproject, '_GetBranch', return_value='junk'):
+      self.assertFalse(superproject.Sync())
 
-  def test_superproject_get_project_shas_mock_clone(self):
-    """Test with _Clone failing."""
-    with self.assertRaises(GitError):
-      with mock.patch.object(self._superproject, '_Clone', return_value=False):
-        self._superproject._GetAllProjectsSHAs(url='localhost')
+  def test_superproject_get_superproject_mock_init(self):
+    """Test with _Init failing."""
+    with mock.patch.object(self._superproject, '_Init', return_value=False):
+      self.assertFalse(self._superproject.Sync())
 
-  def test_superproject_get_project_shas_mock_fetch(self):
+  def test_superproject_get_superproject_mock_fetch(self):
     """Test with _Fetch failing."""
-    with self.assertRaises(GitError):
-      with mock.patch.object(self._superproject, '_Clone', return_value=True):
-        with mock.patch.object(self._superproject, '_Fetch', return_value=False):
-          self._superproject._GetAllProjectsSHAs(url='localhost')
+    with mock.patch.object(self._superproject, '_Init', return_value=True):
+      os.mkdir(self._superproject._superproject_path)
+      with mock.patch.object(self._superproject, '_Fetch', return_value=False):
+        self.assertFalse(self._superproject.Sync())
 
-  def test_superproject_get_project_shas_mock_ls_tree(self):
+  def test_superproject_get_all_project_commit_ids_mock_ls_tree(self):
     """Test with LsTree being a mock."""
     data = ('120000 blob 158258bdf146f159218e2b90f8b699c4d85b5804\tAndroid.bp\x00'
             '160000 commit 2c2724cb36cd5a9cec6c852c681efc3b7c6b86ea\tart\x00'
             '160000 commit e9d25da64d8d365dbba7c8ee00fe8c4473fe9a06\tbootable/recovery\x00'
             '120000 blob acc2cbdf438f9d2141f0ae424cec1d8fc4b5d97f\tbootstrap.bash\x00'
             '160000 commit ade9b7a0d874e25fff4bf2552488825c6f111928\tbuild/bazel\x00')
-    with mock.patch.object(self._superproject, '_Clone', return_value=True):
-      with mock.patch.object(self._superproject, '_LsTree', return_value=data):
-        shas = self._superproject._GetAllProjectsSHAs(url='localhost', branch='junk')
-        self.assertEqual(shas, {
-            'art': '2c2724cb36cd5a9cec6c852c681efc3b7c6b86ea',
-            'bootable/recovery': 'e9d25da64d8d365dbba7c8ee00fe8c4473fe9a06',
-            'build/bazel': 'ade9b7a0d874e25fff4bf2552488825c6f111928'
-        })
+    with mock.patch.object(self._superproject, '_Init', return_value=True):
+      with mock.patch.object(self._superproject, '_Fetch', return_value=True):
+        with mock.patch.object(self._superproject, '_LsTree', return_value=data):
+          commit_ids = self._superproject._GetAllProjectsCommitIds()
+          self.assertEqual(commit_ids, {
+              'art': '2c2724cb36cd5a9cec6c852c681efc3b7c6b86ea',
+              'bootable/recovery': 'e9d25da64d8d365dbba7c8ee00fe8c4473fe9a06',
+              'build/bazel': 'ade9b7a0d874e25fff4bf2552488825c6f111928'
+          })
 
   def test_superproject_write_manifest_file(self):
     """Test with writing manifest to a file after setting revisionId."""
-    manifest = self.getXmlManifest("""
-<manifest>
-  <remote name="default-remote" fetch="http://localhost" />
-  <default remote="default-remote" revision="refs/heads/main" />
-  <project name="test-name"/>
-</manifest>
-""")
-    self.assertEqual(len(manifest.projects), 1)
-    project = manifest.projects[0]
+    self.assertEqual(len(self._superproject._manifest.projects), 1)
+    project = self._superproject._manifest.projects[0]
     project.SetRevisionId('ABCDEF')
     # Create temporary directory so that it can write the file.
     os.mkdir(self._superproject._superproject_path)
-    manifest_path = self._superproject._WriteManfiestFile(manifest)
+    manifest_path = self._superproject._WriteManfiestFile()
     self.assertIsNotNone(manifest_path)
     with open(manifest_path, 'r') as fp:
       manifest_xml = fp.read()
@@ -124,29 +144,25 @@ 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="test-name" revision="ABCDEF"/>' +
+        '<project name="platform/art" path="art" revision="ABCDEF" ' +
+        'groups="notdefault,platform-' + self.platform + '"/>' +
+        '<superproject name="superproject"/>' +
         '</manifest>')
 
   def test_superproject_update_project_revision_id(self):
     """Test with LsTree being a mock."""
-    manifest = self.getXmlManifest("""
-<manifest>
-  <remote name="default-remote" fetch="http://localhost" />
-  <default remote="default-remote" revision="refs/heads/main" />
-  <project path="art" name="platform/art" />
-</manifest>
-""")
-    self.assertEqual(len(manifest.projects), 1)
-    projects = manifest.projects
+    self.assertEqual(len(self._superproject._manifest.projects), 1)
+    projects = self._superproject._manifest.projects
     data = ('160000 commit 2c2724cb36cd5a9cec6c852c681efc3b7c6b86ea\tart\x00'
             '160000 commit e9d25da64d8d365dbba7c8ee00fe8c4473fe9a06\tbootable/recovery\x00')
-    with mock.patch.object(self._superproject, '_Clone', return_value=True):
+    with mock.patch.object(self._superproject, '_Init', return_value=True):
       with mock.patch.object(self._superproject, '_Fetch', return_value=True):
-        with mock.patch.object(self._superproject, '_LsTree', return_value=data):
+        with mock.patch.object(self._superproject,
+                               '_LsTree',
+                               return_value=data):
           # Create temporary directory so that it can write the file.
           os.mkdir(self._superproject._superproject_path)
-          manifest_path = self._superproject.UpdateProjectsRevisionId(
-              manifest, projects, url='localhost')
+          manifest_path = self._superproject.UpdateProjectsRevisionId(projects)
           self.assertIsNotNone(manifest_path)
           with open(manifest_path, 'r') as fp:
             manifest_xml = fp.read()
@@ -156,7 +172,9 @@ 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
@@ -24,6 +25,73 @@ import error
 import manifest_xml
 
 
+# Invalid paths that we don't want in the filesystem.
+INVALID_FS_PATHS = (
+    '',
+    '.',
+    '..',
+    '../',
+    './',
+    './/',
+    'foo/',
+    './foo',
+    '../foo',
+    'foo/./bar',
+    'foo/../../bar',
+    '/foo',
+    './../foo',
+    '.git/foo',
+    # Check case folding.
+    '.GIT/foo',
+    'blah/.git/foo',
+    '.repo/foo',
+    '.repoconfig',
+    # Block ~ due to 8.3 filenames on Windows filesystems.
+    '~',
+    'foo~',
+    'blah/foo~',
+    # Block Unicode characters that get normalized out by filesystems.
+    u'foo\u200Cbar',
+)
+
+# Make sure platforms that use path separators (e.g. Windows) are also
+# rejected properly.
+if os.path.sep != '/':
+  INVALID_FS_PATHS += tuple(x.replace('/', os.path.sep) for x in INVALID_FS_PATHS)
+
+
+class ManifestParseTestCase(unittest.TestCase):
+  """TestCase for parsing manifests."""
+
+  def setUp(self):
+    self.tempdir = tempfile.mkdtemp(prefix='repo_tests')
+    self.repodir = os.path.join(self.tempdir, '.repo')
+    self.manifest_dir = os.path.join(self.repodir, 'manifests')
+    self.manifest_file = os.path.join(
+        self.repodir, manifest_xml.MANIFEST_FILE_NAME)
+    self.local_manifest_dir = os.path.join(
+        self.repodir, manifest_xml.LOCAL_MANIFESTS_DIR_NAME)
+    os.mkdir(self.repodir)
+    os.mkdir(self.manifest_dir)
+
+    # The manifest parsing really wants a git repo currently.
+    gitdir = os.path.join(self.repodir, 'manifests.git')
+    os.mkdir(gitdir)
+    with open(os.path.join(gitdir, 'config'), 'w') as fp:
+      fp.write("""[remote "origin"]
+        url = https://localhost:0/manifest
+""")
+
+  def tearDown(self):
+    shutil.rmtree(self.tempdir, ignore_errors=True)
+
+  def getXmlManifest(self, data):
+    """Helper to initialize a manifest for testing."""
+    with open(self.manifest_file, 'w') as fp:
+      fp.write(data)
+    return manifest_xml.XmlManifest(self.repodir, self.manifest_file)
+
+
 class ManifestValidateFilePaths(unittest.TestCase):
   """Check _ValidateFilePaths helper.
 
@@ -54,36 +122,7 @@ class ManifestValidateFilePaths(unittest.TestCase):
 
   def test_bad_paths(self):
     """Make sure bad paths (src & dest) are rejected."""
-    PATHS = (
-        '..',
-        '../',
-        './',
-        'foo/',
-        './foo',
-        '../foo',
-        'foo/./bar',
-        'foo/../../bar',
-        '/foo',
-        './../foo',
-        '.git/foo',
-        # Check case folding.
-        '.GIT/foo',
-        'blah/.git/foo',
-        '.repo/foo',
-        '.repoconfig',
-        # Block ~ due to 8.3 filenames on Windows filesystems.
-        '~',
-        'foo~',
-        'blah/foo~',
-        # Block Unicode characters that get normalized out by filesystems.
-        u'foo\u200Cbar',
-    )
-    # Make sure platforms that use path separators (e.g. Windows) are also
-    # rejected properly.
-    if os.path.sep != '/':
-      PATHS += tuple(x.replace('/', os.path.sep) for x in PATHS)
-
-    for path in PATHS:
+    for path in INVALID_FS_PATHS:
       self.assertRaises(
           error.ManifestInvalidPathError, self.check_both, path, 'a')
       self.assertRaises(
@@ -146,37 +185,9 @@ class ValueTests(unittest.TestCase):
       manifest_xml.XmlInt(node, 'a')
 
 
-class XmlManifestTests(unittest.TestCase):
+class XmlManifestTests(ManifestParseTestCase):
   """Check manifest processing."""
 
-  def setUp(self):
-    self.tempdir = tempfile.mkdtemp(prefix='repo_tests')
-    self.repodir = os.path.join(self.tempdir, '.repo')
-    self.manifest_dir = os.path.join(self.repodir, 'manifests')
-    self.manifest_file = os.path.join(
-        self.repodir, manifest_xml.MANIFEST_FILE_NAME)
-    self.local_manifest_dir = os.path.join(
-        self.repodir, manifest_xml.LOCAL_MANIFESTS_DIR_NAME)
-    os.mkdir(self.repodir)
-    os.mkdir(self.manifest_dir)
-
-    # The manifest parsing really wants a git repo currently.
-    gitdir = os.path.join(self.repodir, 'manifests.git')
-    os.mkdir(gitdir)
-    with open(os.path.join(gitdir, 'config'), 'w') as fp:
-      fp.write("""[remote "origin"]
-        url = https://localhost:0/manifest
-""")
-
-  def tearDown(self):
-    shutil.rmtree(self.tempdir, ignore_errors=True)
-
-  def getXmlManifest(self, data):
-    """Helper to initialize a manifest for testing."""
-    with open(self.manifest_file, 'w') as fp:
-      fp.write(data)
-    return manifest_xml.XmlManifest(self.repodir, self.manifest_file)
-
   def test_empty(self):
     """Parse an 'empty' manifest file."""
     manifest = self.getXmlManifest(
@@ -221,67 +232,6 @@ class XmlManifestTests(unittest.TestCase):
     self.assertEqual(manifest.repo_hooks_project.name, 'repohooks')
     self.assertEqual(manifest.repo_hooks_project.enabled_repo_hooks, ['a', 'b'])
 
-  def test_superproject(self):
-    """Check superproject settings."""
-    manifest = self.getXmlManifest("""
-<manifest>
-  <remote name="test-remote" fetch="http://localhost" />
-  <default remote="test-remote" revision="refs/heads/main" />
-  <superproject name="superproject"/>
-</manifest>
-""")
-    self.assertEqual(manifest.superproject['name'], 'superproject')
-    self.assertEqual(manifest.superproject['remote'].name, 'test-remote')
-    self.assertEqual(manifest.superproject['remote'].url, 'http://localhost/superproject')
-    self.assertEqual(
-        manifest.ToXml().toxml(),
-        '<?xml version="1.0" ?><manifest>' +
-        '<remote name="test-remote" fetch="http://localhost"/>' +
-        '<default remote="test-remote" revision="refs/heads/main"/>' +
-        '<superproject name="superproject"/>' +
-        '</manifest>')
-
-  def test_superproject_with_remote(self):
-    """Check superproject settings."""
-    manifest = self.getXmlManifest("""
-<manifest>
-  <remote name="default-remote" fetch="http://localhost" />
-  <remote name="superproject-remote" fetch="http://localhost" />
-  <default remote="default-remote" revision="refs/heads/main" />
-  <superproject name="platform/superproject" remote="superproject-remote"/>
-</manifest>
-""")
-    self.assertEqual(manifest.superproject['name'], 'platform/superproject')
-    self.assertEqual(manifest.superproject['remote'].name, 'superproject-remote')
-    self.assertEqual(manifest.superproject['remote'].url, 'http://localhost/platform/superproject')
-    self.assertEqual(
-        manifest.ToXml().toxml(),
-        '<?xml version="1.0" ?><manifest>' +
-        '<remote name="default-remote" fetch="http://localhost"/>' +
-        '<remote name="superproject-remote" fetch="http://localhost"/>' +
-        '<default remote="default-remote" revision="refs/heads/main"/>' +
-        '<superproject name="platform/superproject" remote="superproject-remote"/>' +
-        '</manifest>')
-
-  def test_superproject_with_defalut_remote(self):
-    """Check superproject settings."""
-    manifest = self.getXmlManifest("""
-<manifest>
-  <remote name="default-remote" fetch="http://localhost" />
-  <default remote="default-remote" revision="refs/heads/main" />
-  <superproject name="superproject" remote="default-remote"/>
-</manifest>
-""")
-    self.assertEqual(manifest.superproject['name'], 'superproject')
-    self.assertEqual(manifest.superproject['remote'].name, 'default-remote')
-    self.assertEqual(
-        manifest.ToXml().toxml(),
-        '<?xml version="1.0" ?><manifest>' +
-        '<remote name="default-remote" fetch="http://localhost"/>' +
-        '<default remote="default-remote" revision="refs/heads/main"/>' +
-        '<superproject name="superproject"/>' +
-        '</manifest>')
-
   def test_unknown_tags(self):
     """Check superproject settings."""
     manifest = self.getXmlManifest("""
@@ -303,51 +253,11 @@ class XmlManifestTests(unittest.TestCase):
         '<superproject name="superproject"/>' +
         '</manifest>')
 
-  def test_project_group(self):
-    """Check project group settings."""
-    manifest = self.getXmlManifest("""
-<manifest>
-  <remote name="test-remote" fetch="http://localhost" />
-  <default remote="test-remote" revision="refs/heads/main" />
-  <project name="test-name" path="test-path"/>
-  <project name="extras" path="path" groups="g1,g2,g1"/>
-</manifest>
-""")
-    self.assertEqual(len(manifest.projects), 2)
-    # Ordering isn't guaranteed.
-    result = {
-        manifest.projects[0].name: manifest.projects[0].groups,
-        manifest.projects[1].name: manifest.projects[1].groups,
-    }
-    project = manifest.projects[0]
-    self.assertCountEqual(
-        result['test-name'],
-        ['name:test-name', 'all', 'path:test-path'])
-    self.assertCountEqual(
-        result['extras'],
-        ['g1', 'g2', 'g1', 'name:extras', 'all', 'path:path'])
 
-  def test_project_set_revision_id(self):
-    """Check setting of project's revisionId."""
-    manifest = self.getXmlManifest("""
-<manifest>
-  <remote name="default-remote" fetch="http://localhost" />
-  <default remote="default-remote" revision="refs/heads/main" />
-  <project name="test-name"/>
-</manifest>
-""")
-    self.assertEqual(len(manifest.projects), 1)
-    project = manifest.projects[0]
-    project.SetRevisionId('ABCDEF')
-    self.assertEqual(
-        manifest.ToXml().toxml(),
-        '<?xml version="1.0" ?><manifest>' +
-        '<remote name="default-remote" fetch="http://localhost"/>' +
-        '<default remote="default-remote" revision="refs/heads/main"/>' +
-        '<project name="test-name" revision="ABCDEF"/>' +
-        '</manifest>')
+class IncludeElementTests(ManifestParseTestCase):
+  """Tests for <include>."""
 
-  def test_include_levels(self):
+  def test_group_levels(self):
     root_m = os.path.join(self.manifest_dir, 'root.xml')
     with open(root_m, 'w') as fp:
       fp.write("""
@@ -389,3 +299,251 @@ class XmlManifestTests(unittest.TestCase):
         self.assertIn('level2-group', proj.groups)
         # Check level2 proj group not removed.
         self.assertIn('l2g1', proj.groups)
+
+  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>
+  <remote name="default-remote" fetch="http://localhost" />
+  <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()
+
+    # Handle empty name explicitly because a different codepath rejects it.
+    with self.assertRaises(error.ManifestParseError):
+      parse('')
+
+    for path in INVALID_FS_PATHS:
+      if not path:
+        continue
+
+      with self.assertRaises(error.ManifestInvalidPathError):
+        parse(path)
+
+
+class ProjectElementTests(ManifestParseTestCase):
+  """Tests for <project>."""
+
+  def test_group(self):
+    """Check project group settings."""
+    manifest = self.getXmlManifest("""
+<manifest>
+  <remote name="test-remote" fetch="http://localhost" />
+  <default remote="test-remote" revision="refs/heads/main" />
+  <project name="test-name" path="test-path"/>
+  <project name="extras" path="path" groups="g1,g2,g1"/>
+</manifest>
+""")
+    self.assertEqual(len(manifest.projects), 2)
+    # Ordering isn't guaranteed.
+    result = {
+        manifest.projects[0].name: manifest.projects[0].groups,
+        manifest.projects[1].name: manifest.projects[1].groups,
+    }
+    project = manifest.projects[0]
+    self.assertCountEqual(
+        result['test-name'],
+        ['name:test-name', 'all', 'path:test-path'])
+    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."""
+    manifest = self.getXmlManifest("""
+<manifest>
+  <remote name="default-remote" fetch="http://localhost" />
+  <default remote="default-remote" revision="refs/heads/main" />
+  <project name="test-name"/>
+</manifest>
+""")
+    self.assertEqual(len(manifest.projects), 1)
+    project = manifest.projects[0]
+    project.SetRevisionId('ABCDEF')
+    self.assertEqual(
+        manifest.ToXml().toxml(),
+        '<?xml version="1.0" ?><manifest>' +
+        '<remote name="default-remote" fetch="http://localhost"/>' +
+        '<default remote="default-remote" revision="refs/heads/main"/>' +
+        '<project name="test-name" revision="ABCDEF"/>' +
+        '</manifest>')
+
+  def test_trailing_slash(self):
+    """Check handling of trailing slashes in attributes."""
+    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>
+""")
+
+    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'))
+
+    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'))
+
+    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):
+      manifest = 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>
+""")
+      # Force the manifest to be parsed.
+      manifest.ToXml()
+
+    # Verify the parser is valid by default to avoid buggy tests below.
+    parse('ok', 'ok')
+
+    # Handle empty name explicitly because a different codepath rejects it.
+    # Empty path is OK because it defaults to the name field.
+    with self.assertRaises(error.ManifestParseError):
+      parse('', 'ok')
+
+    for path in INVALID_FS_PATHS:
+      if not path or path.endswith('/'):
+        continue
+
+      with self.assertRaises(error.ManifestInvalidPathError):
+        parse(path, 'ok')
+
+      # We have a dedicated test for path=".".
+      if path not in {'.'}:
+        with self.assertRaises(error.ManifestInvalidPathError):
+          parse('ok', path)
+
+
+class SuperProjectElementTests(ManifestParseTestCase):
+  """Tests for <superproject>."""
+
+  def test_superproject(self):
+    """Check superproject settings."""
+    manifest = self.getXmlManifest("""
+<manifest>
+  <remote name="test-remote" fetch="http://localhost" />
+  <default remote="test-remote" revision="refs/heads/main" />
+  <superproject name="superproject"/>
+</manifest>
+""")
+    self.assertEqual(manifest.superproject['name'], 'superproject')
+    self.assertEqual(manifest.superproject['remote'].name, 'test-remote')
+    self.assertEqual(manifest.superproject['remote'].url, 'http://localhost/superproject')
+    self.assertEqual(
+        manifest.ToXml().toxml(),
+        '<?xml version="1.0" ?><manifest>' +
+        '<remote name="test-remote" fetch="http://localhost"/>' +
+        '<default remote="test-remote" revision="refs/heads/main"/>' +
+        '<superproject name="superproject"/>' +
+        '</manifest>')
+
+  def test_remote(self):
+    """Check superproject settings with a remote."""
+    manifest = self.getXmlManifest("""
+<manifest>
+  <remote name="default-remote" fetch="http://localhost" />
+  <remote name="superproject-remote" fetch="http://localhost" />
+  <default remote="default-remote" revision="refs/heads/main" />
+  <superproject name="platform/superproject" remote="superproject-remote"/>
+</manifest>
+""")
+    self.assertEqual(manifest.superproject['name'], 'platform/superproject')
+    self.assertEqual(manifest.superproject['remote'].name, 'superproject-remote')
+    self.assertEqual(manifest.superproject['remote'].url, 'http://localhost/platform/superproject')
+    self.assertEqual(
+        manifest.ToXml().toxml(),
+        '<?xml version="1.0" ?><manifest>' +
+        '<remote name="default-remote" fetch="http://localhost"/>' +
+        '<remote name="superproject-remote" fetch="http://localhost"/>' +
+        '<default remote="default-remote" revision="refs/heads/main"/>' +
+        '<superproject name="platform/superproject" remote="superproject-remote"/>' +
+        '</manifest>')
+
+  def test_defalut_remote(self):
+    """Check superproject settings with a default remote."""
+    manifest = self.getXmlManifest("""
+<manifest>
+  <remote name="default-remote" fetch="http://localhost" />
+  <default remote="default-remote" revision="refs/heads/main" />
+  <superproject name="superproject" remote="default-remote"/>
+</manifest>
+""")
+    self.assertEqual(manifest.superproject['name'], 'superproject')
+    self.assertEqual(manifest.superproject['remote'].name, 'default-remote')
+    self.assertEqual(
+        manifest.ToXml().toxml(),
+        '<?xml version="1.0" ?><manifest>' +
+        '<remote name="default-remote" fetch="http://localhost"/>' +
+        '<default remote="default-remote" revision="refs/heads/main"/>' +
+        '<superproject name="superproject"/>' +
+        '</manifest>')

tests/test_project.py

@@ -46,7 +46,7 @@ def TempGitTree():
       templatedir = tempfile.mkdtemp(prefix='.test-template')
       with open(os.path.join(templatedir, 'HEAD'), 'w') as fp:
         fp.write('ref: refs/heads/main\n')
-      cmd += ['--template=', templatedir]
+      cmd += ['--template', templatedir]
     subprocess.check_call(cmd, cwd=tempdir)
     yield tempdir
   finally:

tests/test_subcmds_init.py

@@ -38,7 +38,7 @@ class InitCommand(unittest.TestCase):
     """Check invalid command line options."""
     ARGV = (
         # Too many arguments.
-        ['asdf'],
+        ['url', 'asdf'],
 
         # Conflicting options.
         ['--mirror', '--archive'],

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()

tox.ini

@@ -15,13 +15,15 @@
 # https://tox.readthedocs.io/
 
 [tox]
-envlist = py36, py37, py38
+envlist = py35, py36, py37, py38, py39
 
 [gh-actions]
 python =
+    3.5: py35
     3.6: py36
     3.7: py37
     3.8: py38
+    3.9: py39
 
 [testenv]
 deps = pytest