Implement partial reroll

It has been a common request to support partial series rerolls where
someone sends an amended patch as a follow-up to a previous series,
e.g.:

[PATCH v3 1/3] Patch one
[PATCH v3 2/3] Patch two
\- Re: [PATCH v3 2/3] Patch two
   Looks good, but please fix this $small_thing

   \- [PATCH v4 2/3] Patch two
[PATCH v3] Patch three

Previously, b4 refused to consider v4 as a complete new series, but now
it will properly perform a partial reroll, but only in the cases where
such patches are sent as follow-ups to the exact same patch number in
the previous series:

[PATCH v3->v4 1/3] Patch one
[PATCH     v4 2/3] Patch two
[PATCH v3->v4 3/3] Patch three

Reported-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Konstantin Ryabitsev <konstantin@linuxfoundation.org>
Link: https://lore.kernel.org/r/CAPcyv4ggbuHbqKV33_TpE7pqxvRag34baJrX3yQe-jXOikoATQ@mail.gmail.com

4 files changed, 124 insertions, 41 deletions

diff --git a/b4/__init__.py b/b4/__init__.py
index 11ad012..1418641 100644
--- a/b4/__init__.py
+++ b/b4/__init__.py
@@ -15,6 +15,7 @@ import requests
 import urllib.parse
 import datetime
 import time
+import copy
 import shutil
 import mailbox
 # noinspection PyCompatibility
@@ -229,7 +230,66 @@ class LoreMailbox:
                     logger.info('Successfully backfilled missing patches')
                     break
 
-    def get_series(self, revision=None, sloppytrailers=False, backfill=True):
+    def partial_reroll(self, revision, sloppytrailers, backfill):
+        # Is it a partial reroll?
+        # To qualify for a partial reroll:
+        # 1. Needs to be version > 1
+        # 2. Replies need to be to the exact X/N of the previous revision
+        if revision <= 1 or revision - 1 not in self.series:
+            return
+        # Are existing patches replies to previous revisions with the same counter?
+        pser = self.get_series(revision-1, sloppytrailers=sloppytrailers, backfill=backfill)
+        lser = self.series[revision]
+        sane = True
+        for patch in lser.patches:
+            if patch is None:
+                continue
+            if patch.in_reply_to is None or patch.in_reply_to not in self.msgid_map:
+                logger.debug('Patch not sent as a reply-to')
+                sane = False
+                break
+            ppatch = self.msgid_map[patch.in_reply_to]
+            found = False
+            while True:
+                if patch.counter == ppatch.counter and patch.expected == ppatch.expected:
+                    logger.debug('Found a previous matching patch in v%s', ppatch.revision)
+                    found = True
+                    break
+                # Do we have another level up?
+                if ppatch.in_reply_to is None or ppatch.in_reply_to not in self.msgid_map:
+                    break
+                ppatch = self.msgid_map[ppatch.in_reply_to]
+
+            if not found:
+                sane = False
+                logger.debug('Patch not a reply to a patch with the same counter/expected (%s/%s != %s/%s)',
+                             patch.counter, patch.expected, ppatch.counter, ppatch.expected)
+                break
+
+        if not sane:
+            logger.debug('Not a sane partial reroll')
+            return
+        logger.info('Partial reroll detected, reconstituting from v%s', pser.revision)
+        logger.debug('Reconstituting a partial reroll')
+        at = 0
+        for patch in lser.patches:
+            if patch is None:
+                ppatch = copy.deepcopy(pser.patches[at])
+                ppatch.revision = lser.revision
+                ppatch.reroll_from_revision = pser.revision
+                lser.patches[at] = ppatch
+            else:
+                patch.reroll_from_revision = lser.revision
+            at += 1
+        if None not in lser.patches[1:]:
+            lser.complete = True
+            lser.partial_reroll = True
+            if lser.patches[0] is not None:
+                lser.has_cover = True
+            lser.subject = pser.subject
+            logger.debug('Reconstituted successfully')
+
+    def get_series(self, revision=None, sloppytrailers=False, backfill=True, reroll=True):
         if revision is None:
             if not len(self.series):
                 return None
@@ -250,11 +310,14 @@ class LoreMailbox:
             logger.critical('All patches in series v%s are missing.', lser.revision)
             return None
 
+        if not lser.complete and reroll:
+            self.partial_reroll(revision, sloppytrailers, backfill)
+
         if not lser.complete and backfill:
             self.backfill(revision)
 
         # Grab our cover letter if we have one
-        if revision in self.covers.keys():
+        if revision in self.covers:
             lser.add_patch(self.covers[revision])
             lser.has_cover = True
         else:
@@ -307,18 +370,15 @@ class LoreMailbox:
                 for trailer in trailers:
                     logger.debug('%s%s: %s', ' ' * (lvl+1), trailer[0], trailer[1])
                 if pmsg.has_diff and not pmsg.reply:
-                    # TODO: See below
                     # We found the patch for these trailers
