diff mbox series

shazam: Add the --add-message-id argument

Message ID 20230216164841.3356-1-palmer@rivosinc.com (mailing list archive)
State Rejected
Headers show
Series shazam: Add the --add-message-id argument | expand

Commit Message

Palmer Dabbelt Feb. 16, 2023, 4:48 p.m. UTC
Some projects (at least QEMU) want Message-ID headers instead of Link
headers.  I couldn't find an easy way to do that, so this just adds an
argument to shazam that mirrors "--add-link" but adds a MessageID
instead.

Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
---
Using a LoreTrailer for something that's not on Lore seems odd, but it
appears to work so I just went with it.
---
 b4/__init__.py | 6 +++++-
 b4/command.py  | 2 ++
 b4/mbox.py     | 2 +-
 man/b4.5.rst   | 2 ++
 4 files changed, 10 insertions(+), 2 deletions(-)

Comments

Konstantin Ryabitsev Feb. 27, 2023, 6:48 p.m. UTC | #1
On Thu, Feb 16, 2023 at 08:48:41AM -0800, Palmer Dabbelt wrote:
> Some projects (at least QEMU) want Message-ID headers instead of Link
> headers.  I couldn't find an easy way to do that, so this just adds an
> argument to shazam that mirrors "--add-link" but adds a MessageID
> instead.

I'd like to go a slightly different way about it. We already have a
"b4.linkmask" parameter, so I suggest we also add "b4.linktrailer", such as
you can specify:

    [b4]
        linktrailer = Message-ID
        linkmask = <%s>

This would additionally allow someone to differentiate Link trailers that
point at code-review links vs. any other links.

Does that sound like a potential alternative to creating another runtime flag?

-K
Palmer Dabbelt Feb. 27, 2023, 7 p.m. UTC | #2
On Mon, 27 Feb 2023 10:48:55 PST (-0800), konstantin@linuxfoundation.org wrote:
> On Thu, Feb 16, 2023 at 08:48:41AM -0800, Palmer Dabbelt wrote:
>> Some projects (at least QEMU) want Message-ID headers instead of Link
>> headers.  I couldn't find an easy way to do that, so this just adds an
>> argument to shazam that mirrors "--add-link" but adds a MessageID
>> instead.
>
> I'd like to go a slightly different way about it. We already have a
> "b4.linkmask" parameter, so I suggest we also add "b4.linktrailer", such as
> you can specify:
>
>     [b4]
>         linktrailer = Message-ID
>         linkmask = <%s>
>
> This would additionally allow someone to differentiate Link trailers that
> point at code-review links vs. any other links.
>
> Does that sound like a potential alternative to creating another runtime flag?

IIUC I can put that in each repo's config?  In that case it's actually 
better, as I don't need to remember to type "-l" in Linux and "-I" in 
QEMU ;)
Konstantin Ryabitsev Feb. 27, 2023, 7:03 p.m. UTC | #3
On Mon, Feb 27, 2023 at 11:00:03AM -0800, Palmer Dabbelt wrote:
> > I'd like to go a slightly different way about it. We already have a
> > "b4.linkmask" parameter, so I suggest we also add "b4.linktrailer", such as
> > you can specify:
> > 
> >     [b4]
> >         linktrailer = Message-ID
> >         linkmask = <%s>
> > 
> > This would additionally allow someone to differentiate Link trailers that
> > point at code-review links vs. any other links.
> > 
> > Does that sound like a potential alternative to creating another runtime flag?
> 
> IIUC I can put that in each repo's config?  In that case it's actually
> better, as I don't need to remember to type "-l" in Linux and "-I" in QEMU
> ;)

Yes, and you can also add it to wtglobs in get_main_config() in init.py, which
should allow setting this in .b4-config for projects that want to add that to
their toplevel.

Do you feel like doing a v2 with this change?

-K
Palmer Dabbelt Feb. 27, 2023, 7:05 p.m. UTC | #4
On Mon, 27 Feb 2023 11:03:48 PST (-0800), konstantin@linuxfoundation.org wrote:
> On Mon, Feb 27, 2023 at 11:00:03AM -0800, Palmer Dabbelt wrote:
>> > I'd like to go a slightly different way about it. We already have a
>> > "b4.linkmask" parameter, so I suggest we also add "b4.linktrailer", such as
>> > you can specify:
>> >
>> >     [b4]
>> >         linktrailer = Message-ID
>> >         linkmask = <%s>
>> >
>> > This would additionally allow someone to differentiate Link trailers that
>> > point at code-review links vs. any other links.
>> >
>> > Does that sound like a potential alternative to creating another runtime flag?
>>
>> IIUC I can put that in each repo's config?  In that case it's actually
>> better, as I don't need to remember to type "-l" in Linux and "-I" in QEMU
>> ;)
>
> Yes, and you can also add it to wtglobs in get_main_config() in init.py, which
> should allow setting this in .b4-config for projects that want to add that to
> their toplevel.
>
> Do you feel like doing a v2 with this change?

Sure, but if someone else wants to take a whack then I'll be more than 
happy as it'll likely happen after the Linux merge window, QEMU soft 
freeze, and GCC branch...
Konstantin Ryabitsev Feb. 27, 2023, 8:09 p.m. UTC | #5
On Mon, Feb 27, 2023 at 11:05:19AM -0800, Palmer Dabbelt wrote:
> > Yes, and you can also add it to wtglobs in get_main_config() in init.py, which
> > should allow setting this in .b4-config for projects that want to add that to
> > their toplevel.
> > 
> > Do you feel like doing a v2 with this change?
> 
> Sure, but if someone else wants to take a whack then I'll be more than happy
> as it'll likely happen after the Linux merge window, QEMU soft freeze, and
> GCC branch...

