[PATCH v2] format-patch: fix From header in cover letter

[PATCH v2] format-patch: fix From header in cover letter

[Mirko Faina]

"git format-patch" takes "--from=<user ident>" command line option and
uses the given ident for patch e-mails, but this is not applied to the
cover letter, the option is ignored and the committer ident of the
current user is used.

Teach the make_cover_letter() function to honor the option, instead of
always using the current committer identity.

Signed-off-by: Mirko Faina <mroik@delayed.space>
---
I'm sorry about the poor quality of the previous commit message, I
realise it did a poor job at explaining what the patch did.

I've applied the changes you suggested and wrote the test.
Thank you for the review.

 builtin/log.c           |  6 +++---
 t/t4014-format-patch.sh | 12 ++++++++++++
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index d43ca693bf..42648dda54 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1332,7 +1332,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
 			      int quiet,
 			      const struct format_config *cfg)
 {
-	const char *committer;
+	const char *from;
 	struct shortlog log;
 	struct strbuf sb = STRBUF_INIT;
 	int i;
@@ -1345,7 +1345,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
 	if (!cmit_fmt_is_mail(rev->commit_format))
 		die(_("cover letter needs email format"));
 
-	committer = git_committer_info(0);
+	from = cfg->from ? cfg->from : git_committer_info(0);
 
 	if (use_separate_file &&
 	    open_next_file(NULL, rev->numbered_files ? NULL : "cover-letter", rev, quiet))
@@ -1368,7 +1368,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
 	pp.date_mode.type = DATE_RFC2822;
 	pp.rev = rev;
 	pp.encode_email_headers = rev->encode_email_headers;
-	pp_user_info(&pp, NULL, &sb, committer, encoding);
+	pp_user_info(&pp, NULL, &sb, from, encoding);
 	prepare_cover_text(&pp, description_file, branch_name, &sb,
 			   encoding, need_8bit_cte, cfg);
 	fprintf(rev->diffopt.file, "%s\n", sb.buf);
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 21d6d0cd9e..cb04a9c47b 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1472,6 +1472,18 @@ test_expect_success '--from uses committer ident' '
 	test_cmp expect patch.head
 '
 
+test_expect_success '--from applies to cover letter' '
+	echo "This is a test text" >file_to_commit &&
+	git format-patch -1 --stdout --cover-letter --from="Foo Bar <author@example.com>" >patch &&
+	cat >expect <<-\EOF &&
+	From: Foo Bar <author@example.com>
+	From: Foo Bar <author@example.com>
+	From: A U Thor <author@example.com>
+	EOF
+	sed -ne "/^From:/p; /^[[:space:]]$/d" patch >patch.head &&
+	test_cmp expect patch.head
+'
+
 test_expect_success '--from omits redundant in-body header' '
 	git format-patch -1 --stdout --from="A U Thor <author@example.com>" >patch &&
 	cat >expect <<-\EOF &&
-- 
2.53.0

Re: [PATCH v2] format-patch: fix From header in cover letter

[Patrick Steinhardt]

On Mon, Feb 16, 2026 at 04:27:30PM +0100, Mirko Faina wrote:
> "git format-patch" takes "--from=<user ident>" command line option and
> uses the given ident for patch e-mails, but this is not applied to the
> cover letter, the option is ignored and the committer ident of the
> current user is used.

I think it makes sense to apply this identity to the cover letter, as
well. After all, I would say that the whole intent is to allow the user
to send those mails from their own mail address, and of course that
would also require us to send the cover letter from that address.

One thing that made me stop though is the folowing sentence in
git-format-patch(1):

  Use ident in the From: header of each commit email.

The option explicitly mentions that we use "--from" for the commit
emails, only, and that may be read as implying that it's not used for
the cover letter.

I don't really know whether that wording is intentional, and I cannot
come up with a good reason why it should be. But I'd say that the
wording is something we should adjust.

> diff --git a/builtin/log.c b/builtin/log.c
> index d43ca693bf..42648dda54 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -1332,7 +1332,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
>  			      int quiet,
>  			      const struct format_config *cfg)
>  {
> -	const char *committer;
> +	const char *from;
>  	struct shortlog log;
>  	struct strbuf sb = STRBUF_INIT;
>  	int i;
> @@ -1345,7 +1345,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
>  	if (!cmit_fmt_is_mail(rev->commit_format))
>  		die(_("cover letter needs email format"));
>  
> -	committer = git_committer_info(0);
> +	from = cfg->from ? cfg->from : git_committer_info(0);
>  
>  	if (use_separate_file &&
>  	    open_next_file(NULL, rev->numbered_files ? NULL : "cover-letter", rev, quiet))
> @@ -1368,7 +1368,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
>  	pp.date_mode.type = DATE_RFC2822;
>  	pp.rev = rev;
>  	pp.encode_email_headers = rev->encode_email_headers;
> -	pp_user_info(&pp, NULL, &sb, committer, encoding);
> +	pp_user_info(&pp, NULL, &sb, from, encoding);
>  	prepare_cover_text(&pp, description_file, branch_name, &sb,
>  			   encoding, need_8bit_cte, cfg);
>  	fprintf(rev->diffopt.file, "%s\n", sb.buf);

The changes here look straight forward.

> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
> index 21d6d0cd9e..cb04a9c47b 100755
> --- a/t/t4014-format-patch.sh
> +++ b/t/t4014-format-patch.sh
> @@ -1472,6 +1472,18 @@ test_expect_success '--from uses committer ident' '
>  	test_cmp expect patch.head
>  '
>  
> +test_expect_success '--from applies to cover letter' '
> +	echo "This is a test text" >file_to_commit &&

This file isn't used.

> +	git format-patch -1 --stdout --cover-letter --from="Foo Bar <author@example.com>" >patch &&
> +	cat >expect <<-\EOF &&
> +	From: Foo Bar <author@example.com>
> +	From: Foo Bar <author@example.com>
> +	From: A U Thor <author@example.com>
> +	EOF
> +	sed -ne "/^From:/p; /^[[:space:]]$/d" patch >patch.head &&
> +	test_cmp expect patch.head
> +'

We're not only testing the cover letter here though, but also the other
generated patch. This makes it somewhat hard to verify that the test
actually works as expected. Would it make sense to maybe use something
like the following instead?

  test_expect_success '--from applies to cover letter' '
  	test_when_finished "rm -rf patches" &&
  	git format-patch -1 --cover-letter --from="Foo Bar <author@example.com>" -o patches &&
  	echo "From: Foo Bar <author@example.com>" >expect &&
  	grep "^From:" patches/0000-cover-letter.patch >patch.head &&
  	test_cmp expect patch.head
  '

Thanks!

Patrick

Re: [PATCH v2] format-patch: fix From header in cover letter

[Jeff King]

On Tue, Feb 17, 2026 at 07:22:08AM +0100, Patrick Steinhardt wrote:

> One thing that made me stop though is the folowing sentence in
> git-format-patch(1):
> 
>   Use ident in the From: header of each commit email.
> 
> The option explicitly mentions that we use "--from" for the commit
> emails, only, and that may be read as implying that it's not used for
> the cover letter.
> 
> I don't really know whether that wording is intentional, and I cannot
> come up with a good reason why it should be. But I'd say that the
> wording is something we should adjust.

I think that wording comes from me back in 2013, and I just never gave
any thought to cover-letter generation by format-patch itself. I never
use it (and I probably forgot it even existed, given that most people
would use send-email's cover letter generation, and I do my own thing
with mutt).

So I think the direction of the patch is good, and I agree it would make
sense to clarify the documentation.

-Peff

Re: [PATCH v2] format-patch: fix From header in cover letter

[D. Ben Knoble]

On Tue, Feb 17, 2026 at 1:34 AM Jeff King <peff@peff.net> wrote:
>
> On Tue, Feb 17, 2026 at 07:22:08AM +0100, Patrick Steinhardt wrote:
>
> > One thing that made me stop though is the folowing sentence in
> > git-format-patch(1):
> >
> >   Use ident in the From: header of each commit email.
> >
> > The option explicitly mentions that we use "--from" for the commit
> > emails, only, and that may be read as implying that it's not used for
> > the cover letter.
> >
> > I don't really know whether that wording is intentional, and I cannot
> > come up with a good reason why it should be. But I'd say that the
> > wording is something we should adjust.
>
> I think that wording comes from me back in 2013, and I just never gave
> any thought to cover-letter generation by format-patch itself. I never
> use it (and I probably forgot it even existed, given that most people
> would use send-email's cover letter generation, and I do my own thing
> with mutt).

Funny: the send-email manual's main mention of cover letters apart
from setting Cc/To fields is in an example…

           $ git format-patch --cover-letter -M origin/master -o outgoing/
           $ edit outgoing/0000-*
           $ git send-email outgoing/*

…using format-patch! (Which is how I generate cover letters, and
probably part of why.)

-- 
D. Ben Knoble

Re: [PATCH v2] format-patch: fix From header in cover letter

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I think that wording comes from me back in 2013, and I just never gave
> any thought to cover-letter generation by format-patch itself.

Figures.  If "--from" was done by somebody who did not know or care
about the cover letter, it is totally understandable omission the
patch addresses.  The log message may want to refer to a9080475
(teach format-patch to place other authors into in-body "From",
2013-07-03).

> I never
> use it (and I probably forgot it even existed, given that most people
> would use send-email's cover letter generation, and I do my own thing
> with mutt).

I use it myself and I really hate the way it lists the patches.  We
should have done "log --oneline --reverse" instead of "shortlog", as
it is hard to understand the reference the cover letter message
makes to individual patches like "the first two patches do X", etc.

> So I think the direction of the patch is good, and I agree it would make
> sense to clarify the documentation.

Yes.  It does make sense to document the change in thinking in the
proposed log message and in documentation.

Thanks.

Re: [PATCH v2] format-patch: fix From header in cover letter

[Mirko Faina]

On Tue, Feb 17, 2026 at 07:22:08AM +0100, Patrick Steinhardt wrote:
> One thing that made me stop though is the folowing sentence in
> git-format-patch(1):
> 
>   Use ident in the From: header of each commit email.
> 
> The option explicitly mentions that we use "--from" for the commit
> emails, only, and that may be read as implying that it's not used for
> the cover letter.
> 
> I don't really know whether that wording is intentional, and I cannot
> come up with a good reason why it should be. But I'd say that the
> wording is something we should adjust.

I don't think any user would reasonably think that it wouldn't apply to
the cover letter as you're sending the whole patch series in bulk, as the
same person.

I will change the documentation to remove any ambiguity.

> We're not only testing the cover letter here though, but also the other
> generated patch. This makes it somewhat hard to verify that the test
> actually works as expected. Would it make sense to maybe use something
> like the following instead?
> 
>   test_expect_success '--from applies to cover letter' '
>   	test_when_finished "rm -rf patches" &&
>   	git format-patch -1 --cover-letter --from="Foo Bar <author@example.com>" -o patches &&
>   	echo "From: Foo Bar <author@example.com>" >expect &&
>   	grep "^From:" patches/0000-cover-letter.patch >patch.head &&
>   	test_cmp expect patch.head
>   '

Yes, makes sense, wouldn't want to generate a false negative on the
compliance of the "--from" option.
I will apply the suggested change.

Thank you

Re: [PATCH v2] format-patch: fix From header in cover letter

[Mirko Faina]

On Tue, Feb 17, 2026 at 07:22:33AM -0800, Junio C Hamano wrote:
> Yes.  It does make sense to document the change in thinking in the
> proposed log message and in documentation.

As I said to Patrick, I don't think anyone would reasonably expect
"--from" to not apply to the cover letter as well. Given this, I'm not
sure we should refer to this as a change in thinking in the commit
message, it should be treated as a bug as if it should've always been
this way.

I will edit the documentation to remove any ambiguity.

Thank you

Re: [PATCH v2] format-patch: fix From header in cover letter

[Jeff King]

On Tue, Feb 17, 2026 at 08:21:32AM -0500, D. Ben Knoble wrote:

> > I think that wording comes from me back in 2013, and I just never gave
> > any thought to cover-letter generation by format-patch itself. I never
> > use it (and I probably forgot it even existed, given that most people
> > would use send-email's cover letter generation, and I do my own thing
> > with mutt).
> 
> Funny: the send-email manual's main mention of cover letters apart
> from setting Cc/To fields is in an example…
> 
>            $ git format-patch --cover-letter -M origin/master -o outgoing/
>            $ edit outgoing/0000-*
>            $ git send-email outgoing/*
> 
> …using format-patch! (Which is how I generate cover letters, and
> probably part of why.)

I think I may be showing my clueless-ness about send-email here. I
thought it had its own cover-letter code, but looking at it briefly, I
may just be wrong. I don't see any code, so I guess people generate the
cover-letter independently with format-patch.

Maybe nobody noticed because most people do not use "--from=<foo>" with
a <foo> that does not match the committer ident in the first place. I'm
not really sure why you'd want the two to differ. Which makes me wonder
why I added that feature in the first place (as opposed to just "--from"
with no options).

Mysteries of the cosmos, I guess.

-Peff

Re: [PATCH v2] format-patch: fix From header in cover letter

[Jeff King]

On Tue, Feb 17, 2026 at 07:22:33AM -0800, Junio C Hamano wrote:

> > I never
> > use it (and I probably forgot it even existed, given that most people
> > would use send-email's cover letter generation, and I do my own thing
> > with mutt).
> 
> I use it myself and I really hate the way it lists the patches.  We
> should have done "log --oneline --reverse" instead of "shortlog", as
> it is hard to understand the reference the cover letter message
> makes to individual patches like "the first two patches do X", etc.

Agreed. I have long hated the shortlog version. According to the commit
history of my personal scripts, I've been sending with:

   [1/3]: subject of the first patch
   [2/3]: the second patch
   [3/3]: and so on

since at least 2009, and nobody has complained. ;)

Sadly I don't think there is an easy way to do so with "log --format",
as it can't just be a single output placeholder (you have to do the
whole traversal to get the "/3" part, so the caller needs to know this
and queue up the output).

I do it locally by piping the format-patch output through something
like:

  sed -ne 's/^Subject: //p' |
  sed -e 's/\[PATCH /[/' \
      -e 's/]/]:/' \
      -e 's/^/  /'

Of course that does not handle rfc822 header continuations, which we
started to do at some point for long subjects. So now the first part is
some gnarly perl. :-/

Probably some combination of "git log --format=%s", "nl", "wc -l" would
be less horrid. I stuck with format-patch because of its somewhat
magical command-line parsing of revisions. E.g., saying "git
format-patch @{upstream}" will give you the patches on the topic branch,
rather than all history down to the roots.

But anyway, if this were happening internally in format-patch's
cover-letter code, it should be quite easy to do (we have the complete
list of patches and their subjects there already).

-Peff

Re: [PATCH v2] format-patch: fix From header in cover letter

[Mirko Faina]

> Maybe nobody noticed because most people do not use "--from=<foo>" with
> a <foo> that does not match the committer ident in the first place. I'm
> not really sure why you'd want the two to differ. Which makes me wonder
> why I added that feature in the first place (as opposed to just "--from"
> with no options).

I usually commit everything under the name Mroik. Having the "--from"
option is useful to send a one off contribution, otherwise I'd have to
change my committer ident everytime I decide to use a different name (I
know most people just use their full name so it's not common to have to
use "--from").

Mirko

Re: [PATCH v2] format-patch: fix From header in cover letter

[D. Ben Knoble]

On Thu, Feb 19, 2026 at 6:29 AM Jeff King <peff@peff.net> wrote:
>
> On Tue, Feb 17, 2026 at 08:21:32AM -0500, D. Ben Knoble wrote:
>
> > > I think that wording comes from me back in 2013, and I just never gave
> > > any thought to cover-letter generation by format-patch itself. I never
> > > use it (and I probably forgot it even existed, given that most people
> > > would use send-email's cover letter generation, and I do my own thing
> > > with mutt).
> >
> > Funny: the send-email manual's main mention of cover letters apart
> > from setting Cc/To fields is in an example…
> >
> >            $ git format-patch --cover-letter -M origin/master -o outgoing/
> >            $ edit outgoing/0000-*
> >            $ git send-email outgoing/*
> >
> > …using format-patch! (Which is how I generate cover letters, and
> > probably part of why.)
>
> I think I may be showing my clueless-ness about send-email here. I
> thought it had its own cover-letter code, but looking at it briefly, I
> may just be wrong. I don't see any code, so I guess people generate the
> cover-letter independently with format-patch.

Yeah, maybe—send-email takes format-patch options, so send-email
--cover--letter should also work?

> Maybe nobody noticed because most people do not use "--from=<foo>" with
> a <foo> that does not match the committer ident in the first place. I'm
> not really sure why you'd want the two to differ. Which makes me wonder
> why I added that feature in the first place (as opposed to just "--from"
> with no options).
>
> Mysteries of the cosmos, I guess.
>
> -Peff

:shrug: :)

-- 
D. Ben Knoble

Re: [PATCH v2] format-patch: fix From header in cover letter

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

>> I use it myself and I really hate the way it lists the patches.  We
>> should have done "log --oneline --reverse" instead of "shortlog", as
>> it is hard to understand the reference the cover letter message
>> makes to individual patches like "the first two patches do X", etc.
>
> Agreed. I have long hated the shortlog version. According to the commit
> history of my personal scripts, I've been sending with:
>
>    [1/3]: subject of the first patch
>    [2/3]: the second patch
>    [3/3]: and so on
>
> since at least 2009, and nobody has complained. ;)
> ...
> But anyway, if this were happening internally in format-patch's
> cover-letter code, it should be quite easy to do (we have the complete
> list of patches and their subjects there already).

A good bite-sized #leftoverbits project for somebody relatively new
to the codebase, perhaps.

Thanks.