[PATCH b4 v2] ez: add subject prefix flag

[PATCH b4 v2] ez: add subject prefix flag

[Andreas Hindborg]

Add a flag `-p PREFIX`/`--add-subject-prefix=PREFIX` to `am` and `shazam`
commands. This flag will add PREFIX to the subject line of each patch before
applying the patch.

One use case for this is when composing trees that consist of patches from many
different sources. Adding a prefix to the subject of each series makes it easy
to see the origin when scanning the commit log.

Another side effect is that git will not remove the `PREFIX: [PATCH vX x/y]`
prefix from the patch when applying it. This might similarly be desirable in
some situations.

Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
Changes in v2:
- Add prefix to dependencies when using `b4 shazam`
- Link to v1: https://patch.msgid.link/20241220-prefix-v1-1-026a7be0b35a@kernel.org
---
 src/b4/__init__.py | 14 ++++++++------
 src/b4/command.py  |  2 ++
 src/b4/mbox.py     |  4 ++--
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/src/b4/__init__.py b/src/b4/__init__.py
index 4d234e01d9932fc600103bf62c1cb109b1eac201..083c81d96c5ab73e6f5eaaf194d23ee10b71363e 100644
--- a/src/b4/__init__.py
+++ b/src/b4/__init__.py
@@ -618,7 +618,7 @@ class LoreSeries:
 
     def get_am_ready(self, noaddtrailers: bool = False, addmysob: bool = False,
                      addlink: bool = False, cherrypick: Optional[List[int]] = None, copyccs: bool = False,
-                     allowbadchars: bool = False, showchecks: bool = False) -> List[email.message.EmailMessage]:
+                     allowbadchars: bool = False, showchecks: bool = False, addprefix: Optional[str] = None) -> List[email.message.EmailMessage]:
 
         usercfg = get_user_config()
         config = get_main_config()
@@ -752,7 +752,7 @@ class LoreSeries:
                 if noaddtrailers:
                     add_trailers = False
                 msg = lmsg.get_am_message(add_trailers=add_trailers, extras=extras, copyccs=copyccs,
-                                          addmysob=addmysob, allowbadchars=allowbadchars)
+                                          addmysob=addmysob, allowbadchars=allowbadchars, addprefix=addprefix)
                 if local_check_cmds:
                     lmsg.load_local_ci_status(local_check_cmds)
                 if lmsg.local_ci_status or lmsg.pw_ci_status in {'success', 'fail', 'warning'}:
@@ -2333,7 +2333,7 @@ class LoreMessage:
 
         self.body = LoreMessage.rebuild_message(bheaders, message, fixtrailers, basement, signature)
 
-    def get_am_subject(self, indicate_reroll: bool = True, use_subject: Optional[str] = None) -> str:
+    def get_am_subject(self, indicate_reroll: bool = True, use_subject: Optional[str] = None, prefix: Optional[str] = None) -> str:
         # Return a clean patch subject
         parts = ['PATCH']
         if self.lsubject.rfc:
@@ -2354,11 +2354,13 @@ class LoreMessage:
         if not use_subject:
             use_subject = self.lsubject.subject
 
-        return '[%s] %s' % (' '.join(parts), use_subject)
+        prefix = "" if prefix is None else prefix
+
+        return '%s: [%s] %s' % (prefix, ' '.join(parts), use_subject)
 
     def get_am_message(self, add_trailers: bool = True, addmysob: bool = False,
                        extras: Optional[List['LoreTrailer']] = None, copyccs: bool = False,
-                       allowbadchars: bool = False) -> email.message.EmailMessage:
+                       allowbadchars: bool = False, addprefix: Optional[str] = None) -> email.message.EmailMessage:
         # Look through the body to make sure there aren't any suspicious unicode control flow chars
         # First, encode into ascii and compare for a quick utf8 presence test
         if not allowbadchars and self.body.encode('ascii', errors='replace') != self.body.encode():
@@ -2403,7 +2405,7 @@ class LoreMessage:
 
         am_msg = email.message.EmailMessage()
         hfrom = format_addrs([(i.get('Author', ''), i.get('Email'))])
-        am_msg.add_header('Subject', self.get_am_subject(indicate_reroll=False, use_subject=i.get('Subject')))
+        am_msg.add_header('Subject', self.get_am_subject(indicate_reroll=False, use_subject=i.get('Subject'), prefix=addprefix))
         am_msg.add_header('From', hfrom)
         am_msg.add_header('Date', i.get('Date'))
         am_msg.add_header('Message-Id', f'<{self.msgid}>')
diff --git a/src/b4/command.py b/src/b4/command.py
index 2bdd785d90004bc0be9c839fe50938dc9cbbe207..910e6461679e6a11af69ad2d6a908b28a78a32ee 100644
--- a/src/b4/command.py
+++ b/src/b4/command.py
@@ -61,6 +61,8 @@ def cmd_am_common_opts(sp):
                     help='Break thread at the msgid specified and ignore any parent messages')
     sp.add_argument('--allow-unicode-control-chars', dest='allowbadchars', action='store_true', default=False,
                     help='Allow unicode control characters (very rarely legitimate)')
