am: add support for --check

Revamp patchwork CI checks and add local checks using checkpatch.pl, if
found. This is still in flux and the UI/UX may change depending on the
feedback I receive from maintainers.

Signed-off-by: Konstantin Ryabitsev <konstantin@linuxfoundation.org>

4 files changed, 120 insertions, 146 deletions

diff --git a/src/b4/__init__.py b/src/b4/__init__.py
index e88de37..47552e0 100644
--- a/src/b4/__init__.py
+++ b/src/b4/__init__.py
@@ -19,6 +19,7 @@ import argparse
 import smtplib
 import shlex
 import textwrap
+import json
 
 import urllib.parse
 import datetime
@@ -629,7 +630,7 @@ class LoreSeries:
 
     def get_am_ready(self, noaddtrailers: bool = False, addmysob: bool = False,
                      addlink: bool = False, cherrypick: Optional[List[int]] = None, copyccs: bool = False,
-                     allowbadchars: bool = False) -> List[email.message.Message]:
+                     allowbadchars: bool = False, showchecks: bool = False) -> List[email.message.Message]:
 
         usercfg = get_user_config()
         config = get_main_config()
@@ -670,45 +671,45 @@ class LoreSeries:
                 logger.debug('Attestation info is not the same')
                 break
 
-        if can_network and config.get('pw-url') and config.get('pw-project'):
-            # Use this to pre-load the CI status of each patch
-            logger.info('Retrieving CI status, may take a moment...')
-            show_ci_checks = True
-            # If there is more than one patch in the series, check the last patch first
-            # and skip checking the rest of the patches if the last patch is still 'pending'
-            if self.expected > 1:
-                lastp = self.patches[-1]
-                if lastp:
-                    # This will be cached, so we don't worry about extra lookups
-                    lastp.load_ci_status()
-                    if not lastp.pw_ci_status or lastp.pw_ci_status == 'pending':
-                        logger.debug('No CI on the last patch, skipping the rest of the checks')
-                        lastp.pw_ci_status = None
-                        show_ci_checks = False
-
-            if show_ci_checks:
-                ci_overall = 'success'
-                series_url = None
+        if showchecks:
+            # Local checks
+            cmdstr = None
+            if config.get('am-perpatch-check-cmd'):
+                cmdstr = config.get('am-perpatch-check-cmd')
+            else:
+                # Use recommended checkpatch defaults if we find checkpatch
+                topdir = git_get_toplevel()
+                checkpatch = os.path.join(topdir, 'scripts', 'checkpatch.pl')
+                if os.access(checkpatch, os.X_OK):
+                    cmdstr = f'{checkpatch} -q --terse --no-summary --mailback'
+            cmdargs = None
+            if cmdstr:
+                sp = shlex.shlex(cmdstr, posix=True)
+                sp.whitespace_split = True
+                cmdargs = list(sp)
+
+            if cmdargs:
+                logger.info('Running local checks using %s, may take a moment...',
+                            os.path.basename(cmdargs[0]))
                 for lmsg in self.patches[1:]:
                     if lmsg is None:
                         continue
-                    lmsg.load_ci_status()
+                    # TODO: Progress bar?
+                    lmsg.load_local_ci_status(cmdargs)
+
+            # Patchwork CI status
+            if can_network and config.get('pw-url') and config.get('pw-project'):
+                # Use this to pre-load the CI status of each patch
+                logger.info('Retrieving patchwork CI status, may take a moment...')
+                # Go in reverse order and skip the rest if any patch still shows "pending"
+                for lmsg in reversed(self.patches[1:]):
+                    if lmsg is None:
+                        continue
+                    # TODO: Progress bar?
+                    lmsg.load_pw_ci_status()
                     if not lmsg.pw_ci_status or lmsg.pw_ci_status == 'pending':
+                        logger.debug('No CI on patch %s, skipping the rest of the checks', lmsg.counter)
                         lmsg.pw_ci_status = None
