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>
diff --git a/main.py b/main.py
index 1c7f0af..bafa64d 100755
--- a/main.py
+++ b/main.py
@@ -32,6 +32,8 @@
import time
import urllib.request
+from repo_logging import RepoLogger
+
try:
import kerberos
@@ -69,6 +71,9 @@
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_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 @@
)
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 @@
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 @@
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 @@
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 @@
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 @@
!!! 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 @@
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 7d05055..5862535 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 @@
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 52f251a..0f6a335 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"),
+ ]
)
