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