-                    # if pmsg.revision != revision:
-                    #     # add this into our trailer map to carry over trailers from
-                    #     # previous revisions to current revision if patch/metadata did
-                    #     # not change
-                    #     pmsg.load_hashes()
-                    #     if pmsg.attestation:
-                    #         attid = pmsg.attestation.attid
-                    #         if attid not in self.trailer_map:
-                    #             self.trailer_map[attid] = list()
-                    #         self.trailer_map[attid] += trailers
+                    if pmsg.revision != revision:
+                        # add this into our trailer map to carry over trailers from
+                        # previous revisions to current revision if patch id did
+                        # not change
+                        if pmsg.pwhash:
+                            if pmsg.pwhash not in self.trailer_map:
+                                self.trailer_map[pmsg.pwhash] = list()
+                            self.trailer_map[pmsg.pwhash] += trailers
                     pmsg.followup_trailers += trailers
                     break
                 if not pmsg.reply:
@@ -334,13 +394,11 @@ class LoreMailbox:
                 break
 
         # Carry over trailers from previous series if patch/metadata did not change
-        # TODO: This needs fixing, because we no longer generate the three hashes
-        # TODO: and patchwork_hash only gives us the hash of the actual patch
-        # for lmsg in lser.patches:
-        #     if lmsg is None or lmsg.patchwork_hash is None:
-        #         continue
-        #     if lmsg.patchwork_hash in self.trailer_map:
-        #         lmsg.followup_trailers += self.trailer_map[lmsg.patchwork_hash]
+        for lmsg in lser.patches:
+            if lmsg is None or lmsg.pwhash is None:
+                continue
+            if lmsg.pwhash in self.trailer_map:
+                lmsg.followup_trailers += self.trailer_map[lmsg.pwhash]
 
         return lser
 
@@ -415,6 +473,7 @@ class LoreSeries:
         self.trailer_mismatches = set()
         self.complete = False
         self.has_cover = False
+        self.partial_reroll = False
         self.subject = '(untitled)'
 
     def __repr__(self):
@@ -424,13 +483,12 @@ class LoreSeries:
         out.append('  expected: %s' % self.expected)
         out.append('  complete: %s' % self.complete)
         out.append('  has_cover: %s' % self.has_cover)
+        out.append('  partial_reroll: %s' % self.partial_reroll)
         out.append('  patches:')
         at = 0
         for member in self.patches:
             if member is not None:
                 out.append('    [%s/%s] %s' % (at, self.expected, member.subject))
-                if member.followup_trailers:
-                    out.append('       Add: %s' % ', '.join(member.followup_trailers))
             else:
                 out.append('    [%s/%s] MISSING' % (at, self.expected))
             at += 1
@@ -547,13 +605,13 @@ class LoreSeries:
 
                 if attsame and not attcrit:
                     if attmark:
-                        logger.info('  %s %s', attmark, lmsg.full_subject)
+                        logger.info('  %s %s', attmark, lmsg.get_am_subject())
                     else:
-                        logger.info('  %s', lmsg.full_subject)
+                        logger.info('  %s', lmsg.get_am_subject())
 
                 else:
                     checkmark, trailers, critical = lmsg.get_attestation_trailers(attpolicy, maxdays)
-                    logger.info('  %s %s', checkmark, lmsg.full_subject)
+                    logger.info('  %s %s', checkmark, lmsg.get_am_subject())
                     for trailer in trailers:
                         logger.info('    %s', trailer)
 
@@ -647,7 +705,7 @@ class LoreSeries:
             logger.critical('Cannot operate on an empty series')
             return None, None
         cachedata = get_cache(msgid, suffix='fakeam')
-        if cachedata:
+        if cachedata and not self.partial_reroll:
             stalecache = False
             chunks = cachedata.strip().split()
             if len(chunks) == 2:
@@ -760,6 +818,7 @@ class LoreMessage:
         self.subject = None
         self.reply = False
         self.revision = 1
+        self.reroll_from_revision = None
         self.counter = 1
         self.expected = 1
         self.revision_inferred = True
@@ -1403,6 +1462,24 @@ class LoreMessage:
             self.body += signature
             self.body += '\n'
 
+    def get_am_subject(self):
+        # Return a clean patch subject
+        parts = ['PATCH']
+        if self.lsubject.rfc:
+            parts.append('RFC')
+        if not self.revision_inferred:
+            if self.reroll_from_revision:
+                if self.reroll_from_revision != self.revision:
+                    parts.append('v%d->v%d' % (self.reroll_from_revision, self.revision))
+                else:
+                    parts.append(' %s  v%d' % (' ' * len(str(self.reroll_from_revision)), self.revision))
+            else:
+                parts.append('v%d' % self.revision)
+        if not self.lsubject.counters_inferred:
+            parts.append('%d/%d' % (self.lsubject.counter, self.lsubject.expected))
+
+        return '[%s] %s' % (' '.join(parts), self.lsubject.subject)
+
     def get_am_message(self, add_trailers=True, trailer_order=None, copyccs=False):
         if add_trailers:
             self.fix_trailers(trailer_order=trailer_order, copyccs=copyccs)
