diff mbox series

shazam: Add the --merge-base argument

Message ID 20221013175727.17139-1-palmer@rivosinc.com (mailing list archive)
State Superseded
Headers show
Series shazam: Add the --merge-base argument | expand

Commit Message

Palmer Dabbelt Oct. 13, 2022, 5:57 p.m. UTC
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/
Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
---
 b4/command.py |  2 ++
 b4/mbox.py    | 13 ++++++++-----
 man/b4.5.rst  |  2 ++
 3 files changed, 12 insertions(+), 5 deletions(-)

Comments

Conor Dooley Jan. 20, 2023, 10:34 p.m. UTC | #1
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>

Thanks,
Conor.

> Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
> ---
>  b4/command.py |  2 ++
>  b4/mbox.py    | 13 ++++++++-----
>  man/b4.5.rst  |  2 ++
>  3 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/b4/command.py b/b4/command.py
> index b16043e..5bb674e 100644
> --- a/b4/command.py
> +++ b/b4/command.py
> @@ -171,6 +171,8 @@ def cmd():
>      sp_sh.add_argument('--guess-lookback', dest='guessdays', type=int, default=21,
>                         help=('(use with -H or -M) When guessing base, go back this many days from the patch date '
>                               '(default: 3 weeks)'))
> +    sp_sh.add_argument('--merge-base', dest='mergebase', type=str, default=None,
> +                       help=('(use with -H or -M) Force this base when merging'))
>      sp_sh.set_defaults(func=cmd_shazam)
>  
>      # b4 pr
> diff --git a/b4/mbox.py b/b4/mbox.py
> index ff96a11..c1a0660 100644
> --- a/b4/mbox.py
> +++ b/b4/mbox.py
> @@ -231,14 +231,17 @@ def make_am(msgs, cmdargs, msgid):
>          logger.critical(' Link: %s', linkurl)
>  
>      base_commit = None
> -    matches = re.search(r'base-commit: .*?([\da-f]+)', first_body, re.MULTILINE)
> -    if matches:
> -        base_commit = matches.groups()[0]
> +    if cmdargs.mergebase is not None:
> +        base_commit = cmdargs.mergebase
>      else:
> -        # Try a more relaxed search
> -        matches = re.search(r'based on .*?([\da-f]{40})', first_body, re.MULTILINE)
> +        matches = re.search(r'base-commit: .*?([\da-f]+)', 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 not base_commit and topdir and cmdargs.guessbase:
>          logger.critical(' Base: attempting to guess base-commit...')
> diff --git a/man/b4.5.rst b/man/b4.5.rst
> index 073f32a..e889542 100644
> --- a/man/b4.5.rst
> +++ b/man/b4.5.rst
> @@ -226,6 +226,8 @@ options:
>                          Attempt to merge series as if it were a pull request (execs git-merge)
>    --guess-lookback GUESSDAYS
>                          (use with -H or -M) When guessing base, go back this many days from the patch date (default: 3 weeks)
> +  --merge-base COMMIT
> +                        (use with -H or -M) Force this base when merging
>  
>  *Example*: b4 shazam -H 20200313231252.64999-1-keescook@chromium.org
>  
> -- 
> 2.38.0
>
Conor Dooley Jan. 21, 2023, 5:18 p.m. UTC | #2
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.

> > Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
> > ---
> >  b4/command.py |  2 ++
> >  b4/mbox.py    | 13 ++++++++-----
> >  man/b4.5.rst  |  2 ++
> >  3 files changed, 12 insertions(+), 5 deletions(-)
> > 
> > diff --git a/b4/command.py b/b4/command.py
> > index b16043e..5bb674e 100644
> > --- a/b4/command.py
> > +++ b/b4/command.py
> > @@ -171,6 +171,8 @@ def cmd():
> >      sp_sh.add_argument('--guess-lookback', dest='guessdays', type=int, default=21,
> >                         help=('(use with -H or -M) When guessing base, go back this many days from the patch date '
> >                               '(default: 3 weeks)'))
> > +    sp_sh.add_argument('--merge-base', dest='mergebase', type=str, default=None,
> > +                       help=('(use with -H or -M) Force this base when merging'))
> >      sp_sh.set_defaults(func=cmd_shazam)
> >  
> >      # b4 pr
> > diff --git a/b4/mbox.py b/b4/mbox.py
> > index ff96a11..c1a0660 100644
> > --- a/b4/mbox.py
> > +++ b/b4/mbox.py
> > @@ -231,14 +231,17 @@ def make_am(msgs, cmdargs, msgid):
> >          logger.critical(' Link: %s', linkurl)
> >  
> >      base_commit = None
> > -    matches = re.search(r'base-commit: .*?([\da-f]+)', first_body, re.MULTILINE)
> > -    if matches:
> > -        base_commit = matches.groups()[0]
> > +    if cmdargs.mergebase is not None:
> > +        base_commit = cmdargs.mergebase
> >      else:
> > -        # Try a more relaxed search
> > -        matches = re.search(r'based on .*?([\da-f]{40})', first_body, re.MULTILINE)
> > +        matches = re.search(r'base-commit: .*?([\da-f]+)', 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 not base_commit and topdir and cmdargs.guessbase:
> >          logger.critical(' Base: attempting to guess base-commit...')
> > diff --git a/man/b4.5.rst b/man/b4.5.rst
> > index 073f32a..e889542 100644
> > --- a/man/b4.5.rst
> > +++ b/man/b4.5.rst
> > @@ -226,6 +226,8 @@ options:
> >                          Attempt to merge series as if it were a pull request (execs git-merge)
> >    --guess-lookback GUESSDAYS
> >                          (use with -H or -M) When guessing base, go back this many days from the patch date (default: 3 weeks)
> > +  --merge-base COMMIT
> > +                        (use with -H or -M) Force this base when merging
> >  
> >  *Example*: b4 shazam -H 20200313231252.64999-1-keescook@chromium.org
> >  
> > -- 
> > 2.38.0
> >
Conor Dooley Jan. 22, 2023, 6:18 p.m. UTC | #3
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
diff mbox series

Patch

diff --git a/b4/command.py b/b4/command.py
index b16043e..5bb674e 100644
--- a/b4/command.py
+++ b/b4/command.py
@@ -171,6 +171,8 @@  def cmd():
     sp_sh.add_argument('--guess-lookback', dest='guessdays', type=int, default=21,
                        help=('(use with -H or -M) When guessing base, go back this many days from the patch date '
                              '(default: 3 weeks)'))
+    sp_sh.add_argument('--merge-base', dest='mergebase', type=str, default=None,
+                       help=('(use with -H or -M) Force this base when merging'))
     sp_sh.set_defaults(func=cmd_shazam)
 
     # b4 pr
diff --git a/b4/mbox.py b/b4/mbox.py
index ff96a11..c1a0660 100644
--- a/b4/mbox.py
+++ b/b4/mbox.py
@@ -231,14 +231,17 @@  def make_am(msgs, cmdargs, msgid):
         logger.critical(' Link: %s', linkurl)
 
     base_commit = None
-    matches = re.search(r'base-commit: .*?([\da-f]+)', first_body, re.MULTILINE)
-    if matches:
-        base_commit = matches.groups()[0]
+    if cmdargs.mergebase is not None:
+        base_commit = cmdargs.mergebase
     else:
-        # Try a more relaxed search
-        matches = re.search(r'based on .*?([\da-f]{40})', first_body, re.MULTILINE)
+        matches = re.search(r'base-commit: .*?([\da-f]+)', 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 not base_commit and topdir and cmdargs.guessbase:
         logger.critical(' Base: attempting to guess base-commit...')
diff --git a/man/b4.5.rst b/man/b4.5.rst
index 073f32a..e889542 100644
--- a/man/b4.5.rst
+++ b/man/b4.5.rst
@@ -226,6 +226,8 @@  options:
                         Attempt to merge series as if it were a pull request (execs git-merge)
   --guess-lookback GUESSDAYS
                         (use with -H or -M) When guessing base, go back this many days from the patch date (default: 3 weeks)
+  --merge-base COMMIT
+                        (use with -H or -M) Force this base when merging
 
 *Example*: b4 shazam -H 20200313231252.64999-1-keescook@chromium.org