-                        break
-                    if series_url is None:
-                        pwdata = lmsg.get_patchwork_info()
-                        if pwdata and pwdata.get('series'):
-                            for series in pwdata.get('series'):
-                                series_url = series.get('web_url')
-                                break
-                    if lmsg.pw_ci_status == 'warning':
-                        ci_overall = 'warning'
-                    elif lmsg.pw_ci_status == 'fail':
-                        ci_overall = 'fail'
-                if ci_overall != 'success':
-                    logger.info('Some CI checks failed, see patchwork for more info:')
-                    logger.info('  %s', series_url)
 
         self.add_cover_trailers()
 
@@ -771,6 +772,16 @@ class LoreSeries:
                     add_trailers = False
                 msg = lmsg.get_am_message(add_trailers=add_trailers, extras=extras, copyccs=copyccs,
                                           addmysob=addmysob, allowbadchars=allowbadchars)
+                if lmsg.local_ci_status or lmsg.pw_ci_status in {'fail', 'warning'}:
+                    if lmsg.local_ci_status:
+                        for flag, status in lmsg.local_ci_status:
+                            logger.info('    %s %s', CI_FLAGS_FANCY[flag], status)
+                    pwurl = config.get('pw-url')
+                    pwproj = config.get('pw-project')
+                    if lmsg.pw_ci_status in {'fail', 'warning'}:
+                        pwlink = f'{pwurl}/project/{pwproj}/patch/{lmsg.msgid}'
+                        logger.info('    %s patchwork: %s: %s', CI_FLAGS_FANCY[lmsg.pw_ci_status],
+                                    str(lmsg.pw_ci_status).upper(), pwlink)
                 msgs.append(msg)
             else:
                 logger.error('  ERROR: missing [%s/%s]!', at, self.expected)
@@ -1180,8 +1191,9 @@ class LoreMessage:
     pr_tip_commit: Optional[str]
     pr_remote_tip_commit: Optional[str]
 
-    # patch-related info
+    # patch CI info
     pw_ci_status: Optional[str]
+    local_ci_status: Optional[List[Tuple[str, str]]]
 
     def __init__(self, msg):
         self.msg = msg
@@ -1204,6 +1216,7 @@ class LoreMessage:
         self.pr_tip_commit = None
         self.pr_remote_tip_commit = None
         self.pw_ci_status = None
+        self.local_ci_status = None
 
         self.msgid = LoreMessage.get_clean_msgid(self.msg)
         self.lsubject = LoreSubject(msg['Subject'])
@@ -1522,6 +1535,42 @@ class LoreMessage:
             self._attestors.append(attestor)
 
     @staticmethod
+    def run_local_check(cmdargs: List[str], ident: str, msg: email.message.Message,
+                        nocache: bool = False) -> List[Tuple[str, str]]:
+        cacheid = ' '.join(cmdargs) + ident
+        cachedata = get_cache(cacheid, suffix='checks')
+        if cachedata and not nocache:
+            return json.loads(cachedata)
+
+        logger.debug('Checking ident=%s using %s', ident, cmdargs[0])
+        bdata = LoreMessage.get_msg_as_bytes(msg)
+        topdir = git_get_toplevel()
+        mycmd = os.path.basename(cmdargs[0])
+        ecode, out, err = _run_command(cmdargs, stdin=bdata, rundir=topdir)
+        out = out.strip()
+        out_lines = out.decode(errors='replace').split('\n') if out else list()
+        report = list()
+        if out_lines:
+            for line in out_lines:
+                if 'ERROR:' in line:
+                    flag = 'fail'
+                elif 'WARNING:' in line:
+                    flag = 'warning'
+                else:
+                    flag = 'success'
+                # Remove '-:' from the start of the line, because it's never useful
+                if line.startswith('-:'):
+                    line = line[2:]
+                report.append((flag, f'{mycmd}: {line}'))
+        save_cache(json.dumps(report), cacheid, suffix='checks')
+        return report
+
+    def load_local_ci_status(self, cmdargs: List[str]) -> None:
+        # TODO: currently ignoring --no-cache
+        if self.local_ci_status is None:
+            self.local_ci_status = LoreMessage.run_local_check(cmdargs, self.msgid, self.msg)
+
+    @staticmethod
     def get_patchwork_data_by_msgid(msgid: str) -> dict:
         config = get_main_config()
         pwkey = config.get('pw-key')
