Raise RepoExitError in place of sys.exit

Bug: b/293344017
Change-Id: Icae4932b00e4068cba502a5ab4a0274fd7854d9d
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/382214
Reviewed-by: Gavin Mak <gavinmak@google.com>
Tested-by: Jason Chang <jasonnc@google.com>
Reviewed-by: Aravind Vasudevan <aravindvasudev@google.com>
Commit-Queue: Jason Chang <jasonnc@google.com>

error.py

@@ -56,6 +56,10 @@ class RepoUnhandledExceptionError(RepoExitError):
         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."""
 
@@ -125,6 +129,10 @@ class DownloadError(RepoExitError):
         return self.reason
 
 
+class InvalidArgumentsError(RepoExitError):
+    """Invalid command Arguments."""
+
+
 class SyncError(RepoExitError):
     """Cannot sync repo."""
 

main.py

@@ -57,6 +57,7 @@ from error import RepoChangedException
 from error import RepoExitError
 from error import RepoUnhandledExceptionError
 from error import RepoError
+from error import SilentRepoExitError
 import gitc_utils
 from manifest_xml import GitcClient, RepoClient
 from pager import RunPager, TerminatePager
@@ -872,16 +873,20 @@ def _Main(argv):
 
         result = repo._Run(name, gopts, argv) or 0
     except RepoExitError as e:
-        exception_name = type(e).__name__
+        if not isinstance(e, SilentRepoExitError):
+            exception_name = type(e).__name__
+            print("fatal: %s" % e, file=sys.stderr)
+            if e.aggregate_errors:
+                print(f"{exception_name} Aggregate Errors")
+                for err in e.aggregate_errors[:MAX_PRINT_ERRORS]:
+                    print(err)
+            if (
+                e.aggregate_errors
+                and len(e.aggregate_errors) > MAX_PRINT_ERRORS
+            ):
+                diff = len(e.aggregate_errors) - MAX_PRINT_ERRORS
+                print(f"+{diff} additional errors ...")
         result = e.exit_code
-        print("fatal: %s" % e, file=sys.stderr)
-        if e.aggregate_errors:
-            print(f"{exception_name} Aggregate Errors")
-            for err in e.aggregate_errors[:MAX_PRINT_ERRORS]:
-                print(err)
-        if len(e.aggregate_errors) > MAX_PRINT_ERRORS:
-            diff = len(e.aggregate_errors) - MAX_PRINT_ERRORS
-            print(f"+{diff} additional errors ...")
     except KeyboardInterrupt:
         print("aborted by user", file=sys.stderr)
         result = KEYBOARD_INTERRUPT_EXIT

project.py

@@ -1733,8 +1733,7 @@ class Project(object):
         cmd.append(
             "refs/changes/%2.2d/%d/%d" % (change_id % 100, change_id, patch_id)
         )
-        if GitCommand(self, cmd, bare=True).Wait() != 0:
+        GitCommand(self, cmd, bare=True, verify_command=True).Wait()
-            return None
         return DownloadedChange(
             self,
             self.GetRevisionId(),
@@ -1911,7 +1910,10 @@ class Project(object):
 
         all_refs = self.bare_ref.all
         if R_HEADS + name in all_refs:
-            return GitCommand(self, ["checkout", "-q", name, "--"]).Wait() == 0
+            GitCommand(
+                self, ["checkout", "-q", name, "--"], verify_command=True
+            ).Wait()
+            return True
 
         branch = self.GetBranch(name)
         branch.remote = self.GetRemote()
@@ -1938,15 +1940,13 @@ class Project(object):
             branch.Save()
             return True
 
-        if (
+        GitCommand(
-            GitCommand(
+            self,
-                self, ["checkout", "-q", "-b", branch.name, revid]
+            ["checkout", "-q", "-b", branch.name, revid],
-            ).Wait()
+            verify_command=True,
-            == 0
+        ).Wait()
-        ):
+        branch.Save()
-            branch.Save()
+        return True
-            return True
-        return False
 
     def CheckoutBranch(self, name):
         """Checkout a local topic branch.
@@ -1955,8 +1955,8 @@ class Project(object):
             name: The name of the branch to checkout.
 
         Returns:
-            True if the checkout succeeded; False if it didn't; None if the
+            True if the checkout succeeded; False if the
-            branch didn't exist.
+            branch doesn't exist.
         """
         rev = R_HEADS + name
         head = self.work_git.GetHead()
@@ -1969,7 +1969,7 @@ class Project(object):
             revid = all_refs[rev]
         except KeyError:
             # Branch does not exist in this project.
-            return None
+            return False
 
         if head.startswith(R_HEADS):
             try:
@@ -1986,15 +1986,14 @@ class Project(object):
             )
             return True
 
-        return (
+        GitCommand(
-            GitCommand(
+            self,
-                self,
+            ["checkout", name, "--"],
-                ["checkout", name, "--"],
+            capture_stdout=True,
-                capture_stdout=True,
+            capture_stderr=True,
-                capture_stderr=True,
+            verify_command=True,
-            ).Wait()
+        ).Wait()
-            == 0
+        return True
-        )
 
     def AbandonBranch(self, name):
         """Destroy a local topic branch.