+    sp.add_argument('-p', '--add-subject-prefix', dest='addprefix', default=None,
+                      help='Add a PREFIX: to every patch subject line')
     sa_g = sp.add_mutually_exclusive_group()
     sa_g.add_argument('-l', '--add-link', dest='addlink', action='store_true', default=False,
                       help='Add a Link: trailer with message-id lookup URL to every patch')
diff --git a/src/b4/mbox.py b/src/b4/mbox.py
index 7061d1493be3e0d116ec504c9a1638be34b65a9a..171da7d8d445052babc335b7f11e9c60e3f518f7 100644
--- a/src/b4/mbox.py
+++ b/src/b4/mbox.py
@@ -159,7 +159,7 @@ def make_am(msgs: List[email.message.EmailMessage], cmdargs: argparse.Namespace,
 
     am_msgs = lser.get_am_ready(noaddtrailers=cmdargs.noaddtrailers, addmysob=cmdargs.addmysob, addlink=cmdargs.addlink,
                                 cherrypick=cherrypick, copyccs=cmdargs.copyccs, allowbadchars=cmdargs.allowbadchars,
-                                showchecks=cmdargs.check)
+                                showchecks=cmdargs.check, addprefix=cmdargs.addprefix)
     logger.info('---')
 
     if cherrypick is None:
@@ -325,7 +325,7 @@ def make_am(msgs: List[email.message.EmailMessage], cmdargs: argparse.Namespace,
             for ppid in lser.prereq_patch_ids:
                 if ppid in pmap:
                     logger.info(' Deps: Applying prerequisite patch: %s', pmap[ppid].full_subject)
-                    pam_msg = pmap[ppid].get_am_message(add_trailers=False)
+                    pam_msg = pmap[ppid].get_am_message(add_trailers=False, addprefix=cmdargs.addprefix )
                     b4.save_mboxrd_mbox([pam_msg], ifh)
 
         b4.save_git_am_mbox(am_msgs, ifh)

---
base-commit: d23a9a2d5684369710bd2e416e41159193df3756
change-id: 20241220-prefix-87d15717e4a4

Best regards,
-- 
Andreas Hindborg <a.hindborg@kernel.org>

Re: [PATCH b4 v2] ez: add subject prefix flag

[Konstantin Ryabitsev]

On Tue, Feb 04, 2025 at 04:30:45PM +0100, Andreas Hindborg wrote:
> Add a flag `-p PREFIX`/`--add-subject-prefix=PREFIX` to `am` and `shazam`
> commands. This flag will add PREFIX to the subject line of each patch before
> applying the patch.
> 
> One use case for this is when composing trees that consist of patches from many
> different sources. Adding a prefix to the subject of each series makes it easy
> to see the origin when scanning the commit log.

I get the reason why you would want this, but I also kinda hate the idea,
because I'm almost 100% sure that this will make it into someone's -next tree
or something like that.

Can we take a step back and try to look at it from a broader perspective? Do
you use this approach during code review, or for some other purpose?

-K

Re: [PATCH b4 v2] ez: add subject prefix flag

[Andreas Hindborg]

"Konstantin Ryabitsev" <konstantin@linuxfoundation.org> writes:

> On Tue, Feb 04, 2025 at 04:30:45PM +0100, Andreas Hindborg wrote:
>> Add a flag `-p PREFIX`/`--add-subject-prefix=PREFIX` to `am` and `shazam`
>> commands. This flag will add PREFIX to the subject line of each patch before
>> applying the patch.
>>
>> One use case for this is when composing trees that consist of patches from many
>> different sources. Adding a prefix to the subject of each series makes it easy
>> to see the origin when scanning the commit log.
>
> I get the reason why you would want this, but I also kinda hate the idea,
> because I'm almost 100% sure that this will make it into someone's -next tree
> or something like that.
>
> Can we take a step back and try to look at it from a broader perspective? Do
> you use this approach during code review, or for some other purpose?

I use it when assembling trees for upstream work. For instance see [1].
It is super valuable for me to know what commits are from list when
scanning down the git log.

For a series prepared with b4 it looks something like this:

f3d69aaf6b6c1 * configfs MAINTAINERS: add entry for configfs Rust abstractions
b813426cf1e74 * rust: configfs: introduce rust support for configfs
d2a88d97552a9 * rust: sync: change `<Arc<T> as ForeignOwnable>::PointedTo` to `T`
935d4487ff8d5 * rust: configfs abstractions
0a2d72888abc3 * LIST: [PATCH v15 1/3] rust: types: add `ForeignOwnable::PointedTo`
988bd1ba2ebb3 * LIST: [PATCH 3/3] kbuild: rust: support `W=e` for Rust
3b39f5625406b * LIST: [PATCH 2/3] kbuild: rust: apply `CONFIG_WERROR` to all Rust targets
89cfa7f9c1099 * LIST: [PATCH 1/3] kbuild: rust: move `-Dwarnings` handling to `Makefile.extrawarn`
2014c95afecee * v6.14-rc1 torvalds/master local/torvalds/master Linux 6.14-rc1

Here commit HEAD~3 is a b4 cover letter.

I'm not married to this approach, it was just the only one I could
think of. I really like that the patches I apply with `b4 shazam ..`
appear with `LIST: [PATCH vX X/Y] ..`.


Best regards,
Andreas Hindborg


[1] https://git.kernel.org/pub/scm/linux/kernel/git/a.hindborg/linux.git/log/?h=rnull-v6.14-rc1