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

[Patrick Steinhardt <ps@pks.im>]

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