| Message ID | 20230330110909.355701-1-conor.dooley@microchip.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | shazam: warn when overriding base-commit with --merge-base | expand |
On Thu, 30 Mar 2023 04:09:10 PDT (-0700), Conor Dooley wrote: > If the --merge-base argument is provided, base-commit information that > was provided by the submitter is ignored. This behaviour is intentional, > but warning the user that this has happened (and what the intended base > was) is helpful. > > To be able to do this, base-commit can't be set from mergebase until > after parsing the submission and attempting to guess it. > > Suggested-by: Palmer Dabbelt <palmer@rivosinc.com> Thanks! > Signed-off-by: Conor Dooley <conor.dooley@microchip.com> > --- > This should be a valid test: > b4 shazam -H --merge-base v6.3-rc1 20230330064321.1008373-2-jeeheng.sia@starfivetech.com > > I wasn't 100% sure if it should go before or after the base commit > guessing code. > --- > b4/mbox.py | 22 +++++++++++++--------- > 1 file changed, 13 insertions(+), 9 deletions(-) > > diff --git a/b4/mbox.py b/b4/mbox.py > index 7060564..a983092 100644 > --- a/b4/mbox.py > +++ b/b4/mbox.py > @@ -211,24 +211,22 @@ def make_am(msgs: List[email.message.Message], cmdargs: argparse.Namespace, msgi > logger.critical(' Link: %s', linkurl) > > base_commit = None > - if cmdargs.mergebase: > - base_commit = cmdargs.mergebase > + > + matches = re.search(r'base-commit: .*?([\da-f]+)', first_body, re.MULTILINE) > + if matches: > + base_commit = matches.groups()[0] > else: > - matches = re.search(r'base-commit: .*?([\da-f]+)', first_body, re.MULTILINE) > + # Try a more relaxed search > + matches = re.search(r'based on .*?([\da-f]{40})', first_body, re.MULTILINE) > if matches: > base_commit = matches.groups()[0] > - else: > - # Try a more relaxed search > - matches = re.search(r'based on .*?([\da-f]{40})', first_body, re.MULTILINE) > - if matches: > - base_commit = matches.groups()[0] > > if base_commit and topdir: > # Does it actually exist in this tree? > if not b4.git_commit_exists(topdir, base_commit): > logger.info(' Base: base-commit %s not known, ignoring', base_commit) > base_commit = None > - else: > + elif not cmdargs.mergebase: > logger.info(' Base: using specified base-commit %s', base_commit) > > if not base_commit and topdir and cmdargs.guessbase: > @@ -246,6 +244,12 @@ def make_am(msgs: List[email.message.Message], cmdargs: argparse.Namespace, msgi > except IndexError: > logger.critical(' Base: failed to guess base') > > + if cmdargs.mergebase: > + if (base_commit): > + logger.warn(' Base: overriding submitter provided base-commit %s', base_commit) > + base_commit = cmdargs.mergebase > + logger.info(' Base: using CLI provided base-commit %s', base_commit) > + > if cmdargs.subcmd == 'shazam': > if not topdir: > logger.critical('Could not figure out where your git dir is, cannot shazam.')
On Thu, 30 Mar 2023 12:09:10 +0100, Conor Dooley wrote: > If the --merge-base argument is provided, base-commit information that > was provided by the submitter is ignored. This behaviour is intentional, > but warning the user that this has happened (and what the intended base > was) is helpful. > > To be able to do this, base-commit can't be set from mergebase until > after parsing the submission and attempting to guess it. > > [...] Applied, thanks! [1/1] shazam: warn when overriding base-commit with --merge-base commit: 91592bf5d05f312225bf4f49d8bcc9508041bee2 Best regards,
diff --git a/b4/mbox.py b/b4/mbox.py index 7060564..a983092 100644 --- a/b4/mbox.py +++ b/b4/mbox.py @@ -211,24 +211,22 @@ def make_am(msgs: List[email.message.Message], cmdargs: argparse.Namespace, msgi logger.critical(' Link: %s', linkurl) base_commit = None - if cmdargs.mergebase: - base_commit = cmdargs.mergebase + + matches = re.search(r'base-commit: .*?([\da-f]+)', first_body, re.MULTILINE) + if matches: + base_commit = matches.groups()[0] else: - matches = re.search(r'base-commit: .*?([\da-f]+)', first_body, re.MULTILINE) + # Try a more relaxed search + matches = re.search(r'based on .*?([\da-f]{40})', first_body, re.MULTILINE) if matches: base_commit = matches.groups()[0] - else: - # Try a more relaxed search - matches = re.search(r'based on .*?([\da-f]{40})', first_body, re.MULTILINE) - if matches: - base_commit = matches.groups()[0] if base_commit and topdir: # Does it actually exist in this tree? if not b4.git_commit_exists(topdir, base_commit): logger.info(' Base: base-commit %s not known, ignoring', base_commit) base_commit = None - else: + elif not cmdargs.mergebase: logger.info(' Base: using specified base-commit %s', base_commit) if not base_commit and topdir and cmdargs.guessbase: @@ -246,6 +244,12 @@ def make_am(msgs: List[email.message.Message], cmdargs: argparse.Namespace, msgi except IndexError: logger.critical(' Base: failed to guess base') + if cmdargs.mergebase: + if (base_commit): + logger.warn(' Base: overriding submitter provided base-commit %s', base_commit) + base_commit = cmdargs.mergebase + logger.info(' Base: using CLI provided base-commit %s', base_commit) + if cmdargs.subcmd == 'shazam': if not topdir: logger.critical('Could not figure out where your git dir is, cannot shazam.')
If the --merge-base argument is provided, base-commit information that was provided by the submitter is ignored. This behaviour is intentional, but warning the user that this has happened (and what the intended base was) is helpful. To be able to do this, base-commit can't be set from mergebase until after parsing the submission and attempting to guess it. Suggested-by: Palmer Dabbelt <palmer@rivosinc.com> Signed-off-by: Conor Dooley <conor.dooley@microchip.com> --- This should be a valid test: b4 shazam -H --merge-base v6.3-rc1 20230330064321.1008373-2-jeeheng.sia@starfivetech.com I wasn't 100% sure if it should go before or after the base commit guessing code. --- b4/mbox.py | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-)