@@ -1533,7 +1582,6 @@ class LoreMessage:
 
         cachedata = get_cache(pwurl + pwproj + msgid, suffix='lookup')
         if cachedata:
-            import json
             return json.loads(cachedata)
 
         pses, url = get_patchwork_session(pwkey, pwurl)
@@ -1562,7 +1610,6 @@ class LoreMessage:
         if not pwdata:
             logger.debug('Not able to look up patchwork data for %s', msgid)
             raise LookupError('Error looking up %s in patchwork' % msgid)
-        import json
         save_cache(json.dumps(pwdata), pwurl + pwproj + msgid, suffix='lookup')
         return pwdata
 
@@ -1574,46 +1621,7 @@ class LoreMessage:
         except LookupError:
             return None
 
-    def get_ci_checks(self) -> list:
-        checks = list()
-        if not self.pw_ci_status or self.pw_ci_status == 'pending':
-            return checks
-        config = get_main_config()
-        pwkey = config.get('pw-key')
-        pwurl = config.get('pw-url')
-        pwdata = self.get_patchwork_info()
-        pw_patch_id = pwdata.get('id')
-        cacheid = pwurl + str(pw_patch_id) + 'checks'
-        cachedata = get_cache(cacheid, suffix='lookup')
-        if cachedata:
-            import json
-            checks = json.loads(cachedata)
-        else:
-            pses, url = get_patchwork_session(pwkey, pwurl)
-            checks_url = '/'.join((url, 'patches', str(pw_patch_id), 'checks'))
-            try:
-                logger.debug('looking up checks for patch_id=%s', pw_patch_id)
-                rsp = pses.get(checks_url, stream=False)
-                rsp.raise_for_status()
-                pdata = rsp.json()
-                for entry in pdata:
-                    if entry.get('state') == 'success' or entry.get('state') not in CI_FLAGS_FANCY:
-                        # We don't care to see these ;)
-                        continue
-                    checks.append((entry.get('state'), entry.get('context'), entry.get('description')))
-                rsp.close()
-            except requests.exceptions.RequestException as ex:
-                logger.debug('Patchwork REST error: %s', ex)
-                return checks
-            if not pwdata:
-                logger.debug('Not able to look up patchwork checks data for %s', self.msgid)
-                return checks
-            import json
-            save_cache(json.dumps(checks), cacheid, suffix='lookup')
-
-        return checks
-
-    def load_ci_status(self) -> None:
+    def load_pw_ci_status(self) -> None:
         logger.debug('Loading CI status for %s', self.msgid)
         pwdata = self.get_patchwork_info()
         if not pwdata:
@@ -2311,8 +2319,7 @@ class LoreMessage:
 
         self.body = LoreMessage.rebuild_message(bheaders, message, fixtrailers, basement, signature)
 
-    def get_am_subject(self, indicate_reroll: bool = True, use_subject: Optional[str] = None,
-                       show_ci_status: bool = True) -> str:
+    def get_am_subject(self, indicate_reroll: bool = True, use_subject: Optional[str] = None) -> str:
         # Return a clean patch subject
         parts = ['PATCH']
         if self.lsubject.rfc:
@@ -2333,9 +2340,6 @@ class LoreMessage:
         if not use_subject:
             use_subject = self.lsubject.subject
 
