Re: [PATCH] shazam: Add the --merge-base argument

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3440 bytes --]

On Sat, Jan 21, 2023 at 05:19:02PM +0000, Conor Dooley wrote:
> On Fri, Jan 20, 2023 at 10:34:01PM +0000, Conor Dooley wrote:
> > Hey Konstantin,
> > 
> > On Thu, Oct 13, 2022 at 10:57:27AM -0700, Palmer Dabbelt wrote:
> > > I was just handling a patch set where the author used English to
> > > describe the dependencies.  They hadn't yet been merged at the time the
> > > patch set was posted so I don't think there's really any way to make
> > > sure the computers always understand the base, this just lets me quickly
> > > override the automatic merge base detection when I run into something
> > > non-canonical.
> > > 
> > > Link: https://lore.kernel.org/all/20220913061817.22564-1-zong.li@sifive.com/
> > 
> > I guess this just got lost somewhere (missing a CC?), or perhaps we've
> > missed some reason why this is not needed?
> > 
> > Doesn't apply cleanly against 0.12.0, but with -3 it does & appears to
> > work - at least in my simple test cases
> > For example, the following gets automagically assigned -rc4 as a base:
> > 
> > /stuff/b4/b4.sh shazam -s -t shazam 20230119094447.21939-3-walker.chen@starfivetech.com -H
> > 
> > But I don't want to merge -rc4 into my for-next branch, and being able
> > to explicitly pass my preferred base of -rc1 is really useful.
> > 
> > Tested-by: Conor Dooley <conor.dooley@microchip.com>
> 
> Heh, the functionality itself is tested - but I get an error doing:
> b4 am -3 20230116074259.22874-4-walker.chen@starfivetech.com
> 
> Traceback (most recent call last):
>   File "/stuff/b4/b4/command.py", line 379, in <module>
>     cmd()
>   File "/stuff/b4/b4/command.py", line 362, in cmd
>     cmdargs.func(cmdargs)
>   File "/stuff/b4/b4/command.py", line 91, in cmd_am
>     b4.mbox.main(cmdargs)
>   File "/stuff/b4/b4/mbox.py", line 710, in main
>     make_am(msgs, cmdargs, msgid)
>   File "/stuff/b4/b4/mbox.py", line 214, in make_am
>     if cmdargs.mergebase is not None:
> AttributeError: 'Namespace' object has no attribute 'mergebase'
> 
> I may go fix that up, since the feature itself is useful to me.

Perhaps it is fixed up by simply doing what is done to "protect" shazam
from am specific options. I'm not a pythonist, but I did try each of am,
and shazam with & without the flag. They all seem to work with this diff
applied:
diff --git a/b4/mbox.py b/b4/mbox.py
index ce04062..0b0fc40 100644
--- a/b4/mbox.py
+++ b/b4/mbox.py
@@ -211,7 +211,7 @@ def make_am(msgs: List[email.message.Message], cmdargs: argparse.Namespace, msgi
         logger.critical(' Link: %s', linkurl)
 
     base_commit = None
-    if cmdargs.mergebase is not None:
+    if cmdargs.mergebase:
         base_commit = cmdargs.mergebase
     else:
         matches = re.search(r'base-commit: .*?([\da-f]+)', first_body, re.MULTILINE)
@@ -673,8 +673,8 @@ def refetch(dest: str) -> None:
 
 
 def main(cmdargs: argparse.Namespace) -> None:
+    # We force some settings
     if cmdargs.subcmd == 'shazam':
-        # We force some settings
         cmdargs.checknewer = True
         cmdargs.threeway = False
         cmdargs.nopartialreroll = False
@@ -686,6 +686,8 @@ def main(cmdargs: argparse.Namespace) -> None:
             cmdargs.guessbase = True
         else:
             cmdargs.guessbase = False
+    else:
+        cmdargs.mergebase = False
 
     if cmdargs.checknewer:
         # Force nocache mode


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]