@@ -4458,9 +4457,12 @@ class ManifestProject(MetaProject):
             syncbuf.Finish()
 
             if is_new or self.CurrentBranch is None:
-                if not self.StartBranch("default"):
+                try:
+                    self.StartBranch("default")
+                except GitError as e:
+                    msg = str(e)
                     print(
-                        "fatal: cannot create default in manifest",
+                        f"fatal: cannot create default in manifest {msg}",
                         file=sys.stderr,
                     )
                     return False

subcmds/checkout.py

@@ -15,8 +15,26 @@
 import functools
 import sys
 
+from typing import NamedTuple
 from command import Command, DEFAULT_LOCAL_JOBS
 from progress import Progress
+from project import Project
+from error import GitError, RepoExitError
+
+
+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 +59,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:
+            for result in results:
-                if status is not None:
+                if result.error is not None:
-                    if status:
+                    err.append(result.error)
-                        success.append(project)
+                    err_projects.append(result.project)
-                    else:
+                elif result.result:
-                        err.append(project)
+                    success.append(result.project)
                 pm.update(msg="")
 
         self.ExecuteInParallel(
@@ -70,13 +95,14 @@ The command is equivalent to:
             ),
         )
 
-        if err:
+        if err_projects:
-            for p in err:
+            for p in err_projects:
                 print(
                     "error: %s/: cannot checkout %s" % (p.relpath, nb),
                     file=sys.stderr,
                 )
-            sys.exit(1)
+            raise CheckoutCommandError(aggregate_errors=err)
         elif not success:
-            print("error: no project has branch %s" % nb, file=sys.stderr)
+            msg = f"error: no project has branch {nb}"
-            sys.exit(1)
+            print(msg, file=sys.stderr)
+            raise MissingBranchError(msg)

subcmds/cherry_pick.py

@@ -16,6 +16,7 @@ import re
 import sys
 from command import Command
 from git_command import GitCommand
+from error import GitError
 
 CHANGE_ID_RE = re.compile(r"^\s*Change-Id: I([0-9a-f]{40})\s*$")
 
@@ -44,18 +45,31 @@ change id will be added.
             ["rev-parse", "--verify", reference],
             capture_stdout=True,
             capture_stderr=True,
+            verify_command=True,
         )
-        if p.Wait() != 0:
+        try:
+            p.Wait()
+        except GitError:
             print(p.stderr, file=sys.stderr)
-            sys.exit(1)
+            raise
+
         sha1 = p.stdout.strip()
 
-        p = GitCommand(None, ["cat-file", "commit", sha1], capture_stdout=True)
+        p = GitCommand(
-        if p.Wait() != 0:
+            None,
+            ["cat-file", "commit", sha1],
+            capture_stdout=True,
+            verify_command=True,
+        )
+
+        try:
+            p.Wait()
+        except GitError:
             print(
                 "error: Failed to retrieve old commit message", file=sys.stderr
             )
-            sys.exit(1)
+            raise
+
         old_msg = self._StripHeader(p.stdout)
 
         p = GitCommand(
@@ -63,37 +77,47 @@ 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:
+            print(str(e))
+            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)
+            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 cherry-pick was applied correctly. We just need to edit the
+        # the commit message.
-            # commit message.
+        new_msg = self._Reformat(old_msg, sha1)
-            new_msg = self._Reformat(old_msg, sha1)
 
-            p = GitCommand(
+        p = GitCommand(
-                None,
+            None,
-                ["commit", "--amend", "-F", "-"],
+            ["commit", "--amend", "-F", "-"],
-                input=new_msg,
+            input=new_msg,
-                capture_stdout=True,
+            capture_stdout=True,
-                capture_stderr=True,
+            capture_stderr=True,
-            )
+            verify_command=True,
-            if p.Wait() != 0:
+        )
-                print("error: Failed to update commit message", file=sys.stderr)
+        try:
-                sys.exit(1)
+            p.Wait()
-
+        except GitError:
-        else:
             print(
-                "NOTE: When committing (please see above) and editing the "
+                "error: Failed to update commit message",
-                "commit message, please remove the old Change-Id-line and add:"
+                file=sys.stderr,
             )
-            print(self._GetReference(sha1), file=sys.stderr)
+            raise
-            print(file=sys.stderr)
 
     def _IsChangeId(self, line):
         return CHANGE_ID_RE.match(line)

subcmds/download.py

@@ -16,11 +16,15 @@ import re
 import sys
 
 from command import Command
-from error import GitError, NoSuchProjectError
+from error import GitError, NoSuchProjectError, RepoExitError
 
 CHANGE_RE = re.compile(r"^([1-9][0-9]*)(?:[/\.-]([1-9][0-9]*))?$")
 
 
