summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKonstantin Ryabitsev <konstantin@linuxfoundation.org>2021-05-17 12:04:09 -0400
committerKonstantin Ryabitsev <konstantin@linuxfoundation.org>2021-05-17 12:04:09 -0400
commitf6f46cd221e35bf80d3b7c24ec4b4d799446399f (patch)
treec773531336afa9a9e49b8d0f0c5c03a41f38bbf3
parent34f5c6886f3e02558101a19cffb479c480ef646e (diff)
downloadb4-f6f46cd221e35bf80d3b7c24ec4b4d799446399f.tar.gz
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
-rw-r--r--b4/__init__.py143
-rw-r--r--b4/command.py4
-rw-r--r--b4/diff.py8
-rw-r--r--b4/mbox.py10
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!')