-        if show_ci_status and self.pw_ci_status:
-            return '[%s %s] %s' % (CI_FLAGS_FANCY[self.pw_ci_status], ' '.join(parts), use_subject)
-
         return '[%s] %s' % (' '.join(parts), use_subject)
 
     def get_am_message(self, add_trailers: bool = True, addmysob: bool = False,
@@ -2938,12 +2942,17 @@ def get_cache_dir(appname: str = 'b4') -> str:
         logger.critical('ERROR: cache-expire must be an integer (minutes): %s', config['cache-expire'])
         expmin = 600
     expage = time.time() - expmin
+    # Expire anything else that is older than 30 days
+    expall = time.time() - 2592000
     for entry in os.listdir(cachedir):
-        if entry.find('.mbx') <= 0 and entry.find('.lookup') <= 0 and entry.find('.msgs') <= 0:
-            continue
+        suffix = entry.split('.')[-1]
+        if suffix in {'mbx', 'lookup', 'msgs'}:
+            explim = expage
+        else:
+            explim = expall
         fullpath = os.path.join(cachedir, entry)
         st = os.stat(fullpath)
-        if st.st_mtime < expage:
+        if st.st_mtime < explim:
             logger.debug('Cleaning up cache: %s', entry)
             if os.path.isdir(fullpath):
                 shutil.rmtree(fullpath)
diff --git a/src/b4/command.py b/src/b4/command.py
index d06e194..3073888 100644
--- a/src/b4/command.py
+++ b/src/b4/command.py
@@ -55,6 +55,8 @@ def cmd_am_common_opts(sp):
                     help='Cherry-pick a subset of patches (e.g. "-P 1-2,4,6-", '
                          '"-P _" to use just the msgid specified, or '
                          '"-P *globbing*" to match on commit subject)')
