project: Rename if deletion fails

If a project contains files not owned by the current user, remove will
fail. In order to ensure repo sync continues to work, rename the
affected project instead, and let user know about it.

Bug: 321273512
Change-Id: I0779d61fc67042308a0226adea7d98167252a5d3
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/404372
Reviewed-by: Mike Frysinger <vapier@google.com>
Tested-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>

.flake8

@@ -11,3 +11,6 @@ extend-ignore =
     E402,
     # E731: do not assign a lambda expression, use a def
     E731,
+exclude =
+    venv,
+    .tox,

.github/workflows/close-pull-request.yml

@@ -0,0 +1,22 @@
+# GitHub actions workflow.
+# https://docs.github.com/en/actions/learn-github-actions/workflow-syntax-for-github-actions
+
+# https://github.com/superbrothers/close-pull-request
+name: Close Pull Request
+
+on:
+  pull_request_target:
+    types: [opened]
+
+jobs:
+  run:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: superbrothers/close-pull-request@v3
+      with:
+        comment: >
+          Thanks for your contribution!
+          Unfortunately, we don't use GitHub pull requests to manage code
+          contributions to this repository.
+          Instead, please see [README.md](../blob/HEAD/SUBMITTING_PATCHES.md)
+          which provides full instructions on how to get involved.

.github/workflows/test-ci.yml

@@ -13,8 +13,9 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        os: [ubuntu-latest, macos-latest, windows-latest]
-        python-version: ['3.6', '3.7', '3.8', '3.9', '3.10']
+        # ubuntu-20.04 is the last version that supports python 3.6
+        os: [ubuntu-20.04, macos-latest, windows-latest]
+        python-version: ['3.6', '3.7', '3.8', '3.9', '3.10', '3.11', '3.12']
     runs-on: ${{ matrix.os }}
 
     steps:

.isort.cfg

@@ -0,0 +1,41 @@
+# Copyright 2023 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.
+
+# Config file for the isort python module.
+# This is used to enforce import sorting standards.
+#
+# https://pycqa.github.io/isort/docs/configuration/options.html
+
+[settings]
+# Be compatible with `black` since it also matches what we want.
+profile = black
+
+line_length = 80
+length_sort = false
+force_single_line = true
+lines_after_imports = 2
+from_first = false
+case_sensitive = false
+force_sort_within_sections = true
+order_by_type = false
+
+# Ignore generated files.
+extend_skip_glob = *_pb2.py
+
+# Allow importing multiple classes on a single line from these modules.
+# https://google.github.io/styleguide/pyguide#s2.2-imports
+single_line_exclusions =
+    abc,
+    collections.abc,
+    typing,

README.md

@@ -8,7 +8,7 @@ that you can put anywhere in your path.
 
 * Homepage: <https://gerrit.googlesource.com/git-repo/>
 * Mailing list: [repo-discuss on Google Groups][repo-discuss]
-* Bug reports: <https://bugs.chromium.org/p/gerrit/issues/list?q=component:Applications%3Erepo>
+* Bug reports: <https://issues.gerritcodereview.com/issues?q=is:open%20componentid:1370071>
 * Source: <https://gerrit.googlesource.com/git-repo/>
 * Overview: <https://source.android.com/source/developing.html>
 * Docs: <https://source.android.com/source/using-repo.html>
@@ -50,6 +50,6 @@ $ chmod a+rx ~/.bin/repo
 ```
 
 
-[new-bug]: https://bugs.chromium.org/p/gerrit/issues/entry?template=Repo+tool+issue
-[issue tracker]: https://bugs.chromium.org/p/gerrit/issues/list?q=component:Applications%3Erepo
+[new-bug]: https://issues.gerritcodereview.com/issues/new?component=1370071
+[issue tracker]: https://issues.gerritcodereview.com/issues?q=is:open%20componentid:1370071
 [repo-discuss]: https://groups.google.com/forum/#!forum/repo-discuss

color.py

@@ -17,6 +17,7 @@ import sys
 
 import pager
 
+
 COLORS = {
     None: -1,
     "normal": -1,
@@ -102,7 +103,7 @@ def SetDefaultColoring(state):
         DEFAULT = "never"
 
 
-class Coloring(object):
+class Coloring:
     def __init__(self, config, section_type):
         self._section = "color.%s" % section_type
         self._config = config
@@ -193,7 +194,7 @@ class Coloring(object):
         if not opt:
             return _Color(fg, bg, attr)
 
-        v = self._config.GetString("%s.%s" % (self._section, opt))
+        v = self._config.GetString(f"{self._section}.{opt}")
         if v is None:
             return _Color(fg, bg, attr)
 

command.py

@@ -13,14 +13,14 @@
 # limitations under the License.
 
 import multiprocessing
-import os
 import optparse
+import os
 import re
-import sys
 
-from event_log import EventLog
-from error import NoSuchProjectError
 from error import InvalidProjectGroupsError
+from error import NoSuchProjectError
+from error import RepoExitError
+from event_log import EventLog
 import progress
 
 
@@ -42,7 +42,11 @@ WORKER_BATCH_SIZE = 32
 DEFAULT_LOCAL_JOBS = min(os.cpu_count(), 8)
 
 
-class Command(object):
+class UsageError(RepoExitError):
+    """Exception thrown with invalid command usage."""
+
+
+class Command:
     """Base class for any command line action in repo."""
 
     # Singleton for all commands to track overall repo command execution and
@@ -71,7 +75,6 @@ class Command(object):
         repodir=None,
         client=None,
         manifest=None,
-        gitc_manifest=None,
         git_event_log=None,
         outer_client=None,
         outer_manifest=None,
@@ -80,7 +83,6 @@ class Command(object):
         self.client = client
         self.outer_client = outer_client or client
         self.manifest = manifest
-        self.gitc_manifest = gitc_manifest
         self.git_event_log = git_event_log
         self.outer_manifest = outer_manifest
 
@@ -215,7 +217,7 @@ class Command(object):
     def Usage(self):
         """Display usage and terminate."""
         self.OptionParser.print_usage()
-        sys.exit(1)
+        raise UsageError()
 
     def CommonValidateOptions(self, opt, args):
         """Validate common options."""
@@ -288,7 +290,7 @@ class Command(object):
                 output.end()
 
     def _ResetPathToProjectMap(self, projects):
-        self._by_path = dict((p.worktree, p) for p in projects)
+        self._by_path = {p.worktree: p for p in projects}
 
     def _UpdatePathToProjectMap(self, project):
         self._by_path[project.worktree] = project
@@ -474,8 +476,7 @@ class Command(object):
             top = self.manifest
         yield top
         if not opt.this_manifest_only:
-            for child in top.all_children:
-                yield child
+            yield from top.all_children
 
 
 class InteractiveCommand(Command):
@@ -496,17 +497,11 @@ class PagedCommand(Command):
         return True
 
 
-class MirrorSafeCommand(object):
+class MirrorSafeCommand:
     """Command permits itself to run within a mirror, and does not require a
     working directory.
     """
 
 
-class GitcAvailableCommand(object):
-    """Command that requires GITC to be available, but does not require the
-    local client to be a GITC client.
-    """
-
-
-class GitcClientCommand(object):
+class GitcClientCommand:
     """Command that requires the local client to be a GITC client."""

docs/internal-fs-layout.md

@@ -42,8 +42,12 @@ For example, if you want to change the manifest branch, you can simply run
     change the git URL/branch that this tracks, re-run `repo init` with the new
     settings.
 
-*   `.repo_fetchtimes.json`: Used by `repo sync` to record stats when syncing
-    the various projects.
+*   `.repo_fetchtimes.json`: Used by `repo sync` to record fetch times when
+    syncing the various projects.
+
+*   `.repo_localsyncstate.json`: Used by `repo sync` to detect and warn on
+    on partial tree syncs.  Partial syncs are allowed by `repo` itself, but are
+    unsupported by many projects where `repo` is used.
 
 ### Manifests
 

docs/manifest-format.md

@@ -109,8 +109,9 @@ following DTD:
   <!ATTLIST extend-project upstream CDATA #IMPLIED>
 
   <!ELEMENT remove-project EMPTY>
-  <!ATTLIST remove-project name  CDATA #REQUIRED>
-  <!ATTLIST remove-project optional  CDATA #IMPLIED>
+  <!ATTLIST remove-project name     CDATA #IMPLIED>
+  <!ATTLIST remove-project path     CDATA #IMPLIED>
+  <!ATTLIST remove-project optional CDATA #IMPLIED>
 
   <!ELEMENT repo-hooks EMPTY>
   <!ATTLIST repo-hooks in-project CDATA #REQUIRED>
@@ -473,7 +474,7 @@ of the repo client.
 
 ### Element remove-project
 
-Deletes the named project from the internal manifest table, possibly
+Deletes a project from the internal manifest table, possibly
 allowing a subsequent project element in the same manifest file to
 replace the project with a different source.
 
@@ -481,6 +482,17 @@ This element is mostly useful in a local manifest file, where
 the user can remove a project, and possibly replace it with their
 own definition.
 
+The project `name` or project `path` can be used to specify the remove target
+meaning one of them is required. If only name is specified, all
+projects with that name are removed.
+
+If both name and path are specified, only projects with the same name and
+path are removed, meaning projects with the same name but in other
+locations are kept.
+
+If only path is specified, a matching project is removed regardless of its
+name. Logic otherwise behaves like both are specified.
+
 Attribute `optional`: Set to true to ignore remove-project elements with no
 matching `project` element.
 

docs/python-support.md

@@ -1,47 +1,92 @@
 # Supported Python Versions
 
-With Python 2.7 officially going EOL on [01 Jan 2020](https://pythonclock.org/),
-we need a support plan for the repo project itself.
-Inevitably, there will be a long tail of users who still want to use Python 2 on
-their old LTS/corp systems and have little power to change the system.
+This documents the current supported Python versions, and tries to provide
+guidance for when we decide to drop support for older versions.
 
 ## Summary
 
-* Python 3.6 (released Dec 2016) is required by default starting with repo-2.x.
-* Older versions of Python (e.g. v2.7) may use the legacy feature-frozen branch
-  based on repo-1.x.
+*   Python 3.6 (released Dec 2016) is required starting with repo-2.0.
+*   Older versions of Python (e.g. v2.7) may use old releases via the repo-1.x
+    branch, but no support is provided.
 
-## Overview
-
-We provide a branch for Python 2 users that is feature-frozen.
-Bugfixes may be added on a best-effort basis or from the community, but largely
-no new features will be added, nor is support guaranteed.
-
-Users can select this during `repo init` time via the [repo launcher].
-Otherwise the default branches (e.g. stable & main) will be used which will
-require Python 3.
-
-This means the [repo launcher] needs to support both Python 2 & Python 3, but
-since it doesn't import any other repo code, this shouldn't be too problematic.
-
-The main branch will require Python 3.6 at a minimum.
-If the system has an older version of Python 3, then users will have to select
-the legacy Python 2 branch instead.
-
-### repo hooks
+## repo hooks
 
 Projects that use [repo hooks] run on independent schedules.
-They might migrate to Python 3 earlier or later than us.
-To support them, we'll probe the shebang of the hook script and if we find an
-interpreter in there that indicates a different version than repo is currently
-running under, we'll attempt to reexec ourselves under that.
+Since it's not possible to detect what version of Python the hooks were written
+or tested against, we always import & exec them with the active Python version.
 
-For example, a hook with a header like `#!/usr/bin/python2` will have repo
-execute `/usr/bin/python2` to execute the hook code specifically if repo is
-currently running Python 3.
+If the user's Python is too new for the [repo hooks], then it is up to the hooks
+maintainer to update.
 
-For more details, consult the [repo hooks] documentation.
+## Repo launcher
+
+The [repo launcher] is an independent script that can support older versions of
+Python without holding back the rest of the codebase.
+If it detects the current version of Python is too old, it will try to reexec
+via a newer version of Python via standard `pythonX.Y` interpreter names.
+
+However, this is provided as a nicety when it is not onerous, and there is no
+official support for older versions of Python than the rest of the codebase.
+
+If your default python interpreters are too old to run the launcher even though
+you have newer versions installed, your choices are:
+
+*   Modify the [repo launcher]'s shebang to suite your environment.
+*   Download an older version of the [repo launcher] and don't upgrade it.
+    Be aware that we do not guarantee old repo launchers will work with current
+    versions of repo.  Bug reports using old launchers will not be accepted.
+
+## When to drop support
+
+So far, Python 3.6 has provided most of the interesting features that we want
+(e.g. typing & f-strings), and there haven't been features in newer versions
+that are critical to us.
+
+That said, let's assume we need functionality that only exists in Python 3.7.
+How do we decide when it's acceptable to drop Python 3.6?
+
+1.  Review the [Project References](./release-process.md#project-references) to
+    see what major distros are using the previous version of Python, and when
+    they go EOL.  Generally we care about Ubuntu LTS & current/previous Debian
+    stable versions.
+    *   If they're all EOL already, then go for it, drop support.
+    *   If they aren't EOL, start a thread on [repo-discuss] to see how the user
+        base feels about the proposal.
+1.  Update the "soft" versions in the codebase.  This will start warning users
+    that the older version is deprecated.
+    *   Update [repo](/repo) if the launcher needs updating.
+        This only helps with people who download newer launchers.
+    *   Update [main.py](/main.py) for the main codebase.
+        This warns for everyone regardless of [repo launcher] version.
+    *   Update [requirements.json](/requirements.json).
+        This allows [repo launcher] to display warnings/errors without having
+        to execute the new codebase.  This helps in case of syntax or module
+        changes where older versions won't even be able to import the new code.
+1.  After some grace period (ideally at least 2 quarters after the first release
+    with the updated soft requirements), update the "hard" versions, and then
+    start using the new functionality.
+
+## Python 2.7 & 3.0-3.5
+
+> **There is no support for these versions.**
+> **Do not file bugs if you are using old Python versions.**
+> **Any such reports will be marked invalid and ignored.**
+> **Upgrade your distro and/or runtime instead.**
+
+Fetch an old version of the [repo launcher]:
+
+```sh
+$ curl https://storage.googleapis.com/git-repo-downloads/repo-2.32 > ~/.bin/repo-2.32
+$ chmod a+rx ~/.bin/repo-2.32
+```
+
+Then initialize an old version of repo:
+
+```sh
+$ repo-2.32 init --repo-rev=repo-1 ...
+```
 
 
+[repo-discuss]: https://groups.google.com/forum/#!forum/repo-discuss
 [repo hooks]: ./repo-hooks.md
 [repo launcher]: ../repo

editor.py

@@ -14,15 +14,15 @@
 
 import os
 import re
-import sys
 import subprocess
+import sys
 import tempfile
 
 from error import EditorError
 import platform_utils
 
 
-class Editor(object):
+class Editor:
     """Manages the user's preferred text editor."""
 
     _editor = None
@@ -104,9 +104,7 @@ least one of these before using this command.""",  # noqa: E501
             try:
                 rc = subprocess.Popen(args, shell=shell).wait()
             except OSError as e:
-                raise EditorError(
-                    "editor failed, %s: %s %s" % (str(e), editor, path)
-                )
+                raise EditorError(f"editor failed, {str(e)}: {editor} {path}")
             if rc != 0:
                 raise EditorError(
                     "editor failed with exit status %d: %s %s"

error.py

@@ -12,8 +12,55 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+from typing import List
 
-class ManifestParseError(Exception):
+
+class BaseRepoError(Exception):
+    """All repo specific exceptions derive from BaseRepoError."""
+
+
+class RepoError(BaseRepoError):
+    """Exceptions thrown inside repo that can be handled."""
+
+    def __init__(self, *args, project: str = None) -> None:
+        super().__init__(*args)
+        self.project = project
+
+
+class RepoExitError(BaseRepoError):
+    """Exception thrown that result in termination of repo program.
+    - Should only be handled in main.py
+    """
+
+    def __init__(
+        self,
+        *args,
+        exit_code: int = 1,
+        aggregate_errors: List[Exception] = None,
+        **kwargs,
+    ) -> None:
+        super().__init__(*args, **kwargs)
+        self.exit_code = exit_code
+        self.aggregate_errors = aggregate_errors
+
+
+class RepoUnhandledExceptionError(RepoExitError):
+    """Exception that maintains error as reason for program exit."""
+
+    def __init__(
+        self,
+        error: BaseException,
+        **kwargs,
+    ) -> None:
+        super().__init__(error, **kwargs)
+        self.error = error
+
+
+class SilentRepoExitError(RepoExitError):
+    """RepoExitError that should no include CLI logging of issue/issues."""
+
+
+class ManifestParseError(RepoExitError):
     """Failed to parse the manifest file."""
 
 
@@ -25,11 +72,11 @@ class ManifestInvalidPathError(ManifestParseError):
     """A path used in <copyfile> or <linkfile> is incorrect."""
 
 
-class NoManifestException(Exception):
+class NoManifestException(RepoExitError):
     """The required manifest does not exist."""
 
-    def __init__(self, path, reason):
-        super().__init__(path, reason)
+    def __init__(self, path, reason, **kwargs):
+        super().__init__(path, reason, **kwargs)
         self.path = path
         self.reason = reason
 
@@ -37,55 +84,72 @@ class NoManifestException(Exception):
         return self.reason
 
 
-class EditorError(Exception):
+class EditorError(RepoError):
     """Unspecified error from the user's text editor."""
 
-    def __init__(self, reason):
-        super().__init__(reason)
+    def __init__(self, reason, **kwargs):
+        super().__init__(reason, **kwargs)
         self.reason = reason
 
     def __str__(self):
         return self.reason
 
 
-class GitError(Exception):
-    """Unspecified internal error from git."""
+class GitError(RepoError):
+    """Unspecified git related error."""
 
-    def __init__(self, command):
-        super().__init__(command)
-        self.command = command
+    def __init__(self, message, command_args=None, **kwargs):
+        super().__init__(message, **kwargs)
+        self.message = message
+        self.command_args = command_args
 
     def __str__(self):
-        return self.command
+        return self.message
 
 
-class UploadError(Exception):
+class GitcUnsupportedError(RepoExitError):
+    """Gitc no longer supported."""
+
+
+class UploadError(RepoError):
     """A bundle upload to Gerrit did not succeed."""
 
-    def __init__(self, reason):
-        super().__init__(reason)
+    def __init__(self, reason, **kwargs):
+        super().__init__(reason, **kwargs)
         self.reason = reason
 
     def __str__(self):
         return self.reason
 
 
-class DownloadError(Exception):
+class DownloadError(RepoExitError):
     """Cannot download a repository."""
 
-    def __init__(self, reason):
-        super().__init__(reason)
+    def __init__(self, reason, **kwargs):
+        super().__init__(reason, **kwargs)
         self.reason = reason
 
     def __str__(self):
         return self.reason
 
 
-class NoSuchProjectError(Exception):
+class InvalidArgumentsError(RepoExitError):
+    """Invalid command Arguments."""
+
+
+class SyncError(RepoExitError):
+    """Cannot sync repo."""
+
+
+class UpdateManifestError(RepoExitError):
+    """Cannot update manifest."""
+
+
+class NoSuchProjectError(RepoExitError):
     """A specified project does not exist in the work tree."""
 
-    def __init__(self, name=None):
-        super().__init__(name)
+    def __init__(self, name=None, **kwargs):
+        super().__init__(**kwargs)
         self.name = name
 
     def __str__(self):
@@ -94,11 +158,11 @@ class NoSuchProjectError(Exception):
         return self.name
 
 
-class InvalidProjectGroupsError(Exception):
+class InvalidProjectGroupsError(RepoExitError):
     """A specified project is not suitable for the specified groups"""
 
-    def __init__(self, name=None):
-        super().__init__(name)
+    def __init__(self, name=None, **kwargs):
+        super().__init__(**kwargs)
         self.name = name
 
     def __str__(self):
@@ -107,7 +171,7 @@ class InvalidProjectGroupsError(Exception):
         return self.name
 
 
-class RepoChangedException(Exception):
+class RepoChangedException(BaseRepoError):
     """Thrown if 'repo sync' results in repo updating its internal
     repo or manifest repositories.  In this special case we must
     use exec to re-execute repo with the new code and manifest.
@@ -118,7 +182,7 @@ class RepoChangedException(Exception):
         self.extra_args = extra_args or []
 
 
-class HookError(Exception):
+class HookError(RepoError):
     """Thrown if a 'repo-hook' could not be run.
 
     The common case is that the file wasn't present when we tried to run it.

event_log.py

@@ -15,12 +15,13 @@
 import json
 import multiprocessing
 
+
 TASK_COMMAND = "command"
 TASK_SYNC_NETWORK = "sync-network"
 TASK_SYNC_LOCAL = "sync-local"
 
 
-class EventLog(object):
+class EventLog:
     """Event log that records events that occurred during a repo invocation.
 
     Events are written to the log as a consecutive JSON entries, one per line.

fetch.py

@@ -19,6 +19,12 @@ import sys
 from urllib.parse import urlparse
 from urllib.request import urlopen
 
+from error import RepoExitError
+
+
+class FetchFileError(RepoExitError):
+    """Exit error when fetch_file fails."""
+
 
 def fetch_file(url, verbose=False):
     """Fetch a file from the specified source using the appropriate protocol.
@@ -29,6 +35,7 @@ def fetch_file(url, verbose=False):
     scheme = urlparse(url).scheme
     if scheme == "gs":
         cmd = ["gsutil", "cat", url]
+        errors = []
         try:
             result = subprocess.run(
                 cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, check=True
@@ -41,9 +48,10 @@ def fetch_file(url, verbose=False):
                 )
             return result.stdout
         except subprocess.CalledProcessError as e:
+            errors.append(e)
             print(
                 'fatal: error running "gsutil": %s' % e.stderr, file=sys.stderr
             )
-        sys.exit(1)
+        raise FetchFileError(aggregate_errors=errors)
     with urlopen(url) as f:
         return f.read()

git_command.py

@@ -13,17 +13,25 @@
 # limitations under the License.
 
 import functools
+import json
 import os
-import sys
+import re
 import subprocess
+import sys
 from typing import Any, Optional
 
 from error import GitError
+from error import RepoExitError
 from git_refs import HEAD
+from git_trace2_event_log_base import BaseEventLog
 import platform_utils
-from repo_trace import REPO_TRACE, IsTrace, Trace
+from repo_logging import RepoLogger
+from repo_trace import IsTrace
+from repo_trace import REPO_TRACE
+from repo_trace import Trace
 from wrapper import Wrapper
 
+
 GIT = "git"
 # NB: These do not need to be kept in sync with the repo launcher script.
 # These may be much newer as it allows the repo launcher to roll between
@@ -40,15 +48,24 @@ GIT_DIR = "GIT_DIR"
 
 LAST_GITDIR = None
 LAST_CWD = None
+DEFAULT_GIT_FAIL_MESSAGE = "git command failure"
+ERROR_EVENT_LOGGING_PREFIX = "RepoGitCommandError"
+# Common line length limit
+GIT_ERROR_STDOUT_LINES = 1
+GIT_ERROR_STDERR_LINES = 10
+INVALID_GIT_EXIT_CODE = 126
+
+logger = RepoLogger(__file__)
 
 
-class _GitCall(object):
+class _GitCall:
     @functools.lru_cache(maxsize=None)
     def version_tuple(self):
         ret = Wrapper().ParseGitVersion()
         if ret is None:
-            print("fatal: unable to detect git version", file=sys.stderr)
-            sys.exit(1)
+            msg = "fatal: unable to detect git version"
+            logger.error(msg)
+            raise GitRequireError(msg)
         return ret
 
     def __getattr__(self, name):
@@ -57,7 +74,7 @@ class _GitCall(object):
         def fun(*cmdv):
             command = [name]
             command.extend(cmdv)
-            return GitCommand(None, command).Wait() == 0
+            return GitCommand(None, command, add_event_log=False).Wait() == 0
 
         return fun
 
@@ -95,7 +112,43 @@ def RepoSourceVersion():
     return ver
 
 
-class UserAgent(object):
+@functools.lru_cache(maxsize=None)
+def GetEventTargetPath():
+    """Get the 'trace2.eventtarget' path from git configuration.
+
+    Returns:
+        path: git config's 'trace2.eventtarget' path if it exists, or None
+    """
+    path = None
+    cmd = ["config", "--get", "trace2.eventtarget"]
+    # TODO(https://crbug.com/gerrit/13706): Use GitConfig when it supports
+    # system git config variables.
+    p = GitCommand(
+        None,
+        cmd,
+        capture_stdout=True,
+        capture_stderr=True,
+        bare=True,
+        add_event_log=False,
+    )
+    retval = p.Wait()
+    if retval == 0:
+        # Strip trailing carriage-return in path.
+        path = p.stdout.rstrip("\n")
+    elif retval != 1:
+        # `git config --get` is documented to produce an exit status of `1`
+        # if the requested variable is not present in the configuration.
+        # Report any other return value as an error.
+        logger.error(
+            "repo: error: 'git config --get' call failed with return code: "
+            "%r, stderr: %r",
+            retval,
+            p.stderr,
+        )
+    return path
+
+
+class UserAgent:
     """Mange User-Agent settings when talking to external services
 
     We follow the style as documented here:
@@ -143,12 +196,10 @@ class UserAgent(object):
     def git(self):
         """The UA when running git."""
         if self._git_ua is None:
-            self._git_ua = "git/%s (%s) git-repo/%s" % (
-                git.version_tuple().full,
-                self.os,
-                RepoSourceVersion(),
+            self._git_ua = (
+                f"git/{git.version_tuple().full} ({self.os}) "
+                f"git-repo/{RepoSourceVersion()}"
             )
-
         return self._git_ua
 
 
@@ -163,10 +214,9 @@ def git_require(min_version, fail=False, msg=""):
         need = ".".join(map(str, min_version))
         if msg:
             msg = " for " + msg
-        print(
-            "fatal: git %s or later required%s" % (need, msg), file=sys.stderr
-        )
-        sys.exit(1)
+        error_msg = f"fatal: git {need} or later required{msg}"
+        logger.error(error_msg)
+        raise GitRequireError(error_msg)
     return False
 
 
@@ -191,7 +241,7 @@ def _build_env(
         env["GIT_SSH"] = ssh_proxy.proxy
         env["GIT_SSH_VARIANT"] = "ssh"
     if "http_proxy" in env and "darwin" == sys.platform:
-        s = "'http.proxy=%s'" % (env["http_proxy"],)
+        s = f"'http.proxy={env['http_proxy']}'"
         p = env.get("GIT_CONFIG_PARAMETERS")
         if p is not None:
             s = p + " " + s
@@ -220,7 +270,7 @@ def _build_env(
     return env
 
 
-class GitCommand(object):
+class GitCommand:
     """Wrapper around a single git invocation."""
 
     def __init__(
@@ -237,6 +287,9 @@ class GitCommand(object):
         cwd=None,
         gitdir=None,
         objdir=None,
+        verify_command=False,
+        add_event_log=True,
+        log_as_error=True,
     ):
         if project:
             if not cwd:
@@ -244,6 +297,11 @@ class GitCommand(object):
             if not gitdir:
                 gitdir = project.gitdir
 
+        self.project = project
+        self.cmdv = cmdv
+        self.verify_command = verify_command
+        self.stdout, self.stderr = None, None
+
         # Git on Windows wants its paths only using / for reliability.
         if platform_utils.isWindows():
             if objdir:
@@ -262,15 +320,67 @@ class GitCommand(object):
         command = [GIT]
         if bare:
             cwd = None
-        command.append(cmdv[0])
+        command_name = cmdv[0]
+        command.append(command_name)
         # Need to use the --progress flag for fetch/clone so output will be
         # displayed as by default git only does progress output if stderr is a
         # TTY.
-        if sys.stderr.isatty() and cmdv[0] in ("fetch", "clone"):
+        if sys.stderr.isatty() and command_name in ("fetch", "clone"):
             if "--progress" not in cmdv and "--quiet" not in cmdv:
                 command.append("--progress")
         command.extend(cmdv[1:])
 
+        event_log = (
+            BaseEventLog(env=env, add_init_count=True)
+            if add_event_log
+            else None
+        )
+
+        try:
+            self._RunCommand(
+                command,
+                env,
+                capture_stdout=capture_stdout,
+                capture_stderr=capture_stderr,
+                merge_output=merge_output,
+                ssh_proxy=ssh_proxy,
+                cwd=cwd,
+                input=input,
+            )
+            self.VerifyCommand()
+        except GitCommandError as e:
+            if event_log is not None:
+                error_info = json.dumps(
+                    {
+                        "ErrorType": type(e).__name__,
+                        "Project": e.project,
+                        "CommandName": command_name,
+                        "Message": str(e),
+                        "ReturnCode": str(e.git_rc)
+                        if e.git_rc is not None
+                        else None,
+                        "IsError": log_as_error,
+                    }
+                )
+                event_log.ErrorEvent(
+                    f"{ERROR_EVENT_LOGGING_PREFIX}:{error_info}"
+                )
+                event_log.Write(GetEventTargetPath())
+            if isinstance(e, GitPopenCommandError):
+                raise
+
+    def _RunCommand(
+        self,
+        command,
+        env,
+        capture_stdout=False,
+        capture_stderr=False,
+        merge_output=False,
+        ssh_proxy=None,
+        cwd=None,
+        input=None,
+    ):
+        # Set subprocess.PIPE for streams that need to be captured.
         stdin = subprocess.PIPE if input else None
         stdout = subprocess.PIPE if capture_stdout else None
         stderr = (
@@ -279,6 +389,30 @@ class GitCommand(object):
             else (subprocess.PIPE if capture_stderr else None)
         )
 
+        # tee_stderr acts like a tee command for stderr, in that, it captures
+        # stderr from the subprocess and streams it back to sys.stderr, while
+        # keeping a copy in-memory.
+        # This allows us to store stderr logs from the subprocess into
+        # GitCommandError.
+        # Certain git operations, such as `git push`, writes diagnostic logs,
+        # such as, progress bar for pushing, into stderr. To ensure we don't
+        # break git's UX, we need to write to sys.stderr as we read from the
+        # subprocess. Setting encoding or errors makes subprocess return
+        # io.TextIOWrapper, which is line buffered. To avoid line-buffering
+        # while tee-ing stderr, we unset these kwargs. See GitCommand._Tee
+        # for tee-ing between the streams.
+        # We tee stderr iff the caller doesn't want to capture any stream to
+        # not disrupt the existing flow.
+        # See go/tee-repo-stderr for more context.
+        tee_stderr = False
+        kwargs = {"encoding": "utf-8", "errors": "backslashreplace"}
+        if not (stdin or stdout or stderr):
+            tee_stderr = True
+            # stderr will be written back to sys.stderr even though it is
+            # piped here.
+            stderr = subprocess.PIPE
+            kwargs = {}
+
         dbg = ""
         if IsTrace():
             global LAST_CWD
@@ -325,14 +459,17 @@ class GitCommand(object):
                     command,
                     cwd=cwd,
                     env=env,
-                    encoding="utf-8",
-                    errors="backslashreplace",
                     stdin=stdin,
                     stdout=stdout,
                     stderr=stderr,
+                    **kwargs,
                 )
             except Exception as e:
-                raise GitError("%s: %s" % (command[1], e))
+                raise GitPopenCommandError(
+                    message=f"{command[1]}: {e}",
+                    project=self.project.name if self.project else None,
+                    command_args=self.cmdv,
+                )
 
             if ssh_proxy:
                 ssh_proxy.add_client(p)
@@ -340,12 +477,45 @@ class GitCommand(object):
             self.process = p
 
             try:
-                self.stdout, self.stderr = p.communicate(input=input)
+                if tee_stderr:
+                    # tee_stderr streams stderr to sys.stderr while capturing
+                    # a copy within self.stderr. tee_stderr is only enabled
+                    # when the caller wants to pipe no stream.
+                    self.stderr = self._Tee(p.stderr, sys.stderr)
+                else:
+                    self.stdout, self.stderr = p.communicate(input=input)
             finally:
                 if ssh_proxy:
                     ssh_proxy.remove_client(p)
             self.rc = p.wait()
 
+    @staticmethod
+    def _Tee(in_stream, out_stream):
+        """Writes text from in_stream to out_stream while recording in buffer.
+
+        Args:
+            in_stream: I/O stream to be read from.
+            out_stream: I/O stream to write to.
+
+        Returns:
+            A str containing everything read from the in_stream.
+        """
+        buffer = ""
+        read_size = 1024 if sys.version_info < (3, 7) else -1
+        chunk = in_stream.read1(read_size)
+        while chunk:
+            # Convert to str.
+            if not hasattr(chunk, "encode"):
+                chunk = chunk.decode("utf-8", "backslashreplace")
+
+            buffer += chunk
+            out_stream.write(chunk)
+            out_stream.flush()
+
+            chunk = in_stream.read1(read_size)
+
+        return buffer
+
     @staticmethod
     def _GetBasicEnv():
         """Return a basic env for running git under.
@@ -365,5 +535,122 @@ class GitCommand(object):
             env.pop(key, None)
         return env
 
+    def VerifyCommand(self):
+        if self.rc == 0:
+            return None
+        stdout = (
+            "\n".join(self.stdout.split("\n")[:GIT_ERROR_STDOUT_LINES])
+            if self.stdout
+            else None
+        )
+        stderr = (
+            "\n".join(self.stderr.split("\n")[:GIT_ERROR_STDERR_LINES])
+            if self.stderr
+            else None
+        )
+        project = self.project.name if self.project else None
+        raise GitCommandError(
+            project=project,
+            command_args=self.cmdv,
+            git_rc=self.rc,
+            git_stdout=stdout,
+            git_stderr=stderr,
+        )
+
     def Wait(self):
+        if self.verify_command:
+            self.VerifyCommand()
         return self.rc
+
+
+class GitRequireError(RepoExitError):
+    """Error raised when git version is unavailable or invalid."""
+
+    def __init__(self, message, exit_code: int = INVALID_GIT_EXIT_CODE):
+        super().__init__(message, exit_code=exit_code)
+
+
+class GitCommandError(GitError):
+    """
+    Error raised from a failed git command.
+    Note that GitError can refer to any Git related error (e.g. branch not
+    specified for project.py 'UploadForReview'), while GitCommandError is
+    raised exclusively from non-zero exit codes returned from git commands.
+    """
+
+    # Tuples with error formats and suggestions for those errors.
+    _ERROR_TO_SUGGESTION = [
+        (
+            re.compile("couldn't find remote ref .*"),
+            "Check if the provided ref exists in the remote.",
+        ),
+        (
+            re.compile("unable to access '.*': .*"),
+            (
+                "Please make sure you have the correct access rights and the "
+                "repository exists."
+            ),
+        ),
+        (
+            re.compile("'.*' does not appear to be a git repository"),
+            "Are you running this repo command outside of a repo workspace?",
+        ),
+        (
+            re.compile("not a git repository"),
+            "Are you running this repo command outside of a repo workspace?",
+        ),
+    ]
+
+    def __init__(
+        self,
+        message: str = DEFAULT_GIT_FAIL_MESSAGE,
+        git_rc: int = None,
+        git_stdout: str = None,
+        git_stderr: str = None,
+        **kwargs,
+    ):
+        super().__init__(
+            message,
+            **kwargs,
+        )
+        self.git_rc = git_rc
+        self.git_stdout = git_stdout
+        self.git_stderr = git_stderr
+
+    @property
+    @functools.lru_cache(maxsize=None)
+    def suggestion(self):
+        """Returns helpful next steps for the given stderr."""
+        if not self.git_stderr:
+            return self.git_stderr
+
+        for err, suggestion in self._ERROR_TO_SUGGESTION:
+            if err.search(self.git_stderr):
+                return suggestion
+
+        return None
+
+    def __str__(self):
+        args = "[]" if not self.command_args else " ".join(self.command_args)
+        error_type = type(self).__name__
+        string = f"{error_type}: '{args}' on {self.project} failed"
+
+        if self.message != DEFAULT_GIT_FAIL_MESSAGE:
+            string += f": {self.message}"
+
+        if self.git_stdout:
+            string += f"\nstdout: {self.git_stdout}"
+
+        if self.git_stderr:
+            string += f"\nstderr: {self.git_stderr}"
+
+        if self.suggestion:
+            string += f"\nsuggestion: {self.suggestion}"
+
+        return string
+
+
+class GitPopenCommandError(GitError):
+    """
+    Error raised when subprocess.Popen fails for a GitCommand
+    """

git_config.py

@@ -15,7 +15,7 @@
 import contextlib
 import datetime
 import errno
-from http.client import HTTPException
+import http.client
 import json
 import os
 import re
@@ -26,11 +26,15 @@ from typing import Union
 import urllib.error
 import urllib.request
 
-from error import GitError, UploadError
+from error import GitError
+from error import UploadError
+from git_command import GitCommand
+from git_refs import R_CHANGES
+from git_refs import R_HEADS
+from git_refs import R_TAGS
 import platform_utils
 from repo_trace import Trace
-from git_command import GitCommand
-from git_refs import R_CHANGES, R_HEADS, R_TAGS
+
 
 # Prefix that is prepended to all the keys of SyncAnalysisState's data
 # that is saved in the config.
@@ -66,7 +70,7 @@ def _key(name):
     return ".".join(parts)
 
 
-class GitConfig(object):
+class GitConfig:
     _ForUser = None
 
     _ForSystem = None
@@ -176,7 +180,7 @@ class GitConfig(object):
             config_dict[key] = self.GetString(key)
         return config_dict
 
-    def GetBoolean(self, name: str) -> Union[str, None]:
+    def GetBoolean(self, name: str) -> Union[bool, None]:
         """Returns a boolean from the configuration file.
 
         Returns:
@@ -366,7 +370,7 @@ class GitConfig(object):
             with Trace(": parsing %s", self.file):
                 with open(self._json) as fd:
                     return json.load(fd)
-        except (IOError, ValueError):
+        except (OSError, ValueError):
             platform_utils.remove(self._json, missing_ok=True)
             return None
 
@@ -374,7 +378,7 @@ class GitConfig(object):
         try:
             with open(self._json, "w") as fd:
                 json.dump(cache, fd, indent=2)
-        except (IOError, TypeError):
+        except (OSError, TypeError):
             platform_utils.remove(self._json, missing_ok=True)
 
     def _ReadGit(self):
@@ -414,7 +418,7 @@ class GitConfig(object):
         if p.Wait() == 0:
             return p.stdout
         else:
-            raise GitError("git config %s: %s" % (str(args), p.stderr))
+            raise GitError(f"git config {str(args)}: {p.stderr}")
 
 
 class RepoConfig(GitConfig):
@@ -426,7 +430,7 @@ class RepoConfig(GitConfig):
         return os.path.join(repo_config_dir, ".repoconfig/config")
 
 
-class RefSpec(object):
+class RefSpec:
     """A Git refspec line, split into its components:
 
     forced:  True if the line starts with '+'
@@ -537,7 +541,7 @@ def GetUrlCookieFile(url, quiet):
     yield cookiefile, None
 
 
-class Remote(object):
+class Remote:
     """Configuration options related to a remote."""
 
     def __init__(self, config, name):
@@ -647,13 +651,11 @@ class Remote(object):
                             userEmail, host, port
                         )
                 except urllib.error.HTTPError as e:
