diff mbox series

[v2] git_range_to_patches: actually add to/cc headers from cover letter to single-commit series

Message ID 20221114-prep-format-patch-single-patch-to-cc-v2-0-70a9019eb90c@theobroma-systems.com (mailing list archive)
State Rejected
Headers show
Series [v2] git_range_to_patches: actually add to/cc headers from cover letter to single-commit series | expand

Commit Message

Quentin Schulz Nov. 15, 2022, 5:23 p.m. UTC
From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

The intent of the original code was to copy the Cc and To headers from
the cover letter, previously added with b4 prep --auto-to-cc. However,
those are added in the commit log which means a newline is inserted
between the subject header and the start of the commit log, "breaking"
the mail parsing because the rest is assumed to be the body of the mail.

Since the To: and Cc: lines are assumed to be in-body, and therefore not
parsed by the mail backend, they cannot be fetched with mail.get('To')
since the header isn't actually set.

Instead, let's use the Cc and To fields returned by get_body_parts of
the covermsg into ctrailers list of LoreTrailer and then add the ones
that are supposed to be To or Cc headers to the single-commit patch
headers.

Fixes: 6c215d83473d ("ez: don't send a cover letter for a 1-patch series")
Fixes: c748abf6ad2a ("ez: don't send a cover letter for a 1-patch series") #v0.10.1
Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
git_range_to_patches: actually add to/cc headers from cover letter to single-commit series

This probably wasn't detected because git-send-email is capable of
adding the Cc: fields in the "snipped" section of a patch (---) even if not in
the headers but not the To: fields.

To: "Kernel.org Tools" <tools@linux.kernel.org>
Cc: Konstantin Ryabitsev <konstantin@linuxfoundation.org>
---
Changes in v2:
- fix From: field,
- remove list cast for ctrailers as unnecessary,
- Link to v1: https://msgid.link/20221114-prep-format-patch-single-patch-to-cc-v1-0-e8dddbd07096@theobroma-systems.com
---
 b4/__init__.py | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)


---
base-commit: 287477e9e0149b42e444471da9dca629ebacf2e1
change-id: 20221114-prep-format-patch-single-patch-to-cc-2a878158d50b

Best regards,

Comments

Konstantin Ryabitsev Nov. 15, 2022, 7:43 p.m. UTC | #1
On Tue, Nov 15, 2022 at 06:31:04PM +0100, Quentin Schulz wrote:
> The issue with this patchset is that it only fixes single-commit series
> (which is already a good start). However, patches generated with b4 prep -p
> patches for bigger series are broken because the Cc: and To: fields
> generated by b4 prep --auto-to-cc do not make it to individual patches, only
> to the cover letter.

This was intentional -- `b4 prep -p` generates minimal headers (i.e. operates
in git-format-patch compatible mode). If you want to generate the series as
ready to sent out, you should use `b4 send -o`, which will populate To/Cc
fields (among other headers).

> I could suggest to *always* add the To: and Cc: fields from the cover letter
> into each individual patch headers but I know some contributors/maintainers
> like to have a patch series with not all patches sent to all people impacted
> by at least one patch.

I did a (totally unscientific) polling and the results were that almost all
maintainers preferred to receive the entire patch series and never just
individual patches, which is why b4 operates this way. The To/Cc destinations
are only tracked in the cover letter and when "b4 send" is invoked, we
populate *all* messages with the same set of addresses to make sure that
everyone gets the entire series.

> I could see what I can do for the workflow where all Cc: and To: fields from
> the mailing lists are added as headers to each individual patch in a series
> if there's some interest for it?

Are you sure this isn't already done by `b4 send -o`?

-K
Quentin Schulz Nov. 16, 2022, 10:45 a.m. UTC | #2
Hi Konstantin,

On 11/15/22 20:43, Konstantin Ryabitsev wrote:
> On Tue, Nov 15, 2022 at 06:31:04PM +0100, Quentin Schulz wrote:
>> The issue with this patchset is that it only fixes single-commit series
>> (which is already a good start). However, patches generated with b4 prep -p
>> patches for bigger series are broken because the Cc: and To: fields
>> generated by b4 prep --auto-to-cc do not make it to individual patches, only
>> to the cover letter.
> 
> This was intentional -- `b4 prep -p` generates minimal headers (i.e. operates
> in git-format-patch compatible mode). If you want to generate the series as
> ready to sent out, you should use `b4 send -o`, which will populate To/Cc
> fields (among other headers).
> 

Ahah! RTFM :)

>> I could suggest to *always* add the To: and Cc: fields from the cover letter
>> into each individual patch headers but I know some contributors/maintainers
>> like to have a patch series with not all patches sent to all people impacted
>> by at least one patch.
> 
> I did a (totally unscientific) polling and the results were that almost all
> maintainers preferred to receive the entire patch series and never just
> individual patches, which is why b4 operates this way. The To/Cc destinations
> are only tracked in the cover letter and when "b4 send" is invoked, we
> populate *all* messages with the same set of addresses to make sure that
> everyone gets the entire series.
>  >> I could see what I can do for the workflow where all Cc: and To: 
fields from
>> the mailing lists are added as headers to each individual patch in a series
>> if there's some interest for it?
> 
> Are you sure this isn't already done by `b4 send -o`?
> 

It is. I think I got confused by the name (I don't want to use b4 for 
sending mails right now) and didn't even look into it. This works fine 
for me, thanks for the pointers.

Sorry for the noise, please carry on :)

Cheers,
Quentin
diff mbox series

Patch

diff --git a/b4/__init__.py b/b4/__init__.py
index f5bbc53..590c756 100644
--- a/b4/__init__.py
+++ b/b4/__init__.py
@@ -2779,13 +2779,11 @@  def git_range_to_patches(gitdir: Optional[str], start: str, end: str,
 
             pbody = LoreMessage.rebuild_message(pheaders, pmessage, ptrailers, newbasement, csignature)
             pmsg.set_payload(pbody, charset='utf-8')
+
             # Add any To: and Cc: headers from the cover_message
-            toh = covermsg.get('To')
-            if toh:
-                pmsg.add_header('To', toh)
-            cch = covermsg.get('Cc')
-            if cch:
-                pmsg.add_header('Cc', cch)
+            for ctrailer in ctrailers:
+                if ctrailer.name in ('To', 'Cc'):
+                    pmsg.add_header(ctrailer.name, ctrailer.value)
             startfrom = 0
         else:
             patches.insert(0, (None, covermsg))