@@ -1493,17 +1570,6 @@ class LoreSubject:
                 self.prefixes.append(chunk)
             subject = re.sub(r'^\s*\[[^]]*]\s*', '', subject)
         self.subject = subject
-        # reconstitute full subject
-        parts = ['PATCH']
-        if self.rfc:
-            parts.append('RFC')
-        if self.resend:
-            parts.append('RESEND')
-        if not self.revision_inferred:
-            parts.append('v%d' % self.revision)
-        if not self.counters_inferred:
-            parts.append('%d/%d' % (self.counter, self.expected))
-        self.full_subject = '[%s] %s' % (' '.join(parts), subject)
 
     def __repr__(self):
         out = list()
@@ -1631,7 +1697,8 @@ class LoreAttestorDKIM(LoreAttestor):
 
 
 class LoreAttestorPatatt(LoreAttestor):
-    def __init__(self, result: bool, identity: str, signtime: str, keysrc: str, keyalgo: str, errors: list) -> None:
+    def __init__(self, result: bool, identity: str, signtime: Optional[any], keysrc: str, keyalgo: str,
+                 errors: list) -> None:
         super().__init__()
         self.mode = 'patatt'
         self.level = 'person'
diff --git a/b4/command.py b/b4/command.py
index 12c7354..5a9d6eb 100644
--- a/b4/command.py
+++ b/b4/command.py
@@ -118,6 +118,8 @@ def cmd():
                        help='Copy all Cc\'d addresses into Cc: trailers')
     sp_am.add_argument('--no-cover', dest='nocover', action='store_true', default=False,
                        help='Do not save the cover letter (on by default when using -o -)')
+    sp_am.add_argument('--no-partial-reroll', dest='nopartialreroll', action='store_true', default=False,
+                       help='Do not reroll partial series when detected')
     sp_am.set_defaults(func=cmd_am)
 
     # b4 attest
@@ -129,7 +131,7 @@ def cmd():
     sp_att.add_argument('-o', '--output', default=None,
                         help='OBSOLETE: this option does nothing and will be removed')
     sp_att.add_argument('-m', '--mutt-filter', default=None,
-                        help='OBSOLETE: this option does nothign and will be removed')
+                        help='OBSOLETE: this option does nothing and will be removed')
     sp_att.add_argument('patchfile', nargs='*', help='Patches to attest')
     sp_att.set_defaults(func=cmd_attest)
 
diff --git a/b4/diff.py b/b4/diff.py
index 10f3159..38d2a9a 100644
--- a/b4/diff.py
+++ b/b4/diff.py
@@ -66,7 +66,7 @@ def diff_same_thread_series(cmdargs):
         lower = min(wantvers)
     else:
         upper = max(lmbx.series.keys())
-        lower = min(lmbx.series.keys())
+        lower = upper - 1
 
     if upper == lower:
         logger.critical('ERROR: Could not auto-find previous revision')
@@ -80,9 +80,15 @@ def diff_same_thread_series(cmdargs):
         return None, None
 
     if not lmbx.series[lower].complete:
+        lmbx.partial_reroll(lower, sloppytrailers=False, backfill=True)
+
+    if not lmbx.series[lower].complete:
         lmbx.backfill(lower)
 
     if not lmbx.series[upper].complete:
+        lmbx.partial_reroll(upper, sloppytrailers=False, backfill=True)
+
+    if not lmbx.series[upper].complete:
         lmbx.backfill(upper)
 
     return lmbx.series[lower], lmbx.series[upper]
diff --git a/b4/mbox.py b/b4/mbox.py
index 6d4c408..f0bae35 100644
--- a/b4/mbox.py
+++ b/b4/mbox.py
@@ -47,7 +47,10 @@ def mbox_to_am(mboxfile, cmdargs, msgid):
     for key, msg in mbx.items():
         lmbx.add_message(msg)
 
-    lser = lmbx.get_series(revision=wantver, sloppytrailers=cmdargs.sloppytrailers)
+    reroll = True
+    if cmdargs.nopartialreroll:
+        reroll = False
+    lser = lmbx.get_series(revision=wantver, sloppytrailers=cmdargs.sloppytrailers, reroll=reroll)
     if lser is None and wantver is None:
         logger.critical('No patches found.')
         return
@@ -156,6 +159,11 @@ def mbox_to_am(mboxfile, cmdargs, msgid):
                 logger.info('Prepared a fake commit range for 3-way merge (%.12s..%.12s)', rstart, rend)
 
     logger.critical('---')
+    if lser.partial_reroll:
+        logger.critical('WARNING: v%s is a partial reroll from previous revisions', lser.revision)
+        logger.critical('         Please carefully review the resulting series to ensure correctness')
+        logger.critical('         Pass --no-partial-reroll to disable')
+        logger.critical('---')
     if not lser.complete and not cmdargs.cherrypick:
         logger.critical('WARNING: Thread incomplete!')