-                    raise UploadError("%s: %s" % (self.review, str(e)))
+                    raise UploadError(f"{self.review}: {str(e)}")
                 except urllib.error.URLError as e:
-                    raise UploadError("%s: %s" % (self.review, str(e)))
-                except HTTPException as e:
-                    raise UploadError(
-                        "%s: %s" % (self.review, e.__class__.__name__)
-                    )
+                    raise UploadError(f"{self.review}: {str(e)}")
+                except http.client.HTTPException as e:
+                    raise UploadError(f"{self.review}: {e.__class__.__name__}")
 
                 REVIEW_CACHE[u] = self._review_url
         return self._review_url + self.projectname
@@ -662,7 +664,7 @@ class Remote(object):
         username = self._config.GetString("review.%s.username" % self.review)
         if username is None:
             username = userEmail.split("@")[0]
-        return "ssh://%s@%s:%s/" % (username, host, port)
+        return f"ssh://{username}@{host}:{port}/"
 
     def ToLocal(self, rev):
         """Convert a remote revision string to something we have locally."""
@@ -711,15 +713,15 @@ class Remote(object):
         self._Set("fetch", list(map(str, self.fetch)))
 
     def _Set(self, key, value):
-        key = "remote.%s.%s" % (self.name, key)
+        key = f"remote.{self.name}.{key}"
         return self._config.SetString(key, value)
 
     def _Get(self, key, all_keys=False):
-        key = "remote.%s.%s" % (self.name, key)
+        key = f"remote.{self.name}.{key}"
         return self._config.GetString(key, all_keys=all_keys)
 
 
-class Branch(object):
+class Branch:
     """Configuration options related to a single branch."""
 
     def __init__(self, config, name):
@@ -758,11 +760,11 @@ class Branch(object):
                     fd.write("\tmerge = %s\n" % self.merge)
 
     def _Set(self, key, value):
-        key = "branch.%s.%s" % (self.name, key)
+        key = f"branch.{self.name}.{key}"
         return self._config.SetString(key, value)
 
     def _Get(self, key, all_keys=False):
-        key = "branch.%s.%s" % (self.name, key)
+        key = f"branch.{self.name}.{key}"
         return self._config.GetString(key, all_keys=all_keys)
 
 
@@ -791,8 +793,8 @@ class SyncAnalysisState:
                 to be logged.
         """
         self._config = config
-        now = datetime.datetime.utcnow()
-        self._Set("main.synctime", now.isoformat(timespec="microseconds") + "Z")
+        now = datetime.datetime.now(datetime.timezone.utc)
+        self._Set("main.synctime", now.isoformat(timespec="microseconds"))
         self._Set("main.version", "1")
         self._Set("sys.argv", sys.argv)
         for key, value in superproject_logging_data.items():

git_refs.py

@@ -13,8 +13,10 @@
 # limitations under the License.
 
 import os
-from repo_trace import Trace
+
 import platform_utils
+from repo_trace import Trace
+
 
 HEAD = "HEAD"
 R_CHANGES = "refs/changes/"
@@ -26,7 +28,7 @@ R_WORKTREE_M = R_WORKTREE + "m/"
 R_M = "refs/remotes/m/"
 
 
-class GitRefs(object):
+class GitRefs:
     def __init__(self, gitdir):
         self._gitdir = gitdir
         self._phyref = None
@@ -103,10 +105,8 @@ class GitRefs(object):
     def _ReadPackedRefs(self):
         path = os.path.join(self._gitdir, "packed-refs")
         try:
-            fd = open(path, "r")
+            fd = open(path)
             mtime = os.path.getmtime(path)
-        except IOError:
-            return
         except OSError:
             return
         try:

git_superproject.py

@@ -22,17 +22,19 @@ Examples:
   UpdateProjectsResult = superproject.UpdateProjectsRevisionId(projects)
 """
 
-import hashlib
 import functools
+import hashlib
 import os
 import sys
 import time
 from typing import NamedTuple
 
-from git_command import git_require, GitCommand
+from git_command import git_require
+from git_command import GitCommand
 from git_config import RepoConfig
 from git_refs import GitRefs
 
+
 _SUPERPROJECT_GIT_NAME = "superproject.git"
 _SUPERPROJECT_MANIFEST_NAME = "superproject_override.xml"
 
@@ -64,12 +66,12 @@ class UpdateProjectsResult(NamedTuple):
     fatal: bool
 
 
-class Superproject(object):
+class Superproject:
     """Get commit ids from superproject.
 
-    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.
+    Initializes a bare 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__(
@@ -233,7 +235,8 @@ class Superproject(object):
         p = GitCommand(
             None,
             cmd,
-            cwd=self._work_git,
+            gitdir=self._work_git,
+            bare=True,
             capture_stdout=True,
             capture_stderr=True,
         )
@@ -269,7 +272,8 @@ class Superproject(object):
         p = GitCommand(
             None,
             cmd,
-            cwd=self._work_git,
+            gitdir=self._work_git,
+            bare=True,
             capture_stdout=True,
             capture_stderr=True,
         )
@@ -379,7 +383,7 @@ class Superproject(object):
         try:
             with open(manifest_path, "w", encoding="utf-8") as fp:
                 fp.write(manifest_str)
-        except IOError as e:
+        except OSError as e:
             self._LogError("cannot write manifest to : {} {}", manifest_path, e)
             return None
         return manifest_path

git_trace2_event_log.py

@@ -1,46 +1,9 @@
-# Copyright (C) 2020 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.
-
-"""Provide event logging in the git trace2 EVENT format.
-
-The git trace2 EVENT format is defined at:
-https://www.kernel.org/pub/software/scm/git/docs/technical/api-trace2.html#_event_format
-https://git-scm.com/docs/api-trace2#_the_event_format_target
-
-  Usage:
-
-  git_trace_log = EventLog()
-  git_trace_log.StartEvent()
-  ...
-  git_trace_log.ExitEvent()
-  git_trace_log.Write()
-"""
+from git_command import GetEventTargetPath
+from git_command import RepoSourceVersion
+from git_trace2_event_log_base import BaseEventLog
 
 
-import datetime
-import errno
-import json
-import os
-import socket
-import sys
-import tempfile
-import threading
-
-from git_command import GitCommand, RepoSourceVersion
-
-
-class EventLog(object):
+class EventLog(BaseEventLog):
     """Event log that records events that occurred during a repo invocation.
 
     Events are written to the log as a consecutive JSON entries, one per line.
@@ -57,318 +20,13 @@ class EventLog(object):
     https://git-scm.com/docs/api-trace2#_event_format
     """
 
-    def __init__(self, env=None):
-        """Initializes the event log."""
-        self._log = []
-        # Try to get session-id (sid) from environment (setup in repo launcher).
-        KEY = "GIT_TRACE2_PARENT_SID"
-        if env is None:
-            env = os.environ
+    def __init__(self, **kwargs):
+        super().__init__(repo_source_version=RepoSourceVersion(), **kwargs)
 
-        self.start = datetime.datetime.utcnow()
-
-        # Save both our sid component and the complete sid.
-        # We use our sid component (self._sid) as the unique filename prefix and
-        # the full sid (self._full_sid) in the log itself.
-        self._sid = "repo-%s-P%08x" % (
-            self.start.strftime("%Y%m%dT%H%M%SZ"),
-            os.getpid(),
-        )
-        parent_sid = env.get(KEY)
-        # Append our sid component to the parent sid (if it exists).
-        if parent_sid is not None:
-            self._full_sid = parent_sid + "/" + self._sid
-        else:
-            self._full_sid = self._sid
-
-        # Set/update the environment variable.
-        # Environment handling across systems is messy.
-        try:
-            env[KEY] = self._full_sid
-        except UnicodeEncodeError:
-            env[KEY] = self._full_sid.encode()
-
-        # Add a version event to front of the log.
-        self._AddVersionEvent()
-
-    @property
-    def full_sid(self):
-        return self._full_sid
-
-    def _AddVersionEvent(self):
-        """Adds a 'version' event at the beginning of current log."""
-        version_event = self._CreateEventDict("version")
-        version_event["evt"] = "2"
-        version_event["exe"] = RepoSourceVersion()
-        self._log.insert(0, version_event)
-
-    def _CreateEventDict(self, event_name):
-        """Returns a dictionary with common keys/values for git trace2 events.
-
-        Args:
-            event_name: The event name.
-
-        Returns:
-            Dictionary with the common event fields populated.
-        """
-        return {
-            "event": event_name,
-            "sid": self._full_sid,
-            "thread": threading.current_thread().name,
-            "time": datetime.datetime.utcnow().isoformat() + "Z",
-        }
-
-    def StartEvent(self):
-        """Append a 'start' event to the current log."""
-        start_event = self._CreateEventDict("start")
-        start_event["argv"] = sys.argv
-        self._log.append(start_event)
-
-    def ExitEvent(self, result):
-        """Append an 'exit' event to the current log.
-
-        Args:
-            result: Exit code of the event
-        """
-        exit_event = self._CreateEventDict("exit")
-
-        # Consider 'None' success (consistent with event_log result handling).
-        if result is None:
-            result = 0
-        exit_event["code"] = result
-        time_delta = datetime.datetime.utcnow() - self.start
-        exit_event["t_abs"] = time_delta.total_seconds()
-        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 LogConfigEvents(self, config, event_dict_name):
-        """Append a |event_dict_name| event for each config key in |config|.
-
-        Args:
-            config: Configuration dictionary.
-            event_dict_name: Name of the event dictionary for items to be logged
-                under.
-        """
-        for param, value in config.items():
-            event = self._CreateEventDict(event_dict_name)
-            event["param"] = param
-            event["value"] = value
-            self._log.append(event)
-
-    def DefParamRepoEvents(self, config):
-        """Append 'def_param' events for repo config keys to the current log.
-
-        This appends one event for each repo.* config key.
-
-        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.")}
-        self.LogConfigEvents(repo_config, "def_param")
-
-    def GetDataEventName(self, value):
-        """Returns 'data-json' if the value is an array else returns 'data'."""
-        return "data-json" if value[0] == "[" and value[-1] == "]" else "data"
-
-    def LogDataConfigEvents(self, config, prefix):
-        """Append a 'data' event for each entry in |config| to the current log.
-
-        For each keyX and valueX of the config, "key" field of the event is
-        '|prefix|/keyX' and the "value" of the "key" field is valueX.
-
-        Args:
-            config: Configuration dictionary.
-            prefix: Prefix for each key that is logged.
-        """
-        for key, value in config.items():
-            event = self._CreateEventDict(self.GetDataEventName(value))
-            event["key"] = f"{prefix}/{key}"
-            event["value"] = value
-            self._log.append(event)
-
-    def ErrorEvent(self, msg, fmt=None):
-        """Append a 'error' event to the current log."""
-        error_event = self._CreateEventDict("error")
-        if fmt is None:
-            fmt = msg
-        error_event["msg"] = msg
-        error_event["fmt"] = fmt
-        self._log.append(error_event)
-
-    def _GetEventTargetPath(self):
-        """Get the 'trace2.eventtarget' path from git configuration.
-
-        Returns:
-            path: git config's 'trace2.eventtarget' path if it exists, or None
-        """
-        path = None
-        cmd = ["config", "--get", "trace2.eventtarget"]
-        # TODO(https://crbug.com/gerrit/13706): Use GitConfig when it supports
-        # system git config variables.
-        p = GitCommand(
-            None, cmd, capture_stdout=True, capture_stderr=True, bare=True
-        )
-        retval = p.Wait()
-        if retval == 0:
-            # Strip trailing carriage-return in path.
-            path = p.stdout.rstrip("\n")
-        elif retval != 1:
-            # `git config --get` is documented to produce an exit status of `1`
-            # if the requested variable is not present in the configuration.
-            # Report any other return value as an error.
-            print(
-                "repo: error: 'git config --get' call failed with return code: "
-                "%r, stderr: %r" % (retval, p.stderr),
-                file=sys.stderr,
-            )
-        return path
-
-    def _WriteLog(self, write_fn):
-        """Writes the log out using a provided writer function.
-
-        Generate compact JSON output for each item in the log, and write it
-        using write_fn.
-
-        Args:
-            write_fn: A function that accepts byts and writes them to a
-                destination.
-        """
-
-        for e in self._log:
-            # Dump in compact encoding mode.
-            # See 'Compact encoding' in Python docs:
-            # https://docs.python.org/3/library/json.html#module-json
-            write_fn(
-                json.dumps(e, indent=None, separators=(",", ":")).encode(
-                    "utf-8"
-                )
-                + b"\n"
-            )
-
-    def Write(self, path=None):
-        """Writes the log out to a file or socket.
-
-        Log is only written if 'path' or 'git config --get trace2.eventtarget'
-        provide a valid path (or socket) to write logs to.
-
-        Logging filename format follows the git trace2 style of being a unique
-        (exclusive writable) file.
-
-        Args:
-            path: Path to where logs should be written. The path may have a
-                prefix of the form "af_unix:[{stream|dgram}:]", in which case
-                the path is treated as a Unix domain socket. See
-                https://git-scm.com/docs/api-trace2#_enabling_a_target for
-                details.
-
-        Returns:
-            log_path: Path to the log file or socket if log is written,
-                otherwise None
-        """
-        log_path = None
-        # If no logging path is specified, get the path from
-        # 'trace2.eventtarget'.
+    def Write(self, path=None, **kwargs):
         if path is None:
             path = self._GetEventTargetPath()
+        return super().Write(path=path, **kwargs)
 
-        # If no logging path is specified, exit.
-        if path is None:
-            return None
-
-        path_is_socket = False
-        socket_type = None
-        if isinstance(path, str):
-            parts = path.split(":", 1)
-            if parts[0] == "af_unix" and len(parts) == 2:
-                path_is_socket = True
-                path = parts[1]
-                parts = path.split(":", 1)
-                if parts[0] == "stream" and len(parts) == 2:
-                    socket_type = socket.SOCK_STREAM
-                    path = parts[1]
-                elif parts[0] == "dgram" and len(parts) == 2:
-                    socket_type = socket.SOCK_DGRAM
-                    path = parts[1]
-            else:
-                # Get absolute path.
-                path = os.path.abspath(os.path.expanduser(path))
-        else:
-            raise TypeError("path: str required but got %s." % type(path))
-
-        # Git trace2 requires a directory to write log to.
-
-        # TODO(https://crbug.com/gerrit/13706): Support file (append) mode also.
-        if not (path_is_socket or os.path.isdir(path)):
-            return None
-
-        if path_is_socket:
-            if socket_type == socket.SOCK_STREAM or socket_type is None:
-                try:
-                    with socket.socket(
-                        socket.AF_UNIX, socket.SOCK_STREAM
-                    ) as sock:
-                        sock.connect(path)
-                        self._WriteLog(sock.sendall)
-                    return f"af_unix:stream:{path}"
-                except OSError as err:
-                    # If we tried to connect to a DGRAM socket using STREAM,
-                    # ignore the attempt and continue to DGRAM below. Otherwise,
-                    # issue a warning.
-                    if err.errno != errno.EPROTOTYPE:
-                        print(
-                            f"repo: warning: git trace2 logging failed: {err}",
-                            file=sys.stderr,
-                        )
-                        return None
-            if socket_type == socket.SOCK_DGRAM or socket_type is None:
-                try:
-                    with socket.socket(
-                        socket.AF_UNIX, socket.SOCK_DGRAM
-                    ) as sock:
-                        self._WriteLog(lambda bs: sock.sendto(bs, path))
-                        return f"af_unix:dgram:{path}"
-                except OSError as err:
-                    print(
-                        f"repo: warning: git trace2 logging failed: {err}",
-                        file=sys.stderr,
-                    )
-                    return None
-            # Tried to open a socket but couldn't connect (SOCK_STREAM) or write
-            # (SOCK_DGRAM).
-            print(
-                "repo: warning: git trace2 logging failed: could not write to "
-                "socket",
-                file=sys.stderr,
-            )
-            return None
-
-        # Path is an absolute path
-        # Use NamedTemporaryFile to generate a unique filename as required by
-        # git trace2.
-        try:
-            with tempfile.NamedTemporaryFile(
-                mode="xb", prefix=self._sid, dir=path, delete=False
-            ) as f:
-                # TODO(https://crbug.com/gerrit/13706): Support writing events
-                # as they occur.
-                self._WriteLog(f.write)
-                log_path = f.name
-        except FileExistsError as err:
-            print(
-                "repo: warning: git trace2 logging failed: %r" % err,
-                file=sys.stderr,
-            )
-            return None
-        return log_path
+    def _GetEventTargetPath(self):
+        return GetEventTargetPath()

git_trace2_event_log_base.py

