threads / discuss / 58366

forcing an in-body From header

Subject: forcing an in-body From header

## tl;dr

5 messages between Aug 26, 2022 and Aug 29, 2022.

replies: 4people: 3as markdown or json

Rasmus Villemoes· Aug 26, 2022, 13:17 UTC · lore
Hi,

Some mailing lists mangle the real From: header, making it a little hard for maintainers to apply patches directly using 'git am'. See e.g. https://lists.openembedded.org/g/openembedded-core/message/166515 . One way to work around that is by having an in-body From: "header".

However, merely setting sendemail.from or format.from is not enough to get such a header, if the value happens to be identical to the patch author (which it would usually be). So, could we get some config knob and/or command line switch to force an in-body from header? Then one could set that on a per-project basis, for projects with such mailing lists.

I looked into the code, and while this is obviously just a hacky patch to see that I found the right spot, it doesn't seem to be too hard to implement properly.

diff --git a/pretty.c b/pretty.c
index ee6114e3f0..8b9ef6f644 100644
--- a/pretty.c
+++ b/pretty.c
@@ -503,7 +503,7 @@ void pp_user_info(struct pretty_print_context *pp,
                map_user(pp->mailmap, &mailbuf, &maillen, &namebuf,
&namelen);

        if (cmit_fmt_is_mail(pp->fmt)) {
-               if (pp->from_ident && ident_cmp(pp->from_ident, &ident)) {
+               if (pp->from_ident && (ident_cmp(pp->from_ident, &ident)
|| getenv("GIT_FORCE_BODY_FROM"))) {
                        struct strbuf buf = STRBUF_INIT;

                        strbuf_addstr(&buf, "From: ");

Thanks,
Rasmus
Konstantin Ryabitsev· Aug 26, 2022, 16:48 UTC · re: Rasmus Villemoes · lore

Re: forcing an in-body From header

On Fri, Aug 26, 2022 at 03:17:34PM +0200, Rasmus Villemoes wrote:
Show 6 quoted lines
> Hi,
> 
> Some mailing lists mangle the real From: header, making it a little hard
> for maintainers to apply patches directly using 'git am'. See e.g.
> https://lists.openembedded.org/g/openembedded-core/message/166515 . One
> way to work around that is by having an in-body From: "header".

This is only tangentially related to the problem at hand, but the list in question is also mirrored on lore.kernel.org:

https://lore.kernel.org/openembedded-core/20220531151052.3667079-1-sean.anderson@seco.com/

The mirroring is done with a direct push hook from groups.io, which is why we get messages without all the groups.io mangling.

Show 5 quoted lines
> However, merely setting sendemail.from or format.from is not enough to
> get such a header, if the value happens to be identical to the patch
> author (which it would usually be). So, could we get some config knob
> and/or command line switch to force an in-body from header? Then one
> could set that on a per-project basis, for projects with such mailing lists.

I agree that it's a nice feature to have, though I would put this into the sendemail config instead of using an env variable, something like:

    [sendemail]
      in-body-headers = From
-K
Junio C Hamano· Aug 26, 2022, 17:20 UTC · re: Konstantin Ryabitsev · lore

Re: forcing an in-body From header

Konstantin Ryabitsev <konstantin@linuxfoundation.org> writes:
Show 5 quoted lines
> I agree that it's a nice feature to have, though I would put this into the
> sendemail config instead of using an env variable, something like:
>
>     [sendemail]
>       in-body-headers = From
A change only for send-email does sound quite attractive.

I encourage folks to run format-patch first, make amends as needed in the output files, proofread the result once again, all before finally handing it to send-email for sending out. If a "force in-body headers" command line option plus a configuration variable added to "git send-email" would work for them, I would be OK with such a change.

There may be folks who do not use "git send-email" to send out their patches, and a change to "git format-patch" may help them, though.

Thanks.
Rasmus Villemoes· Aug 29, 2022, 11:56 UTC · re: Junio C Hamano · lore

Re: forcing an in-body From header

On 26/08/2022 19.20, Junio C Hamano wrote:
Show 13 quoted lines
> Konstantin Ryabitsev <konstantin@linuxfoundation.org> writes:
> 
>> I agree that it's a nice feature to have, though I would put this into the
>> sendemail config instead of using an env variable, something like:
>>
>>     [sendemail]
>>       in-body-headers = From
> 
> A change only for send-email does sound quite attractive.
> 
> I encourage folks to run format-patch first, make amends as needed
> in the output files, proofread the result once again, all before
> finally handing it to send-email for sending out. 
That's at least my usual workflow, yes.
 If a "force
> in-body headers" command line option plus a configuration variable
> added to "git send-email" would work for them, I would be OK with
> such a change.

It would work for me, except that if I am to rely on git send-email to munge the body of the file(s) I pass to it, I'd really like a way for --dry-run to reassure me that that will actually happen. Currently that only prints the headers, which is quite useful to check that the To and Cc are as expected (especially when one has some to-cmd or cc-cmd configured).

> There may be folks who do not use "git send-email" to send out their
> patches, and a change to "git format-patch" may help them, though.

Maybe, yes. I also stumbled on this paragraph in git format-patch --help which would probably need some adjustment

           Note that this option is only useful if you are actually
sending the
           emails and want to identify yourself as the sender, but
retain the
           original author (and git am will correctly pick up the in-body
           header). Note also that git send-email already handles this
           transformation for you, and this option should not be used if
you are
           feeding the result to git send-email.

I don't know how git send-email behaves if there already is an in-body From, or if it is different than the one that send-email was about to add itself.

Somehow I got the cover letter of the mini-series, but neither of the two actual patches. I can't answer the big-picture question of whether this belongs in format-patch, send-email or both, but:

> * Should it be "inbody" or "in-body"?
in-body, I think.
> * Should it have a corresponding configuration variable?

Most definitely yes, it's something I'd want to set for certain projects in the local .git/config file, and not a command line option I want to have to remember when doing the occasional contribution for those projects.

Thanks for the quick responses and initial draft work on this.
Rasmus
Junio C Hamano· Aug 26, 2022, 17:11 UTC · re: Rasmus Villemoes · lore

Re: forcing an in-body From header

Rasmus Villemoes <rv@rasmusvillemoes.dk> writes:
> I looked into the code, and while this is obviously just a hacky patch
> to see that I found the right spot, it doesn't seem to be too hard to
> implement properly.

I agree with you on all points in the above sentence ;-) I do not think anybody minds if a command line option to "git format-patch" added in builtin/log.c::cmd_format_patch() and a new configuration variable taught to builtin/log.c::git_format_config() that gives the default value for the new "force_in_body_from" member that will be added to "struct rev_info" and relayed to "pretty_print_context" when log-tree.c::fmt_output_commit() calls format_commit_messages().

Or something like that.

← back to recent threads