From a73711ad65e0f5d9ad155713fea2e609cd7fc188 Mon Sep 17 00:00:00 2001 From: Konstantin Ryabitsev Date: Fri, 22 May 2020 17:55:05 -0400 Subject: 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 --- b4/__init__.py | 14 ++++----- b4/command.py | 4 +++ b4/diff.py | 90 ++++++++++++++++++++++++++++++++++++++++------------------ b4/mbox.py | 20 ++++++------- 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 -- cgit v1.2.3