From 1a3612fe6d347e458a53d7a9e920a91ea502e6ba Mon Sep 17 00:00:00 2001 From: Jason Chang Date: Tue, 8 Aug 2023 14:12:53 -0700 Subject: [PATCH] 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 Tested-by: Jason Chang Reviewed-by: Aravind Vasudevan Commit-Queue: Jason Chang --- error.py | 8 ++++ main.py | 23 +++++++----- project.py | 54 ++++++++++++++------------- subcmds/checkout.py | 50 +++++++++++++++++++------ subcmds/cherry_pick.py | 76 +++++++++++++++++++++++++------------- subcmds/download.py | 23 +++++++----- subcmds/grep.py | 84 +++++++++++++++++++++++++++++------------- subcmds/help.py | 7 +++- subcmds/selfupdate.py | 10 ++++- subcmds/start.py | 38 +++++++++++++------ 10 files changed, 251 insertions(+), 122 deletions(-) diff --git a/error.py b/error.py index ed4a90b7..cee977f9 100644 --- a/error.py +++ b/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.""" diff --git a/main.py b/main.py index b213f0a4..ffed0b72 100755 --- a/main.py +++ b/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 diff --git a/project.py b/project.py index c3eb09c9..6e6a605e 100644 --- a/project.py +++ b/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: - return None + GitCommand(self, cmd, bare=True, verify_command=True).Wait() 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( - self, ["checkout", "-q", "-b", branch.name, revid] - ).Wait() - == 0 - ): - branch.Save() - return True - return False + GitCommand( + self, + ["checkout", "-q", "-b", branch.name, revid], + verify_command=True, + ).Wait() + branch.Save() + return True 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 - branch didn't exist. + True if the checkout succeeded; False if the + 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( - self, - ["checkout", name, "--"], - capture_stdout=True, - capture_stderr=True, - ).Wait() - == 0 - ) + GitCommand( + self, + ["checkout", name, "--"], + capture_stdout=True, + capture_stderr=True, + verify_command=True, + ).Wait() + 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 diff --git a/subcmds/checkout.py b/subcmds/checkout.py index 6448518f..033fd349 100644 --- a/subcmds/checkout.py +++ b/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: - 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( @@ -70,13 +95,14 @@ The command is equivalent to: ), ) - if err: - for p in err: + if err_projects: + 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) - sys.exit(1) + msg = f"error: no project has branch {nb}" + print(msg, file=sys.stderr) + raise MissingBranchError(msg) diff --git a/subcmds/cherry_pick.py b/subcmds/cherry_pick.py index 4cfb8c88..7a4dd09e 100644 --- a/subcmds/cherry_pick.py +++ b/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) - if p.Wait() != 0: + p = GitCommand( + 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 - # 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: + p = GitCommand( + None, + ["commit", "--amend", "-F", "-"], + input=new_msg, + capture_stdout=True, + capture_stderr=True, + verify_command=True, + ) + try: + p.Wait() + except GitError: print( - "NOTE: When committing (please see above) and editing the " - "commit message, please remove the old Change-Id-line and add:" + "error: Failed to update commit message", + file=sys.stderr, ) - print(self._GetReference(sha1), file=sys.stderr) - print(file=sys.stderr) + raise def _IsChangeId(self, line): return CHANGE_ID_RE.match(line) diff --git a/subcmds/download.py b/subcmds/download.py index 475c0bc2..18e555be 100644 --- a/subcmds/download.py +++ b/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 diff --git a/subcmds/grep.py b/subcmds/grep.py index 5cd33763..9ebd776c 100644 --- a/subcmds/grep.py +++ b/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: - 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 +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( - "fatal: cannot combine --cached and --revision", - file=sys.stderr, - ) - sys.exit(1) + msg = "fatal: cannot combine --cached and --revision" + print(msg, file=sys.stderr) + 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 +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) - else: - sys.exit(1) + raise GrepCommandError(aggregate_errors=errors) diff --git a/subcmds/help.py b/subcmds/help.py index 50a48047..593bf676 100644 --- a/subcmds/help.py +++ b/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) diff --git a/subcmds/selfupdate.py b/subcmds/selfupdate.py index d5d0a838..00376b66 100644 --- a/subcmds/selfupdate.py +++ b/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) diff --git a/subcmds/start.py b/subcmds/start.py index f6355126..67ac7df9 100644 --- a/subcmds/start.py +++ b/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 - return (ret, project) + error = e + 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: - 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( @@ -161,13 +175,15 @@ revision specified in the manifest. ), ) - if err: - for p in err: + if err_projects: + 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) - sys.exit(1) + self.git_event_log.ErrorEvent( + msg_fmt % (len(err_projects)), msg_fmt + ) + raise StartError(aggregate_errors=err)