llvm_tools: use tiny_render in the nightly revert checker

Now that we have `tiny_render` imported into `cros_utils`, we can use
it. This lets us easily linkify and stylize our emails, and makes
comments like tcwang's in Ibc3ef1d2e3b5a3301366e971cacf53396a6ca2aa much
easier to address.

BUG=chromium:1046988
TEST=unittests; sent an email

Change-Id: I538efd2e349a48bd433c826359eb0dd94b422966
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/toolchain-utils/+/2169010
Reviewed-by: Tiancong Wang <tcwang@google.com>
Tested-by: George Burgess <gbiv@chromium.org>

2 files changed, 98 insertions, 32 deletions

diff --git a/llvm_tools/nightly_revert_checker.py b/llvm_tools/nightly_revert_checker.py
index b7b87fad..24130e0a 100755
--- a/llvm_tools/nightly_revert_checker.py
+++ b/llvm_tools/nightly_revert_checker.py
@@ -24,6 +24,8 @@ import sys
 import typing as t
 
 import cros_utils.email_sender as email_sender
+import cros_utils.tiny_render as tiny_render
+import get_llvm_hash
 import revert_checker
 
 State = t.Any
@@ -77,35 +79,46 @@ def _find_interesting_shas(chromeos_base: str) -> t.List[t.Tuple[str]]:
 
 _Email = t.NamedTuple('_Email', [
     ('subject', str),
-    ('body', str),
+    ('body', tiny_render.Piece),
 ])
 
 
 def _generate_revert_email(
     friendly_name: str, sha: str,
+    prettify_sha: t.Callable[[str], tiny_render.Piece],
     new_reverts: t.List[revert_checker.Revert]) -> _Email:
-  email_lines = [
-      'It looks like there may be %s across %s (%s).' % (
+  email_pieces = [
+      'It looks like there may be %s across %s (' % (
           'a new revert' if len(new_reverts) == 1 else 'new reverts',
           friendly_name,
-          sha,
       ),
-      '',
+      prettify_sha(sha),
+      ').',
+      tiny_render.line_break,
+      tiny_render.line_break,
       'That is:' if len(new_reverts) == 1 else 'These are:',
   ]
 
+  revert_listing = []
   for revert in sorted(new_reverts, key=lambda r: r.sha):
-    email_lines.append(
-        '\t- %s (appears to revert %s)' % (revert.sha, revert.reverted_sha))
-
-  email_lines.append('')
-  email_lines.append('PTAL and consider reverting them locally.')
+    revert_listing.append([
+        prettify_sha(revert.sha),
+        ' (appears to revert ',
+        prettify_sha(revert.reverted_sha),
+        ')',
+    ])
+
+  email_pieces.append(tiny_render.UnorderedList(items=revert_listing))
+  email_pieces += [
+      tiny_render.line_break,
+      'PTAL and consider reverting them locally.',
+  ]
   return _Email(
       subject='[revert-checker] new %s discovered across %s' % (
           'revert' if len(new_reverts) == 1 else 'reverts',
           friendly_name,
       ),
-      body='\n'.join(email_lines),
+      body=email_pieces,
   )
 
 
@@ -113,9 +126,11 @@ def _send_revert_email(email: _Email) -> None:
   email_sender.EmailSender().SendX20Email(
       subject=email.subject,
       identifier='revert-checker',
-      well_known_recipients=['mage'],
+      well_known_recipients=[],
       direct_recipients=['gbiv@google.com'],
-      text_body=email.body)
+      text_body=tiny_render.render_text_pieces(email.body),
+      html_body=tiny_render.render_html_pieces(email.body),
+  )
 
 
 def _write_state(state_file: str, new_state: State) -> None:
@@ -165,19 +180,31 @@ def main(argv: t.List[str]) -> None:
 
   state_file = opts.state_file
   dry_run = opts.dry_run
+  llvm_dir = opts.llvm_dir
 
   state = _read_state(state_file)
 
   interesting_shas = _find_interesting_shas(opts.chromeos_dir)
   logging.info('Interesting SHAs were %r', interesting_shas)
 
+  def prettify_sha(sha: str) -> tiny_render.Piece:
+    rev = get_llvm_hash.GetVersionFrom(llvm_dir, sha)
+
+    # 12 is arbitrary, but should be unambiguous enough.
+    short_sha = sha[:12]
+    return tiny_render.Switch(
+        text='r%s (%s)' % (rev, short_sha),
+        html=tiny_render.Link(
+            href='https://reviews.llvm.org/rG' + sha, inner='r' + str(rev)),
+    )
+
   new_state: State = {}
   revert_emails_to_send: t.List[t.Tuple[str, t.List[revert_checker
                                                     .Revert]]] = []
   for friendly_name, sha in interesting_shas:
     logging.info('Finding reverts across %s (%s)', friendly_name, sha)
     all_reverts = revert_checker.find_reverts(
-        opts.llvm_dir, sha, root='origin/master')
+        llvm_dir, sha, root='origin/master')
     logging.info('Detected the following revert(s) across %s:\n%s',
                  friendly_name, pprint.pformat(all_reverts))
 