+    sp.add_argument('-k', '--check', action='store_true', default=False,
+                    help='Run local checks for every patch (e.g. checkpatch)')
     sp.add_argument('--cc-trailers', dest='copyccs', action='store_true', default=False,
                     help='Copy all Cc\'d addresses into Cc: trailers')
     sp.add_argument('--no-parent', dest='noparent', action='store_true', default=False,
diff --git a/src/b4/ez.py b/src/b4/ez.py
index 2d9b553..3d370e2 100644
--- a/src/b4/ez.py
+++ b/src/b4/ez.py
@@ -1176,18 +1176,6 @@ def get_addresses_from_cmd(cmdargs: List[str], msgbytes: bytes) -> List[Tuple[st
     return utils.getaddresses(addrs.split('\n'))
 
 
-def get_check_results_from_cmd(cmdargs: List[str], msgbytes: bytes) -> List[str]:
-    if not cmdargs:
-        return list()
-    # Run this command from git toplevel
-    topdir = b4.git_get_toplevel()
-    ecode, out, err = b4._run_command(cmdargs, stdin=msgbytes, rundir=topdir)  # noqa
-    check_report = out.strip().decode(errors='ignore')
-    if not check_report:
-        return list()
-    return check_report.split('\n')
-
-
 def get_series_details(start_commit: Optional[str] = None, usebranch: Optional[str] = None
                        ) -> Tuple[str, str, str, List[str], str, str]:
     if usebranch:
@@ -1609,21 +1597,19 @@ def format_patch(output_dir: str) -> None:
 
 
 def check(cmdargs: argparse.Namespace) -> None:
-    # Use recommended checkpatch defaults if we find checkpatch
-    topdir = b4.git_get_toplevel()
-    checkpatch = os.path.join(topdir, 'scripts', 'checkpatch.pl')
     config = b4.get_main_config()
     ppcmdstr = None
     if config.get('prep-perpatch-check-cmd'):
         ppcmdstr = config.get('prep-perpatch-check-cmd')
-    elif os.access(checkpatch, os.X_OK):
-        ppcmdstr = f'{checkpatch} -q --terse --no-summary --mailback --showfile'
-    ppcmd = list()
-    if ppcmdstr:
-        sp = shlex.shlex(ppcmdstr, posix=True)
-        sp.whitespace_split = True
-        ppcmd = list(sp)
-        logger.info('Will run per-patch checks using %s', os.path.basename(ppcmd[0]))
+    else:
+        # Use recommended checkpatch defaults if we find checkpatch
+        topdir = b4.git_get_toplevel()
+        checkpatch = os.path.join(topdir, 'scripts', 'checkpatch.pl')
+        if os.access(checkpatch, os.X_OK):
+            ppcmdstr = f'{checkpatch} -q --terse --no-summary --mailback --showfile'
+    if not ppcmdstr:
+        logger.critical('Not able to find checkpatch and no custom command defined.')
+        sys.exit(1)
     # TODO: support for a whole-series check command, (pytest, etc)
 
     try:
@@ -1632,61 +1618,37 @@ def check(cmdargs: argparse.Namespace) -> None:
         logger.critical('CRITICAL: Failed to convert range to patches: %s', ex)
         sys.exit(1)
 
-    cover, tracking = load_cover()
-    if cmdargs.nocache:
-        checks_cache = dict()
-    else:
-        cached = b4.get_cache(tracking['series']['change-id'], suffix='checks')
-        checks_cache = json.loads(cached) if cached else dict()
-        if checks_cache.get('prep-perpatch-check-cmd', '') != ppcmdstr:
-            logger.debug('Ignoring cached checks info because the check command has changed')
-            b4.clear_cache(tracking['series']['change-id'], suffix='checks')
-            checks_cache = dict()
-    checks_cache['prep-perpatch-check-cmd'] = ppcmdstr
+    logger.info('Checking patches using:')
+    logger.info(f'  {ppcmdstr}')
+    sp = shlex.shlex(ppcmdstr, posix=True)
+    sp.whitespace_split = True
+    ppcmdargs = list(sp)
 
-    logger.info('---')
-    seen = set()
     summary = {
+        'success': 0,
         'warning': 0,
         'fail': 0,
-        'success': 0
     }
+    logger.info('---')
     for commit, msg in patches:
         if not msg or not commit:
             continue
-        seen.add(commit)
+        report = b4.LoreMessage.run_local_check(ppcmdargs, commit, msg, nocache=cmdargs.nocache)
         lsubject = b4.LoreSubject(msg.get('Subject', ''))
         csubject = f'{commit[:12]}: {lsubject.subject}'
-        if 'commits' not in checks_cache:
-            checks_cache['commits'] = dict()
-        if commit not in checks_cache['commits']:
-            logger.debug('Checking: %s', lsubject.subject)
-            bdata = b4.LoreMessage.get_msg_as_bytes(msg)
-            report = get_check_results_from_cmd(ppcmd, bdata)
-            checks_cache['commits'][commit] = report
-        else:
-            logger.debug('Using cached checks for %s', commit)
-            report = checks_cache['commits'][commit]
         if not report:
             logger.info('%s %s', b4.CI_FLAGS_FANCY['success'], csubject)
             summary['success'] += 1
             continue
         worst = 'warning'
-        for line in report:
-            if 'ERROR:' in line:
+        for flag, status in report:
+            if flag == 'fail':
                 worst = 'fail'
                 break
         logger.info('%s %s', b4.CI_FLAGS_FANCY[worst], csubject)
-        for line in report:
-            flag = 'fail' if 'ERROR:' in line else 'warning'
+        for flag, status in report:
             summary[flag] += 1
-            logger.info('  %s %s', b4.CI_FLAGS_FANCY[flag], line)
-    # Throw out any stale checks
-    for commit in list(checks_cache['commits'].keys()):
-        if commit not in seen:
-            logger.debug('Removing stale check cache for non-existent commit %s', commit)
-            del(checks_cache['commits'][commit])
-    b4.save_cache(json.dumps(checks_cache), tracking['series']['change-id'], suffix='checks')
+            logger.info('  %s %s', b4.CI_FLAGS_FANCY[flag], status)
     logger.info('---')
     logger.info('Success: %s, Warning: %s, Error: %s', summary['success'], summary['warning'], summary['fail'])
 
diff --git a/src/b4/mbox.py b/src/b4/mbox.py
index af80b97..1fcd652 100644
--- a/src/b4/mbox.py
+++ b/src/b4/mbox.py
@@ -153,7 +153,8 @@ def make_am(msgs: List[email.message.Message], cmdargs: argparse.Namespace, msgi
         cherrypick = None
 
     am_msgs = lser.get_am_ready(noaddtrailers=cmdargs.noaddtrailers, addmysob=cmdargs.addmysob, addlink=cmdargs.addlink,
-                                cherrypick=cherrypick, copyccs=cmdargs.copyccs, allowbadchars=cmdargs.allowbadchars)
+                                cherrypick=cherrypick, copyccs=cmdargs.copyccs, allowbadchars=cmdargs.allowbadchars,
+                                showchecks=cmdargs.check)
     logger.info('---')
 
     if cherrypick is None: