Add ability to diff arbitrary mbox files

Sometimes we are unable to properly look up previous series -- usually
because the cover letter title changes. For those cases, it is now
possible to diff two arbitrary mbox files prepared with "b4 am -T".

There's also a smattering of other fixes in there because I'm too
lazy to properly stage my patches.

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

4 files changed, 83 insertions, 45 deletions

diff --git a/b4/__init__.py b/b4/__init__.py
index 414f669..b6f25fe 100644
--- a/b4/__init__.py
+++ b/b4/__init__.py
@@ -335,19 +335,19 @@ class LoreMailbox:
         logger.debug('Looking at: %s', lmsg.full_subject)
         self.msgid_map[lmsg.msgid] = lmsg
 
-        if lmsg.counter == 0 and lmsg.has_diffstat:
-            # Cover letter
-            # Add it to covers -- we'll deal with them later
-            logger.debug('  adding as v%s cover letter', lmsg.revision)
-            self.covers[lmsg.revision] = lmsg
-            return
-
         if lmsg.reply:
             # We'll figure out where this belongs later
             logger.debug('  adding to followups')
             self.followups.append(lmsg)
             return
 
+        if lmsg.counter == 0 and (not lmsg.counters_inferred or lmsg.has_diffstat):
+            # Cover letter
+            # Add it to covers -- we'll deal with them later
+            logger.debug('  adding as v%s cover letter', lmsg.revision)
+            self.covers[lmsg.revision] = lmsg
+            return
+
         if re.search(r'^Comment: att-fmt-ver:', lmsg.body, re.I | re.M):
             logger.debug('Found attestation message')
             LoreAttestationDocument.load_from_string(lmsg.msgid, lmsg.body)
diff --git a/b4/command.py b/b4/command.py
index 80b072a..2964f54 100644
--- a/b4/command.py
+++ b/b4/command.py
@@ -106,6 +106,8 @@ def cmd():
                        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_am.add_argument('-g', '--guess-base', dest='guessbase', action='store_true', default=False,
+                       help='Try to guess the base of the series (if not specified)')
     sp_am.set_defaults(func=cmd_am)
 
     # b4 attest
@@ -187,6 +189,8 @@ def cmd():
                          help='Save diff into this file instead of outputting to stdout')
     sp_diff.add_argument('-c', '--color', dest='color', action='store_true', default=False,
                          help='Force color output even when writing to file')
+    sp_diff.add_argument('-m', '--compare-am-mboxes', dest='ambox', nargs=2, default=None,
+                         help='Compare two mbx files prepared with "b4 am"')
     sp_diff.set_defaults(func=cmd_diff)
 
     cmdargs = parser.parse_args()
diff --git a/b4/diff.py b/b4/diff.py
index d2f467d..e9766c9 100644
--- a/b4/diff.py
+++ b/b4/diff.py
@@ -69,6 +69,10 @@ def make_fake_commit_range(gitdir, lser):
         for lmsg in lser.patches[1:]:
             logger.debug('Looking at %s', lmsg.full_subject)
             lmsg.load_hashes()
+            if not len(lmsg.blob_indexes):
+                logger.critical('ERROR: some patches do not have indexes')
+                logger.critical('       automatic range-diff would be misleading')
+                return None, None
             for fn, fi in lmsg.blob_indexes:
                 if fn in seenfiles:
                     # We already processed this file, so this blob won't match
@@ -123,9 +127,10 @@ def make_fake_commit_range(gitdir, lser):
     return start_commit, end_commit
 
 
-def main(cmdargs):
+def diff_same_thread_series(cmdargs):
     msgid = b4.get_msgid(cmdargs)
-    if cmdargs.wantvers and len(cmdargs.wantvers) > 2:
+    wantvers = cmdargs.wantvers
+    if wantvers and len(wantvers) > 2:
         logger.critical('Can only compare two versions at a time')
         sys.exit(1)
 
@@ -133,9 +138,9 @@ def main(cmdargs):
     savefile = mkstemp('b4-diff-to')[1]
     # Do we have a cache of this lookup?
     cachedir = b4.get_cache_dir()
-    if cmdargs.wantvers:
+    if wantvers:
         cachefile = os.path.join(cachedir, '%s-%s.diff.mbx' % (urllib.parse.quote_plus(msgid),
-                                                               '-'.join([str(x) for x in cmdargs.wantvers])))
+                                                               '-'.join([str(x) for x in wantvers])))
     else:
         cachefile = os.path.join(cachedir, '%s-latest.diff.mbx' % urllib.parse.quote_plus(msgid))
     if os.path.exists(cachefile) and not cmdargs.nocache:
@@ -147,10 +152,9 @@ def main(cmdargs):
         if mboxfile is None:
             logger.critical('Unable to retrieve thread: %s', msgid)
             return
-        logger.info('Retrieved %s messages in the thread', len(mboxfile))
-        b4.mbox.get_extra_series(mboxfile, direction=-1, wantvers=cmdargs.wantvers)
+        b4.mbox.get_extra_series(mboxfile, direction=-1, wantvers=wantvers)
+        shutil.copyfile(mboxfile, cachefile)
 
-    shutil.copyfile(mboxfile, cachefile)
     mbx = mailbox.mbox(mboxfile)
     count = len(mbx)
     logger.info('---')
@@ -158,51 +162,81 @@ def main(cmdargs):
     lmbx = b4.LoreMailbox()
     for key, msg in mbx.items():
         lmbx.add_message(msg)
-    if cmdargs.wantvers and len(cmdargs.wantvers) == 1:
+
+    mbx.close()
+    os.unlink(mboxfile)
+
+    if wantvers and len(wantvers) == 1:
         upper = max(lmbx.series.keys())
-        lower = cmdargs.wantvers[0]
-    elif cmdargs.wantvers and len(cmdargs.wantvers) == 2:
-        upper = max(cmdargs.wantvers)
-        lower = min(cmdargs.wantvers)
+        lower = wantvers[0]
+    elif wantvers and len(wantvers) == 2:
+        upper = max(wantvers)
+        lower = min(wantvers)
     else:
         upper = max(lmbx.series.keys())
         lower = min(lmbx.series.keys())
 
     if upper == lower:
         logger.critical('Could not find previous revision')
-        os.unlink(mboxfile)
-        sys.exit(1)
+        return None, None
 
     if upper not in lmbx.series:
-        logger.critical('Could not find revision %s', upper)
-        os.unlink(mboxfile)
-        sys.exit(1)
+        return None, None
+
     if lower not in lmbx.series:
-        logger.critical('Could not find revision %s', lower)
-        os.unlink(mboxfile)
+        return None, None
+
+    return lmbx.series[lower], lmbx.series[upper]
+
+
+def diff_mboxes(cmdargs):
+    chunks = list()
+    for mboxfile in cmdargs.ambox:
+        if not os.path.exists(mboxfile):
+            logger.critical('Cannot open %s', mboxfile)
+            return None, None
+
+        mbx = mailbox.mbox(mboxfile)
+        count = len(mbx)
+        logger.info('Loading %s messages from %s', count, mboxfile)
+        lmbx = b4.LoreMailbox()
+        for key, msg in mbx.items():
+            lmbx.add_message(msg)
+        if len(lmbx.series) > 1:
+            logger.critical('More than one series version in %s, will use latest', mboxfile)
+
+        chunks.append(lmbx.series[max(lmbx.series.keys())])
+
+    return chunks
+
+
+def main(cmdargs):
+    if cmdargs.ambox is not None:
+        lser, user = diff_mboxes(cmdargs)
+    else:
+        lser, user = diff_same_thread_series(cmdargs)
+
+    if lser is None or user is None:
         sys.exit(1)
 
     # Prepare the lower fake-am range
-    lsc, lec = make_fake_commit_range(cmdargs.gitdir, lmbx.series[lower])
+    lsc, lec = make_fake_commit_range(cmdargs.gitdir, lser)
     if lsc is None or lec is None:
         logger.critical('---')
-        logger.critical('Could not create fake-am range for lower series v%s', lower)
-        os.unlink(mboxfile)
+        logger.critical('Could not create fake-am range for lower series v%s', lser.revision)
         sys.exit(1)
     # Prepare the upper fake-am range
-    usc, uec = make_fake_commit_range(cmdargs.gitdir, lmbx.series[upper])
+    usc, uec = make_fake_commit_range(cmdargs.gitdir, user)
     if usc is None or uec is None:
         logger.critical('---')
-        logger.critical('Could not create fake-am range for upper series v%s', upper)
-        os.unlink(mboxfile)
+        logger.critical('Could not create fake-am range for upper series v%s', user.revision)
         sys.exit(1)
-    logger.info('---')
     grdcmd = 'git range-diff %.12s..%.12s %.12s..%.12s' % (lsc, lec, usc, uec)
     if cmdargs.nodiff:
-        logger.info('Success, to compare v%s and v%s:', lower, upper)
+        logger.info('Success, to compare v%s and v%s:', lser.revision, user.revision)
         logger.info(f'    {grdcmd}')
         sys.exit(0)
-    logger.info('Diffing v%s and v%s', lower, upper)
+    logger.info('Diffing v%s and v%s', lser.revision, user.revision)
     logger.info('    Running: %s', grdcmd)
     gitargs = ['range-diff', f'{lsc}..{lec}', f'{usc}..{uec}']
     if cmdargs.outdiff is None or cmdargs.color:
diff --git a/b4/mbox.py b/b4/mbox.py
index e03ef6c..abcad74 100644
--- a/b4/mbox.py
+++ b/b4/mbox.py
@@ -173,7 +173,7 @@ def mbox_to_am(mboxfile, cmdargs):
             checked, mismatches = lser.check_applies_clean(topdir)
             if mismatches == 0 and checked != mismatches:
                 cleanmsg = ' (applies clean to current tree)'
-            else:
+            elif cmdargs.guessbase:
                 # Look at the last 10 tags and see if it applies cleanly to
                 # any of them. I'm not sure how useful this is, but I'm going
                 # to put it in for now and maybe remove later if it causes
@@ -324,18 +324,18 @@ def get_extra_series(mboxfile, direction=1, wantvers=None, nocache=False):
         # Ignore replies or counters above 1
         if lsub.reply or lsub.counter > 1:
             continue
-        if latest_revision is None or lsub.revision > latest_revision:
-            # New revision
+        if base_msg is not None:
+            logger.debug('Current base_msg: %s', base_msg['Subject'])
+        logger.debug('Checking the subject on %s', lsub.full_subject)
+        if latest_revision is None or lsub.revision >= latest_revision:
             latest_revision = lsub.revision
-            if lsub.counter == 0:
+            if lsub.counter == 0 and not lsub.counters_inferred:
                 # And a cover letter, nice. This is the easy case
                 base_msg = msg
                 seen_covers.append(latest_revision)
-                continue
-            if lsub.counter == 1:
-                if latest_revision not in seen_covers:
-                    # A patch/series without a cover letter
-                    base_msg = msg
+            elif lsub.counter == 1 and latest_revision not in seen_covers:
+                # A patch/series without a cover letter
+                base_msg = msg
 
     # Get subject info from base_msg again
     lsub = b4.LoreSubject(base_msg['Subject'])
@@ -371,7 +371,7 @@ def get_extra_series(mboxfile, direction=1, wantvers=None, nocache=False):
     try:
         tree = xml.etree.ElementTree.fromstring(resp.content)
     except xml.etree.ElementTree.ParseError as ex:
-        logger.debug('Unable to parse results, ignoring', ex)
+        logger.debug('Unable to parse results, ignoring: %s', ex)
         resp.close()
         mbx.close()
         return