@@ -0,0 +1,354 @@
+# Copyright (C) 2020 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.
+
+"""Provide event logging in the git trace2 EVENT format.
+
+The git trace2 EVENT format is defined at:
+https://www.kernel.org/pub/software/scm/git/docs/technical/api-trace2.html#_event_format
+https://git-scm.com/docs/api-trace2#_the_event_format_target
+
+  Usage:
+
+  git_trace_log = EventLog()
+  git_trace_log.StartEvent()
+  ...
+  git_trace_log.ExitEvent()
+  git_trace_log.Write()
+"""
+
+
+import datetime
+import errno
+import json
+import os
+import socket
+import sys
+import tempfile
+import threading
+
+
+# Timeout when sending events via socket (applies to connect, send)
+SOCK_TIMEOUT = 0.5  # in seconds
+# BaseEventLog __init__ Counter that is consistent within the same process
+p_init_count = 0
+
+
+class BaseEventLog:
+    """Event log that records events that occurred during a repo invocation.
+
+    Events are written to the log as a consecutive JSON entries, one per line.
+    Entries follow the git trace2 EVENT format.
+
+    Each entry contains the following common keys:
+    - event: The event name
+    - sid: session-id - Unique string to allow process instance to be
+          identified.
+    - thread: The thread name.
+    - time: is the UTC time of the event.
+
+    Valid 'event' names and event specific fields are documented here:
+    https://git-scm.com/docs/api-trace2#_event_format
+    """
+
+    def __init__(
+        self, env=None, repo_source_version=None, add_init_count=False
+    ):
+        """Initializes the event log."""
+        global p_init_count
+        p_init_count += 1
+        self._log = []
+        # Try to get session-id (sid) from environment (setup in repo launcher).
+        KEY = "GIT_TRACE2_PARENT_SID"
+        if env is None:
+            env = os.environ
+
+        self.start = datetime.datetime.now(datetime.timezone.utc)
+
+        # Save both our sid component and the complete sid.
+        # We use our sid component (self._sid) as the unique filename prefix and
+        # the full sid (self._full_sid) in the log itself.
+        self._sid = (
+            f"repo-{self.start.strftime('%Y%m%dT%H%M%SZ')}-P{os.getpid():08x}"
+        )
+
+        if add_init_count:
+            self._sid = f"{self._sid}-{p_init_count}"
+
+        parent_sid = env.get(KEY)
+        # Append our sid component to the parent sid (if it exists).
+        if parent_sid is not None:
+            self._full_sid = parent_sid + "/" + self._sid
+        else:
+            self._full_sid = self._sid
+
+        # Set/update the environment variable.
+        # Environment handling across systems is messy.
+        try:
+            env[KEY] = self._full_sid
+        except UnicodeEncodeError:
+            env[KEY] = self._full_sid.encode()
+
+        if repo_source_version is not None:
+            # Add a version event to front of the log.
+            self._AddVersionEvent(repo_source_version)
+
+    @property
+    def full_sid(self):
+        return self._full_sid
+
+    def _AddVersionEvent(self, repo_source_version):
+        """Adds a 'version' event at the beginning of current log."""
+        version_event = self._CreateEventDict("version")
+        version_event["evt"] = "2"
+        version_event["exe"] = repo_source_version
+        self._log.insert(0, version_event)
+
+    def _CreateEventDict(self, event_name):
+        """Returns a dictionary with common keys/values for git trace2 events.
+
+        Args:
+            event_name: The event name.
+
+        Returns:
+            Dictionary with the common event fields populated.
+        """
+        return {
+            "event": event_name,
+            "sid": self._full_sid,
+            "thread": threading.current_thread().name,
+            "time": datetime.datetime.now(datetime.timezone.utc).isoformat(),
+        }
+
+    def StartEvent(self):
+        """Append a 'start' event to the current log."""
+        start_event = self._CreateEventDict("start")
+        start_event["argv"] = sys.argv
+        self._log.append(start_event)
+
+    def ExitEvent(self, result):
+        """Append an 'exit' event to the current log.
+
+        Args:
+            result: Exit code of the event
+        """
+        exit_event = self._CreateEventDict("exit")
+
+        # Consider 'None' success (consistent with event_log result handling).
+        if result is None:
+            result = 0
+        exit_event["code"] = result
+        time_delta = datetime.datetime.now(datetime.timezone.utc) - self.start
+        exit_event["t_abs"] = time_delta.total_seconds()
+        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 LogConfigEvents(self, config, event_dict_name):
+        """Append a |event_dict_name| event for each config key in |config|.
+
+        Args:
+            config: Configuration dictionary.
+            event_dict_name: Name of the event dictionary for items to be logged
+                under.
+        """
+        for param, value in config.items():
+            event = self._CreateEventDict(event_dict_name)
+            event["param"] = param
+            event["value"] = value
+            self._log.append(event)
+
+    def DefParamRepoEvents(self, config):
+        """Append 'def_param' events for repo config keys to the current log.
+
+        This appends one event for each repo.* config key.
+
+        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.")}
+        self.LogConfigEvents(repo_config, "def_param")
+
+    def GetDataEventName(self, value):
+        """Returns 'data-json' if the value is an array else returns 'data'."""
+        return "data-json" if value[0] == "[" and value[-1] == "]" else "data"
+
+    def LogDataConfigEvents(self, config, prefix):
+        """Append a 'data' event for each entry in |config| to the current log.
+
+        For each keyX and valueX of the config, "key" field of the event is
+        '|prefix|/keyX' and the "value" of the "key" field is valueX.
+
+        Args:
+            config: Configuration dictionary.
+            prefix: Prefix for each key that is logged.
+        """
+        for key, value in config.items():
+            event = self._CreateEventDict(self.GetDataEventName(value))
+            event["key"] = f"{prefix}/{key}"
+            event["value"] = value
+            self._log.append(event)
+
+    def ErrorEvent(self, msg, fmt=None):
+        """Append a 'error' event to the current log."""
+        error_event = self._CreateEventDict("error")
+        if fmt is None:
+            fmt = msg
+        error_event["msg"] = f"RepoErrorEvent:{msg}"
+        error_event["fmt"] = f"RepoErrorEvent:{fmt}"
+        self._log.append(error_event)
+
+    def _WriteLog(self, write_fn):
+        """Writes the log out using a provided writer function.
+
+        Generate compact JSON output for each item in the log, and write it
+        using write_fn.
+
+        Args:
+            write_fn: A function that accepts byts and writes them to a
+                destination.
+        """
+
+        for e in self._log:
+            # Dump in compact encoding mode.
+            # See 'Compact encoding' in Python docs:
+            # https://docs.python.org/3/library/json.html#module-json
+            write_fn(
+                json.dumps(e, indent=None, separators=(",", ":")).encode(
+                    "utf-8"
+                )
+                + b"\n"
+            )
+
+    def Write(self, path=None):
+        """Writes the log out to a file or socket.
+
+        Log is only written if 'path' or 'git config --get trace2.eventtarget'
+        provide a valid path (or socket) to write logs to.
+
+        Logging filename format follows the git trace2 style of being a unique
+        (exclusive writable) file.
+
+        Args:
+            path: Path to where logs should be written. The path may have a
+                prefix of the form "af_unix:[{stream|dgram}:]", in which case
+                the path is treated as a Unix domain socket. See
+                https://git-scm.com/docs/api-trace2#_enabling_a_target for
+                details.
+
+        Returns:
+            log_path: Path to the log file or socket if log is written,
+                otherwise None
+        """
+        log_path = None
+        # If no logging path is specified, exit.
+        if path is None:
+            return None
+
+        path_is_socket = False
+        socket_type = None
+        if isinstance(path, str):
+            parts = path.split(":", 1)
+            if parts[0] == "af_unix" and len(parts) == 2:
+                path_is_socket = True
+                path = parts[1]
+                parts = path.split(":", 1)
+                if parts[0] == "stream" and len(parts) == 2:
+                    socket_type = socket.SOCK_STREAM
+                    path = parts[1]
+                elif parts[0] == "dgram" and len(parts) == 2:
+                    socket_type = socket.SOCK_DGRAM
+                    path = parts[1]
+            else:
+                # Get absolute path.
+                path = os.path.abspath(os.path.expanduser(path))
+        else:
+            raise TypeError("path: str required but got %s." % type(path))
+
+        # Git trace2 requires a directory to write log to.
+
+        # TODO(https://crbug.com/gerrit/13706): Support file (append) mode also.
+        if not (path_is_socket or os.path.isdir(path)):
+            return None
+
+        if path_is_socket:
+            if socket_type == socket.SOCK_STREAM or socket_type is None:
+                try:
+                    with socket.socket(
+                        socket.AF_UNIX, socket.SOCK_STREAM
+                    ) as sock:
+                        sock.settimeout(SOCK_TIMEOUT)
+                        sock.connect(path)
+                        self._WriteLog(sock.sendall)
+                    return f"af_unix:stream:{path}"
+                except OSError as err:
+                    # If we tried to connect to a DGRAM socket using STREAM,
+                    # ignore the attempt and continue to DGRAM below. Otherwise,
+                    # issue a warning.
+                    if err.errno != errno.EPROTOTYPE:
+                        print(
+                            f"repo: warning: git trace2 logging failed: {err}",
+                            file=sys.stderr,
+                        )
+                        return None
+            if socket_type == socket.SOCK_DGRAM or socket_type is None:
+                try:
+                    with socket.socket(
+                        socket.AF_UNIX, socket.SOCK_DGRAM
+                    ) as sock:
+                        self._WriteLog(lambda bs: sock.sendto(bs, path))
+                        return f"af_unix:dgram:{path}"
+                except OSError as err:
+                    print(
+                        f"repo: warning: git trace2 logging failed: {err}",
+                        file=sys.stderr,
+                    )
+                    return None
+            # Tried to open a socket but couldn't connect (SOCK_STREAM) or write
+            # (SOCK_DGRAM).
+            print(
+                "repo: warning: git trace2 logging failed: could not write to "
+                "socket",
+                file=sys.stderr,
+            )
+            return None
+
+        # Path is an absolute path
+        # Use NamedTemporaryFile to generate a unique filename as required by
+        # git trace2.
+        try:
+            with tempfile.NamedTemporaryFile(
+                mode="xb", prefix=self._sid, dir=path, delete=False
+            ) as f:
+                # TODO(https://crbug.com/gerrit/13706): Support writing events
+                # as they occur.
+                self._WriteLog(f.write)
+                log_path = f.name
+        except FileExistsError as err:
+            print(
+                "repo: warning: git trace2 logging failed: %r" % err,
+                file=sys.stderr,
+            )
+            return None
+        return log_path

gitc_utils.py

@@ -1,166 +0,0 @@
-# Copyright (C) 2015 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.
-
-import os
-import multiprocessing
-import re
-import sys
-import time
-
-import git_command
-import git_config
-import wrapper
-
-from error import ManifestParseError
-
-NUM_BATCH_RETRIEVE_REVISIONID = 32
-
-
-def get_gitc_manifest_dir():
-    return wrapper.Wrapper().get_gitc_manifest_dir()
-
-
-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.
-
-    Because of the limit of open file descriptors allowed, length of projects
-    should not be overly large. Recommend calling this function multiple times
-    with each call not exceeding NUM_BATCH_RETRIEVE_REVISIONID projects.
-
-    Args:
-        projects: List of project objects to set the revionExpr for.
-    """
-    # Retrieve the commit id for each project based off of its current
-    # revisionExpr and it is not already a commit id.
-    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):
-    """Generate a manifest for shafsd to use for this GITC client.
-
-    Args:
-        gitc_manifest: Current gitc manifest, or None if there isn't one yet.
-        manifest: A GitcManifest object loaded with the current repo manifest.
-        paths: List of project paths we want to update.
-    """
-
-    print(
-        "Generating GITC Manifest by fetching revision SHAs for each "
-        "project."
-    )
-    if paths is None:
-        paths = list(manifest.paths.keys())
-
-    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]
-    projects = [p for p in projects if p.MatchesGroups(groups)]
-
-    if gitc_manifest is not None:
-        for path, proj in manifest.paths.items():
-            if not proj.MatchesGroups(groups):
-                continue
-
-            if not proj.upstream and not git_config.IsId(proj.revisionExpr):
-                proj.upstream = proj.revisionExpr
-
-            if path not in gitc_manifest.paths:
-                # Any new projects need their first revision, even if we weren't
-                # asked for them.
-                projects.append(proj)
-            elif path not in paths:
-                # And copy revisions from the previous manifest if we're not
-                # updating them now.
-                gitc_proj = gitc_manifest.paths[path]
-                if gitc_proj.old_revision:
-                    proj.revisionExpr = None
-                    proj.old_revision = gitc_proj.old_revision
-                else:
-                    proj.revisionExpr = gitc_proj.revisionExpr
-
-    _set_project_revisions(projects)
-
-    if gitc_manifest is not None:
-        for path, proj in gitc_manifest.paths.items():
-            if proj.old_revision and path in paths:
-                # If we updated a project that has been started, keep the
-                # old-revision updated.
-                repo_proj = manifest.paths[path]
-                repo_proj.old_revision = repo_proj.revisionExpr
-                repo_proj.revisionExpr = None
-
-    # Convert URLs from relative to absolute.
-    for _name, remote in manifest.remotes.items():
-        remote.fetchUrl = remote.resolvedFetchUrl
-
-    # Save the manifest.
-    save_manifest(manifest)
-
-
-def save_manifest(manifest, client_dir=None):
-    """Save the manifest file in the client_dir.
-
-    Args:
-        manifest: Manifest object to save.
-        client_dir: Client directory to save the manifest in.
-    """
-    if not client_dir:
-        manifest_file = manifest.manifestFile
-    else:
-        manifest_file = os.path.join(client_dir, ".manifest")
-    with open(manifest_file, "w") as f:
-        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)

hooks.py

@@ -12,11 +12,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import errno
-import json
 import os
 import re
-import subprocess
 import sys
 import traceback
 import urllib.parse
@@ -25,7 +22,7 @@ from error import HookError
 from git_refs import HEAD
 
 
-class RepoHook(object):
+class RepoHook:
     """A RepoHook contains information about a script to run as a hook.
 
     Hooks are used to run a python script before running an upload (for
@@ -183,7 +180,7 @@ class RepoHook(object):
                 abort_if_user_denies was passed to the consturctor.
         """
         hooks_config = self._hooks_project.config
-        git_approval_key = "repo.hooks.%s.%s" % (self._hook_type, subkey)
+        git_approval_key = f"repo.hooks.{self._hook_type}.{subkey}"
 
         # Get the last value that the user approved for this hook; may be None.
         old_val = hooks_config.GetString(git_approval_key)
@@ -196,7 +193,7 @@ class RepoHook(object):
             else:
                 # Give the user a reason why we're prompting, since they last
                 # told us to "never ask again".
-                prompt = "WARNING: %s\n\n" % (changed_prompt,)
+                prompt = f"WARNING: {changed_prompt}\n\n"
         else:
             prompt = ""
 
@@ -244,9 +241,8 @@ class RepoHook(object):
         return self._CheckForHookApprovalHelper(
             "approvedmanifest",
             self._manifest_url,
-            "Run hook scripts from %s" % (self._manifest_url,),
-            "Manifest URL has changed since %s was allowed."
-            % (self._hook_type,),
+            f"Run hook scripts from {self._manifest_url}",
+            f"Manifest URL has changed since {self._hook_type} was allowed.",
         )
 
     def _CheckForHookApprovalHash(self):
@@ -265,7 +261,7 @@ class RepoHook(object):
             "approvedhash",
             self._GetHash(),
             prompt % (self._GetMustVerb(), self._script_fullpath),
-            "Scripts have changed since %s was allowed." % (self._hook_type,),
+            f"Scripts have changed since {self._hook_type} was allowed.",
         )
 
     @staticmethod
@@ -298,43 +294,6 @@ class RepoHook(object):
 
         return interp
 
-    def _ExecuteHookViaReexec(self, interp, context, **kwargs):
-        """Execute the hook script through |interp|.
-
-        Note: Support for this feature should be dropped ~Jun 2021.
-
-        Args:
-            interp: The Python program to run.
-            context: Basic Python context to execute the hook inside.
-            kwargs: Arbitrary arguments to pass to the hook script.
-
-        Raises:
-            HookError: When the hooks failed for any reason.
-        """
-        # This logic needs to be kept in sync with _ExecuteHookViaImport below.
-        script = """
-import json, os, sys
-path = '''%(path)s'''
-kwargs = json.loads('''%(kwargs)s''')
-context = json.loads('''%(context)s''')
-sys.path.insert(0, os.path.dirname(path))
-data = open(path).read()
-exec(compile(data, path, 'exec'), context)
-context['main'](**kwargs)
-""" % {
-            "path": self._script_fullpath,
-            "kwargs": json.dumps(kwargs),
-            "context": json.dumps(context),
-        }
-
-        # We pass the script via stdin to avoid OS argv limits.  It also makes
-        # unhandled exception tracebacks less verbose/confusing for users.
-        cmd = [interp, "-c", "import sys; exec(sys.stdin.read())"]
-        proc = subprocess.Popen(cmd, stdin=subprocess.PIPE)
-        proc.communicate(input=script.encode("utf-8"))
-        if proc.returncode:
-            raise HookError("Failed to run %s hook." % (self._hook_type,))
-
     def _ExecuteHookViaImport(self, data, context, **kwargs):
         """Execute the hook code in |data| directly.
 
@@ -412,30 +371,13 @@ context['main'](**kwargs)
             # See what version of python the hook has been written against.
             data = open(self._script_fullpath).read()
             interp = self._ExtractInterpFromShebang(data)
-            reexec = False
             if interp:
                 prog = os.path.basename(interp)
-                if prog.startswith("python2") and sys.version_info.major != 2:
-                    reexec = True
-                elif prog.startswith("python3") and sys.version_info.major == 2:
-                    reexec = True
-
-            # Attempt to execute the hooks through the requested version of
-            # Python.
-            if reexec:
-                try:
-                    self._ExecuteHookViaReexec(interp, context, **kwargs)
-                except OSError as e:
-                    if e.errno == errno.ENOENT:
-                        # We couldn't find the interpreter, so fallback to
-                        # importing.
-                        reexec = False
-                    else:
-                        raise
+                if prog.startswith("python2"):
+                    raise HookError("Python 2 is not supported")
 
             # Run the hook by importing directly.
-            if not reexec:
-                self._ExecuteHookViaImport(data, context, **kwargs)
+            self._ExecuteHookViaImport(data, context, **kwargs)
         finally:
             # Restore sys.path and CWD.
             sys.path = orig_syspath

hooks/commit-msg

@@ -1,5 +1,5 @@
 #!/bin/sh
-# From Gerrit Code Review 3.6.1 c67916dbdc07555c44e32a68f92ffc484b9b34f0
+# From Gerrit Code Review 3.10.0 d5403dbf335ba7d48977fc95170c3f7027c34659
 #
 # Part of Gerrit Code Review (https://www.gerritcodereview.com/)
 #
@@ -31,14 +31,21 @@ if test ! -f "$1" ; then
 fi
 
 # Do not create a change id if requested
-if test "false" = "$(git config --bool --get gerrit.createChangeId)" ; then
-  exit 0
-fi
+create_setting=$(git config --get gerrit.createChangeId)
+case "$create_setting" in
+  false)
+    exit 0
+    ;;
+  always)
+    ;;
+  *)
+    # Do not create a change id for squash/fixup commits.
+    if head -n1 "$1" | LC_ALL=C grep -q '^[a-z][a-z]*! '; then
+      exit 0
+    fi
+    ;;
+esac
 
-# Do not create a change id for squash commits.
-if head -n1 "$1" | grep -q '^squash! '; then
-  exit 0
-fi
 
 if git rev-parse --verify HEAD >/dev/null 2>&1; then
   refhash="$(git rev-parse HEAD)"
@@ -51,7 +58,7 @@ dest="$1.tmp.${random}"
 
 trap 'rm -f "$dest" "$dest-2"' EXIT
 
-if ! git stripspace --strip-comments < "$1" > "${dest}" ; then
+if ! cat "$1" | sed -e '/>8/q' | git stripspace --strip-comments > "${dest}" ; then
    echo "cannot strip comments from $1"
    exit 1
 fi
@@ -65,7 +72,7 @@ reviewurl="$(git config --get gerrit.reviewUrl)"
 if test -n "${reviewurl}" ; then
   token="Link"
   value="${reviewurl%/}/id/I$random"
-  pattern=".*/id/I[0-9a-f]\{40\}$"
+  pattern=".*/id/I[0-9a-f]\{40\}"
 else
   token="Change-Id"
   value="I$random"
@@ -92,7 +99,7 @@ fi
 # Avoid the --where option which only appeared in Git 2.15
 if ! git -c trailer.where=before interpret-trailers \
          --trailer "Signed-off-by: $token: $value" < "$dest-2" |
-     sed -re "s/^Signed-off-by: ($token: )/\1/" \
+     sed -e "s/^Signed-off-by: \($token: \)/\1/" \
          -e "/^Signed-off-by: SENTINEL/d" > "$dest" ; then
   echo "cannot insert $token line in $1"
   exit 1

main.py

@@ -21,44 +21,58 @@ which takes care of execing this entry point.
 """
 
 import getpass
+import json
 import netrc
 import optparse
 import os
 import shlex
+import signal
 import sys
 import textwrap
 import time
 import urllib.request
 
+from repo_logging import RepoLogger
+
+
 try:
     import kerberos
 except ImportError:
     kerberos = None
 
 from color import SetDefaultColoring
-import event_log
-from repo_trace import SetTrace, Trace, SetTraceToStderr
-from git_command import user_agent
-from git_config import RepoConfig
-from git_trace2_event_log import EventLog
 from command import InteractiveCommand
 from command import MirrorSafeCommand
-from command import GitcAvailableCommand, GitcClientCommand
-from subcmds.version import Version
 from editor import Editor
 from error import DownloadError
+from error import GitcUnsupportedError
 from error import InvalidProjectGroupsError
 from error import ManifestInvalidRevisionError
 from error import ManifestParseError
 from error import NoManifestException
 from error import NoSuchProjectError
 from error import RepoChangedException
-import gitc_utils
-from manifest_xml import GitcClient, RepoClient
-from pager import RunPager, TerminatePager
-from wrapper import WrapperPath, Wrapper
-
+from error import RepoError
+from error import RepoExitError
+from error import RepoUnhandledExceptionError
+from error import SilentRepoExitError
+import event_log
+from git_command import user_agent
+from git_config import RepoConfig
+from git_trace2_event_log import EventLog
+from manifest_xml import RepoClient
+from pager import RunPager
+from pager import TerminatePager
+from repo_trace import SetTrace
+from repo_trace import SetTraceToStderr
+from repo_trace import Trace
 from subcmds import all_commands
+from subcmds.version import Version
+from wrapper import Wrapper
+from wrapper import WrapperPath
+
+
+logger = RepoLogger(__file__)
 
 
 # NB: These do not need to be kept in sync with the repo launcher script.
@@ -73,28 +87,22 @@ from subcmds import all_commands
 MIN_PYTHON_VERSION_SOFT = (3, 6)
 MIN_PYTHON_VERSION_HARD = (3, 6)
 