+class DownloadCommandError(RepoExitError):
+    """Error raised when download command fails."""
+
+
 class Download(Command):
     COMMON = True
     helpSummary = "Download and checkout a change"
@@ -137,15 +141,16 @@ 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(
@@ -201,4 +206,4 @@ If no project is specified try to use current directory as a project.
                     % (project.name, mode, dl.commit),
                     file=sys.stderr,
                 )
-                sys.exit(1)
+                raise

subcmds/grep.py

@@ -17,8 +17,10 @@ import sys
 
 from color import Coloring
 from command import DEFAULT_LOCAL_JOBS, PagedCommand
-from error import GitError
+from error import GitError, InvalidArgumentsError, SilentRepoExitError
 from git_command import GitCommand
+from typing import NamedTuple
+from project import Project
 
 
 class GrepColoring(Coloring):
@@ -28,6 +30,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 +264,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 +283,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:
+        for result in results:
-            if rc < 0:
+            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 result.stderr:
-                    if have_rev and "fatal: ambiguous argument" in 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 +324,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 +338,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 +367,14 @@ contain a line that matches both expressions:
         have_rev = False
         if opt.revision:
             if "--cached" in cmd_argv:
-                print(
+                msg = "fatal: cannot combine --cached and --revision"
-                    "fatal: cannot combine --cached and --revision",
+                print(msg, file=sys.stderr)
-                    file=sys.stderr,
+                raise InvalidArgumentsError(msg)
-                )
-                sys.exit(1)
             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 +386,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)
+        raise GrepCommandError(aggregate_errors=errors)
-        else:
-            sys.exit(1)

subcmds/help.py

@@ -26,6 +26,11 @@ from command import (
 )
 import gitc_utils
 from wrapper import Wrapper
+from error import RepoExitError
+
+
+class InvalidHelpCommand(RepoExitError):
+    """Invalid command passed into help."""
 
 
 class Help(PagedCommand, MirrorSafeCommand):
@@ -202,7 +207,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/selfupdate.py

@@ -18,6 +18,11 @@ import sys
 from command import Command, MirrorSafeCommand
 from subcmds.sync import _PostRepoUpgrade
 from subcmds.sync import _PostRepoFetch
+from error import RepoExitError
+
+
+class SelfupdateError(RepoExitError):
+    """Exit error for failed selfupdate command."""
 
 
 class Selfupdate(Command, MirrorSafeCommand):
@@ -58,9 +63,10 @@ need to be performed by an end-user.
             _PostRepoUpgrade(self.manifest)
 
         else:
-            if not rp.Sync_NetworkHalf().success:
+            result = rp.Sync_NetworkHalf()
+            if result.error:
                 print("error: can't update repo", file=sys.stderr)
-                sys.exit(1)
+                raise SelfupdateError(aggregate_errors=[result.error])
 
             rp.bare_git.gc("--auto")
             _PostRepoFetch(rp, repo_verify=opt.repo_verify, verbose=True)

subcmds/start.py

@@ -21,7 +21,18 @@ from git_config import IsImmutable
 from git_command import git
 import gitc_utils
 from progress import Progress
-from project import SyncBuffer
+from project import SyncBuffer, Project
+from typing import NamedTuple
+from error import RepoExitError
+
+
+class ExecuteOneResult(NamedTuple):
+    project: Project
+    error: Exception
+
+
+class StartError(RepoExitError):
+    """Exit error for failed start command."""
 
 
 class Start(Command):
@@ -73,6 +84,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,7 +92,7 @@ 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:
@@ -88,11 +100,12 @@ revision specified in the manifest.
                 "error: unable to checkout %s: %s" % (project.name, e),
                 file=sys.stderr,
             )
-            ret = False
+            error = e
-        return (ret, project)
+        return ExecuteOneResult(project, error)
 
     def Execute(self, opt, args):
         nb = args[0]
+        err_projects = []
         err = []
         projects = []
         if not opt.all:
@@ -146,9 +159,10 @@ revision specified in the manifest.
             pm.end()
 
         def _ProcessResults(_pool, pm, results):
-            for result, project in results:
+            for result in results:
-                if not result:
+                if result.error:
-                    err.append(project)
+                    err_projects.append(result.project)
+                    err.append(result.error)
                 pm.update(msg="")
 
         self.ExecuteInParallel(
@@ -161,13 +175,15 @@ revision specified in the manifest.
             ),
         )
 
-        if err:
+        if err_projects:
-            for p in err:
+            for p in err_projects:
                 print(
                     "error: %s/: cannot start %s"
                     % (p.RelPath(local=opt.this_manifest_only), nb),
                     file=sys.stderr,
                 )
             msg_fmt = "cannot start %d project(s)"
-            self.git_event_log.ErrorEvent(msg_fmt % (len(err)), msg_fmt)
+            self.git_event_log.ErrorEvent(
-            sys.exit(1)
+                msg_fmt % (len(err_projects)), msg_fmt
+            )
+            raise StartError(aggregate_errors=err)