@@ -195,14 +222,14 @@ def main(argv: t.List[str]) -> None:
       continue
 
     revert_emails_to_send.append(
-        _generate_revert_email(friendly_name, sha, new_reverts))
+        _generate_revert_email(friendly_name, sha, prettify_sha, new_reverts))
 
   # We want to be as free of obvious side-effects as possible in case something
   # above breaks. Hence, send the email as late as possible.
   for email in revert_emails_to_send:
     if dry_run:
       logging.info('Would send email:\nSubject: %s\nBody:\n%s\n', email.subject,
-                   email.body)
+                   tiny_render.render_text_pieces(email.body))
     else:
       logging.info('Sending email with subject %r...', email.subject)
       _send_revert_email(email)
diff --git a/llvm_tools/nightly_revert_checker_test.py b/llvm_tools/nightly_revert_checker_test.py
index 81cfbd2c..dce29343 100755
--- a/llvm_tools/nightly_revert_checker_test.py
+++ b/llvm_tools/nightly_revert_checker_test.py
@@ -8,22 +8,28 @@
 
 from __future__ import print_function
 
-# pylint: disable=protected-access
-
 import io
 import unittest
 
+import cros_utils.tiny_render as tiny_render
 import nightly_revert_checker
 import revert_checker
 
+# pylint: disable=protected-access
+
 
 class Test(unittest.TestCase):
   """Tests for nightly_revert_checker."""
 
   def test_email_rendering_works_for_singular_revert(self):
+
+    def prettify_sha(sha: str) -> tiny_render.Piece:
+      return 'pretty_' + sha
+
     email = nightly_revert_checker._generate_revert_email(
         friendly_name='${name}',
         sha='${sha}',
+        prettify_sha=prettify_sha,
         new_reverts=[
             revert_checker.Revert(
                 sha='${revert_sha}', reverted_sha='${reverted_sha}')
@@ -31,21 +37,34 @@ class Test(unittest.TestCase):
 
     expected_email = nightly_revert_checker._Email(
         subject='[revert-checker] new revert discovered across ${name}',
-        body='\n'.join((
-            'It looks like there may be a new revert across ${name} (${sha}).',
-            '',
+        body=[
+            'It looks like there may be a new revert across ${name} (',
+            'pretty_${sha}',
+            ').',
+            tiny_render.line_break,
+            tiny_render.line_break,
             'That is:',
-            '\t- ${revert_sha} (appears to revert ${reverted_sha})',
-            '',
+            tiny_render.UnorderedList([[
+                'pretty_${revert_sha}',
+                ' (appears to revert ',
+                'pretty_${reverted_sha}',
+                ')',
+            ]]),
+            tiny_render.line_break,
             'PTAL and consider reverting them locally.',
-        )))
+        ])
 
     self.assertEqual(email, expected_email)
 
   def test_email_rendering_works_for_multiple_reverts(self):
+
+    def prettify_sha(sha: str) -> tiny_render.Piece:
+      return 'pretty_' + sha
+
     email = nightly_revert_checker._generate_revert_email(
         friendly_name='${name}',
         sha='${sha}',
+        prettify_sha=prettify_sha,
         new_reverts=[
             revert_checker.Revert(
                 sha='${revert_sha1}', reverted_sha='${reverted_sha1}'),
@@ -58,16 +77,36 @@ class Test(unittest.TestCase):
 
     expected_email = nightly_revert_checker._Email(
         subject='[revert-checker] new reverts discovered across ${name}',
-        body='\n'.join((
-            'It looks like there may be new reverts across ${name} (${sha}).',
-            '',
+        body=[
+            'It looks like there may be new reverts across ${name} (',
+            'pretty_${sha}',
+            ').',
+            tiny_render.line_break,
+            tiny_render.line_break,
             'These are:',
-            '\t- ${revert_sha0} (appears to revert ${reverted_sha0})',
-            '\t- ${revert_sha1} (appears to revert ${reverted_sha1})',
-            '\t- ${revert_sha2} (appears to revert ${reverted_sha2})',
-            '',
+            tiny_render.UnorderedList([
+                [
+                    'pretty_${revert_sha0}',
+                    ' (appears to revert ',
+                    'pretty_${reverted_sha0}',
+                    ')',
+                ],
+                [
+                    'pretty_${revert_sha1}',
+                    ' (appears to revert ',
+                    'pretty_${reverted_sha1}',
+                    ')',
+                ],
+                [
+                    'pretty_${revert_sha2}',
+                    ' (appears to revert ',
+                    'pretty_${reverted_sha2}',
+                    ')',
+                ],
+            ]),
+            tiny_render.line_break,
             'PTAL and consider reverting them locally.',
-        )))
+        ])
 
     self.assertEqual(email, expected_email)