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 <jasonnc@google.com>
Commit-Queue: Aravind Vasudevan <aravindvasudev@google.com>
Tested-by: Aravind Vasudevan <aravindvasudev@google.com>

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),
+        "Please upgrade to Python %d.%d+.",
-        file=sys.stderr,
+        *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),
+            "Please upgrade to Python %d.%d+.",
-            file=sys.stderr,
+            *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),
+            "Please upgrade to Python %d.%d+.",
-            file=sys.stderr,
+            *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(
+            logger.error(
-                "repo: '%s' is not a repo command.  See 'repo help'." % name,
+                "repo: '%s' is not a repo command.  See 'repo help'.", name
-                file=sys.stderr,
             )
             return 1
 
         Editor.globalConfig = cmd.client.globalConfig
 
         if not isinstance(cmd, MirrorSafeCommand) and cmd.manifest.IsMirror:
-            print(
+            logger.error("fatal: '%s' requires a working directory", name)
-                "fatal: '%s' requires a working directory" % name,
-                file=sys.stderr,
-            )
             return 1
 
         try:
             copts, cargs = cmd.OptionParser.parse_args(argv)
             copts = cmd.ReadEnvironmentOptions(copts)
         except NoManifestException as e:
-            print(
+            logger.error("error: in `%s`: %s", " ".join([name] + argv), e)
-                "error: in `%s`: %s" % (" ".join([name] + argv), str(e)),
+            logger.error(
-                file=sys.stderr,
+                "error: manifest missing or unreadable -- please run init"
-            )
-            print(
-                "error: manifest missing or unreadable -- please run init",
-                file=sys.stderr,
             )
             return 1
 
@@ -453,34 +450,28 @@ class _Repo(object):
             ManifestInvalidRevisionError,
             NoManifestException,
         ) as e:
-            print(
+            logger.error("error: in `%s`: %s", " ".join([name] + argv), e)
-                "error: in `%s`: %s" % (" ".join([name] + argv), str(e)),
-                file=sys.stderr,
-            )
             if isinstance(e, NoManifestException):
-                print(
+                logger.error(
-                    "error: manifest missing or unreadable -- please run init",
+                    "error: manifest missing or unreadable -- please run init"
-                    file=sys.stderr,
                 )
             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(
+                logger.error(
-                    "error: project group must be enabled for project %s"
+                    "error: project group must be enabled for project %s",
-                    % e.name,
+                    e.name,
-                    file=sys.stderr,
                 )
             else:
-                print(
+                logger.error(
                     "error: project group must be enabled for the project in "
-                    "the current directory",
+                    "the current directory"
-                    file=sys.stderr,
                 )
             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),
+            ver_str,
-            file=sys.stderr,
+            min_str,
+            exp_str,
+            WrapperPath(),
+            repo_path,
         )
         sys.exit(1)
 
     if exp > ver:
-        print(
+        logger.warn("\n... A new version of repo (%s) is available.", exp_str)
-            "\n... A new version of repo (%s) is available." % (exp_str,),
-            file=sys.stderr,
-        )
         if os.access(repo_path, os.W_OK):
-            print(
+            logger.warn(
                 """\
 ... You should upgrade soon:
     cp %s %s
-"""
+""",
-                % (WrapperPath(), repo_path),
+                WrapperPath(),
-                file=sys.stderr,
+                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),
+                WrapperPath(),
-                file=sys.stderr,
+                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__
+            logger.log_aggregated_errors(e)
-            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
     except KeyboardInterrupt:
         print("aborted by user", file=sys.stderr)

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)
+        self.error(SEPARATOR)
-        super().error("Repo command failed due to following errors:")
+
-        super().error("\n".join(str(e) for e in errors))
+        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)

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):
+    @mock.patch.object(RepoLogger, "error")
-        """Test if log_aggregated_errors outputs aggregated errors."""
+    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",
+                mock.call("=" * 80),
-                "Never gonna let you down",
+                mock.call(
-                "Never gonna run around and desert you",
+                    "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(
+    @mock.patch.object(RepoLogger, "error")
-            result,
+    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",
+                mock.call("=" * 80),
-                "Never gonna let you down",
+                mock.call("Repo command failed: %s", "RepoExitError"),
-                "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"
-                ),
-            ],
         )