-if sys.version_info.major < 3:
-    print(
-        "repo: error: Python 2 is no longer supported; "
-        "Please upgrade to Python {}.{}+.".format(*MIN_PYTHON_VERSION_SOFT),
-        file=sys.stderr,
+if sys.version_info < MIN_PYTHON_VERSION_HARD:
+    logger.error(
+        "repo: error: Python version is too old; "
+        "Please upgrade to Python %d.%d+.",
+        *MIN_PYTHON_VERSION_SOFT,
     )
     sys.exit(1)
-else:
-    if sys.version_info < MIN_PYTHON_VERSION_HARD:
-        print(
-            "repo: error: Python 3 version is too old; "
-            "Please upgrade to Python {}.{}+.".format(*MIN_PYTHON_VERSION_SOFT),
-            file=sys.stderr,
-        )
-        sys.exit(1)
-    elif sys.version_info < MIN_PYTHON_VERSION_SOFT:
-        print(
-            "repo: warning: your Python 3 version is no longer supported; "
-            "Please upgrade to Python {}.{}+.".format(*MIN_PYTHON_VERSION_SOFT),
-            file=sys.stderr,
-        )
+elif sys.version_info < MIN_PYTHON_VERSION_SOFT:
+    logger.error(
+        "repo: warning: your Python version is no longer supported; "
+        "Please upgrade to Python %d.%d+.",
+        *MIN_PYTHON_VERSION_SOFT,
+    )
 
+KEYBOARD_INTERRUPT_EXIT = 128 + signal.SIGINT
+MAX_PRINT_ERRORS = 5
 
 global_options = optparse.OptionParser(
     usage="repo [-p|--paginate|--no-pager] COMMAND [ARGS]",
@@ -179,7 +187,7 @@ global_options.add_option(
 )
 
 
-class _Repo(object):
+class _Repo:
     def __init__(self, repodir):
         self.repodir = repodir
         self.commands = all_commands
@@ -191,9 +199,8 @@ class _Repo(object):
         if short:
             commands = " ".join(sorted(self.commands))
             wrapped_commands = textwrap.wrap(commands, width=77)
-            print(
-                "Available commands:\n  %s" % ("\n  ".join(wrapped_commands),)
-            )
+            help_commands = "".join(f"\n  {x}" for x in wrapped_commands)
+            print(f"Available commands:{help_commands}")
             print("\nRun `repo help <command>` for command-specific details.")
             print("Bug reports:", Wrapper().BUG_URL)
         else:
@@ -229,7 +236,7 @@ class _Repo(object):
         if name in self.commands:
             return name, []
 
-        key = "alias.%s" % (name,)
+        key = f"alias.{name}"
         alias = RepoConfig.ForRepository(self.repodir).GetString(key)
         if alias is None:
             alias = RepoConfig.ForUser().GetString(key)
@@ -297,11 +304,10 @@ class _Repo(object):
                 submanifest_path=gopts.submanifest_path,
                 outer_client=outer_client,
             )
-        gitc_manifest = None
-        gitc_client_name = gitc_utils.parse_clientdir(os.getcwd())
-        if gitc_client_name:
-            gitc_manifest = GitcClient(self.repodir, gitc_client_name)
-            repo_client.isGitcClient = True
+
+        if Wrapper().gitc_parse_clientdir(os.getcwd()):
+            logger.error("GITC is not supported.")
+            raise GitcUnsupportedError()
 
         try:
             cmd = self.commands[name](
@@ -310,50 +316,27 @@ class _Repo(object):
                 manifest=repo_client.manifest,
                 outer_client=outer_client,
                 outer_manifest=outer_client.manifest,
-                gitc_manifest=gitc_manifest,
                 git_event_log=git_trace2_event_log,
             )
         except KeyError:
-            print(
-                "repo: '%s' is not a repo command.  See 'repo help'." % name,
-                file=sys.stderr,
+            logger.error(
+                "repo: '%s' is not a repo command.  See 'repo help'.", name
             )
             return 1
 
         Editor.globalConfig = cmd.client.globalConfig
 
         if not isinstance(cmd, MirrorSafeCommand) and cmd.manifest.IsMirror:
-            print(
-                "fatal: '%s' requires a working directory" % name,
-                file=sys.stderr,
-            )
-            return 1
-
-        if (
-            isinstance(cmd, GitcAvailableCommand)
-            and not gitc_utils.get_gitc_manifest_dir()
-        ):
-            print(
-                "fatal: '%s' requires GITC to be available" % name,
-                file=sys.stderr,
-            )
-            return 1
-
-        if isinstance(cmd, GitcClientCommand) and not gitc_client_name:
-            print("fatal: '%s' requires a GITC client" % name, file=sys.stderr)
+            logger.error("fatal: '%s' requires a working directory", name)
             return 1
 
         try:
             copts, cargs = cmd.OptionParser.parse_args(argv)
             copts = cmd.ReadEnvironmentOptions(copts)
         except NoManifestException as e:
-            print(
-                "error: in `%s`: %s" % (" ".join([name] + argv), str(e)),
-                file=sys.stderr,
-            )
-            print(
-                "error: manifest missing or unreadable -- please run init",
-                file=sys.stderr,
+            logger.error("error: in `%s`: %s", " ".join([name] + argv), e)
+            logger.error(
+                "error: manifest missing or unreadable -- please run init"
             )
             return 1
 
@@ -374,7 +357,11 @@ class _Repo(object):
         git_trace2_event_log.StartEvent()
         git_trace2_event_log.CommandEvent(name="repo", subcommands=[name])
 
-        try:
+        def execute_command_helper():
+            """
+            Execute the subcommand.
+            """
+            nonlocal result
             cmd.CommonValidateOptions(copts, cargs)
             cmd.ValidateOptions(copts, cargs)
 
@@ -409,45 +396,90 @@ class _Repo(object):
                     if hasattr(copts, "manifest_branch"):
                         child_argv.extend(["--manifest-branch", spec.revision])
                     result = self._Run(name, gopts, child_argv) or result
+
+        def execute_command():
+            """
+            Execute the command and log uncaught exceptions.
+            """
+            try:
+                execute_command_helper()
+            except (
+                KeyboardInterrupt,
+                SystemExit,
+                Exception,
+                RepoExitError,
+            ) as e:
+                ok = isinstance(e, SystemExit) and not e.code
+                exception_name = type(e).__name__
+                if isinstance(e, RepoUnhandledExceptionError):
+                    exception_name = type(e.error).__name__
+                if isinstance(e, RepoExitError):
+                    aggregated_errors = e.aggregate_errors or []
+                    for error in aggregated_errors:
+                        project = None
+                        if isinstance(error, RepoError):
+                            project = error.project
+                        error_info = json.dumps(
+                            {
+                                "ErrorType": type(error).__name__,
+                                "Project": project,
+                                "Message": str(error),
+                            }
+                        )
+                        git_trace2_event_log.ErrorEvent(
+                            f"AggregateExitError:{error_info}"
+                        )
+                if not ok:
+                    git_trace2_event_log.ErrorEvent(
+                        f"RepoExitError:{exception_name}"
+                    )
+                raise
+
+        try:
+            execute_command()
         except (
             DownloadError,
             ManifestInvalidRevisionError,
+            ManifestParseError,
             NoManifestException,
         ) as e:
-            print(
-                "error: in `%s`: %s" % (" ".join([name] + argv), str(e)),
-                file=sys.stderr,
-            )
+            logger.error("error: in `%s`: %s", " ".join([name] + argv), e)
             if isinstance(e, NoManifestException):
-                print(
-                    "error: manifest missing or unreadable -- please run init",
-                    file=sys.stderr,
+                logger.error(
+                    "error: manifest missing or unreadable -- please run init"
                 )
-            result = 1
+            result = e.exit_code
         except NoSuchProjectError as e:
             if e.name:
-                print("error: project %s not found" % e.name, file=sys.stderr)
+                logger.error("error: project %s not found", e.name)
             else:
-                print("error: no project in current directory", file=sys.stderr)
-            result = 1
+                logger.error("error: no project in current directory")
+            result = e.exit_code
         except InvalidProjectGroupsError as e:
             if e.name:
-                print(
-                    "error: project group must be enabled for project %s"
-                    % e.name,
-                    file=sys.stderr,
+                logger.error(
+                    "error: project group must be enabled for project %s",
+                    e.name,
                 )
             else:
-                print(
+                logger.error(
                     "error: project group must be enabled for the project in "
-                    "the current directory",
-                    file=sys.stderr,
+                    "the current directory"
                 )
-            result = 1
+            result = e.exit_code
         except SystemExit as e:
             if e.code:
                 result = e.code
             raise
+        except KeyboardInterrupt:
+            result = KEYBOARD_INTERRUPT_EXIT
+            raise
+        except RepoExitError as e:
+            result = e.exit_code
+            raise
+        except Exception:
+            result = 1
+            raise
         finally:
             finish = time.time()
             elapsed = finish - start
@@ -499,7 +531,7 @@ def _CheckWrapperVersion(ver_str, repo_path):
         repo_path = "~/bin/repo"
 
     if not ver_str:
-        print("no --wrapper-version argument", file=sys.stderr)
+        logger.error("no --wrapper-version argument")
         sys.exit(1)
 
     # Pull out the version of the repo launcher we know about to compare.
@@ -508,7 +540,7 @@ def _CheckWrapperVersion(ver_str, repo_path):
 
     exp_str = ".".join(map(str, exp))
     if ver < MIN_REPO_VERSION:
-        print(
+        logger.error(
             """
 repo: error:
 !!! Your version of repo %s is too old.
@@ -517,42 +549,44 @@ repo: error:
 !!! You must upgrade before you can continue:
 
     cp %s %s
-"""
-            % (ver_str, min_str, exp_str, WrapperPath(), repo_path),
-            file=sys.stderr,
+""",
+            ver_str,
+            min_str,
+            exp_str,
+            WrapperPath(),
+            repo_path,
         )
         sys.exit(1)
 
     if exp > ver:
-        print(
-            "\n... A new version of repo (%s) is available." % (exp_str,),
-            file=sys.stderr,
+        logger.warning(
+            "\n... A new version of repo (%s) is available.", exp_str
         )
         if os.access(repo_path, os.W_OK):
-            print(
+            logger.warning(
                 """\
 ... You should upgrade soon:
     cp %s %s
-"""
-                % (WrapperPath(), repo_path),
-                file=sys.stderr,
+""",
+                WrapperPath(),
+                repo_path,
             )
         else:
-            print(
+            logger.warning(
                 """\
 ... New version is available at: %s
 ... The launcher is run from: %s
 !!! The launcher is not writable.  Please talk to your sysadmin or distro
 !!! to get an update installed.
-"""
-                % (WrapperPath(), repo_path),
-                file=sys.stderr,
+""",
+                WrapperPath(),
+                repo_path,
             )
 
 
 def _CheckRepoDir(repo_dir):
     if not repo_dir:
-        print("no --repo-dir argument", file=sys.stderr)
+        logger.error("no --repo-dir argument")
         sys.exit(1)
 
 
@@ -756,7 +790,7 @@ def init_http():
             mgr.add_password(p[1], "https://%s/" % host, p[0], p[2])
     except netrc.NetrcParseError:
         pass
-    except IOError:
+    except OSError:
         pass
     handlers.append(_BasicAuthHandler(mgr))
     handlers.append(_DigestAuthHandler(mgr))
@@ -811,19 +845,19 @@ def _Main(argv):
             SetTraceToStderr()
 
         result = repo._Run(name, gopts, argv) or 0
+    except RepoExitError as e:
+        if not isinstance(e, SilentRepoExitError):
+            logger.log_aggregated_errors(e)
+        result = e.exit_code
     except KeyboardInterrupt:
         print("aborted by user", file=sys.stderr)
-        result = 1
-    except ManifestParseError as mpe:
-        print("fatal: %s" % mpe, file=sys.stderr)
-        result = 1
+        result = KEYBOARD_INTERRUPT_EXIT
     except RepoChangedException as rce:
         # If repo changed, re-exec ourselves.
-        #
         argv = list(sys.argv)
         argv.extend(rce.extra_args)
         try:
-            os.execv(sys.executable, [__file__] + argv)
+            os.execv(sys.executable, [sys.executable, __file__] + argv)
         except OSError as e:
             print("fatal: cannot restart repo after upgrade", file=sys.stderr)
             print("fatal: %s" % e, file=sys.stderr)

man/repo.1

@@ -1,5 +1,5 @@
 .\" DO NOT MODIFY THIS FILE!  It was generated by help2man.
-.TH REPO "1" "November 2022" "repo" "Repo Manual"
+.TH REPO "1" "June 2023" "repo" "Repo Manual"
 .SH NAME
 repo \- repository management tool built on top of git
 .SH SYNOPSIS
@@ -137,4 +137,4 @@ version
 Display the version of repo
 .PP
 See 'repo help <command>' for more information on a specific command.
-Bug reports: https://bugs.chromium.org/p/gerrit/issues/entry?template=Repo+tool+issue
+Bug reports: https://issues.gerritcodereview.com/issues/new?component=1370071

manifest_xml.py

@@ -18,28 +18,25 @@ import os
 import platform
 import re
 import sys
-import xml.dom.minidom
 import urllib.parse
+import xml.dom.minidom
 
-import gitc_utils
+from error import ManifestInvalidPathError
+from error import ManifestInvalidRevisionError
+from error import ManifestParseError
 from git_config import GitConfig
-from git_refs import R_HEADS, HEAD
+from git_refs import HEAD
+from git_refs import R_HEADS
 from git_superproject import Superproject
 import platform_utils
-from project import (
-    Annotation,
-    RemoteSpec,
-    Project,
-    RepoProject,
-    ManifestProject,
-)
-from error import (
-    ManifestParseError,
-    ManifestInvalidPathError,
-    ManifestInvalidRevisionError,
-)
+from project import Annotation
+from project import ManifestProject
+from project import Project
+from project import RemoteSpec
+from project import RepoProject
 from wrapper import Wrapper
 
+
 MANIFEST_FILE_NAME = "manifest.xml"
 LOCAL_MANIFEST_NAME = "local_manifest.xml"
 LOCAL_MANIFESTS_DIR_NAME = "local_manifests"
@@ -117,12 +114,40 @@ def XmlInt(node, attr, default=None):
     try:
         return int(value)
     except ValueError:
-        raise ManifestParseError(
-            'manifest: invalid %s="%s" integer' % (attr, value)
-        )
+        raise ManifestParseError(f'manifest: invalid {attr}="{value}" integer')
 
 
-class _Default(object):
+def normalize_url(url: str) -> str:
+    """Mutate input 'url' into normalized form:
+
+    * remove trailing slashes
+    * convert SCP-like syntax to SSH URL
+
+    Args:
+        url: URL to modify
+
+    Returns:
+        The normalized URL.
+    """
+
+    url = url.rstrip("/")
+    parsed_url = urllib.parse.urlparse(url)
+
+    # This matches patterns like "git@github.com:foo".
+    scp_like_url_re = r"^[^/:]+@[^/:]+:[^/]+"
+
+    # If our URL is missing a schema and matches git's
+    # SCP-like syntax we should convert it to a proper
+    # SSH URL instead to make urljoin() happier.
+    #
+    # See: https://git-scm.com/docs/git-clone#URLS
+    if not parsed_url.scheme and re.match(scp_like_url_re, url):
+        return "ssh://" + url.replace(":", "/", 1)
+
+    return url
+
+
+class _Default:
     """Project defaults within the manifest."""
 
     revisionExpr = None
@@ -145,7 +170,7 @@ class _Default(object):
         return self.__dict__ != other.__dict__
 
 
-class _XmlRemote(object):
+class _XmlRemote:
     def __init__(
         self,
         name,
@@ -185,20 +210,22 @@ class _XmlRemote(object):
     def _resolveFetchUrl(self):
         if self.fetchUrl is None:
             return ""
-        url = self.fetchUrl.rstrip("/")
-        manifestUrl = self.manifestUrl.rstrip("/")
-        # urljoin will gets confused over quite a few things.  The ones we care
-        # about here are:
-        # * no scheme in the base url, like <hostname:port>
-        # We handle no scheme by replacing it with an obscure protocol, gopher
-        # and then replacing it with the original when we are done.
 
-        if manifestUrl.find(":") != manifestUrl.find("/") - 1:
-            url = urllib.parse.urljoin("gopher://" + manifestUrl, url)
-            url = re.sub(r"^gopher://", "", url)
+        fetch_url = normalize_url(self.fetchUrl)
+        manifest_url = normalize_url(self.manifestUrl)
+
+        # urljoin doesn't like URLs with no scheme in the base URL
+        # such as file paths.  We handle this by prefixing it with
+        # an obscure protocol, gopher, and replacing it with the
+        # original after urljoin
+        if manifest_url.find(":") != manifest_url.find("/") - 1:
+            fetch_url = urllib.parse.urljoin(
+                "gopher://" + manifest_url, fetch_url
+            )
+            fetch_url = re.sub(r"^gopher://", "", fetch_url)
         else:
-            url = urllib.parse.urljoin(manifestUrl, url)
-        return url
+            fetch_url = urllib.parse.urljoin(manifest_url, fetch_url)
+        return fetch_url
 
     def ToRemoteSpec(self, projectName):
         fetchUrl = self.resolvedFetchUrl.rstrip("/")
@@ -278,7 +305,7 @@ class _XmlSubmanifest:
             parent.repodir,
             linkFile,
             parent_groups=",".join(groups) or "",
-            submanifest_path=self.relpath,
+            submanifest_path=os.path.join(parent.path_prefix, self.relpath),
             outer_client=outer_client,
             default_groups=default_groups,
         )
@@ -357,7 +384,7 @@ class SubmanifestSpec:
         self.groups = groups or []
 
 
-class XmlManifest(object):
+class XmlManifest:
     """manages the repo configuration file"""
 
     def __init__(
@@ -730,10 +757,10 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
             self._output_manifest_project_extras(p, e)
 
             if p.subprojects:
-                subprojects = set(subp.name for subp in p.subprojects)
+                subprojects = {subp.name for subp in p.subprojects}
                 output_projects(p, e, list(sorted(subprojects)))
 
-        projects = set(p.name for p in self._paths.values() if not p.parent)
+        projects = {p.name for p in self._paths.values() if not p.parent}
         output_projects(None, root, list(sorted(projects)))
 
         if self._repo_hooks_project:
@@ -803,17 +830,17 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
             for child in node.childNodes:
                 if child.nodeType == xml.dom.Node.ELEMENT_NODE:
                     attrs = child.attributes
-                    element = dict(
-                        (attrs.item(i).localName, attrs.item(i).value)
+                    element = {
+                        attrs.item(i).localName: attrs.item(i).value
                         for i in range(attrs.length)
-                    )
+                    }
                     if child.nodeName in SINGLE_ELEMENTS:
                         ret[child.nodeName] = element
                     elif child.nodeName in MULTI_ELEMENTS:
                         ret.setdefault(child.nodeName, []).append(element)
                     else:
                         raise ManifestParseError(
-                            'Unhandled element "%s"' % (child.nodeName,)
+                            f'Unhandled element "{child.nodeName}"'
                         )
 
                     append_children(element, child)
@@ -860,8 +887,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
         self._Load()
         outer = self._outer_client
         yield outer
-        for tree in outer.all_children:
-            yield tree
+        yield from outer.all_children
 
     @property
     def all_children(self):
@@ -870,8 +896,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
         for child in self._submanifests.values():
             if child.repo_client:
                 yield child.repo_client
-                for tree in child.repo_client.all_children:
-                    yield tree
+                yield from child.repo_client.all_children
 
     @property
     def path_prefix(self):
@@ -990,7 +1015,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
     @property
     def PartialCloneExclude(self):
         exclude = self.manifest.manifestProject.partial_clone_exclude or ""
-        return set(x.strip() for x in exclude.split(","))
+        return {x.strip() for x in exclude.split(",")}
 
     def SetManifestOverride(self, path):
         """Override manifestFile.  The caller must call Unload()"""
@@ -1263,18 +1288,19 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
         try:
             root = xml.dom.minidom.parse(path)
         except (OSError, xml.parsers.expat.ExpatError) as e:
-            raise ManifestParseError(
-                "error parsing manifest %s: %s" % (path, e)
-            )
+            raise ManifestParseError(f"error parsing manifest {path}: {e}")
 
         if not root or not root.childNodes:
-            raise ManifestParseError("no root node in %s" % (path,))
+            raise ManifestParseError(f"no root node in {path}")
 
         for manifest in root.childNodes:
-            if manifest.nodeName == "manifest":
+            if (
+                manifest.nodeType == manifest.ELEMENT_NODE
+                and manifest.nodeName == "manifest"
+            ):
                 break
         else:
-            raise ManifestParseError("no <manifest> in %s" % (path,))
+            raise ManifestParseError(f"no <manifest> in {path}")
 
         nodes = []
         for node in manifest.childNodes:
@@ -1284,7 +1310,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
                     msg = self._CheckLocalPath(name)
                     if msg:
                         raise ManifestInvalidPathError(
-                            '<include> invalid "name": %s: %s' % (name, msg)
+                            f'<include> invalid "name": {name}: {msg}'
                         )
                 include_groups = ""
                 if parent_groups:
@@ -1316,7 +1342,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
                     raise
                 except Exception as e:
                     raise ManifestParseError(
-                        "failed parsing included manifest %s: %s" % (name, e)
+                        f"failed parsing included manifest {name}: {e}"
                     )
             else:
                 if parent_groups and node.nodeName == "project":
@@ -1535,22 +1561,45 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
                 self._contactinfo = ContactInfo(bugurl)
 
             if node.nodeName == "remove-project":
-                name = self._reqatt(node, "name")
+                name = node.getAttribute("name")
+                path = node.getAttribute("path")
 
-                if name in self._projects:
-                    for p in self._projects[name]:
-                        del self._paths[p.relpath]
-                    del self._projects[name]
+                # Name or path needed.
+                if not name and not path:
+                    raise ManifestParseError(
+                        "remove-project must have name and/or path"
+                    )
 
-                    # If the manifest removes the hooks project, treat it as if
-                    # it deleted
-                    # the repo-hooks element too.
-                    if repo_hooks_project == name:
-                        repo_hooks_project = None
-                elif not XmlBool(node, "optional", False):
+                removed_project = ""
+
+                # Find and remove projects based on name and/or path.
+                for projname, projects in list(self._projects.items()):
+                    for p in projects:
+                        if name == projname and not path:
+                            del self._paths[p.relpath]
+                            if not removed_project:
+                                del self._projects[name]
+                                removed_project = name
+                        elif path == p.relpath and (
+                            name == projname or not name
+                        ):
+                            self._projects[projname].remove(p)
+                            del self._paths[p.relpath]
+                            removed_project = p.name
+
+                # If the manifest removes the hooks project, treat it as if
+                # it deleted the repo-hooks element too.
+                if (
+                    removed_project
+                    and removed_project not in self._projects
+                    and repo_hooks_project == removed_project
+                ):
+                    repo_hooks_project = None
+
+                if not removed_project and not XmlBool(node, "optional", False):
                     raise ManifestParseError(
                         "remove-project element specifies non-existent "
-                        "project: %s" % name
+                        "project: %s" % node.toxml()
                     )
 
         # Store repo hooks project information.
@@ -1744,13 +1793,13 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
                 msg = self._CheckLocalPath(name)
                 if msg:
                     raise ManifestInvalidPathError(
-                        '<submanifest> invalid "name": %s: %s' % (name, msg)
+                        f'<submanifest> invalid "name": {name}: {msg}'
                     )
         else:
             msg = self._CheckLocalPath(path)
             if msg:
                 raise ManifestInvalidPathError(
-                    '<submanifest> invalid "path": %s: %s' % (path, msg)
+                    f'<submanifest> invalid "path": {path}: {msg}'
                 )
 
         submanifest = _XmlSubmanifest(
@@ -1785,7 +1834,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
         msg = self._CheckLocalPath(name, dir_ok=True)
         if msg:
             raise ManifestInvalidPathError(
-                '<project> invalid "name": %s: %s' % (name, msg)
+                f'<project> invalid "name": {name}: {msg}'
             )
         if parent:
             name = self._JoinName(parent.name, name)
@@ -1795,7 +1844,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
             remote = self._default.remote
         if remote is None:
             raise ManifestParseError(
-                "no remote for project %s within %s" % (name, self.manifestFile)
+                f"no remote for project {name} within {self.manifestFile}"
             )
 
         revisionExpr = node.getAttribute("revision") or remote.revision
@@ -1816,7 +1865,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
             msg = self._CheckLocalPath(path, dir_ok=True, cwd_dot_ok=True)
             if msg:
                 raise ManifestInvalidPathError(
-                    '<project> invalid "path": %s: %s' % (path, msg)
+                    f'<project> invalid "path": {path}: {msg}'
                 )
 
         rebase = XmlBool(node, "rebase", True)
@@ -2073,7 +2122,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
         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,)
+                    return f"bad component: {part}"
 
         if not dir_ok and resep.match(path[-1]):
             return "dirs not allowed"
@@ -2109,7 +2158,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
         msg = cls._CheckLocalPath(dest)
         if msg:
             raise ManifestInvalidPathError(
-                '<%s> invalid "dest": %s: %s' % (element, dest, msg)
+                f'<{element}> invalid "dest": {dest}: {msg}'
             )
 
         # |src| is the file we read from or path we point to for symlinks.
@@ -2120,7 +2169,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
         )
         if msg:
             raise ManifestInvalidPathError(
-                '<%s> invalid "src": %s: %s' % (element, src, msg)
+                f'<{element}> invalid "src": {src}: {msg}'
             )
 
     def _ParseCopyFile(self, project, node):
@@ -2164,7 +2213,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
         v = self._remotes.get(name)
         if not v:
             raise ManifestParseError(
-                "remote %s not defined in %s" % (name, self.manifestFile)
+                f"remote {name} not defined in {self.manifestFile}"
             )
         return v
 
@@ -2190,7 +2239,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
         toProjects = manifest.paths
 
         fromKeys = sorted(fromProjects.keys())
-        toKeys = sorted(toProjects.keys())
+        toKeys = set(toProjects.keys())
 
         diff = {
             "added": [],
@@ -2201,13 +2250,13 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
         }
 
         for proj in fromKeys:
+            fromProj = fromProjects[proj]
             if proj not in toKeys:
-                diff["removed"].append(fromProjects[proj])
-            elif not fromProjects[proj].Exists:
+                diff["removed"].append(fromProj)
+            elif not fromProj.Exists:
                 diff["missing"].append(toProjects[proj])
                 toKeys.remove(proj)
             else:
-                fromProj = fromProjects[proj]
                 toProj = toProjects[proj]
                 try:
                     fromRevId = fromProj.GetCommitRevisionId()
@@ -2219,27 +2268,11 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
                         diff["changed"].append((fromProj, toProj))
                 toKeys.remove(proj)
 
-        for proj in toKeys:
-            diff["added"].append(toProjects[proj])
+        diff["added"].extend(toProjects[proj] for proj in sorted(toKeys))
 
         return diff
 
 
-class GitcManifest(XmlManifest):
-    """Parser for GitC (git-in-the-cloud) manifests."""
-
-    def _ParseProject(self, node, parent=None):
-        """Override _ParseProject and add support for GITC specific attributes."""  # noqa: E501
-        return super()._ParseProject(
-            node, parent=parent, old_revision=node.getAttribute("old-revision")
-        )
-
-    def _output_manifest_project_extras(self, p, e):
-        """Output GITC Specific Project attributes"""
-        if p.old_revision:
-            e.setAttribute("old-revision", str(p.old_revision))
-
-
 class RepoClient(XmlManifest):
     """Manages a repo client checkout."""
 
@@ -2292,19 +2325,3 @@ class RepoClient(XmlManifest):
 
         # TODO: Completely separate manifest logic out of the client.
         self.manifest = self
-
-
-class GitcClient(RepoClient, GitcManifest):
-    """Manages a GitC client checkout."""
-
-    def __init__(self, repodir, gitc_client_name):
-        """Initialize the GitcManifest object."""
-        self.gitc_client_name = gitc_client_name
-        self.gitc_client_dir = os.path.join(
-            gitc_utils.get_gitc_manifest_dir(), gitc_client_name
-        )
-
-        super().__init__(
-            repodir, os.path.join(self.gitc_client_dir, ".manifest")
-        )
-        self.isGitcClient = True

pager.py

@@ -19,6 +19,7 @@ import sys
 
 import platform_utils
 
+
 active = False
 pager_process = None
 old_stdout = None

platform_utils.py

@@ -57,8 +57,8 @@ def _validate_winpath(path):
     if _winpath_is_valid(path):
         return path
     raise ValueError(
-        'Path "{}" must be a relative path or an absolute '
-        "path starting with a drive letter".format(path)
+        f'Path "{path}" must be a relative path or an absolute '
+        "path starting with a drive letter"
     )
 
 
@@ -193,10 +193,9 @@ def _walk_windows_impl(top, topdown, onerror, followlinks):
     for name in dirs:
         new_path = os.path.join(top, name)
         if followlinks or not islink(new_path):
-            for x in _walk_windows_impl(
+            yield from _walk_windows_impl(
                 new_path, topdown, onerror, followlinks
-            ):
-                yield x
+            )
     if not topdown:
         yield top, dirs, nondirs
 

platform_utils_win32.py

@@ -12,12 +12,28 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+from ctypes import addressof
+from ctypes import byref
+from ctypes import c_buffer
+from ctypes import c_ubyte
+from ctypes import FormatError
+from ctypes import get_last_error
+from ctypes import Structure
+from ctypes import Union
+from ctypes import WinDLL
+from ctypes import WinError
+from ctypes.wintypes import BOOL
+from ctypes.wintypes import BOOLEAN
+from ctypes.wintypes import DWORD
+from ctypes.wintypes import HANDLE
+from ctypes.wintypes import LPCWSTR
+from ctypes.wintypes import LPDWORD
+from ctypes.wintypes import LPVOID
+from ctypes.wintypes import ULONG
+from ctypes.wintypes import USHORT
+from ctypes.wintypes import WCHAR
 import errno
 
-from ctypes import WinDLL, get_last_error, FormatError, WinError, addressof
-from ctypes import c_buffer, c_ubyte, Structure, Union, byref
-from ctypes.wintypes import BOOL, BOOLEAN, LPCWSTR, DWORD, HANDLE
-from ctypes.wintypes import WCHAR, USHORT, LPVOID, ULONG, LPDWORD
 
 kernel32 = WinDLL("kernel32", use_last_error=True)
 
@@ -170,9 +186,7 @@ def _create_symlink(source, link_name, dwFlags):
             error_desc = FormatError(code).strip()
             if code == ERROR_PRIVILEGE_NOT_HELD:
                 raise OSError(errno.EPERM, error_desc, link_name)
-            _raise_winerror(
-                code, 'Error creating symbolic link "{}"'.format(link_name)
-            )
+            _raise_winerror(code, f'Error creating symbolic link "{link_name}"')
 
 
 def islink(path):
@@ -194,7 +208,7 @@ def readlink(path):
     )
     if reparse_point_handle == INVALID_HANDLE_VALUE:
         _raise_winerror(
-            get_last_error(), 'Error opening symbolic link "{}"'.format(path)
+            get_last_error(), f'Error opening symbolic link "{path}"'
         )
     target_buffer = c_buffer(MAXIMUM_REPARSE_DATA_BUFFER_SIZE)
     n_bytes_returned = DWORD()
@@ -211,7 +225,7 @@ def readlink(path):
     CloseHandle(reparse_point_handle)
     if not io_result:
         _raise_winerror(
-            get_last_error(), 'Error reading symbolic link "{}"'.format(path)
+            get_last_error(), f'Error reading symbolic link "{path}"'
         )
     rdb = REPARSE_DATA_BUFFER.from_buffer(target_buffer)
     if rdb.ReparseTag == IO_REPARSE_TAG_SYMLINK:
@@ -220,11 +234,11 @@ def readlink(path):
         return rdb.MountPointReparseBuffer.PrintName
     # Unsupported reparse point type.
     _raise_winerror(
-        ERROR_NOT_SUPPORTED, 'Error reading symbolic link "{}"'.format(path)
+        ERROR_NOT_SUPPORTED, f'Error reading symbolic link "{path}"'
     )
 
 
 def _raise_winerror(code, error_desc):
     win_error_desc = FormatError(code).strip()
-    error_desc = "{0}: {1}".format(error_desc, win_error_desc)
+    error_desc = f"{error_desc}: {win_error_desc}"
     raise WinError(code, error_desc)

progress.py

@@ -16,6 +16,7 @@ import os
 import sys
 import time
 
+
 try:
     import threading as _threading
 except ImportError:
@@ -23,7 +24,8 @@ except ImportError:
 
 from repo_trace import IsTraceToStderr
 
-_NOT_TTY = not os.isatty(2)
+
+_TTY = sys.stderr.isatty()
 
 # This will erase all content in the current line (wherever the cursor is).
 # It does not move the cursor, so this is usually followed by \r to move to
@@ -50,11 +52,11 @@ def duration_str(total):
     uses microsecond resolution.  This makes for noisy output.
     """
     hours, mins, secs = convert_to_hms(total)
-    ret = "%.3fs" % (secs,)
+    ret = f"{secs:.3f}s"
     if mins:
-        ret = "%im%s" % (mins, ret)
+        ret = f"{mins}m{ret}"
     if hours:
-        ret = "%ih%s" % (hours, ret)
+        ret = f"{hours}h{ret}"
     return ret
 
 
@@ -80,7 +82,7 @@ def jobs_str(total):
     return f"{total} job{'s' if total > 1 else ''}"
 
 
-class Progress(object):
+class Progress:
     def __init__(
         self,
         title,
@@ -97,7 +99,8 @@ class Progress(object):
         self._start = time.time()
         self._show = not delay
         self._units = units
-        self._elide = elide
+        self._elide = elide and _TTY
+
         # Only show the active jobs section if we run more than one in parallel.
         self._show_jobs = False
         self._active = 0
@@ -129,7 +132,7 @@ class Progress(object):
     def _write(self, s):
         s = "\r" + s
         if self._elide:
-            col = os.get_terminal_size().columns
+            col = os.get_terminal_size(sys.stderr.fileno()).columns
             if len(s) > col:
                 s = s[: col - 1] + ".."
         sys.stderr.write(s)
@@ -157,7 +160,7 @@ class Progress(object):
             msg = self._last_msg
         self._last_msg = msg
 
-        if _NOT_TTY or IsTraceToStderr():
+        if not _TTY or IsTraceToStderr():
             return
 
         elapsed_sec = time.time() - self._start
@@ -199,7 +202,7 @@ class Progress(object):
 
     def end(self):
         self._update_event.set()
-        if _NOT_TTY or IsTraceToStderr() or not self._show:
+        if not _TTY or IsTraceToStderr() or not self._show:
             return
 
         duration = duration_str(time.time() - self._start)

pyproject.toml

@@ -15,4 +15,4 @@
 [tool.black]
 line-length = 80
 # NB: Keep in sync with tox.ini.
-target-version = ['py36', 'py37', 'py38', 'py39', 'py310']
+target-version = ['py36', 'py37', 'py38', 'py39', 'py310', 'py311'] #, 'py312'

release/update-manpages

@@ -22,4 +22,5 @@ import sys
 
 import update_manpages
 
+
 sys.exit(update_manpages.main(sys.argv[1:]))

release/update_manpages.py

@@ -17,17 +17,18 @@
 Most code lives in this module so it can be unittested.
 """
 
-from pathlib import Path
-from functools import partial
 import argparse
+import functools
 import multiprocessing
 import os
+from pathlib import Path
 import re
 import shutil
 import subprocess
 import sys
 import tempfile
 
+
 TOPDIR = Path(__file__).resolve().parent.parent
 MANDIR = TOPDIR.joinpath("man")
 
@@ -112,7 +113,9 @@ def main(argv):
 
         # Run all cmd in parallel, and wait for them to finish.
         with multiprocessing.Pool() as pool:
-            pool.map(partial(worker, cwd=tempdir, check=True), cmdlist)
+            pool.map(
+                functools.partial(worker, cwd=tempdir, check=True), cmdlist
+            )
 
     for tmp_path in MANDIR.glob("*.1.tmp"):
         path = tmp_path.parent / tmp_path.stem

repo_logging.py

@@ -0,0 +1,93 @@
+# Copyright (C) 2023 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.
+
+"""Logic for printing user-friendly logs in repo."""
+
+import logging
+
+from color import Coloring
+from error import RepoExitError
+
+
+SEPARATOR = "=" * 80
+MAX_PRINT_ERRORS = 5
+
+
+class _ConfigMock:
+    """Default coloring config to use when Logging.config is not set."""
+
+    def __init__(self):
+        self.default_values = {"color.ui": "auto"}
+
+    def GetString(self, x):
+        return self.default_values.get(x, None)
+
+
+class _LogColoring(Coloring):
+    """Coloring outstream for logging."""
+
+    def __init__(self, config):
+        super().__init__(config, "logs")
+        self.error = self.colorer("error", fg="red")
+        self.warning = self.colorer("warn", fg="yellow")
+        self.levelMap = {
+            "WARNING": self.warning,
+            "ERROR": self.error,
+        }
+
+
+class _LogColoringFormatter(logging.Formatter):
+    """Coloring formatter for logging."""
+
+    def __init__(self, config=None, *args, **kwargs):
+        self.config = config if config else _ConfigMock()
+        self.colorer = _LogColoring(self.config)
+        super().__init__(*args, **kwargs)
+
+    def format(self, record):
+        """Formats |record| with color."""
+        msg = super().format(record)
+        colorer = self.colorer.levelMap.get(record.levelname)
+        return msg if not colorer else colorer(msg)
+
+
+class RepoLogger(logging.Logger):
+    """Repo Logging Module."""
+
+    def __init__(self, name: str, config=None, **kwargs):
+        super().__init__(name, **kwargs)
+        handler = logging.StreamHandler()
+        handler.setFormatter(_LogColoringFormatter(config))
+        self.addHandler(handler)
+
+    def log_aggregated_errors(self, err: RepoExitError):
+        """Print all aggregated logs."""
+        self.error(SEPARATOR)
+
+        if not err.aggregate_errors:
+            self.error("Repo command failed: %s", type(err).__name__)
+            self.error("\t%s", str(err))
+            return
+
+        self.error(
+            "Repo command failed due to the following `%s` errors:",
+            type(err).__name__,
+        )
+        self.error(
+            "\n".join(str(e) for e in err.aggregate_errors[:MAX_PRINT_ERRORS])
+        )
+
+        diff = len(err.aggregate_errors) - MAX_PRINT_ERRORS
+        if diff > 0:
+            self.error("+%d additional errors...", diff)

repo_trace.py

@@ -20,14 +20,15 @@ Temporary: Tracing is always on. Set `REPO_TRACE=0` to turn off.
 To also include trace outputs in stderr do `repo --trace_to_stderr ...`
 """
 
-import sys
+import contextlib
 import os
-import time
+import sys
 import tempfile
-from contextlib import ContextDecorator
+import time
 
 import platform_utils
 
+
 # Env var to implicitly turn on tracing.
 REPO_TRACE = "REPO_TRACE"
 
@@ -68,7 +69,7 @@ def _SetTraceFile(quiet):
     _TRACE_FILE = _GetTraceFile(quiet)
 
 
-class Trace(ContextDecorator):
+class Trace(contextlib.ContextDecorator):
     """Used to capture and save git traces."""
 
     def _time(self):
@@ -141,7 +142,7 @@ def _GetTraceFile(quiet):
 def _ClearOldTraces():
     """Clear the oldest commands if trace file is too big."""
     try:
-        with open(_TRACE_FILE, "r", errors="ignore") as f:
+        with open(_TRACE_FILE, errors="ignore") as f:
             if os.path.getsize(f.name) / (1024 * 1024) <= _MAX_SIZE:
                 return
             trace_lines = f.readlines()

run_tests

@@ -18,6 +18,7 @@
 import os
 import subprocess
 import sys
+
 import pytest
 
 
@@ -26,8 +27,16 @@ ROOT_DIR = os.path.dirname(os.path.realpath(__file__))
 
 def run_black():
     """Returns the exit code from black."""
+    # Black by default only matches .py files.  We have to list standalone
+    # scripts manually.
+    extra_programs = [
+        "repo",
+        "run_tests",
+        "release/update-manpages",
+    ]
     return subprocess.run(
-        [sys.executable, "-m", "black", "--check", ROOT_DIR], check=False
+        [sys.executable, "-m", "black", "--check", ROOT_DIR] + extra_programs,
+        check=False,
     ).returncode
 
 
@@ -38,12 +47,20 @@ def run_flake8():
     ).returncode
 
 
+def run_isort():
+    """Returns the exit code from isort."""
+    return subprocess.run(
+        [sys.executable, "-m", "isort", "--check", ROOT_DIR], check=False
+    ).returncode
+
+
 def main(argv):
     """The main entry."""
     checks = (
         lambda: pytest.main(argv),
         run_black,
         run_flake8,
+        run_isort,
     )
     return 0 if all(not c() for c in checks) else 1
 

run_tests.vpython3

@@ -123,3 +123,8 @@ wheel: <
   name: "infra/python/wheels/pycodestyle-py2_py3"
   version: "version:2.10.0"
 >
+
+wheel: <
+  name: "infra/python/wheels/isort-py3"
+  version: "version:5.10.1"
+>

setup.py

@@ -16,6 +16,7 @@
 """Python packaging for repo."""
 
 import os
+
 import setuptools
 
 
@@ -40,7 +41,7 @@ setuptools.setup(
     long_description_content_type="text/plain",
     url="https://gerrit.googlesource.com/git-repo/",
     project_urls={
-        "Bug Tracker": "https://bugs.chromium.org/p/gerrit/issues/list?q=component:Applications%3Erepo",  # noqa: E501
+        "Bug Tracker": "https://issues.gerritcodereview.com/issues?q=is:open%20componentid:1370071",  # noqa: E501
     },
     # https://pypi.org/classifiers/
     classifiers=[

ssh.py

@@ -165,7 +165,7 @@ class ProxyManager:
         # Check to see whether we already think that the master is running; if
         # we think it's already running, return right away.
         if port is not None:
-            key = "%s:%s" % (host, port)
+            key = f"{host}:{port}"
         else:
             key = host
 

subcmds/__init__.py

@@ -14,8 +14,10 @@
 
 import os
 
+
 # A mapping of the subcommand name to the class that implements it.
 all_commands = {}
+all_modules = []
 
 my_dir = os.path.dirname(__file__)
 for py in os.listdir(my_dir):
@@ -35,13 +37,12 @@ for py in os.listdir(my_dir):
         try:
             cmd = getattr(mod, clsn)
         except AttributeError:
-            raise SyntaxError(
-                "%s/%s does not define class %s" % (__name__, py, clsn)
-            )
+            raise SyntaxError(f"{__name__}/{py} does not define class {clsn}")
 
         name = name.replace("_", "-")
         cmd.NAME = name
         all_commands[name] = cmd
+        all_modules.append(mod)
 
 # Add 'branch' as an alias for 'branches'.
 all_commands["branch"] = all_commands["branches"]

subcmds/abandon.py

@@ -12,14 +12,24 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from collections import defaultdict
+import collections
 import functools
 import itertools
-import sys
 
-from command import Command, DEFAULT_LOCAL_JOBS
+from command import Command
+from command import DEFAULT_LOCAL_JOBS
+from error import RepoError
+from error import RepoExitError
 from git_command import git
 from progress import Progress
+from repo_logging import RepoLogger
+
+
+logger = RepoLogger(__file__)
+
+
+class AbandonError(RepoExitError):
+    """Exit error when abandon command fails."""
 
 
 class Abandon(Command):
@@ -68,28 +78,37 @@ It is equivalent to "git branch -D <branchname>".
             branches = nb
 
         ret = {}
+        errors = []
         for name in branches:
-            status = project.AbandonBranch(name)
+            status = None
+            try:
+                status = project.AbandonBranch(name)
+            except RepoError as e:
+                status = False
+                errors.append(e)
             if status is not None:
                 ret[name] = status
-        return (ret, project)
+
+        return (ret, project, errors)
 
     def Execute(self, opt, args):
         nb = args[0].split()
-        err = defaultdict(list)
-        success = defaultdict(list)
+        err = collections.defaultdict(list)
+        success = collections.defaultdict(list)
+        aggregate_errors = []
         all_projects = self.GetProjects(
             args[1:], all_manifests=not opt.this_manifest_only
         )
         _RelPath = lambda p: p.RelPath(local=opt.this_manifest_only)
 
         def _ProcessResults(_pool, pm, states):
-            for results, project in states:
+            for results, project, errors in states:
                 for branch, status in results.items():
                     if status:
                         success[branch].append(project)
                     else:
                         err[branch].append(project)
+                aggregate_errors.extend(errors)
                 pm.update(msg="")
 
         self.ExecuteInParallel(
@@ -98,7 +117,7 @@ It is equivalent to "git branch -D <branchname>".
             all_projects,
             callback=_ProcessResults,
             output=Progress(
-                "Abandon %s" % (nb,), len(all_projects), quiet=opt.quiet
+                f"Abandon {nb}", len(all_projects), quiet=opt.quiet
             ),
         )
 
@@ -110,19 +129,13 @@ It is equivalent to "git branch -D <branchname>".
         if err:
             for br in err.keys():
                 err_msg = "error: cannot abandon %s" % br
-                print(err_msg, file=sys.stderr)
+                logger.error(err_msg)
                 for proj in err[br]:
-                    print(
-                        " " * len(err_msg) + " | %s" % _RelPath(proj),
-                        file=sys.stderr,
-                    )
-            sys.exit(1)
+                    logger.error(" " * len(err_msg) + " | %s", _RelPath(proj))
+            raise AbandonError(aggregate_errors=aggregate_errors)
         elif not success:
-            print(
-                "error: no project has local branch(es) : %s" % nb,
-                file=sys.stderr,
-            )
-            sys.exit(1)
+            logger.error("error: no project has local branch(es) : %s", nb)
+            raise AbandonError(aggregate_errors=aggregate_errors)
         else:
             # Everything below here is displaying status.
             if opt.quiet:
@@ -139,4 +152,4 @@ It is equivalent to "git branch -D <branchname>".
                             _RelPath(p) for p in success[br]
                         )
                     )
-                print("%s%s| %s\n" % (br, " " * (width - len(br)), result))
+                print(f"{br}{' ' * (width - len(br))}| {result}\n")

subcmds/branches.py

@@ -16,7 +16,8 @@ import itertools
 import sys
 
 from color import Coloring
-from command import Command, DEFAULT_LOCAL_JOBS
+from command import Command
+from command import DEFAULT_LOCAL_JOBS
 
 
 class BranchColoring(Coloring):
@@ -27,7 +28,7 @@ class BranchColoring(Coloring):
         self.notinproject = self.printer("notinproject", fg="red")
 
 
-class BranchInfo(object):
+class BranchInfo:
     def __init__(self, name):
         self.name = name
         self.current = 0
@@ -173,7 +174,7 @@ is shown, then the branch appears in all projects.
                         if _RelPath(p) not in have:
                             paths.append(_RelPath(p))
 
-                s = " %s %s" % (in_type, ", ".join(paths))
+                s = f" {in_type} {', '.join(paths)}"
                 if not i.IsSplitCurrent and (width + 7 + len(s) < 80):
                     fmt = out.current if i.IsCurrent else fmt
                     fmt(s)

subcmds/checkout.py

@@ -13,10 +13,33 @@
 # limitations under the License.
 
 import functools
-import sys
+from typing import NamedTuple
 
-from command import Command, DEFAULT_LOCAL_JOBS
+from command import Command
+from command import DEFAULT_LOCAL_JOBS
+from error import GitError
+from error import RepoExitError
 from progress import Progress
+from project import Project
+from repo_logging import RepoLogger
+
+
+logger = RepoLogger(__file__)
+
+
+class CheckoutBranchResult(NamedTuple):
+    # Whether the Project is on the branch (i.e. branch exists and no errors)
+    result: bool
+    project: Project
+    error: Exception
+
+
+class CheckoutCommandError(RepoExitError):
+    """Exception thrown when checkout command fails."""
+
+
+class MissingBranchError(RepoExitError):
+    """Exception thrown when no project has specified branch."""
 
 
 class Checkout(Command):
@@ -41,23 +64,30 @@ The command is equivalent to:
 
     def _ExecuteOne(self, nb, project):
         """Checkout one project."""
-        return (project.CheckoutBranch(nb), project)
+        error = None
+        result = None
+        try:
+            result = project.CheckoutBranch(nb)
+        except GitError as e:
+            error = e
+        return CheckoutBranchResult(result, project, error)
 
     def Execute(self, opt, args):
         nb = args[0]
         err = []
+        err_projects = []
         success = []
         all_projects = self.GetProjects(
             args[1:], all_manifests=not opt.this_manifest_only
         )
 
         def _ProcessResults(_pool, pm, results):
-            for status, project in results:
-                if status is not None:
-                    if status:
-                        success.append(project)
-                    else:
-                        err.append(project)
+            for result in results:
+                if result.error is not None:
+                    err.append(result.error)
+                    err_projects.append(result.project)
+                elif result.result:
+                    success.append(result.project)
                 pm.update(msg="")
 
         self.ExecuteInParallel(
@@ -66,17 +96,15 @@ The command is equivalent to:
             all_projects,
             callback=_ProcessResults,
             output=Progress(
-                "Checkout %s" % (nb,), len(all_projects), quiet=opt.quiet
+                f"Checkout {nb}", len(all_projects), quiet=opt.quiet
             ),
         )
 
-        if err:
-            for p in err:
-                print(
-                    "error: %s/: cannot checkout %s" % (p.relpath, nb),
-                    file=sys.stderr,
-                )
-            sys.exit(1)
+        if err_projects:
+            for p in err_projects:
+                logger.error("error: %s/: cannot checkout %s", p.relpath, nb)
+            raise CheckoutCommandError(aggregate_errors=err)
         elif not success:
-            print("error: no project has branch %s" % nb, file=sys.stderr)
-            sys.exit(1)
+            msg = f"error: no project has branch {nb}"
+            logger.error(msg)
+            raise MissingBranchError(msg)

subcmds/cherry_pick.py

@@ -14,10 +14,15 @@
 
 import re
 import sys
+
 from command import Command
+from error import GitError
 from git_command import GitCommand
+from repo_logging import RepoLogger
+
 
 CHANGE_ID_RE = re.compile(r"^\s*Change-Id: I([0-9a-f]{40})\s*$")
+logger = RepoLogger(__file__)
 
 
 class CherryPick(Command):
@@ -44,18 +49,29 @@ change id will be added.
             ["rev-parse", "--verify", reference],
             capture_stdout=True,
             capture_stderr=True,
+            verify_command=True,
         )
-        if p.Wait() != 0:
-            print(p.stderr, file=sys.stderr)
-            sys.exit(1)
+        try:
+            p.Wait()
+        except GitError:
+            logger.error(p.stderr)
+            raise
+
         sha1 = p.stdout.strip()
 
-        p = GitCommand(None, ["cat-file", "commit", sha1], capture_stdout=True)
-        if p.Wait() != 0:
-            print(
-                "error: Failed to retrieve old commit message", file=sys.stderr
-            )
-            sys.exit(1)
+        p = GitCommand(
+            None,
+            ["cat-file", "commit", sha1],
+            capture_stdout=True,
+            verify_command=True,
+        )
+
+        try:
+            p.Wait()
+        except GitError:
+            logger.error("error: Failed to retrieve old commit message")
+            raise
+
         old_msg = self._StripHeader(p.stdout)
 
         p = GitCommand(
@@ -63,37 +79,43 @@ change id will be added.
             ["cherry-pick", sha1],
             capture_stdout=True,
             capture_stderr=True,
+            verify_command=True,
         )
-        status = p.Wait()
+
+        try:
+            p.Wait()
+        except GitError as e:
+            logger.error(e)
+            logger.warning(
+                "NOTE: When committing (please see above) and editing the "
+                "commit message, please remove the old Change-Id-line and "
+                "add:\n%s",
+                self._GetReference(sha1),
+            )
+            raise
 
         if p.stdout:
             print(p.stdout.strip(), file=sys.stdout)
         if p.stderr:
             print(p.stderr.strip(), file=sys.stderr)
 
-        if status == 0:
-            # The cherry-pick was applied correctly. We just need to edit the
-            # commit message.
-            new_msg = self._Reformat(old_msg, sha1)
+        # The cherry-pick was applied correctly. We just need to edit
+        # the commit message.
+        new_msg = self._Reformat(old_msg, sha1)
 
-            p = GitCommand(
-                None,
-                ["commit", "--amend", "-F", "-"],
-                input=new_msg,
-                capture_stdout=True,
-                capture_stderr=True,
-            )
-            if p.Wait() != 0:
-                print("error: Failed to update commit message", file=sys.stderr)
-                sys.exit(1)
-
-        else:
-            print(
-                "NOTE: When committing (please see above) and editing the "
-                "commit message, please remove the old Change-Id-line and add:"
-            )
-            print(self._GetReference(sha1), file=sys.stderr)
-            print(file=sys.stderr)
+        p = GitCommand(
+            None,
+            ["commit", "--amend", "-F", "-"],
+            input=new_msg,
+            capture_stdout=True,
+            capture_stderr=True,
+            verify_command=True,
+        )
+        try:
+            p.Wait()
+        except GitError:
+            logger.error("error: Failed to update commit message")
+            raise
 
     def _IsChangeId(self, line):
         return CHANGE_ID_RE.match(line)

subcmds/diff.py

@@ -15,7 +15,8 @@
 import functools
 import io
 
-from command import DEFAULT_LOCAL_JOBS, PagedCommand
+from command import DEFAULT_LOCAL_JOBS
+from command import PagedCommand
 
 
 class Diff(PagedCommand):

subcmds/diffmanifests.py

@@ -87,25 +87,17 @@ synced and their revisions won't be found.
     def _printRawDiff(self, diff, pretty_format=None, local=False):
         _RelPath = lambda p: p.RelPath(local=local)
         for project in diff["added"]:
-            self.printText(
-                "A %s %s" % (_RelPath(project), project.revisionExpr)
-            )
+            self.printText(f"A {_RelPath(project)} {project.revisionExpr}")
             self.out.nl()
 
         for project in diff["removed"]:
-            self.printText(
-                "R %s %s" % (_RelPath(project), project.revisionExpr)
-            )
+            self.printText(f"R {_RelPath(project)} {project.revisionExpr}")
             self.out.nl()
 
         for project, otherProject in diff["changed"]:
             self.printText(
-                "C %s %s %s"
-                % (
-                    _RelPath(project),
-                    project.revisionExpr,
-                    otherProject.revisionExpr,
-                )
+                f"C {_RelPath(project)} {project.revisionExpr} "
+                f"{otherProject.revisionExpr}"
             )
             self.out.nl()
             self._printLogs(
@@ -118,12 +110,8 @@ synced and their revisions won't be found.
 
         for project, otherProject in diff["unreachable"]:
             self.printText(
-                "U %s %s %s"
-                % (
-                    _RelPath(project),
-                    project.revisionExpr,
-                    otherProject.revisionExpr,
-                )
+                f"U {_RelPath(project)} {project.revisionExpr} "
+                f"{otherProject.revisionExpr}"
             )
             self.out.nl()
 

subcmds/download.py

@@ -16,9 +16,18 @@ import re
 import sys
 
 from command import Command
-from error import GitError, NoSuchProjectError
+from error import GitError
+from error import NoSuchProjectError
+from error import RepoExitError
+from repo_logging import RepoLogger
+
 
 CHANGE_RE = re.compile(r"^([1-9][0-9]*)(?:[/\.-]([1-9][0-9]*))?$")
+logger = RepoLogger(__file__)
+
+
+class DownloadCommandError(RepoExitError):
+    """Error raised when download command fails."""
 
 
 class Download(Command):
@@ -102,23 +111,18 @@ If no project is specified try to use current directory as a project.
                     except NoSuchProjectError:
                         project = None
                     if project not in projects:
-                        print(
+                        logger.error(
                             "error: %s matches too many projects; please "
-                            "re-run inside the project checkout." % (a,),
-                            file=sys.stderr,
+                            "re-run inside the project checkout.",
+                            a,
                         )
                         for project in projects:
-                            print(
-                                "  %s/ @ %s"
-                                % (
-                                    project.RelPath(
-                                        local=opt.this_manifest_only
-                                    ),
-                                    project.revisionExpr,
-                                ),
-                                file=sys.stderr,
+                            logger.error(
+                                "  %s/ @ %s",
+                                project.RelPath(local=opt.this_manifest_only),
+                                project.revisionExpr,
                             )
-                        sys.exit(1)
+                        raise NoSuchProjectError()
                 else:
                     project = projects[0]
                     print("Defaulting to cwd project", project.name)
@@ -137,29 +141,33 @@ If no project is specified try to use current directory as a project.
                 )
 
     def Execute(self, opt, args):
+        try:
+            self._ExecuteHelper(opt, args)
+        except Exception as e:
+            if isinstance(e, RepoExitError):
+                raise e
+            raise DownloadCommandError(aggregate_errors=[e])
+
+    def _ExecuteHelper(self, opt, args):
         for project, change_id, ps_id in self._ParseChangeIds(opt, args):
             dl = project.DownloadPatchSet(change_id, ps_id)
-            if not dl:
-                print(
-                    "[%s] change %d/%d not found"
-                    % (project.name, change_id, ps_id),
-                    file=sys.stderr,
-                )
-                sys.exit(1)
 
             if not opt.revert and not dl.commits:
-                print(
-                    "[%s] change %d/%d has already been merged"
-                    % (project.name, change_id, ps_id),
-                    file=sys.stderr,
+                logger.error(
+                    "[%s] change %d/%d has already been merged",
+                    project.name,
+                    change_id,
+                    ps_id,
                 )
                 continue
 
             if len(dl.commits) > 1:
-                print(
-                    "[%s] %d/%d depends on %d unmerged changes:"
-                    % (project.name, change_id, ps_id, len(dl.commits)),
-                    file=sys.stderr,
+                logger.error(
+                    "[%s] %d/%d depends on %d unmerged changes:",
+                    project.name,
+                    change_id,
+                    ps_id,
+                    len(dl.commits),
                 )
                 for c in dl.commits:
                     print("  %s" % (c), file=sys.stderr)
@@ -196,9 +204,10 @@ If no project is specified try to use current directory as a project.
                         project._Checkout(dl.commit)
 
             except GitError:
-                print(
-                    "[%s] Could not complete the %s of %s"
-                    % (project.name, mode, dl.commit),
-                    file=sys.stderr,
+                logger.error(
+                    "[%s] Could not complete the %s of %s",
+                    project.name,
+                    mode,
+                    dl.commit,
                 )
-                sys.exit(1)
+                raise

subcmds/forall.py

@@ -16,21 +16,22 @@ import errno
 import functools
 import io
 import multiprocessing
-import re
 import os
+import re
 import signal
-import sys
 import subprocess
+import sys
 
 from color import Coloring
-from command import (
-    DEFAULT_LOCAL_JOBS,
-    Command,
-    MirrorSafeCommand,
-    WORKER_BATCH_SIZE,
-)
+from command import Command
+from command import DEFAULT_LOCAL_JOBS
+from command import MirrorSafeCommand
+from command import WORKER_BATCH_SIZE
 from error import ManifestInvalidRevisionError
+from repo_logging import RepoLogger
 
+
+logger = RepoLogger(__file__)
 _CAN_COLOR = [
     "branch",
     "diff",
@@ -294,10 +295,10 @@ without iterating through the remaining projects.
             rc = rc or errno.EINTR
         except Exception as e:
             # Catch any other exceptions raised
-            print(
-                "forall: unhandled error, terminating the pool: %s: %s"
-                % (type(e).__name__, e),
-                file=sys.stderr,
+            logger.error(
+                "forall: unhandled error, terminating the pool: %s: %s",
+                type(e).__name__,
+                e,
             )
             rc = rc or getattr(e, "errno", 1)
         if rc != 0:

subcmds/gitc_delete.py

@@ -1,52 +0,0 @@
-# Copyright (C) 2015 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.
-
-import sys
-
-from command import Command, GitcClientCommand
-import platform_utils
-
-
-class GitcDelete(Command, GitcClientCommand):
-    COMMON = True
-    visible_everywhere = False
-    helpSummary = "Delete a GITC Client."
-    helpUsage = """
-%prog
-"""
-    helpDescription = """
-This subcommand deletes the current GITC client, deleting the GITC manifest
-and all locally downloaded sources.
-"""
-
-    def _Options(self, p):
-        p.add_option(
-            "-f",
-            "--force",
-            dest="force",
-            action="store_true",
-            help="force the deletion (no prompt)",
-        )
-
-    def Execute(self, opt, args):
-        if not opt.force:
-            prompt = (
-                "This will delete GITC client: %s\nAre you sure? (yes/no) "
-                % self.gitc_manifest.gitc_client_name
-            )
-            response = input(prompt).lower()
-            if not response == "yes":
-                print('Response was not "yes"\n Exiting...')
-                sys.exit(1)
-        platform_utils.rmtree(self.gitc_manifest.gitc_client_dir)

subcmds/gitc_init.py

@@ -1,87 +0,0 @@
-# Copyright (C) 2015 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.
-
-import os
-import sys
-
-import gitc_utils
-from command import GitcAvailableCommand
-from manifest_xml import GitcManifest
-from subcmds import init
-import wrapper
-
-
-class GitcInit(init.Init, GitcAvailableCommand):
-    COMMON = True
-    MULTI_MANIFEST_SUPPORT = False
-    helpSummary = "Initialize a GITC Client."
-    helpUsage = """
-%prog [options] [client name]
-"""
-    helpDescription = """
-The '%prog' command is ran to initialize a new GITC client for use
-with the GITC file system.
-
-This command will setup the client directory, initialize repo, just
-like repo init does, and then downloads the manifest collection
-and installs it in the .repo/directory of the GITC client.
-
-Once this is done, a GITC manifest is generated by pulling the HEAD
-SHA for each project and generates the properly formatted XML file
-and installs it as .manifest in the GITC client directory.
-
-The -c argument is required to specify the GITC client name.
-
-The optional -f argument can be used to specify the manifest file to
-use for this GITC client.
-"""
-
-    def _Options(self, p):
-        super()._Options(p, gitc_init=True)
-
-    def Execute(self, opt, args):
-        gitc_client = gitc_utils.parse_clientdir(os.getcwd())
-        if not gitc_client or (
-            opt.gitc_client and gitc_client != opt.gitc_client
-        ):
-            print(
-                "fatal: Please update your repo command. See go/gitc for "
-                "instructions.",
-                file=sys.stderr,
-            )
-            sys.exit(1)
-        self.client_dir = os.path.join(
-            gitc_utils.get_gitc_manifest_dir(), gitc_client
-        )
-        super().Execute(opt, args)
-
-        manifest_file = self.manifest.manifestFile
-        if opt.manifest_file:
-            if not os.path.exists(opt.manifest_file):
-                print(
-                    "fatal: Specified manifest file %s does not exist."
-                    % opt.manifest_file
-                )
-                sys.exit(1)
-            manifest_file = opt.manifest_file
-
-        manifest = GitcManifest(
-            self.repodir, os.path.join(self.client_dir, ".manifest")
-        )
-        manifest.Override(manifest_file)
-        gitc_utils.generate_gitc_manifest(None, manifest)
-        print(
-            "Please run `cd %s` to view your GITC client."
-            % os.path.join(wrapper.Wrapper().GITC_FS_ROOT_DIR, gitc_client)
-        )

subcmds/grep.py

@@ -14,11 +14,20 @@
 
 import functools
 import sys
+from typing import NamedTuple
 
 from color import Coloring
-from command import DEFAULT_LOCAL_JOBS, PagedCommand
+from command import DEFAULT_LOCAL_JOBS
+from command import PagedCommand
 from error import GitError
+from error import InvalidArgumentsError
+from error import SilentRepoExitError
 from git_command import GitCommand
+from project import Project
+from repo_logging import RepoLogger
+
+
+logger = RepoLogger(__file__)
 
 
 class GrepColoring(Coloring):
@@ -28,6 +37,22 @@ class GrepColoring(Coloring):
         self.fail = self.printer("fail", fg="red")
 
 
+class ExecuteOneResult(NamedTuple):
+    """Result from an execute instance."""
+
+    project: Project
+    rc: int
+    stdout: str
+    stderr: str
+    error: GitError
+
+
+class GrepCommandError(SilentRepoExitError):
+    """Grep command failure. Since Grep command
+    output already outputs errors ensure that
+    aggregate errors exit silently."""
+
+
 class Grep(PagedCommand):
     COMMON = True
     helpSummary = "Print lines matching a pattern"
@@ -246,11 +271,18 @@ contain a line that matches both expressions:
                 bare=False,
                 capture_stdout=True,
                 capture_stderr=True,
+                verify_command=True,
             )
         except GitError as e:
-            return (project, -1, None, str(e))
+            return ExecuteOneResult(project, -1, None, str(e), e)
 
-        return (project, p.Wait(), p.stdout, p.stderr)
+        try:
+            error = None
+            rc = p.Wait()
+        except GitError as e:
+            rc = 1
+            error = e
+        return ExecuteOneResult(project, rc, p.stdout, p.stderr, error)
 
     @staticmethod
     def _ProcessResults(full_name, have_rev, opt, _pool, out, results):
@@ -258,31 +290,40 @@ contain a line that matches both expressions:
         bad_rev = False
         have_match = False
         _RelPath = lambda p: p.RelPath(local=opt.this_manifest_only)
+        errors = []
 
-        for project, rc, stdout, stderr in results:
-            if rc < 0:
+        for result in results:
+            if result.rc < 0:
                 git_failed = True
-                out.project("--- project %s ---" % _RelPath(project))
+                out.project("--- project %s ---" % _RelPath(result.project))
                 out.nl()
-                out.fail("%s", stderr)
+                out.fail("%s", result.stderr)
                 out.nl()
+                errors.append(result.error)
                 continue
 
-            if rc:
+            if result.rc:
                 # no results
-                if stderr:
-                    if have_rev and "fatal: ambiguous argument" in stderr:
+                if result.stderr:
+                    if (
+                        have_rev
+                        and "fatal: ambiguous argument" in result.stderr
+                    ):
                         bad_rev = True
                     else:
-                        out.project("--- project %s ---" % _RelPath(project))
+                        out.project(
+                            "--- project %s ---" % _RelPath(result.project)
+                        )
                         out.nl()
-                        out.fail("%s", stderr.strip())
+                        out.fail("%s", result.stderr.strip())
                         out.nl()
+                    if result.error is not None:
+                        errors.append(result.error)
                 continue
             have_match = True
 
             # We cut the last element, to avoid a blank line.
-            r = stdout.split("\n")
+            r = result.stdout.split("\n")
             r = r[0:-1]
 
             if have_rev and full_name:
@@ -290,13 +331,13 @@ contain a line that matches both expressions:
                     rev, line = line.split(":", 1)
                     out.write("%s", rev)
                     out.write(":")
-                    out.project(_RelPath(project))
+                    out.project(_RelPath(result.project))
                     out.write("/")
                     out.write("%s", line)
                     out.nl()
             elif full_name:
                 for line in r:
-                    out.project(_RelPath(project))
+                    out.project(_RelPath(result.project))
                     out.write("/")
                     out.write("%s", line)
                     out.nl()
@@ -304,7 +345,7 @@ contain a line that matches both expressions:
                 for line in r:
                     print(line)
 
-        return (git_failed, bad_rev, have_match)
+        return (git_failed, bad_rev, have_match, errors)
 
     def Execute(self, opt, args):
         out = GrepColoring(self.manifest.manifestProject.config)
@@ -333,16 +374,14 @@ contain a line that matches both expressions:
         have_rev = False
         if opt.revision:
             if "--cached" in cmd_argv:
-                print(
-                    "fatal: cannot combine --cached and --revision",
-                    file=sys.stderr,
-                )
-                sys.exit(1)
+                msg = "fatal: cannot combine --cached and --revision"
+                logger.error(msg)
+                raise InvalidArgumentsError(msg)
             have_rev = True
             cmd_argv.extend(opt.revision)
         cmd_argv.append("--")
 
-        git_failed, bad_rev, have_match = self.ExecuteInParallel(
+        git_failed, bad_rev, have_match, errors = self.ExecuteInParallel(
             opt.jobs,
             functools.partial(self._ExecuteOne, cmd_argv),
             projects,
@@ -354,12 +393,12 @@ contain a line that matches both expressions:
         )
 
         if git_failed:
-            sys.exit(1)
+            raise GrepCommandError(
+                "error: git failures", aggregate_errors=errors
+            )
         elif have_match:
             sys.exit(0)
         elif have_rev and bad_rev:
             for r in opt.revision:
-                print("error: can't search revision %s" % r, file=sys.stderr)
-            sys.exit(1)
-        else:
-            sys.exit(1)
+                logger.error("error: can't search revision %s", r)
+        raise GrepCommandError(aggregate_errors=errors)

subcmds/help.py

@@ -16,18 +16,18 @@ import re
 import sys
 import textwrap
 
-from subcmds import all_commands
 from color import Coloring
-from command import (
-    PagedCommand,
-    MirrorSafeCommand,
-    GitcAvailableCommand,
-    GitcClientCommand,
-)
-import gitc_utils
+from command import MirrorSafeCommand
+from command import PagedCommand
+from error import RepoExitError
+from subcmds import all_commands
 from wrapper import Wrapper
 
 
+class InvalidHelpCommand(RepoExitError):
+    """Invalid command passed into help."""
+
+
 class Help(PagedCommand, MirrorSafeCommand):
     COMMON = False
     helpSummary = "Display detailed help on a command"
@@ -74,26 +74,9 @@ Displays detailed usage information about a command.
     def PrintCommonCommandsBody(self):
         print("The most commonly used repo commands are:")
 
-        def gitc_supported(cmd):
-            if not isinstance(cmd, GitcAvailableCommand) and not isinstance(
-                cmd, GitcClientCommand
-            ):
-                return True
-            if self.client.isGitcClient:
-                return True
-            if isinstance(cmd, GitcClientCommand):
-                return False
-            if gitc_utils.get_gitc_manifest_dir():
-                return True
-            return False
-
         commandNames = list(
             sorted(
-                [
-                    name
-                    for name, command in all_commands.items()
-                    if command.COMMON and gitc_supported(command)
-                ]
+                name for name, command in all_commands.items() if command.COMMON
             )
         )
         self._PrintCommands(commandNames)
@@ -167,7 +150,7 @@ Displays detailed usage information about a command.
     def _PrintAllCommandHelp(self):
         for name in sorted(all_commands):
             cmd = all_commands[name](manifest=self.manifest)
-            self._PrintCommandHelp(cmd, header_prefix="[%s] " % (name,))
+            self._PrintCommandHelp(cmd, header_prefix=f"[{name}] ")
 
     def _Options(self, p):
         p.add_option(
@@ -202,7 +185,7 @@ Displays detailed usage information about a command.
                 print(
                     "repo: '%s' is not a repo command." % name, file=sys.stderr
                 )
-                sys.exit(1)
+                raise InvalidHelpCommand(name)
 
             self._PrintCommandHelp(cmd)
 

subcmds/info.py

@@ -14,9 +14,10 @@
 
 import optparse
 
-from command import PagedCommand
 from color import Coloring
-from git_refs import R_M, R_HEADS
+from command import PagedCommand
+from git_refs import R_HEADS
+from git_refs import R_M
 
 
 class _Coloring(Coloring):
@@ -96,7 +97,9 @@ class Info(PagedCommand):
             self.headtext(self.manifest.default.revisionExpr)
         self.out.nl()
         self.heading("Manifest merge branch: ")
-        self.headtext(mergeBranch)
+        # The manifest might not have a merge branch if it isn't in a git repo,
+        # e.g. if `repo init --standalone-manifest` is used.
+        self.headtext(mergeBranch or "")
         self.out.nl()
         self.heading("Manifest groups: ")
         self.headtext(manifestGroups)
@@ -247,7 +250,7 @@ class Info(PagedCommand):
 
             for commit in commits:
                 split = commit.split()
-                self.text("{0:38}{1} ".format("", "-"))
+                self.text(f"{'':38}{'-'} ")
                 self.sha(split[0] + " ")
                 self.text(" ".join(split[1:]))
                 self.out.nl()

subcmds/init.py

@@ -16,10 +16,19 @@ import os
 import sys
 
 from color import Coloring
-from command import InteractiveCommand, MirrorSafeCommand
-from git_command import git_require, MIN_GIT_VERSION_SOFT, MIN_GIT_VERSION_HARD
+from command import InteractiveCommand
+from command import MirrorSafeCommand
+from error import RepoUnhandledExceptionError
+from error import UpdateManifestError
+from git_command import git_require
+from git_command import MIN_GIT_VERSION_HARD
+from git_command import MIN_GIT_VERSION_SOFT
+from repo_logging import RepoLogger
 from wrapper import Wrapper
 
+
+logger = RepoLogger(__file__)
+
 _REPO_ALLOW_SHALLOW = os.environ.get("REPO_ALLOW_SHALLOW")
 
 
@@ -82,8 +91,8 @@ to update the working directory files.
     def _CommonOptions(self, p):
         """Disable due to re-use of Wrapper()."""
 
-    def _Options(self, p, gitc_init=False):
-        Wrapper().InitParser(p, gitc_init=gitc_init)
+    def _Options(self, p):
+        Wrapper().InitParser(p)
         m = p.add_option_group("Multi-manifest")
         m.add_option(
             "--outer-manifest",
@@ -156,7 +165,10 @@ to update the working directory files.
             git_event_log=self.git_event_log,
             manifest_name=opt.manifest_name,
         ):
-            sys.exit(1)
+            manifest_name = opt.manifest_name
+            raise UpdateManifestError(
+                f"Unable to sync manifest {manifest_name}"
+            )
 
     def _Prompt(self, prompt, value):
         print("%-10s [%s]: " % (prompt, value), end="", flush=True)
@@ -203,7 +215,7 @@ to update the working directory files.
 
             if not opt.quiet:
                 print()
-            print("Your identity is: %s <%s>" % (name, email))
+            print(f"Your identity is: {name} <{email}>")
             print("is this correct [y/N]? ", end="", flush=True)
             a = sys.stdin.readline().strip().lower()
             if a in ("yes", "y", "t", "true"):
@@ -321,11 +333,11 @@ to update the working directory files.
     def Execute(self, opt, args):
         git_require(MIN_GIT_VERSION_HARD, fail=True)
         if not git_require(MIN_GIT_VERSION_SOFT):
-            print(
-                "repo: warning: git-%s+ will soon be required; please upgrade "
-                "your version of git to maintain support."
-                % (".".join(str(x) for x in MIN_GIT_VERSION_SOFT),),
-                file=sys.stderr,
+            logger.warning(
+                "repo: warning: git-%s+ will soon be required; "
+                "please upgrade your version of git to maintain "
+                "support.",
+                ".".join(str(x) for x in MIN_GIT_VERSION_SOFT),
             )
 
         rp = self.manifest.repoProject
@@ -341,19 +353,17 @@ to update the working directory files.
             wrapper = Wrapper()
             try:
                 remote_ref, rev = wrapper.check_repo_rev(
-                    rp.gitdir,
+                    rp.worktree,
                     opt.repo_rev,
                     repo_verify=opt.repo_verify,
                     quiet=opt.quiet,
                 )
-            except wrapper.CloneFailure:
+            except wrapper.CloneFailure as e:
                 err_msg = "fatal: double check your --repo-rev setting."
-                print(
-                    err_msg,
-                    file=sys.stderr,
-                )
+                logger.error(err_msg)
                 self.git_event_log.ErrorEvent(err_msg)
-                sys.exit(1)
+                raise RepoUnhandledExceptionError(e)
+
             branch = rp.GetBranch("default")
             branch.merge = remote_ref
             rp.work_git.reset("--hard", rev)

subcmds/list.py

@@ -14,7 +14,8 @@
 
 import os
 
-from command import Command, MirrorSafeCommand
+from command import Command
+from command import MirrorSafeCommand
 
 
 class List(Command, MirrorSafeCommand):
@@ -130,7 +131,7 @@ This is similar to running: repo forall -c 'echo "$REPO_PATH : $REPO_PROJECT"'.
             elif opt.path_only and not opt.name_only:
                 lines.append("%s" % (_getpath(project)))
             else:
-                lines.append("%s : %s" % (_getpath(project), project.name))
+                lines.append(f"{_getpath(project)} : {project.name}")
 
         if lines:
             lines.sort()

subcmds/manifest.py

@@ -17,6 +17,10 @@ import os
 import sys
 
 from command import PagedCommand
+from repo_logging import RepoLogger
+
+
+logger = RepoLogger(__file__)
 
 
 class Manifest(PagedCommand):
@@ -132,7 +136,7 @@ to indicate the remote ref to push changes to via 'repo upload'.
             manifest.SetUseLocalManifests(not opt.ignore_local_manifests)
 
             if opt.json:
-                print("warning: --json is experimental!", file=sys.stderr)
+                logger.warning("warning: --json is experimental!")
                 doc = manifest.ToDict(
                     peg_rev=opt.peg_rev,
                     peg_rev_upstream=opt.peg_rev_upstream,
@@ -159,13 +163,13 @@ to indicate the remote ref to push changes to via 'repo upload'.
             if output_file != "-":
                 fd.close()
                 if manifest.path_prefix:
-                    print(
-                        f"Saved {manifest.path_prefix} submanifest to "
-                        f"{output_file}",
-                        file=sys.stderr,
+                    logger.warning(
+                        "Saved %s submanifest to %s",
+                        manifest.path_prefix,
+                        output_file,
                     )
                 else:
-                    print(f"Saved manifest to {output_file}", file=sys.stderr)
+                    logger.warning("Saved manifest to %s", output_file)
 
     def ValidateOptions(self, opt, args):
         if args:

subcmds/prune.py

@@ -15,7 +15,8 @@
 import itertools
 
 from color import Coloring
-from command import DEFAULT_LOCAL_JOBS, PagedCommand
+from command import DEFAULT_LOCAL_JOBS
+from command import PagedCommand
 
 
 class Prune(PagedCommand):
@@ -82,9 +83,7 @@ class Prune(PagedCommand):
             )
 
             if not branch.base_exists:
-                print(
-                    "(ignoring: tracking branch is gone: %s)" % (branch.base,)
-                )
+                print(f"(ignoring: tracking branch is gone: {branch.base})")
             else:
                 commits = branch.commits
                 date = branch.date

subcmds/rebase.py

@@ -17,6 +17,10 @@ import sys
 from color import Coloring
 from command import Command
 from git_command import GitCommand
+from repo_logging import RepoLogger
+
+
+logger = RepoLogger(__file__)
 
 
 class RebaseColoring(Coloring):
@@ -104,17 +108,15 @@ branch but need to incorporate new upstream changes "underneath" them.
         one_project = len(all_projects) == 1
 
         if opt.interactive and not one_project:
-            print(
-                "error: interactive rebase not supported with multiple "
-                "projects",
-                file=sys.stderr,
+            logger.error(
+                "error: interactive rebase not supported with multiple projects"
             )
+
             if len(args) == 1:
-                print(
-                    "note: project %s is mapped to more than one path"
-                    % (args[0],),
-                    file=sys.stderr,
+                logger.warning(
+                    "note: project %s is mapped to more than one path", args[0]
                 )
+
             return 1
 
         # Setup the common git rebase args that we use for all projects.
@@ -145,10 +147,9 @@ branch but need to incorporate new upstream changes "underneath" them.
             cb = project.CurrentBranch
             if not cb:
                 if one_project:
-                    print(
-                        "error: project %s has a detached HEAD"
-                        % _RelPath(project),
-                        file=sys.stderr,
+                    logger.error(
+                        "error: project %s has a detached HEAD",
+                        _RelPath(project),
                     )
                     return 1
                 # Ignore branches with detached HEADs.
@@ -157,10 +158,9 @@ branch but need to incorporate new upstream changes "underneath" them.
             upbranch = project.GetBranch(cb)
             if not upbranch.LocalMerge:
                 if one_project:
-                    print(
-                        "error: project %s does not track any remote branches"
-                        % _RelPath(project),
-                        file=sys.stderr,
+                    logger.error(
+                        "error: project %s does not track any remote branches",
+                        _RelPath(project),
                     )
                     return 1
                 # Ignore branches without remotes.

subcmds/selfupdate.py

@@ -12,12 +12,21 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from optparse import SUPPRESS_HELP
-import sys
+import optparse
 
-from command import Command, MirrorSafeCommand
-from subcmds.sync import _PostRepoUpgrade
+from command import Command
+from command import MirrorSafeCommand
+from error import RepoExitError
+from repo_logging import RepoLogger
 from subcmds.sync import _PostRepoFetch
+from subcmds.sync import _PostRepoUpgrade
+
+
+logger = RepoLogger(__file__)
+
+
+class SelfupdateError(RepoExitError):
+    """Exit error for failed selfupdate command."""
 
 
 class Selfupdate(Command, MirrorSafeCommand):
@@ -47,7 +56,7 @@ need to be performed by an end-user.
             "--repo-upgraded",
             dest="repo_upgraded",
             action="store_true",
-            help=SUPPRESS_HELP,
+            help=optparse.SUPPRESS_HELP,
         )
 
     def Execute(self, opt, args):
@@ -58,9 +67,10 @@ need to be performed by an end-user.
             _PostRepoUpgrade(self.manifest)
 
         else:
-            if not rp.Sync_NetworkHalf().success:
-                print("error: can't update repo", file=sys.stderr)
-                sys.exit(1)
+            result = rp.Sync_NetworkHalf()
+            if result.error:
+                logger.error("error: can't update repo")
+                raise SelfupdateError(aggregate_errors=[result.error])
 
             rp.bare_git.gc("--auto")
             _PostRepoFetch(rp, repo_verify=opt.repo_verify, verbose=True)

subcmds/stage.py

@@ -17,6 +17,10 @@ import sys
 from color import Coloring
 from command import InteractiveCommand
 from git_command import GitCommand
+from repo_logging import RepoLogger
+
+
+logger = RepoLogger(__file__)
 
 
 class _ProjectList(Coloring):
@@ -62,7 +66,7 @@ The '%prog' command stages files to prepare the next commit.
             if p.IsDirty()
         ]
         if not all_projects:
-            print("no projects have uncommitted modifications", file=sys.stderr)
+            logger.error("no projects have uncommitted modifications")
             return
 
         out = _ProjectList(self.manifest.manifestProject.config)

subcmds/start.py

@@ -13,15 +13,28 @@
 # limitations under the License.
 
 import functools
-import os
-import sys
+from typing import NamedTuple
 
-from command import Command, DEFAULT_LOCAL_JOBS
-from git_config import IsImmutable
+from command import Command
+from command import DEFAULT_LOCAL_JOBS
+from error import RepoExitError
 from git_command import git
-import gitc_utils
+from git_config import IsImmutable
 from progress import Progress
-from project import SyncBuffer
+from project import Project
+from repo_logging import RepoLogger
+
+
+logger = RepoLogger(__file__)
+
+
+class ExecuteOneResult(NamedTuple):
+    project: Project
+    error: Exception
+
+
+class StartError(RepoExitError):
+    """Exit error for failed start command."""
 
 
 class Start(Command):
@@ -73,6 +86,7 @@ revision specified in the manifest.
         # a change, then we can't push back to it. Substitute with
         # dest_branch, if defined; or with manifest default revision instead.
         branch_merge = ""
+        error = None
         if IsImmutable(project.revisionExpr):
             if project.dest_branch:
                 branch_merge = project.dest_branch
@@ -80,19 +94,17 @@ revision specified in the manifest.
                 branch_merge = self.manifest.default.revisionExpr
 
         try:
-            ret = project.StartBranch(
+            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)
+            logger.error("error: unable to checkout %s: %s", project.name, e)
+            error = e
+        return ExecuteOneResult(project, error)
 
     def Execute(self, opt, args):
         nb = args[0]
+        err_projects = []
         err = []
         projects = []
         if not opt.all:
@@ -102,53 +114,14 @@ revision specified in the manifest.
 
         all_projects = self.GetProjects(
             projects,
-            missing_ok=bool(self.gitc_manifest),
             all_manifests=not opt.this_manifest_only,
         )
 
-        # This must happen after we find all_projects, since GetProjects may
-        # need the local directory, which will disappear once we save the GITC
-        # manifest.
-        if self.gitc_manifest:
-            gitc_projects = self.GetProjects(
-                projects, manifest=self.gitc_manifest, missing_ok=True
-            )
-            for project in gitc_projects:
-                if project.old_revision:
-                    project.already_synced = True
-                else:
-                    project.already_synced = False
-                    project.old_revision = project.revisionExpr
-                project.revisionExpr = None
-            # Save the GITC manifest.
-            gitc_utils.save_manifest(self.gitc_manifest)
-
-            # Make sure we have a valid CWD.
-            if not os.path.exists(os.getcwd()):
-                os.chdir(self.manifest.topdir)
-
-            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:
-                    proj_localdir = os.path.join(
-                        self.gitc_manifest.gitc_client_dir, project.relpath
-                    )
-                    project.worktree = proj_localdir
-                    if not os.path.exists(proj_localdir):
-                        os.makedirs(proj_localdir)
-                    project.Sync_NetworkHalf()
-                    sync_buf = SyncBuffer(self.manifest.manifestProject.config)
-                    project.Sync_LocalHalf(sync_buf)
-                    project.revisionId = gitc_project.old_revision
-                pm.update(msg="")
-            pm.end()
-
         def _ProcessResults(_pool, pm, results):
-            for result, project in results:
-                if not result:
-                    err.append(project)
+            for result in results:
+                if result.error:
+                    err_projects.append(result.project)
+                    err.append(result.error)
                 pm.update(msg="")
 
         self.ExecuteInParallel(
@@ -157,17 +130,19 @@ revision specified in the manifest.
             all_projects,
             callback=_ProcessResults,
             output=Progress(
-                "Starting %s" % (nb,), len(all_projects), quiet=opt.quiet
+                f"Starting {nb}", len(all_projects), quiet=opt.quiet
             ),
         )
 
-        if err:
-            for p in err:
-                print(
-                    "error: %s/: cannot start %s"
-                    % (p.RelPath(local=opt.this_manifest_only), nb),
-                    file=sys.stderr,
+        if err_projects:
+            for p in err_projects:
+                logger.error(
+                    "error: %s/: cannot start %s",
+                    p.RelPath(local=opt.this_manifest_only),
+                    nb,
                 )
             msg_fmt = "cannot start %d project(s)"
-            self.git_event_log.ErrorEvent(msg_fmt % (len(err)), msg_fmt)
-            sys.exit(1)
+            self.git_event_log.ErrorEvent(
+                msg_fmt % (len(err_projects)), msg_fmt
+            )
+            raise StartError(aggregate_errors=err)

subcmds/status.py

@@ -17,9 +17,9 @@ import glob
 import io
 import os
 
-from command import DEFAULT_LOCAL_JOBS, PagedCommand
-
 from color import Coloring
+from command import DEFAULT_LOCAL_JOBS
+from command import PagedCommand
 import platform_utils
 
 

subcmds/upload.py

@@ -19,16 +19,26 @@ import re
 import sys
 from typing import List
 
-from command import DEFAULT_LOCAL_JOBS, InteractiveCommand
+from command import DEFAULT_LOCAL_JOBS
+from command import InteractiveCommand
 from editor import Editor
+from error import GitError
+from error import SilentRepoExitError
 from error import UploadError
 from git_command import GitCommand
 from git_refs import R_HEADS
 from hooks import RepoHook
 from project import ReviewableBranch
+from repo_logging import RepoLogger
+from subcmds.sync import LocalSyncState
 
 
 _DEFAULT_UNUSUAL_COMMIT_THRESHOLD = 5
+logger = RepoLogger(__file__)
+
+
+class UploadExitError(SilentRepoExitError):
+    """Indicates that there is an upload command error requiring a sys exit."""
 
 
 def _VerifyPendingCommits(branches: List[ReviewableBranch]) -> bool:
@@ -62,16 +72,16 @@ def _VerifyPendingCommits(branches: List[ReviewableBranch]) -> bool:
     # If any branch has many commits, prompt the user.
     if many_commits:
         if len(branches) > 1:
-            print(
+            logger.warning(
                 "ATTENTION: One or more branches has an unusually high number "
                 "of commits."
             )
         else:
-            print(
+            logger.warning(
                 "ATTENTION: You are uploading an unusually high number of "
                 "commits."
             )
-        print(
+        logger.warning(
             "YOU PROBABLY DO NOT MEAN TO DO THIS. (Did you rebase across "
             "branches?)"
         )
@@ -85,8 +95,8 @@ def _VerifyPendingCommits(branches: List[ReviewableBranch]) -> bool:
 
 def _die(fmt, *args):
     msg = fmt % args
-    print("error: %s" % msg, file=sys.stderr)
-    sys.exit(1)
+    logger.error("error: %s", msg)
+    raise UploadExitError(msg)
 
 
 def _SplitEmails(values):
@@ -528,137 +538,137 @@ Gerrit Code Review:  https://www.gerritcodereview.com/
         except (AttributeError, IndexError):
             return ""
 
-    def _UploadAndReport(self, opt, todo, original_people):
+    def _UploadBranch(self, opt, branch, original_people):
+        """Upload Branch."""
+        people = copy.deepcopy(original_people)
+        self._AppendAutoList(branch, people)
+
+        # Check if there are local changes that may have been forgotten.
+        changes = branch.project.UncommitedFiles()
+        if opt.ignore_untracked_files:
+            untracked = set(branch.project.UntrackedFiles())
+            changes = [x for x in changes if x not in untracked]
+
+        if changes:
+            key = "review.%s.autoupload" % branch.project.remote.review
+            answer = branch.project.config.GetBoolean(key)
+
+            # If they want to auto upload, let's not ask because it
+            # could be automated.
+            if answer is None:
+                print()
+                print(
+                    "Uncommitted changes in %s (did you forget to "
+                    "amend?):" % branch.project.name
+                )
+                print("\n".join(changes))
+                print("Continue uploading? (y/N) ", end="", flush=True)
+                if opt.yes:
+                    print("<--yes>")
+                    a = "yes"
+                else:
+                    a = sys.stdin.readline().strip().lower()
+                if a not in ("y", "yes", "t", "true", "on"):
+                    print("skipping upload", file=sys.stderr)
+                    branch.uploaded = False
+                    branch.error = "User aborted"
+                    return
+
+        # Check if topic branches should be sent to the server during
+        # upload.
+        if opt.auto_topic is not True:
+            key = "review.%s.uploadtopic" % branch.project.remote.review
+            opt.auto_topic = branch.project.config.GetBoolean(key)
+
+        def _ExpandCommaList(value):
+            """Split |value| up into comma delimited entries."""
+            if not value:
+                return
+            for ret in value.split(","):
+                ret = ret.strip()
+                if ret:
+                    yield ret
+
+        # Check if hashtags should be included.
+        key = "review.%s.uploadhashtags" % branch.project.remote.review
+        hashtags = set(_ExpandCommaList(branch.project.config.GetString(key)))
+        for tag in opt.hashtags:
+            hashtags.update(_ExpandCommaList(tag))
+        if opt.hashtag_branch:
+            hashtags.add(branch.name)
+
+        # Check if labels should be included.
+        key = "review.%s.uploadlabels" % branch.project.remote.review
+        labels = set(_ExpandCommaList(branch.project.config.GetString(key)))
+        for label in opt.labels:
+            labels.update(_ExpandCommaList(label))
+
+        # Handle e-mail notifications.
+        if opt.notify is False:
+            notify = "NONE"
+        else:
+            key = "review.%s.uploadnotify" % branch.project.remote.review
+            notify = branch.project.config.GetString(key)
+
+        destination = opt.dest_branch or branch.project.dest_branch
+
+        if branch.project.dest_branch and not opt.dest_branch:
+            merge_branch = self._GetMergeBranch(
+                branch.project, local_branch=branch.name
+            )
+
+            full_dest = destination
+            if not full_dest.startswith(R_HEADS):
+                full_dest = R_HEADS + full_dest
+
+            # If the merge branch of the local branch is different from
+            # the project's revision AND destination, this might not be
+            # intentional.
+            if (
+                merge_branch
+                and merge_branch != branch.project.revisionExpr
+                and merge_branch != full_dest
+            ):
+                print(
+                    f"For local branch {branch.name}: merge branch "
+                    f"{merge_branch} does not match destination branch "
+                    f"{destination}"
+                )
+                print("skipping upload.")
+                print(
+                    f"Please use `--destination {destination}` if this "
+                    "is intentional"
+                )
+                branch.uploaded = False
+                return
+
+        branch.UploadForReview(
+            people,
+            dryrun=opt.dryrun,
+            auto_topic=opt.auto_topic,
+            hashtags=hashtags,
+            labels=labels,
+            private=opt.private,
+            notify=notify,
+            wip=opt.wip,
+            ready=opt.ready,
+            dest_branch=destination,
+            validate_certs=opt.validate_certs,
+            push_options=opt.push_options,
+        )
+
+        branch.uploaded = True
+
+    def _UploadAndReport(self, opt, todo, people):
         have_errors = False
+        aggregate_errors = []
         for branch in todo:
             try:
-                people = copy.deepcopy(original_people)
-                self._AppendAutoList(branch, people)
-
-                # Check if there are local changes that may have been forgotten.
-                changes = branch.project.UncommitedFiles()
-                if opt.ignore_untracked_files:
-                    untracked = set(branch.project.UntrackedFiles())
-                    changes = [x for x in changes if x not in untracked]
-
-                if changes:
-                    key = "review.%s.autoupload" % branch.project.remote.review
-                    answer = branch.project.config.GetBoolean(key)
-
-                    # If they want to auto upload, let's not ask because it
-                    # could be automated.
-                    if answer is None:
-                        print()
-                        print(
-                            "Uncommitted changes in %s (did you forget to "
-                            "amend?):" % branch.project.name
-                        )
-                        print("\n".join(changes))
-                        print("Continue uploading? (y/N) ", end="", flush=True)
-                        if opt.yes:
-                            print("<--yes>")
-                            a = "yes"
-                        else:
-                            a = sys.stdin.readline().strip().lower()
-                        if a not in ("y", "yes", "t", "true", "on"):
-                            print("skipping upload", file=sys.stderr)
-                            branch.uploaded = False
-                            branch.error = "User aborted"
-                            continue
-
-                # Check if topic branches should be sent to the server during
-                # upload.
-                if opt.auto_topic is not True:
-                    key = "review.%s.uploadtopic" % branch.project.remote.review
-                    opt.auto_topic = branch.project.config.GetBoolean(key)
-
-                def _ExpandCommaList(value):
-                    """Split |value| up into comma delimited entries."""
-                    if not value:
-                        return
-                    for ret in value.split(","):
-                        ret = ret.strip()
-                        if ret:
-                            yield ret
-
-                # Check if hashtags should be included.
-                key = "review.%s.uploadhashtags" % branch.project.remote.review
-                hashtags = set(
-                    _ExpandCommaList(branch.project.config.GetString(key))
-                )
-                for tag in opt.hashtags:
-                    hashtags.update(_ExpandCommaList(tag))
-                if opt.hashtag_branch:
-                    hashtags.add(branch.name)
-
-                # Check if labels should be included.
-                key = "review.%s.uploadlabels" % branch.project.remote.review
-                labels = set(
-                    _ExpandCommaList(branch.project.config.GetString(key))
-                )
-                for label in opt.labels:
-                    labels.update(_ExpandCommaList(label))
-
-                # Handle e-mail notifications.
-                if opt.notify is False:
-                    notify = "NONE"
-                else:
-                    key = (
-                        "review.%s.uploadnotify" % branch.project.remote.review
-                    )
-                    notify = branch.project.config.GetString(key)
-
-                destination = opt.dest_branch or branch.project.dest_branch
-
-                if branch.project.dest_branch and not opt.dest_branch:
-                    merge_branch = self._GetMergeBranch(
-                        branch.project, local_branch=branch.name
-                    )
-
-                    full_dest = destination
-                    if not full_dest.startswith(R_HEADS):
-                        full_dest = R_HEADS + full_dest
-
-                    # If the merge branch of the local branch is different from
-                    # the project's revision AND destination, this might not be
-                    # intentional.
-                    if (
-                        merge_branch
-                        and merge_branch != branch.project.revisionExpr
-                        and merge_branch != full_dest
-                    ):
-                        print(
-                            f"For local branch {branch.name}: merge branch "
-                            f"{merge_branch} does not match destination branch "
-                            f"{destination}"
-                        )
-                        print("skipping upload.")
-                        print(
-                            f"Please use `--destination {destination}` if this "
-                            "is intentional"
-                        )
-                        branch.uploaded = False
-                        continue
-
-                branch.UploadForReview(
-                    people,
-                    dryrun=opt.dryrun,
-                    auto_topic=opt.auto_topic,
-                    hashtags=hashtags,
-                    labels=labels,
-                    private=opt.private,
-                    notify=notify,
-                    wip=opt.wip,
-                    ready=opt.ready,
-                    dest_branch=destination,
-                    validate_certs=opt.validate_certs,
-                    push_options=opt.push_options,
-                )
-
-                branch.uploaded = True
-            except UploadError as e:
+                self._UploadBranch(opt, branch, people)
+            except (UploadError, GitError) as e:
                 self.git_event_log.ErrorEvent(f"upload error: {e}")
                 branch.error = e
+                aggregate_errors.append(e)
                 branch.uploaded = False
                 have_errors = True
 
@@ -697,7 +707,7 @@ Gerrit Code Review:  https://www.gerritcodereview.com/
                 )
 
         if have_errors:
-            sys.exit(1)
+            raise UploadExitError(aggregate_errors=aggregate_errors)
 
     def _GetMergeBranch(self, project, local_branch=None):
         if local_branch is None:
@@ -740,16 +750,13 @@ Gerrit Code Review:  https://www.gerritcodereview.com/
             for result in results:
                 project, avail = result
                 if avail is None:
-                    print(
+                    logger.error(
                         'repo: error: %s: Unable to upload branch "%s". '
                         "You might be able to fix the branch by running:\n"
-                        "  git branch --set-upstream-to m/%s"
-                        % (
-                            project.RelPath(local=opt.this_manifest_only),
-                            project.CurrentBranch,
-                            project.manifest.branch,
-                        ),
-                        file=sys.stderr,
+                        "  git branch --set-upstream-to m/%s",
+                        project.RelPath(local=opt.this_manifest_only),
+                        project.CurrentBranch,
+                        project.manifest.branch,
                     )
                 elif avail:
                     pending.append(result)
@@ -764,14 +771,11 @@ Gerrit Code Review:  https://www.gerritcodereview.com/
 
         if not pending:
             if opt.branch is None:
-                print(
-                    "repo: error: no branches ready for upload", file=sys.stderr
-                )
+                logger.error("repo: error: no branches ready for upload")
             else:
-                print(
-                    'repo: error: no branches named "%s" ready for upload'
-                    % (opt.branch,),
-                    file=sys.stderr,
+                logger.error(
+                    'repo: error: no branches named "%s" ready for upload',
+                    opt.branch,
                 )
             return 1
 
@@ -800,6 +804,11 @@ Gerrit Code Review:  https://www.gerritcodereview.com/
             if not hook.Run(
                 project_list=pending_proj_names, worktree_list=pending_worktrees
             ):
+                if LocalSyncState(manifest).IsPartiallySynced():
+                    logger.error(
+                        "Partially synced tree detected. Syncing all projects "
+                        "may resolve issues you're seeing."
+                    )
                 ret = 1
         if ret:
             return ret

subcmds/version.py

@@ -15,8 +15,11 @@
 import platform
 import sys
 
-from command import Command, MirrorSafeCommand
-from git_command import git, RepoSourceVersion, user_agent
+from command import Command
+from command import MirrorSafeCommand
+from git_command import git
+from git_command import RepoSourceVersion
+from git_command import user_agent
 from git_refs import HEAD
 from wrapper import Wrapper
 
@@ -39,35 +42,28 @@ class Version(Command, MirrorSafeCommand):
         # These might not be the same.  Report them both.
         src_ver = RepoSourceVersion()
         rp_ver = rp.bare_git.describe(HEAD)
-        print("repo version %s" % rp_ver)
-        print("       (from %s)" % rem.url)
-        print("       (tracking %s)" % branch.merge)
-        print("       (%s)" % rp.bare_git.log("-1", "--format=%cD", HEAD))
+        print(f"repo version {rp_ver}")
+        print(f"       (from {rem.url})")
+        print(f"       (tracking {branch.merge})")
+        print(f"       ({rp.bare_git.log('-1', '--format=%cD', HEAD)})")
 
         if self.wrapper_path is not None:
-            print("repo launcher version %s" % self.wrapper_version)
-            print("       (from %s)" % self.wrapper_path)
+            print(f"repo launcher version {self.wrapper_version}")
+            print(f"       (from {self.wrapper_path})")
 
             if src_ver != rp_ver:
-                print("       (currently at %s)" % src_ver)
+                print(f"       (currently at {src_ver})")
 
-        print("repo User-Agent %s" % user_agent.repo)
-        print("git %s" % git.version_tuple().full)
-        print("git User-Agent %s" % user_agent.git)
-        print("Python %s" % sys.version)
+        print(f"repo User-Agent {user_agent.repo}")
+        print(f"git {git.version_tuple().full}")
+        print(f"git User-Agent {user_agent.git}")
+        print(f"Python {sys.version}")
         uname = platform.uname()
         if sys.version_info.major < 3:
             # Python 3 returns a named tuple, but Python 2 is simpler.
             print(uname)
         else:
-            print(
-                "OS %s %s (%s)" % (uname.system, uname.release, uname.version)
-            )
-            print(
-                "CPU %s (%s)"
-                % (
-                    uname.machine,
-                    uname.processor if uname.processor else "unknown",
-                )
-            )
+            print(f"OS {uname.system} {uname.release} ({uname.version})")
+            processor = uname.processor if uname.processor else "unknown"
+            print(f"CPU {uname.machine} ({processor})")
         print("Bug reports:", Wrapper().BUG_URL)

tests/conftest.py

@@ -14,8 +14,11 @@
 
 """Common fixtures for pytests."""
 
+import pathlib
+
 import pytest
 
+import platform_utils
 import repo_trace
 
 
@@ -23,3 +26,58 @@ import repo_trace
 def disable_repo_trace(tmp_path):
     """Set an environment marker to relax certain strict checks for test code."""  # noqa: E501
     repo_trace._TRACE_FILE = str(tmp_path / "TRACE_FILE_from_test")
+
+
+# adapted from pytest-home 0.5.1
+def _set_home(monkeypatch, path: pathlib.Path):
+    """
+    Set the home dir using a pytest monkeypatch context.
+    """
+    win = platform_utils.isWindows()
+    vars = ["HOME"] + win * ["USERPROFILE"]
+    for var in vars:
+        monkeypatch.setenv(var, str(path))
+    return path
+
+
+# copied from
+# https://github.com/pytest-dev/pytest/issues/363#issuecomment-1335631998
+@pytest.fixture(scope="session")
+def monkeysession():
+    with pytest.MonkeyPatch.context() as mp:
+        yield mp
+
+
+@pytest.fixture(autouse=True, scope="session")
+def session_tmp_home_dir(tmp_path_factory, monkeysession):
+    """Set HOME to a temporary directory, avoiding user's .gitconfig.
+
+    b/302797407
+
+    Set home at session scope to take effect prior to
+    ``test_wrapper.GitCheckoutTestCase.setUpClass``.
+    """
+    return _set_home(monkeysession, tmp_path_factory.mktemp("home"))
+
+
+# adapted from pytest-home 0.5.1
+@pytest.fixture(autouse=True)
+def tmp_home_dir(monkeypatch, tmp_path_factory):
+    """Set HOME to a temporary directory.
+
+    Ensures that state doesn't accumulate in $HOME across tests.
+
+    Note that in conjunction with session_tmp_homedir, the HOME
+    dir is patched twice, once at session scope, and then again at
+    the function scope.
+    """
+    return _set_home(monkeypatch, tmp_path_factory.mktemp("home"))
+
+
+@pytest.fixture(autouse=True)
+def setup_user_identity(monkeysession, scope="session"):
+    """Set env variables for author and committer name and email."""
+    monkeysession.setenv("GIT_AUTHOR_NAME", "Foo Bar")
+    monkeysession.setenv("GIT_COMMITTER_NAME", "Foo Bar")
+    monkeysession.setenv("GIT_AUTHOR_EMAIL", "foo@bar.baz")
+    monkeysession.setenv("GIT_COMMITTER_EMAIL", "foo@bar.baz")

tests/test_error.py

@@ -18,7 +18,21 @@ import inspect
 import pickle
 import unittest
 
+import command
 import error
+import fetch
+import git_command
+import project
+from subcmds import all_modules
+
+
+imports = all_modules + [
+    error,
+    project,
+    git_command,
+    fetch,
+    command,
+]
 
 
 class PickleTests(unittest.TestCase):
@@ -26,10 +40,11 @@ class PickleTests(unittest.TestCase):
 
     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
+        for entry in imports:
+            for name in dir(entry):
+                cls = getattr(entry, name)
+                if isinstance(cls, type) and issubclass(cls, Exception):
+                    yield cls
 
     def testExceptionLookup(self):
         """Make sure our introspection logic works."""

tests/test_git_command.py

@@ -14,14 +14,12 @@
 
 """Unittests for the git_command.py module."""
 
-import re
+import io
 import os
+import re
+import subprocess
 import unittest
-
-try:
-    from unittest import mock
-except ImportError:
-    import mock
+from unittest import mock
 
 import git_command
 import wrapper
@@ -65,6 +63,169 @@ class GitCommandTest(unittest.TestCase):
         )
 
 
+class GitCommandWaitTest(unittest.TestCase):
+    """Tests the GitCommand class .Wait()"""
+
+    def setUp(self):
+        class MockPopen:
+            rc = 0
+
+            def __init__(self):
+                self.stdout = io.BufferedReader(io.BytesIO())
+                self.stderr = io.BufferedReader(io.BytesIO())
+
+            def communicate(
+                self, input: str = None, timeout: float = None
+            ) -> [str, str]:
+                """Mock communicate fn."""
+                return ["", ""]
+
+            def wait(self, timeout=None):
+                return self.rc
+
+        self.popen = popen = MockPopen()
+
+        def popen_mock(*args, **kwargs):
+            return popen
+
+        def realpath_mock(val):
+            return val
+
+        mock.patch.object(subprocess, "Popen", side_effect=popen_mock).start()
+
+        mock.patch.object(
+            os.path, "realpath", side_effect=realpath_mock
+        ).start()
+
+    def tearDown(self):
+        mock.patch.stopall()
+
+    def test_raises_when_verify_non_zero_result(self):
+        self.popen.rc = 1
+        r = git_command.GitCommand(None, ["status"], verify_command=True)
+        with self.assertRaises(git_command.GitCommandError):
+            r.Wait()
+
+    def test_returns_when_no_verify_non_zero_result(self):
+        self.popen.rc = 1
+        r = git_command.GitCommand(None, ["status"], verify_command=False)
+        self.assertEqual(1, r.Wait())
+
+    def test_default_returns_non_zero_result(self):
+        self.popen.rc = 1
+        r = git_command.GitCommand(None, ["status"])
+        self.assertEqual(1, r.Wait())
+
+
+class GitCommandStreamLogsTest(unittest.TestCase):
+    """Tests the GitCommand class stderr log streaming cases."""
+
+    def setUp(self):
+        self.mock_process = mock.MagicMock()
+        self.mock_process.communicate.return_value = (None, None)
+        self.mock_process.wait.return_value = 0
+
+        self.mock_popen = mock.MagicMock()
+        self.mock_popen.return_value = self.mock_process
+        mock.patch("subprocess.Popen", self.mock_popen).start()
+
+    def tearDown(self):
+        mock.patch.stopall()
+
+    def test_does_not_stream_logs_when_input_is_set(self):
+        git_command.GitCommand(None, ["status"], input="foo")
+
+        self.mock_popen.assert_called_once_with(
+            ["git", "status"],
+            cwd=None,
+            env=mock.ANY,
+            encoding="utf-8",
+            errors="backslashreplace",
+            stdin=subprocess.PIPE,
+            stdout=None,
+            stderr=None,
+        )
+        self.mock_process.communicate.assert_called_once_with(input="foo")
+        self.mock_process.stderr.read1.assert_not_called()
+
+    def test_does_not_stream_logs_when_stdout_is_set(self):
+        git_command.GitCommand(None, ["status"], capture_stdout=True)
+
+        self.mock_popen.assert_called_once_with(
+            ["git", "status"],
+            cwd=None,
+            env=mock.ANY,
+            encoding="utf-8",
+            errors="backslashreplace",
+            stdin=None,
+            stdout=subprocess.PIPE,
+            stderr=None,
+        )
+        self.mock_process.communicate.assert_called_once_with(input=None)
+        self.mock_process.stderr.read1.assert_not_called()
+
+    def test_does_not_stream_logs_when_stderr_is_set(self):
+        git_command.GitCommand(None, ["status"], capture_stderr=True)
+
+        self.mock_popen.assert_called_once_with(
+            ["git", "status"],
+            cwd=None,
+            env=mock.ANY,
+            encoding="utf-8",
+            errors="backslashreplace",
+            stdin=None,
+            stdout=None,
+            stderr=subprocess.PIPE,
+        )
+        self.mock_process.communicate.assert_called_once_with(input=None)
+        self.mock_process.stderr.read1.assert_not_called()
+
+    def test_does_not_stream_logs_when_merge_output_is_set(self):
+        git_command.GitCommand(None, ["status"], merge_output=True)
+
+        self.mock_popen.assert_called_once_with(
+            ["git", "status"],
+            cwd=None,
+            env=mock.ANY,
+            encoding="utf-8",
+            errors="backslashreplace",
+            stdin=None,
+            stdout=None,
+            stderr=subprocess.STDOUT,
+        )
+        self.mock_process.communicate.assert_called_once_with(input=None)
+        self.mock_process.stderr.read1.assert_not_called()
+
+    @mock.patch("sys.stderr")
+    def test_streams_stderr_when_no_stream_is_set(self, mock_stderr):
+        logs = "\n".join(
+            [
+                "Enumerating objects: 5, done.",
+                "Counting objects: 100% (5/5), done.",
+                "Writing objects: 100% (3/3), 330 bytes | 330 KiB/s, done.",
+                "remote: Processing changes: refs: 1, new: 1, done ",
+                "remote: SUCCESS",
+            ]
+        )
+        self.mock_process.stderr = io.BufferedReader(
+            io.BytesIO(bytes(logs, "utf-8"))
+        )
+
+        cmd = git_command.GitCommand(None, ["push"])
+
+        self.mock_popen.assert_called_once_with(
+            ["git", "push"],
+            cwd=None,
+            env=mock.ANY,
+            stdin=None,
+            stdout=None,
+            stderr=subprocess.PIPE,
+        )
+        self.mock_process.communicate.assert_not_called()
+        mock_stderr.write.assert_called_once_with(logs)
+        self.assertEqual(cmd.stderr, logs)
+
+
 class GitCallUnitTest(unittest.TestCase):
     """Tests the _GitCall class (via git_command.git)."""
 
@@ -153,12 +314,31 @@ class GitRequireTests(unittest.TestCase):
 
     def test_older_fatal(self):
         """Test fatal require calls with old versions."""
-        with self.assertRaises(SystemExit) as e:
+        with self.assertRaises(git_command.GitRequireError) as e:
             git_command.git_require((2,), fail=True)
             self.assertNotEqual(0, e.code)
 
     def test_older_fatal_msg(self):
         """Test fatal require calls with old versions and message."""
-        with self.assertRaises(SystemExit) as e:
+        with self.assertRaises(git_command.GitRequireError) as e:
             git_command.git_require((2,), fail=True, msg="so sad")
             self.assertNotEqual(0, e.code)
+
+
+class GitCommandErrorTest(unittest.TestCase):
+    """Test for the GitCommandError class."""
+
+    def test_augument_stderr(self):
+        self.assertEqual(
+            git_command.GitCommandError(
+                git_stderr="couldn't find remote ref refs/heads/foo"
+            ).suggestion,
+            "Check if the provided ref exists in the remote.",
+        )
+
+        self.assertEqual(
+            git_command.GitCommandError(
+                git_stderr="'foobar' does not appear to be a git repository"
+            ).suggestion,
+            "Are you running this repo command outside of a repo workspace?",
+        )

tests/test_git_config.py

@@ -100,7 +100,7 @@ class GitConfigReadOnlyTests(unittest.TestCase):
             ("intg", 10737418240),
         )
         for key, value in TESTS:
-            self.assertEqual(value, self.config.GetInt("section.%s" % (key,)))
+            self.assertEqual(value, self.config.GetInt(f"section.{key}"))
 
 
 class GitConfigReadWriteTests(unittest.TestCase):

tests/test_git_superproject.py

@@ -21,10 +21,11 @@ import tempfile
 import unittest
 from unittest import mock
 
+from test_manifest_xml import sort_attributes
+
 import git_superproject
 import git_trace2_event_log
 import manifest_xml
-from test_manifest_xml import sort_attributes
 
 
 class SuperprojectTestCase(unittest.TestCase):
@@ -33,7 +34,7 @@ class SuperprojectTestCase(unittest.TestCase):
     PARENT_SID_KEY = "GIT_TRACE2_PARENT_SID"
     PARENT_SID_VALUE = "parent_sid"
     SELF_SID_REGEX = r"repo-\d+T\d+Z-.*"
-    FULL_SID_REGEX = r"^%s/%s" % (PARENT_SID_VALUE, SELF_SID_REGEX)
+    FULL_SID_REGEX = rf"^{PARENT_SID_VALUE}/{SELF_SID_REGEX}"
 
     def setUp(self):
         """Set up superproject every time."""
@@ -107,7 +108,9 @@ class SuperprojectTestCase(unittest.TestCase):
             self.assertRegex(log_entry["sid"], self.FULL_SID_REGEX)
         else:
             self.assertRegex(log_entry["sid"], self.SELF_SID_REGEX)
-        self.assertRegex(log_entry["time"], r"^\d+-\d+-\d+T\d+:\d+:\d+\.\d+Z$")
+        self.assertRegex(
+            log_entry["time"], r"^\d+-\d+-\d+T\d+:\d+:\d+\.\d+\+00:00$"
+        )
 
     def readLog(self, log_path):
         """Helper function to read log data into a list."""
@@ -246,7 +249,7 @@ class SuperprojectTestCase(unittest.TestCase):
         os.mkdir(self._superproject._superproject_path)
         manifest_path = self._superproject._WriteManifestFile()
         self.assertIsNotNone(manifest_path)
-        with open(manifest_path, "r") as fp:
+        with open(manifest_path) as fp:
             manifest_xml_data = fp.read()
         self.assertEqual(
             sort_attributes(manifest_xml_data),
@@ -281,7 +284,7 @@ class SuperprojectTestCase(unittest.TestCase):
                     )
                     self.assertIsNotNone(update_result.manifest_path)
                     self.assertFalse(update_result.fatal)
-                    with open(update_result.manifest_path, "r") as fp:
+                    with open(update_result.manifest_path) as fp:
                         manifest_xml_data = fp.read()
                     self.assertEqual(
                         sort_attributes(manifest_xml_data),
@@ -368,7 +371,7 @@ class SuperprojectTestCase(unittest.TestCase):
                     )
                     self.assertIsNotNone(update_result.manifest_path)
                     self.assertFalse(update_result.fatal)
-                    with open(update_result.manifest_path, "r") as fp:
+                    with open(update_result.manifest_path) as fp:
                         manifest_xml_data = fp.read()
                     # Verify platform/vendor/x's project revision hasn't
                     # changed.
@@ -433,7 +436,7 @@ class SuperprojectTestCase(unittest.TestCase):
                     )
                     self.assertIsNotNone(update_result.manifest_path)
                     self.assertFalse(update_result.fatal)
-                    with open(update_result.manifest_path, "r") as fp:
+                    with open(update_result.manifest_path) as fp:
                         manifest_xml_data = fp.read()
                     # Verify platform/vendor/x's project revision hasn't
                     # changed.
@@ -489,7 +492,9 @@ class SuperprojectTestCase(unittest.TestCase):
 
                     self.assertTrue(self._superproject._Fetch())
                     self.assertEqual(
-                        mock_git_command.call_args.args,
+                        # TODO: Once we require Python 3.8+,
+                        #  use 'mock_git_command.call_args.args'.
+                        mock_git_command.call_args[0],
                         (
                             None,
                             [
@@ -509,7 +514,9 @@ class SuperprojectTestCase(unittest.TestCase):
                     # If branch for revision exists, set as --negotiation-tip.
                     self.assertTrue(self._superproject._Fetch())
                     self.assertEqual(
-                        mock_git_command.call_args.args,
+                        # TODO: Once we require Python 3.8+,
+                        #  use 'mock_git_command.call_args.args'.
+                        mock_git_command.call_args[0],
                         (
                             None,
                             [

tests/test_git_trace2_event_log.py

@@ -61,7 +61,7 @@ class EventLogTestCase(unittest.TestCase):
     PARENT_SID_KEY = "GIT_TRACE2_PARENT_SID"
     PARENT_SID_VALUE = "parent_sid"
     SELF_SID_REGEX = r"repo-\d+T\d+Z-.*"
-    FULL_SID_REGEX = r"^%s/%s" % (PARENT_SID_VALUE, SELF_SID_REGEX)
+    FULL_SID_REGEX = rf"^{PARENT_SID_VALUE}/{SELF_SID_REGEX}"
 
     def setUp(self):
         """Load the event_log module every time."""
@@ -90,7 +90,9 @@ class EventLogTestCase(unittest.TestCase):
             self.assertRegex(log_entry["sid"], self.FULL_SID_REGEX)
         else:
             self.assertRegex(log_entry["sid"], self.SELF_SID_REGEX)
-        self.assertRegex(log_entry["time"], r"^\d+-\d+-\d+T\d+:\d+:\d+\.\d+Z$")
+        self.assertRegex(
+            log_entry["time"], r"^\d+-\d+-\d+T\d+:\d+:\d+\.\d+\+00:00$"
+        )
 
     def readLog(self, log_path):
         """Helper function to read log data into a list."""
@@ -338,8 +340,8 @@ class EventLogTestCase(unittest.TestCase):
         # Check for 'error' event specific fields.
         self.assertIn("msg", error_event)
         self.assertIn("fmt", error_event)
-        self.assertEqual(error_event["msg"], msg)
-        self.assertEqual(error_event["fmt"], fmt)
+        self.assertEqual(error_event["msg"], f"RepoErrorEvent:{msg}")
+        self.assertEqual(error_event["fmt"], f"RepoErrorEvent:{fmt}")
 
     def test_write_with_filename(self):
         """Test Write() with a path to a file exits with None."""

tests/test_hooks.py

@@ -14,9 +14,10 @@
 
 """Unittests for the hooks.py module."""
 
-import hooks
 import unittest
 
+import hooks
+
 
 class RepoHookShebang(unittest.TestCase):
     """Check shebang parsing in RepoHook."""

tests/test_manifest_xml.py

@@ -198,13 +198,13 @@ class ValueTests(unittest.TestCase):
     def test_bool_true(self):
         """Check XmlBool true values."""
         for value in ("yes", "true", "1"):
-            node = self._get_node('<node a="%s"/>' % (value,))
+            node = self._get_node(f'<node a="{value}"/>')
             self.assertTrue(manifest_xml.XmlBool(node, "a"))
 
     def test_bool_false(self):
         """Check XmlBool false values."""
         for value in ("no", "false", "0"):
-            node = self._get_node('<node a="%s"/>' % (value,))
+            node = self._get_node(f'<node a="{value}"/>')
             self.assertFalse(manifest_xml.XmlBool(node, "a"))
 
     def test_int_default(self):
@@ -220,7 +220,7 @@ class ValueTests(unittest.TestCase):
     def test_int_good(self):
         """Check XmlInt numeric handling."""
         for value in (-1, 0, 1, 50000):
-            node = self._get_node('<node a="%s"/>' % (value,))
+            node = self._get_node(f'<node a="{value}"/>')
             self.assertEqual(value, manifest_xml.XmlInt(node, "a"))
 
     def test_int_invalid(self):
@@ -385,6 +385,21 @@ class XmlManifestTests(ManifestParseTestCase):
             "</manifest>",
         )
 
+    def test_parse_with_xml_doctype(self):
+        """Check correct manifest parse with DOCTYPE node present."""
+        manifest = self.getXmlManifest(
+            """<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE manifest []>
+<manifest>
+  <remote name="test-remote" fetch="http://localhost" />
+  <default remote="test-remote" revision="refs/heads/main" />
+  <project name="test-project" path="src/test-project"/>
+</manifest>
+"""
+        )
+        self.assertEqual(len(manifest.projects), 1)
+        self.assertEqual(manifest.projects[0].name, "test-project")
+
 
 class IncludeElementTests(ManifestParseTestCase):
     """Tests for <include>."""
@@ -996,6 +1011,44 @@ class RemoveProjectElementTests(ManifestParseTestCase):
         )
         self.assertEqual(manifest.projects, [])
 
+    def test_remove_using_path_attrib(self):
+        manifest = self.getXmlManifest(
+            """
+<manifest>
+  <remote name="default-remote" fetch="http://localhost" />
+  <default remote="default-remote" revision="refs/heads/main" />
+  <project name="project1" path="tests/path1" />
+  <project name="project1" path="tests/path2" />
+  <project name="project2" />
+  <project name="project3" />
+  <project name="project4" path="tests/path3" />
+  <project name="project4" path="tests/path4" />
+  <project name="project5" />
+  <project name="project6" path="tests/path6" />
+
+  <remove-project name="project1" path="tests/path2" />
+  <remove-project name="project3" />
+  <remove-project name="project4" />
+  <remove-project path="project5" />
+  <remove-project path="tests/path6" />
+</manifest>
+"""
+        )
+        found_proj1_path1 = False
+        found_proj2 = False
+        for proj in manifest.projects:
+            if proj.name == "project1":
+                found_proj1_path1 = True
+                self.assertEqual(proj.relpath, "tests/path1")
+            if proj.name == "project2":
+                found_proj2 = True
+            self.assertNotEqual(proj.name, "project3")
+            self.assertNotEqual(proj.name, "project4")
+            self.assertNotEqual(proj.name, "project5")
+            self.assertNotEqual(proj.name, "project6")
+        self.assertTrue(found_proj1_path1)
+        self.assertTrue(found_proj2)
+
 
 class ExtendProjectElementTests(ManifestParseTestCase):
     """Tests for <extend-project>."""
@@ -1075,3 +1128,79 @@ class ExtendProjectElementTests(ManifestParseTestCase):
         )
         self.assertEqual(len(manifest.projects), 1)
         self.assertEqual(manifest.projects[0].upstream, "bar")
+
+
+class NormalizeUrlTests(ManifestParseTestCase):
+    """Tests for normalize_url() in manifest_xml.py"""
+
+    def test_has_trailing_slash(self):
+        url = "http://foo.com/bar/baz/"
+        self.assertEqual(
+            "http://foo.com/bar/baz", manifest_xml.normalize_url(url)
+        )
+
+        url = "http://foo.com/bar/"
+        self.assertEqual("http://foo.com/bar", manifest_xml.normalize_url(url))
+
+    def test_has_leading_slash(self):
+        """SCP-like syntax except a / comes before the : which git disallows."""
+        url = "/git@foo.com:bar/baf"
+        self.assertEqual(url, manifest_xml.normalize_url(url))
+
+        url = "gi/t@foo.com:bar/baf"
+        self.assertEqual(url, manifest_xml.normalize_url(url))
+
+        url = "git@fo/o.com:bar/baf"
+        self.assertEqual(url, manifest_xml.normalize_url(url))
+
+    def test_has_no_scheme(self):
+        """Deal with cases where we have no scheme, but we also
+        aren't dealing with the git SCP-like syntax
+        """
+        url = "foo.com/baf/bat"
+        self.assertEqual(url, manifest_xml.normalize_url(url))
+
+        url = "foo.com/baf"
+        self.assertEqual(url, manifest_xml.normalize_url(url))
+
+        url = "git@foo.com/baf/bat"
+        self.assertEqual(url, manifest_xml.normalize_url(url))
+
+        url = "git@foo.com/baf"
+        self.assertEqual(url, manifest_xml.normalize_url(url))
+
+        url = "/file/path/here"
+        self.assertEqual(url, manifest_xml.normalize_url(url))
+
+    def test_has_no_scheme_matches_scp_like_syntax(self):
+        url = "git@foo.com:bar/baf"
+        self.assertEqual(
+            "ssh://git@foo.com/bar/baf", manifest_xml.normalize_url(url)
+        )
+
+        url = "git@foo.com:bar/"
+        self.assertEqual(
+            "ssh://git@foo.com/bar", manifest_xml.normalize_url(url)
+        )
+
+    def test_remote_url_resolution(self):
+        remote = manifest_xml._XmlRemote(
+            name="foo",
+            fetch="git@github.com:org2/",
+            manifestUrl="git@github.com:org2/custom_manifest.git",
+        )
+        self.assertEqual("ssh://git@github.com/org2", remote.resolvedFetchUrl)
+
+        remote = manifest_xml._XmlRemote(
+            name="foo",
+            fetch="ssh://git@github.com/org2/",
+            manifestUrl="git@github.com:org2/custom_manifest.git",
+        )
+        self.assertEqual("ssh://git@github.com/org2", remote.resolvedFetchUrl)
+
+        remote = manifest_xml._XmlRemote(
+            name="foo",
+            fetch="git@github.com:org2/",
+            manifestUrl="ssh://git@github.com/org2/custom_manifest.git",
+        )
+        self.assertEqual("ssh://git@github.com/org2", remote.resolvedFetchUrl)

tests/test_project.py

@@ -22,9 +22,9 @@ import tempfile
 import unittest
 
 import error
-import manifest_xml
 import git_command
 import git_config
+import manifest_xml
 import platform_utils
 import project
 
@@ -48,7 +48,7 @@ def TempGitTree():
         yield tempdir
 
 
-class FakeProject(object):
+class FakeProject:
     """A fake for Project for basic functionality."""
 
     def __init__(self, worktree):
@@ -151,7 +151,7 @@ class CopyLinkTestCase(unittest.TestCase):
                     # "".
                     break
                 result = os.path.exists(path)
-                msg.append("\tos.path.exists(%s): %s" % (path, result))
+                msg.append(f"\tos.path.exists({path}): {result}")
                 if result:
                     msg.append("\tcontents: %r" % os.listdir(path))
                     break
@@ -507,7 +507,10 @@ class ManifestPropertiesFetchedCorrectly(unittest.TestCase):
             self.assertFalse(fakeproj.partial_clone)
 
             fakeproj.config.SetString("repo.depth", "48")
-            self.assertEqual(fakeproj.depth, "48")
+            self.assertEqual(fakeproj.depth, 48)
+
+            fakeproj.config.SetString("repo.depth", "invalid_depth")
+            self.assertEqual(fakeproj.depth, None)
 
             fakeproj.config.SetString("repo.clonefilter", "blob:limit=10M")
             self.assertEqual(fakeproj.clone_filter, "blob:limit=10M")

tests/test_repo_logging.py

@@ -0,0 +1,64 @@
+# Copyright (C) 2023 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.
+
+"""Unit test for repo_logging module."""
+import unittest
+from unittest import mock
+
+from error import RepoExitError
+from repo_logging import RepoLogger
+
+
+class TestRepoLogger(unittest.TestCase):
+    @mock.patch.object(RepoLogger, "error")
+    def test_log_aggregated_errors_logs_aggregated_errors(self, mock_error):
+        """Test if log_aggregated_errors logs a list of aggregated errors."""
+        logger = RepoLogger(__name__)
+        logger.log_aggregated_errors(
+            RepoExitError(
+                aggregate_errors=[
+                    Exception("foo"),
+                    Exception("bar"),
+                    Exception("baz"),
+                    Exception("hello"),
+                    Exception("world"),
+                    Exception("test"),
+                ]
+            )
+        )
+
+        mock_error.assert_has_calls(
+            [
+                mock.call("=" * 80),
+                mock.call(
+                    "Repo command failed due to the following `%s` errors:",
+                    "RepoExitError",
+                ),
+                mock.call("foo\nbar\nbaz\nhello\nworld"),
+                mock.call("+%d additional errors...", 1),
+            ]
+        )
+
+    @mock.patch.object(RepoLogger, "error")
+    def test_log_aggregated_errors_logs_single_error(self, mock_error):
+        """Test if log_aggregated_errors logs empty aggregated_errors."""
+        logger = RepoLogger(__name__)
+        logger.log_aggregated_errors(RepoExitError())
+
+        mock_error.assert_has_calls(
+            [
+                mock.call("=" * 80),
+                mock.call("Repo command failed: %s", "RepoExitError"),
+            ]
+        )

tests/test_subcmds_sync.py

@@ -14,12 +14,18 @@
 """Unittests for the subcmds/sync.py module."""
 
 import os
+import shutil
+import tempfile
+import time
 import unittest
 from unittest import mock
 
 import pytest
 
 import command
+from error import GitError
+from error import RepoExitError
+from project import SyncNetworkHalfResult
 from subcmds import sync
 
 
@@ -104,6 +110,200 @@ def test_cli_jobs(argv, jobs_manifest, jobs, jobs_net, jobs_check):
             assert opts.jobs_checkout == jobs_check
 
 
+class LocalSyncState(unittest.TestCase):
+    """Tests for LocalSyncState."""
+
+    _TIME = 10
+
+    def setUp(self):
+        """Common setup."""
+        self.topdir = tempfile.mkdtemp("LocalSyncState")
+        self.repodir = os.path.join(self.topdir, ".repo")
+        os.makedirs(self.repodir)
+
+        self.manifest = mock.MagicMock(
+            topdir=self.topdir,
+            repodir=self.repodir,
+            repoProject=mock.MagicMock(relpath=".repo/repo"),
+        )
+        self.state = self._new_state()
+
+    def tearDown(self):
+        """Common teardown."""
+        shutil.rmtree(self.topdir)
+
+    def _new_state(self, time=_TIME):
+        with mock.patch("time.time", return_value=time):
+            return sync.LocalSyncState(self.manifest)
+
+    def test_set(self):
+        """Times are set."""
+        p = mock.MagicMock(relpath="projA")
+        self.state.SetFetchTime(p)
+        self.state.SetCheckoutTime(p)
+        self.assertEqual(self.state.GetFetchTime(p), self._TIME)
+        self.assertEqual(self.state.GetCheckoutTime(p), self._TIME)
+
+    def test_update(self):
+        """Times are updated."""
+        with open(self.state._path, "w") as f:
+            f.write(
+                """
+            {
+              "projB": {
+                "last_fetch": 5,
+                "last_checkout": 7
+              }
+            }
+            """
+            )
+
+        # Initialize state to read from the new file.
+        self.state = self._new_state()
+        projA = mock.MagicMock(relpath="projA")
+        projB = mock.MagicMock(relpath="projB")
+        self.assertEqual(self.state.GetFetchTime(projA), None)
+        self.assertEqual(self.state.GetFetchTime(projB), 5)
+        self.assertEqual(self.state.GetCheckoutTime(projB), 7)
+
+        self.state.SetFetchTime(projA)
+        self.state.SetFetchTime(projB)
+        self.assertEqual(self.state.GetFetchTime(projA), self._TIME)
+        self.assertEqual(self.state.GetFetchTime(projB), self._TIME)
+        self.assertEqual(self.state.GetCheckoutTime(projB), 7)
+
+    def test_save_to_file(self):
+        """Data is saved under repodir."""
+        p = mock.MagicMock(relpath="projA")
+        self.state.SetFetchTime(p)
+        self.state.Save()
+        self.assertEqual(
+            os.listdir(self.repodir), [".repo_localsyncstate.json"]
+        )
+
+    def test_partial_sync(self):
+        """Partial sync state is detected."""
+        with open(self.state._path, "w") as f:
+            f.write(
+                """
+            {
+              "projA": {
+                "last_fetch": 5,
+                "last_checkout": 5
+              },
+              "projB": {
+                "last_fetch": 5,
+                "last_checkout": 5
+              }
+            }
+            """
+            )
+
+        # Initialize state to read from the new file.
+        self.state = self._new_state()
+        projB = mock.MagicMock(relpath="projB")
+        self.assertEqual(self.state.IsPartiallySynced(), False)
+
+        self.state.SetFetchTime(projB)
+        self.state.SetCheckoutTime(projB)
+        self.assertEqual(self.state.IsPartiallySynced(), True)
+
+    def test_ignore_repo_project(self):
+        """Sync data for repo project is ignored when checking partial sync."""
+        p = mock.MagicMock(relpath="projA")
+        self.state.SetFetchTime(p)
+        self.state.SetCheckoutTime(p)
+        self.state.SetFetchTime(self.manifest.repoProject)
+        self.state.Save()
+        self.assertEqual(self.state.IsPartiallySynced(), False)
+
+        self.state = self._new_state(self._TIME + 1)
+        self.state.SetFetchTime(self.manifest.repoProject)
+        self.assertEqual(
+            self.state.GetFetchTime(self.manifest.repoProject), self._TIME + 1
+        )
+        self.assertEqual(self.state.GetFetchTime(p), self._TIME)
+        self.assertEqual(self.state.IsPartiallySynced(), False)
+
+    def test_nonexistent_project(self):
+        """Unsaved projects don't have data."""
+        p = mock.MagicMock(relpath="projC")
+        self.assertEqual(self.state.GetFetchTime(p), None)
+        self.assertEqual(self.state.GetCheckoutTime(p), None)
+
+    def test_prune_removed_projects(self):
+        """Removed projects are pruned."""
+        with open(self.state._path, "w") as f:
+            f.write(
+                """
+            {
+              "projA": {
+                "last_fetch": 5
+              },
+              "projB": {
+                "last_fetch": 7
+              }
+            }
+            """
+            )
+
+        def mock_exists(path):
+            if "projA" in path:
+                return False
+            return True
+
+        projA = mock.MagicMock(relpath="projA")
+        projB = mock.MagicMock(relpath="projB")
+        self.state = self._new_state()
+        self.assertEqual(self.state.GetFetchTime(projA), 5)
+        self.assertEqual(self.state.GetFetchTime(projB), 7)
+        with mock.patch("os.path.exists", side_effect=mock_exists):
+            self.state.PruneRemovedProjects()
+        self.assertIsNone(self.state.GetFetchTime(projA))
+
+        self.state = self._new_state()
+        self.assertIsNone(self.state.GetFetchTime(projA))
+        self.assertEqual(self.state.GetFetchTime(projB), 7)
+
+    def test_prune_removed_and_symlinked_projects(self):
+        """Removed projects that still exists on disk as symlink are pruned."""
+        with open(self.state._path, "w") as f:
+            f.write(
+                """
+            {
+              "projA": {
+                "last_fetch": 5
+              },
+              "projB": {
+                "last_fetch": 7
+              }
+            }
+            """
+            )
+
+        def mock_exists(path):
+            return True
+
+        def mock_islink(path):
+            if "projB" in path:
+                return True
+            return False
+
+        projA = mock.MagicMock(relpath="projA")
+        projB = mock.MagicMock(relpath="projB")
+        self.state = self._new_state()
+        self.assertEqual(self.state.GetFetchTime(projA), 5)
+        self.assertEqual(self.state.GetFetchTime(projB), 7)
+        with mock.patch("os.path.exists", side_effect=mock_exists):
+            with mock.patch("os.path.islink", side_effect=mock_islink):
+                self.state.PruneRemovedProjects()
+        self.assertIsNone(self.state.GetFetchTime(projB))
+
+        self.state = self._new_state()
+        self.assertIsNone(self.state.GetFetchTime(projB))
+        self.assertEqual(self.state.GetFetchTime(projA), 5)
+
+
 class GetPreciousObjectsState(unittest.TestCase):
     """Tests for _GetPreciousObjectsState."""
 
@@ -158,3 +358,83 @@ class GetPreciousObjectsState(unittest.TestCase):
         self.assertFalse(
             self.cmd._GetPreciousObjectsState(self.project, self.opt)
         )
+
+
+class SyncCommand(unittest.TestCase):
+    """Tests for cmd.Execute."""
+
+    def setUp(self):
+        """Common setup."""
+        self.repodir = tempfile.mkdtemp(".repo")
+        self.manifest = manifest = mock.MagicMock(
+            repodir=self.repodir,
+        )
+
+        git_event_log = mock.MagicMock(ErrorEvent=mock.Mock(return_value=None))
+        self.outer_client = outer_client = mock.MagicMock()
+        outer_client.manifest.IsArchive = True
+        manifest.manifestProject.worktree = "worktree_path/"
+        manifest.repoProject.LastFetch = time.time()
+        self.sync_network_half_error = None
+        self.sync_local_half_error = None
+        self.cmd = sync.Sync(
+            manifest=manifest,
+            outer_client=outer_client,
+            git_event_log=git_event_log,
+        )
+
+        def Sync_NetworkHalf(*args, **kwargs):
+            return SyncNetworkHalfResult(True, self.sync_network_half_error)
+
+        def Sync_LocalHalf(*args, **kwargs):
+            if self.sync_local_half_error:
+                raise self.sync_local_half_error
+
+        self.project = p = mock.MagicMock(
+            use_git_worktrees=False,
+            UseAlternates=False,
+            name="project",
+            Sync_NetworkHalf=Sync_NetworkHalf,
+            Sync_LocalHalf=Sync_LocalHalf,
+            RelPath=mock.Mock(return_value="rel_path"),
+        )
+        p.manifest.GetProjectsWithName.return_value = [p]
+
+        mock.patch.object(
+            sync,
+            "_PostRepoFetch",
+            return_value=None,
+        ).start()
+
+        mock.patch.object(
+            self.cmd, "GetProjects", return_value=[self.project]
+        ).start()
+
+        opt, _ = self.cmd.OptionParser.parse_args([])
+        opt.clone_bundle = False
+        opt.jobs = 4
+        opt.quiet = True
+        opt.use_superproject = False
+        opt.current_branch_only = True
+        opt.optimized_fetch = True
+        opt.retry_fetches = 1
+        opt.prune = False
+        opt.auto_gc = False
+        opt.repo_verify = False
+        self.opt = opt
+
+    def tearDown(self):
+        mock.patch.stopall()
+
+    def test_command_exit_error(self):
+        """Ensure unsuccessful commands raise expected errors."""
+        self.sync_network_half_error = GitError(
+            "sync_network_half_error error", project=self.project
+        )
+        self.sync_local_half_error = GitError(
+            "sync_local_half_error", project=self.project
+        )
+        with self.assertRaises(RepoExitError) as e:
+            self.cmd.Execute(self.opt, [])
+            self.assertIn(self.sync_local_half_error, e.aggregate_errors)
+            self.assertIn(self.sync_network_half_error, e.aggregate_errors)

tests/test_subcmds_upload.py

@@ -0,0 +1,70 @@
+# Copyright (C) 2023 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 subcmds/upload.py module."""
+
+import unittest
+from unittest import mock
+
+from error import GitError
+from error import UploadError
+from subcmds import upload
+
+
+class UnexpectedError(Exception):
+    """An exception not expected by upload command."""
+
+
+class UploadCommand(unittest.TestCase):
+    """Check registered all_commands."""
+
+    def setUp(self):
+        self.cmd = upload.Upload()
+        self.branch = mock.MagicMock()
+        self.people = mock.MagicMock()
+        self.opt, _ = self.cmd.OptionParser.parse_args([])
+        mock.patch.object(
+            self.cmd, "_AppendAutoList", return_value=None
+        ).start()
+        mock.patch.object(self.cmd, "git_event_log").start()
+
+    def tearDown(self):
+        mock.patch.stopall()
+
+    def test_UploadAndReport_UploadError(self):
+        """Check UploadExitError raised when UploadError encountered."""
+        side_effect = UploadError("upload error")
+        with mock.patch.object(
+            self.cmd, "_UploadBranch", side_effect=side_effect
+        ):
+            with self.assertRaises(upload.UploadExitError):
+                self.cmd._UploadAndReport(self.opt, [self.branch], self.people)
+
+    def test_UploadAndReport_GitError(self):
+        """Check UploadExitError raised when GitError encountered."""
+        side_effect = GitError("some git error")
+        with mock.patch.object(
+            self.cmd, "_UploadBranch", side_effect=side_effect
+        ):
+            with self.assertRaises(upload.UploadExitError):
+                self.cmd._UploadAndReport(self.opt, [self.branch], self.people)
+
+    def test_UploadAndReport_UnhandledError(self):
+        """Check UnexpectedError passed through."""
+        side_effect = UnexpectedError("some os error")
+        with mock.patch.object(
+            self.cmd, "_UploadBranch", side_effect=side_effect
+        ):
+            with self.assertRaises(type(side_effect)):
+                self.cmd._UploadAndReport(self.opt, [self.branch], self.people)

tests/test_wrapper.py

@@ -14,7 +14,7 @@
 
 """Unittests for the wrapper.py module."""
 
-from io import StringIO
+import io
 import os
 import re
 import sys
@@ -47,8 +47,10 @@ class RepoWrapperUnitTest(RepoWrapperTestCase):
     def test_version(self):
         """Make sure _Version works."""
         with self.assertRaises(SystemExit) as e:
-            with mock.patch("sys.stdout", new_callable=StringIO) as stdout:
-                with mock.patch("sys.stderr", new_callable=StringIO) as stderr:
+            with mock.patch("sys.stdout", new_callable=io.StringIO) as stdout:
+                with mock.patch(
+                    "sys.stderr", new_callable=io.StringIO
+                ) as stderr:
                     self.wrapper._Version()
         self.assertEqual(0, e.exception.code)
         self.assertEqual("", stderr.getvalue())
@@ -76,11 +78,9 @@ class RepoWrapperUnitTest(RepoWrapperTestCase):
         self.assertIsNone(opts.manifest_url)
 
     def test_gitc_init_parser(self):
-        """Make sure 'gitc-init' GetParser works."""
-        parser = self.wrapper.GetParser(gitc_init=True)
-        opts, args = parser.parse_args([])
-        self.assertEqual([], args)
-        self.assertIsNone(opts.manifest_file)
+        """Make sure 'gitc-init' GetParser raises."""
+        with self.assertRaises(SystemExit):
+            self.wrapper.GetParser(gitc_init=True)
 
     def test_get_gitc_manifest_dir_no_gitc(self):
         """
@@ -418,7 +418,7 @@ class SetupGnuPG(RepoWrapperTestCase):
                 self.wrapper.home_dot_repo, "gnupg"
             )
             self.assertTrue(self.wrapper.SetupGnuPG(True))
-            with open(os.path.join(tempdir, "keyring-version"), "r") as fp:
+            with open(os.path.join(tempdir, "keyring-version")) as fp:
                 data = fp.read()
             self.assertEqual(
                 ".".join(str(x) for x in self.wrapper.KEYRING_VERSION),

tox.ini

@@ -15,7 +15,8 @@
 # https://tox.readthedocs.io/
 
 [tox]
-envlist = lint, py36, py37, py38, py39, py310
+envlist = lint, py36, py37, py38, py39, py310, py311, py312
+requires = virtualenv<20.22.0
 
 [gh-actions]
 python =
@@ -24,11 +25,14 @@ python =
     3.8: py38
     3.9: py39
     3.10: py310
+    3.11: py311
+    3.12: py312
 
 [testenv]
 deps =
     black
     flake8
+    isort
     pytest
     pytest-timeout
 commands = {envpython} run_tests {posargs}
@@ -54,6 +58,3 @@ deps =
 commands =
     black {posargs:.}
     flake8
-
-[pytest]
-timeout = 300