I can make this change -- should be straightforward. Thanks for the
suggestion.

-K
Palmer Dabbelt Feb. 27, 2023, 8:26 p.m. UTC | #6
On Mon, 27 Feb 2023 12:09:30 PST (-0800), konstantin@linuxfoundation.org wrote:
> On Mon, Feb 27, 2023 at 11:05:19AM -0800, Palmer Dabbelt wrote:
>> > Yes, and you can also add it to wtglobs in get_main_config() in init.py, which
>> > should allow setting this in .b4-config for projects that want to add that to
>> > their toplevel.
>> >
>> > Do you feel like doing a v2 with this change?
>>
>> Sure, but if someone else wants to take a whack then I'll be more than happy
>> as it'll likely happen after the Linux merge window, QEMU soft freeze, and
>> GCC branch...
>
> I can make this change -- should be straightforward. Thanks for the
> suggestion.

Awesome, thanks.  No rush on my end, I've still got a few other patches 
in the stack so I'll be running my local copy for a bit either way.
Konstantin Ryabitsev Feb. 27, 2023, 8:47 p.m. UTC | #7
On Mon, Feb 27, 2023 at 12:26:04PM -0800, Palmer Dabbelt wrote:
> > > Sure, but if someone else wants to take a whack then I'll be more than happy
> > > as it'll likely happen after the Linux merge window, QEMU soft freeze, and
> > > GCC branch...
> > 
> > I can make this change -- should be straightforward. Thanks for the
> > suggestion.
> 
> Awesome, thanks.  No rush on my end, I've still got a few other patches in
> the stack so I'll be running my local copy for a bit either way.

It's in master if you want to try it out. I ended up doing it slightly
differently, since we use linkmask in a couple of places, so you'll need to
set the following config variable in your QEMU repo:

[b4]
   linktrailermask = Message-ID: <%s>

-K
diff mbox series

Patch

diff --git a/b4/__init__.py b/b4/__init__.py
index 4d5a6c9..5e5a5a3 100644
--- a/b4/__init__.py
+++ b/b4/__init__.py
@@ -519,7 +519,7 @@  class LoreSeries:
             self.add_extra_trailers(self.patches[0].followup_trailers)  # noqa
 
     def get_am_ready(self, noaddtrailers=False, covertrailers=False, addmysob=False, addlink=False,
-                     linkmask=None, cherrypick=None, copyccs=False, allowbadchars=False) -> List[email.message.Message]:
+                     linkmask=None, cherrypick=None, copyccs=False, allowbadchars=False, addmessageid=False) -> List[email.message.Message]:
 
         usercfg = get_user_config()
         config = get_main_config()
@@ -585,6 +585,10 @@  class LoreSeries:
                     lltr = LoreTrailer(name='Link', value=linkval)
                     extras.append(lltr)
 
+                if addmessageid:
+                    msgid = "<%s>" % (lmsg.msgid)
+                    extras.append(LoreTrailer(name='Message-ID', value=msgid))
+
                 if attsame and not attcrit:
                     if attmark:
                         logger.info('  %s %s', attmark, lmsg.get_am_subject())
diff --git a/b4/command.py b/b4/command.py
index e384031..7d73aa5 100644
--- a/b4/command.py
+++ b/b4/command.py
@@ -51,6 +51,8 @@  def cmd_am_common_opts(sp):
                     help='Add your own signed-off-by to every patch')
     sp.add_argument('-l', '--add-link', dest='addlink', action='store_true', default=False,
                     help='Add a Link: with message-id lookup URL to every patch')
+    sp.add_argument('-I', '--add-message-id', dest='addmessageid', action='store_true', default=False,
+                    help='Add a Message-ID: with message-id to every patch')
     sp.add_argument('-P', '--cherry-pick', dest='cherrypick', default=None,
                     help='Cherry-pick a subset of patches (e.g. "-P 1-2,4,6-", '
                          '"-P _" to use just the msgid specified, or '
diff --git a/b4/mbox.py b/b4/mbox.py
index 48b037a..0e988a8 100644
--- a/b4/mbox.py
+++ b/b4/mbox.py
@@ -101,7 +101,7 @@  def make_am(msgs: List[email.message.Message], cmdargs: argparse.Namespace, msgi
         am_msgs = lser.get_am_ready(noaddtrailers=cmdargs.noaddtrailers,
                                     covertrailers=covertrailers, addmysob=cmdargs.addmysob,
                                     addlink=cmdargs.addlink, linkmask=config['linkmask'], cherrypick=cherrypick,
-                                    copyccs=cmdargs.copyccs, allowbadchars=cmdargs.allowbadchars)
+                                    copyccs=cmdargs.copyccs, allowbadchars=cmdargs.allowbadchars, addmessageid=cmdargs.addmessageid)
     except KeyError:
         sys.exit(1)
 
diff --git a/man/b4.5.rst b/man/b4.5.rst
index 90db4d7..07d2692 100644
--- a/man/b4.5.rst
+++ b/man/b4.5.rst
@@ -155,6 +155,8 @@  options:
                         Add your own signed-off-by to every patch
   -l, --add-link
                         Add a Link: with message-id lookup URL to every patch
+  -I, --add-message-id
+                        Add a Message-ID: with message-id to every patch
   -P CHERRYPICK, --cherry-pick CHERRYPICK
                         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)
   --cc-trailers