From b8fd19215f59f7f8dbe69528aefca700a2190ecd Mon Sep 17 00:00:00 2001 From: Aravind Vasudevan Date: Thu, 14 Sep 2023 22:54:04 +0000 Subject: [PATCH] main: Use repo logger Bug: b/292704435 Change-Id: Ica02e4c00994a2f64083bb36e8f4ee8aa45d76bd Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/386454 Reviewed-by: Jason Chang Commit-Queue: Aravind Vasudevan Tested-by: Aravind Vasudevan --- main.py | 120 ++++++++++++++++--------------------- repo_logging.py | 25 ++++++-- tests/test_repo_logging.py | 66 ++++++++++---------- 3 files changed, 104 insertions(+), 107 deletions(-) diff --git a/main.py b/main.py index 1c7f0af6..bafa64df 100755 --- a/main.py +++ b/main.py @@ -32,6 +32,8 @@ import textwrap import time import urllib.request +from repo_logging import RepoLogger + try: import kerberos @@ -69,6 +71,9 @@ 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. # These may be much newer as it allows the repo launcher to roll between # different repo releases while source versions might require a newer python. @@ -82,25 +87,25 @@ MIN_PYTHON_VERSION_SOFT = (3, 6) MIN_PYTHON_VERSION_HARD = (3, 6) if sys.version_info.major < 3: - print( + logger.error( "repo: error: Python 2 is no longer supported; " - "Please upgrade to Python {}.{}+.".format(*MIN_PYTHON_VERSION_SOFT), - file=sys.stderr, + "Please upgrade to Python %d.%d+.", + *MIN_PYTHON_VERSION_SOFT, ) sys.exit(1) else: if sys.version_info < MIN_PYTHON_VERSION_HARD: - print( + logger.error( "repo: error: Python 3 version is too old; " - "Please upgrade to Python {}.{}+.".format(*MIN_PYTHON_VERSION_SOFT), - file=sys.stderr, + "Please upgrade to Python %d.%d+.", + *MIN_PYTHON_VERSION_SOFT, ) sys.exit(1) elif sys.version_info < MIN_PYTHON_VERSION_SOFT: - print( + logger.error( "repo: warning: your Python 3 version is no longer supported; " - "Please upgrade to Python {}.{}+.".format(*MIN_PYTHON_VERSION_SOFT), - file=sys.stderr, + "Please upgrade to Python %d.%d+.", + *MIN_PYTHON_VERSION_SOFT, ) KEYBOARD_INTERRUPT_EXIT = 128 + signal.SIGINT @@ -309,7 +314,7 @@ class _Repo(object): ) if Wrapper().gitc_parse_clientdir(os.getcwd()): - print("GITC is not supported.", file=sys.stderr) + logger.error("GITC is not supported.") raise GitcUnsupportedError() try: @@ -322,32 +327,24 @@ class _Repo(object): 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, - ) + 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 @@ -453,34 +450,28 @@ class _Repo(object): ManifestInvalidRevisionError, 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 = 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) + 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 = e.exit_code except SystemExit as e: @@ -547,7 +538,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. @@ -556,7 +547,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. @@ -565,42 +556,42 @@ 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.warn("\n... A new version of repo (%s) is available.", exp_str) if os.access(repo_path, os.W_OK): - print( + logger.warn( """\ ... You should upgrade soon: cp %s %s -""" - % (WrapperPath(), repo_path), - file=sys.stderr, +""", + WrapperPath(), + repo_path, ) else: - print( + logger.warn( """\ ... 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) @@ -861,18 +852,7 @@ def _Main(argv): result = repo._Run(name, gopts, argv) or 0 except RepoExitError as e: 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 ...") + logger.log_aggregated_errors(e) result = e.exit_code except KeyboardInterrupt: print("aborted by user", file=sys.stderr) diff --git a/repo_logging.py b/repo_logging.py index 7d050555..58625351 100644 --- a/repo_logging.py +++ b/repo_logging.py @@ -15,12 +15,13 @@ """Logic for printing user-friendly logs in repo.""" import logging -from typing import List from color import Coloring +from error import RepoExitError SEPARATOR = "=" * 80 +MAX_PRINT_ERRORS = 5 class _ConfigMock: @@ -70,8 +71,22 @@ class RepoLogger(logging.Logger): handler.setFormatter(_LogColoringFormatter(config)) self.addHandler(handler) - def log_aggregated_errors(self, errors: List[Exception]): + def log_aggregated_errors(self, err: RepoExitError): """Print all aggregated logs.""" - super().error(SEPARATOR) - super().error("Repo command failed due to following errors:") - super().error("\n".join(str(e) for e in errors)) + self.error(SEPARATOR) + + if not err.aggregate_errors: + self.error("Repo command failed: %s", type(err).__name__) + 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: + self.error("+%d additional errors...", diff) diff --git a/tests/test_repo_logging.py b/tests/test_repo_logging.py index 52f251a7..0f6a3355 100644 --- a/tests/test_repo_logging.py +++ b/tests/test_repo_logging.py @@ -16,47 +16,49 @@ import unittest from unittest import mock +from error import RepoExitError from repo_logging import RepoLogger class TestRepoLogger(unittest.TestCase): - def test_log_aggregated_errors_logs_aggregated_errors(self): - """Test if log_aggregated_errors outputs aggregated errors.""" + @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__) - result = [] - - def mock_handler(log): - nonlocal result - result.append(log.getMessage()) - - mock_out = mock.MagicMock() - mock_out.level = 0 - mock_out.handle = mock_handler - logger.addHandler(mock_out) - - logger.error("Never gonna give you up") - logger.error("Never gonna let you down") - logger.error("Never gonna run around and desert you") logger.log_aggregated_errors( + RepoExitError( + aggregate_errors=[ + Exception("foo"), + Exception("bar"), + Exception("baz"), + Exception("hello"), + Exception("world"), + Exception("test"), + ] + ) + ) + + mock_error.assert_has_calls( [ - "Never gonna give you up", - "Never gonna let you down", - "Never gonna run around and desert you", + 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), ] ) - self.assertEqual( - result, + @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( [ - "Never gonna give you up", - "Never gonna let you down", - "Never gonna run around and desert you", - "=" * 80, - "Repo command failed due to following errors:", - ( - "Never gonna give you up\n" - "Never gonna let you down\n" - "Never gonna run around and desert you" - ), - ], + mock.call("=" * 80), + mock.call("Repo command failed: %s", "RepoExitError"), + ] )