[PATCH 0/2] send-email: add --header-cmd option

[PATCH 0/2] send-email: add --header-cmd option

[Maxim Cournoyer]

Hi,

This adds a new --header-cmd option to the send-email command, joining
in the ranks of '--cc-cmd' and '--to-cmd'.  The header-cmd script
provided as argument should output 'header: value' lines, such as:

  X-Debbugs-Cc: someone@example.com
  AnotherHeader: somevalue
  [...]

This change was motivated by the use case of easing collaboration
using Debbugs in GNU Guix [0].

[0]  https://issues.guix.gnu.org/58813

Thanks,

Maxim Cournoyer (2):
  send-email: extract execute_cmd from recipients_cmd
  send-email: add --header-cmd option

 Documentation/config/sendemail.txt |  1 +
 Documentation/git-send-email.txt   |  5 ++++
 git-send-email.perl                | 38 +++++++++++++++++++++++-------
 t/t9001-send-email.sh              | 21 +++++++++++++++--
 4 files changed, 54 insertions(+), 11 deletions(-)


base-commit: 7580f92ffa970b9484ac214f7b53cec5e26ca4bc
-- 
2.39.2

[PATCH 1/2] send-email: extract execute_cmd from recipients_cmd

[Maxim Cournoyer]

This refactor is to pave the way for the addition of the new
'--header-cmd' option to the send-email command.
---
 git-send-email.perl | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index fd8cd0d46f..d2febbda1f 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -2,6 +2,7 @@
 #
 # Copyright 2002,2005 Greg Kroah-Hartman <greg@kroah.com>
 # Copyright 2005 Ryan Anderson <ryan@michonline.com>
+# Copyright 2023 Maxim Cournoyer <maxim.cournoyer@gmail.com>
 #
 # GPL v2 (See COPYING)
 #
@@ -2006,15 +2007,30 @@ sub process_file {
 	}
 }
 
+# Execute a command (e.g., $x_cmd) and return its output lines as an
+# array.
+sub execute_cmd {
+	my ($prefix, $cmd, $file) = @_;
+	my @lines = ();
+	open my $fh, "-|", "$cmd \Q$file\E"
+		or die sprintf(__("(%s) Could not execute '%s'"), $prefix, $cmd);
+	while (my $line = <$fh>) {
+		last if $line =~ /^$/;
+		push @lines, $line;
+	}
+	close $fh
+	    or die sprintf(__("(%s) failed to close pipe to '%s'"), $prefix, $cmd);
+	return @lines;
+}
+
 # Execute a command (e.g. $to_cmd) to get a list of email addresses
 # and return a results array
 sub recipients_cmd {
 	my ($prefix, $what, $cmd, $file) = @_;
-
+	my @lines = ();
 	my @addresses = ();
-	open my $fh, "-|", "$cmd \Q$file\E"
-	    or die sprintf(__("(%s) Could not execute '%s'"), $prefix, $cmd);
-	while (my $address = <$fh>) {
+	@lines = execute_cmd($prefix, $cmd, $file);
+	for my $address (@lines) {
 		$address =~ s/^\s*//g;
 		$address =~ s/\s*$//g;
 		$address = sanitize_address($address);
@@ -2023,8 +2039,6 @@ sub recipients_cmd {
 		printf(__("(%s) Adding %s: %s from: '%s'\n"),
 		       $prefix, $what, $address, $cmd) unless $quiet;
 		}
-	close $fh
-	    or die sprintf(__("(%s) failed to close pipe to '%s'"), $prefix, $cmd);
 	return @addresses;
 }
 
-- 
2.39.2

[PATCH 2/2] send-email: add --header-cmd option

[Maxim Cournoyer]

Sometimes, adding a header different than CC or TO is desirable; for
example, when using Debbugs, it is best to use 'X-Debbugs-Cc' headers
to keep people in CC; this is an example use case enabled by the new
'--header-cmd' option.
---
 Documentation/config/sendemail.txt |  1 +
 Documentation/git-send-email.txt   |  5 +++++
 git-send-email.perl                | 12 +++++++++---
 t/t9001-send-email.sh              | 21 +++++++++++++++++++--
 4 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/Documentation/config/sendemail.txt b/Documentation/config/sendemail.txt
index 51da7088a8..3d0f516520 100644
--- a/Documentation/config/sendemail.txt
+++ b/Documentation/config/sendemail.txt
@@ -58,6 +58,7 @@ sendemail.annotate::
 sendemail.bcc::
 sendemail.cc::
 sendemail.ccCmd::
+sendemail.headerCmd::
 sendemail.chainReplyTo::
 sendemail.envelopeSender::
 sendemail.from::
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index b0f438ec99..354c0d06db 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -320,6 +320,11 @@ Automating
 	Output of this command must be single email address per line.
 	Default is the value of `sendemail.ccCmd` configuration value.
 
+--header-cmd=<command>::
+	Specify a command to execute once per patch file which should
+	generate arbitrary, patch file specific header entries.
+	Default is the value of `sendemail.headerCmd` configuration value.
+
 --[no-]chain-reply-to::
 	If this is set, each email will be sent as a reply to the previous
 	email sent.  If disabled with "--no-chain-reply-to", all emails after
diff --git a/git-send-email.perl b/git-send-email.perl
index d2febbda1f..676dd83d89 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -88,8 +88,9 @@ sub usage {
 
   Automating:
     --identity              <str>  * Use the sendemail.<id> options.
-    --to-cmd                <str>  * Email To: via `<str> \$patch_path`
-    --cc-cmd                <str>  * Email Cc: via `<str> \$patch_path`
+    --to-cmd                <str>  * Email To: via `<str> \$patch_path`.
+    --cc-cmd                <str>  * Email Cc: via `<str> \$patch_path`.
+    --header-cmd            <str>  * Add headers via `<str> \$patch_path`.
     --suppress-cc           <str>  * author, self, sob, cc, cccmd, body, bodycc, misc-by, all.
     --[no-]cc-cover                * Email Cc: addresses in the cover letter.
     --[no-]to-cover                * Email To: addresses in the cover letter.
@@ -270,7 +271,7 @@ sub do_edit {
 # Variables with corresponding config settings
 my ($suppress_from, $signed_off_by_cc);
 my ($cover_cc, $cover_to);
-my ($to_cmd, $cc_cmd);
+my ($to_cmd, $cc_cmd, $header_cmd);
 my ($smtp_server, $smtp_server_port, @smtp_server_options);
 my ($smtp_authuser, $smtp_encryption, $smtp_ssl_cert_path);
 my ($batch_size, $relogin_delay);
@@ -319,6 +320,7 @@ sub do_edit {
     "tocmd" => \$to_cmd,
     "cc" => \@config_cc,
     "cccmd" => \$cc_cmd,
+    "headercmd" => \$header_cmd,
     "aliasfiletype" => \$aliasfiletype,
     "bcc" => \@config_bcc,
     "suppresscc" => \@suppress_cc,
@@ -520,6 +522,7 @@ sub config_regexp {
 		    "compose" => \$compose,
 		    "quiet" => \$quiet,
 		    "cc-cmd=s" => \$cc_cmd,
+		    "header-cmd=s" => \$header_cmd,
 		    "suppress-from!" => \$suppress_from,
 		    "no-suppress-from" => sub {$suppress_from = 0},
 		    "suppress-cc=s" => \@suppress_cc,
@@ -1777,6 +1780,9 @@ sub process_file {
 			push(@header, $_);
 		}
 	}
+	# Add computed headers, if applicable.
+	push @header, execute_cmd("header-cmd", $header_cmd, $t)
+		if defined $header_cmd;
 	# Now parse the header
 	foreach(@header) {
 		if (/^From /) {
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 0de83b5d2b..3393725107 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -374,13 +374,16 @@ test_expect_success $PREREQ,!AUTOIDENT 'broken implicit ident aborts send-email'
 	)
 '
 
-test_expect_success $PREREQ 'setup tocmd and cccmd scripts' '
+test_expect_success $PREREQ 'setup cmd scripts' '
 	write_script tocmd-sed <<-\EOF &&
 	sed -n -e "s/^tocmd--//p" "$1"
 	EOF
-	write_script cccmd-sed <<-\EOF
+	write_script cccmd-sed <<-\EOF &&
 	sed -n -e "s/^cccmd--//p" "$1"
 	EOF
+	write_script headercmd-sed <<-\EOF
+	sed -n -e "s/^headercmd--//p" "$1"
+	EOF
 '
 
 test_expect_success $PREREQ 'tocmd works' '
@@ -410,6 +413,20 @@ test_expect_success $PREREQ 'cccmd works' '
 	grep "^	cccmd@example.com" msgtxt1
 '
 
+test_expect_success $PREREQ 'headercmd works' '
+	clean_fake_sendmail &&
+	cp $patches headercmd.patch &&
+	echo "headercmd--X-Debbugs-CC: dummy@example.com" >>headercmd.patch &&
+	git send-email \
+		--from="Example <nobody@example.com>" \
+		--to=nobody@example.com \
+		--header-cmd=./headercmd-sed \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		headercmd.patch \
+		&&
+	grep "^X-Debbugs-CC: dummy@example.com" msgtxt1
+'
+
 test_expect_success $PREREQ 'reject long lines' '
 	z8=zzzzzzzz &&
 	z64=$z8$z8$z8$z8$z8$z8$z8$z8 &&
-- 
2.39.2

Re: [PATCH 0/2] send-email: add --header-cmd option

[Junio C Hamano]

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> This adds a new --header-cmd option to the send-email command, joining
> in the ranks of '--cc-cmd' and '--to-cmd'.

It sounds more like it makes these old two unnecessary, which is a
very good thing if it is the case ;-).

Re: [PATCH 1/2] send-email: extract execute_cmd from recipients_cmd

[Junio C Hamano]

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> This refactor is to pave the way for the addition of the new
> '--header-cmd' option to the send-email command.
> ---
>  git-send-email.perl | 26 ++++++++++++++++++++------
>  1 file changed, 20 insertions(+), 6 deletions(-)

Missing sign-off?

> diff --git a/git-send-email.perl b/git-send-email.perl
> index fd8cd0d46f..d2febbda1f 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -2,6 +2,7 @@
>  #
>  # Copyright 2002,2005 Greg Kroah-Hartman <greg@kroah.com>
>  # Copyright 2005 Ryan Anderson <ryan@michonline.com>
> +# Copyright 2023 Maxim Cournoyer <maxim.cournoyer@gmail.com>
>  #
>  # GPL v2 (See COPYING)
>  #
> @@ -2006,15 +2007,30 @@ sub process_file {
>  	}
>  }
>  
> +# Execute a command (e.g., $x_cmd) and return its output lines as an
> +# array.
> +sub execute_cmd {
> +	my ($prefix, $cmd, $file) = @_;
> +	my @lines = ();
> +	open my $fh, "-|", "$cmd \Q$file\E"
> +		or die sprintf(__("(%s) Could not execute '%s'"), $prefix, $cmd);
> +	while (my $line = <$fh>) {
> +		last if $line =~ /^$/;
> +		push @lines, $line;
> +	}
> +	close $fh
> +	    or die sprintf(__("(%s) failed to close pipe to '%s'"), $prefix, $cmd);
> +	return @lines;
> +}
> +
>  # Execute a command (e.g. $to_cmd) to get a list of email addresses
>  # and return a results array
>  sub recipients_cmd {
>  	my ($prefix, $what, $cmd, $file) = @_;
> -
> +	my @lines = ();
>  	my @addresses = ();
> -	open my $fh, "-|", "$cmd \Q$file\E"
> -	    or die sprintf(__("(%s) Could not execute '%s'"), $prefix, $cmd);
> -	while (my $address = <$fh>) {
> +	@lines = execute_cmd($prefix, $cmd, $file);
> +	for my $address (@lines) {
>  		$address =~ s/^\s*//g;
>  		$address =~ s/\s*$//g;
>  		$address = sanitize_address($address);
> @@ -2023,8 +2039,6 @@ sub recipients_cmd {
>  		printf(__("(%s) Adding %s: %s from: '%s'\n"),
>  		       $prefix, $what, $address, $cmd) unless $quiet;
>  		}
> -	close $fh
> -	    or die sprintf(__("(%s) failed to close pipe to '%s'"), $prefix, $cmd);
>  	return @addresses;
>  }

Re: [PATCH 2/2] send-email: add --header-cmd option

[Junio C Hamano]

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> Sometimes, adding a header different than CC or TO is desirable; for
> example, when using Debbugs, it is best to use 'X-Debbugs-Cc' headers
> to keep people in CC; this is an example use case enabled by the new
> '--header-cmd' option.
> ---

Missing sign-off?

>  Documentation/config/sendemail.txt |  1 +
>  Documentation/git-send-email.txt   |  5 +++++
>  git-send-email.perl                | 12 +++++++++---
>  t/t9001-send-email.sh              | 21 +++++++++++++++++++--
>  4 files changed, 34 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/config/sendemail.txt b/Documentation/config/sendemail.txt
> index 51da7088a8..3d0f516520 100644
> --- a/Documentation/config/sendemail.txt
> +++ b/Documentation/config/sendemail.txt
> @@ -58,6 +58,7 @@ sendemail.annotate::
>  sendemail.bcc::
>  sendemail.cc::
>  sendemail.ccCmd::
> +sendemail.headerCmd::
>  sendemail.chainReplyTo::
>  sendemail.envelopeSender::
>  sendemail.from::

Why here?

Asking because existing other entries look sorted lexicographically.

> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
> index b0f438ec99..354c0d06db 100644
> --- a/Documentation/git-send-email.txt
> +++ b/Documentation/git-send-email.txt
> @@ -320,6 +320,11 @@ Automating
>  	Output of this command must be single email address per line.
>  	Default is the value of `sendemail.ccCmd` configuration value.
>  
> +--header-cmd=<command>::
> +	Specify a command to execute once per patch file which should
> +	generate arbitrary, patch file specific header entries.

"arbitrary, patch file specific" sounds like a problematic thing to
say here.  If it is truly arbitrary, then it is up to the user to
emit identical output for all patches and there is no reason to
inisist it has to be ptach file specific.  I am sure you meant "you
do not have to add the same set of headres with the same values for
all messages", but that is very much obvious once you said "command
to execute once per patch file".

By the way, does it apply also to the cover-letter, which is not a
patch file?  I presume it does, in which case we shouldn't be saying
"once per patch file", but something like "once per outgoing message"
or something.

Also, its output is not really arbitrary.  It has to emit RFC-2822
style header lines.  Emitting a block of lines, with an empty line
in it, would be a disaster, isn't it?  The expected output format
for the <command> this option specifies needs to be described a bit
better.

	Specify a command that is executed once per outgoing message
	and output RFC-2822 style header lines to be inserted into
	them.

or something like that?

> +	Default is the value of `sendemail.headerCmd` configuration value.

Make it clear what you mean by the Default here.  If you configure
the variable, will the command be always used without any way to
turn it off?  Or does it specify the default value to be used when
"git send-email ---header-cmd" option is used without any value?

If it is the former, there should be a way to turn it off from the
command line, probably.

> diff --git a/git-send-email.perl b/git-send-email.perl
> index d2febbda1f..676dd83d89 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -88,8 +88,9 @@ sub usage {
>  
>    Automating:
>      --identity              <str>  * Use the sendemail.<id> options.
> -    --to-cmd                <str>  * Email To: via `<str> \$patch_path`
> -    --cc-cmd                <str>  * Email Cc: via `<str> \$patch_path`
> +    --to-cmd                <str>  * Email To: via `<str> \$patch_path`.
> +    --cc-cmd                <str>  * Email Cc: via `<str> \$patch_path`.
> +    --header-cmd            <str>  * Add headers via `<str> \$patch_path`.
>      --suppress-cc           <str>  * author, self, sob, cc, cccmd, body, bodycc, misc-by, all.
>      --[no-]cc-cover                * Email Cc: addresses in the cover letter.
>      --[no-]to-cover                * Email To: addresses in the cover letter.
> @@ -270,7 +271,7 @@ sub do_edit {
>  # Variables with corresponding config settings
>  my ($suppress_from, $signed_off_by_cc);
>  my ($cover_cc, $cover_to);
> -my ($to_cmd, $cc_cmd);
> +my ($to_cmd, $cc_cmd, $header_cmd);
>  my ($smtp_server, $smtp_server_port, @smtp_server_options);
>  my ($smtp_authuser, $smtp_encryption, $smtp_ssl_cert_path);
>  my ($batch_size, $relogin_delay);
> @@ -319,6 +320,7 @@ sub do_edit {
>      "tocmd" => \$to_cmd,
>      "cc" => \@config_cc,
>      "cccmd" => \$cc_cmd,
> +    "headercmd" => \$header_cmd,
>      "aliasfiletype" => \$aliasfiletype,
>      "bcc" => \@config_bcc,
>      "suppresscc" => \@suppress_cc,
> @@ -520,6 +522,7 @@ sub config_regexp {
>  		    "compose" => \$compose,
>  		    "quiet" => \$quiet,
>  		    "cc-cmd=s" => \$cc_cmd,
> +		    "header-cmd=s" => \$header_cmd,
>  		    "suppress-from!" => \$suppress_from,
>  		    "no-suppress-from" => sub {$suppress_from = 0},
>  		    "suppress-cc=s" => \@suppress_cc,
> @@ -1777,6 +1780,9 @@ sub process_file {
>  			push(@header, $_);
>  		}
>  	}
> +	# Add computed headers, if applicable.
> +	push @header, execute_cmd("header-cmd", $header_cmd, $t)
> +		if defined $header_cmd;

While execute_cmd() may be a good enough interface to be used
without much post-processing to read cc-cmd and to-cmd output (but
notice that even there it needs post-processing), I do not think it
is a good interface to directly use to read header lines without any
postprocessing like patch [2/2] does.  Its use in recipients_cmd()
is OK primarily because it is about just reading bunch of values
placed on Cc: or To: lines.  If you are going to use it in arbitrary
sets of header lines, it is very likely that you would need to
handle header folding (see what the loop before "# Now parse the
header" is doing to preprocess <$fh>, which is not done for lines
you read into @header in [2/2]).


Thanks.

Re: [PATCH 0/2] send-email: add --header-cmd option

[Maxim Cournoyer]

Hi Junio,

Junio C Hamano <gitster@pobox.com> writes:

> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>
>> This adds a new --header-cmd option to the send-email command, joining
>> in the ranks of '--cc-cmd' and '--to-cmd'.
>
> It sounds more like it makes these old two unnecessary, which is a
> very good thing if it is the case ;-).

You are right, that --header-cmd can be considered as some kind of
generalization of the more specific --cc-cmd and --to-cmd options; I
hadn't given it much thought myself :-).

-- 
Thanks,
Maxim

Re: [PATCH 1/2] send-email: extract execute_cmd from recipients_cmd

[Maxim Cournoyer]

Hello,

And thanks for offering a review!

Junio C Hamano <gitster@pobox.com> writes:

> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>
>> This refactor is to pave the way for the addition of the new
>> '--header-cmd' option to the send-email command.
>> ---
>>  git-send-email.perl | 26 ++++++++++++++++++++------
>>  1 file changed, 20 insertions(+), 6 deletions(-)
>
> Missing sign-off?

Added locally; thanks.

-- 
Thanks,
Maxim

Re: [PATCH 2/2] send-email: add --header-cmd option

[Maxim Cournoyer]

Hi,

Junio C Hamano <gitster@pobox.com> writes:

> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>
>> Sometimes, adding a header different than CC or TO is desirable; for
>> example, when using Debbugs, it is best to use 'X-Debbugs-Cc' headers
>> to keep people in CC; this is an example use case enabled by the new
>> '--header-cmd' option.
>> ---
>
> Missing sign-off?

Added.

>>  Documentation/config/sendemail.txt |  1 +
>>  Documentation/git-send-email.txt   |  5 +++++
>>  git-send-email.perl                | 12 +++++++++---
>>  t/t9001-send-email.sh              | 21 +++++++++++++++++++--
>>  4 files changed, 34 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/config/sendemail.txt b/Documentation/config/sendemail.txt
>> index 51da7088a8..3d0f516520 100644
>> --- a/Documentation/config/sendemail.txt
>> +++ b/Documentation/config/sendemail.txt
>> @@ -58,6 +58,7 @@ sendemail.annotate::
>>  sendemail.bcc::
>>  sendemail.cc::
>>  sendemail.ccCmd::
>> +sendemail.headerCmd::
>>  sendemail.chainReplyTo::
>>  sendemail.envelopeSender::
>>  sendemail.from::
>
> Why here?
>
> Asking because existing other entries look sorted lexicographically.

Oops.  Fixed.


>> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
>> index b0f438ec99..354c0d06db 100644
>> --- a/Documentation/git-send-email.txt
>> +++ b/Documentation/git-send-email.txt
>> @@ -320,6 +320,11 @@ Automating
>>  	Output of this command must be single email address per line.
>>  	Default is the value of `sendemail.ccCmd` configuration value.
>>
>> +--header-cmd=<command>::
>> +	Specify a command to execute once per patch file which should
>> +	generate arbitrary, patch file specific header entries.
>
> "arbitrary, patch file specific" sounds like a problematic thing to
> say here.  If it is truly arbitrary, then it is up to the user to
> emit identical output for all patches and there is no reason to
> inisist it has to be ptach file specific.  I am sure you meant "you
> do not have to add the same set of headres with the same values for
> all messages", but that is very much obvious once you said "command
> to execute once per patch file".

That is indeed what I meant.

> By the way, does it apply also to the cover-letter, which is not a
> patch file?  I presume it does, in which case we shouldn't be saying
> "once per patch file", but something like "once per outgoing message"
> or something.

I think it happens for every message (the logic is in the
'process_files' procedure), so it'd also apply to the cover letter
(which is produced with the extension .patch by format-patch, although
it isn't a patch as you noted :-)).

> Also, its output is not really arbitrary.  It has to emit RFC-2822
> style header lines.  Emitting a block of lines, with an empty line
> in it, would be a disaster, isn't it?  The expected output format
> for the <command> this option specifies needs to be described a bit
> better.

I'm not too familiar with the email format; but I presume an empty line
would signal the end of a message?  That'd be bad yes, but I think it
cannot currently happen given the 'last if $line =~ /^$/;' guard at in
execute_cmd around line 2023; it'd means headers following an empty line
would be discarded though.  The expected use case is indeed for a user's
script to produce RFC 2822 style headers to messages.

> 	Specify a command that is executed once per outgoing message
> 	and output RFC-2822 style header lines to be inserted into
> 	them.
>
> or something like that?

That's much clearer, thanks.  I've reworded the text following your
suggestion.

>> +	Default is the value of `sendemail.headerCmd` configuration value.
>
> Make it clear what you mean by the Default here.  If you configure
> the variable, will the command be always used without any way to
> turn it off?  Or does it specify the default value to be used when
> "git send-email ---header-cmd" option is used without any value?
>
> If it is the former, there should be a way to turn it off from the
> command line, probably.

The former (a true default with no way to turn it off other than
redefining it), which I believe is the same behavior as for --cc-cmd or
--to-cmd.  There are no '--no-cc-cmd' or '--no-to-cmd' options, although
their result can be filtered via the '--no-cc' and '--no-to' options.
Looking in the source, options supporting '--no-' always appear to be
boolean toggles (on/off) though, so I'm not sure how a '--no-header-cmd'
that take a value can currently be implemented.  Perhaps it could be
added later if there is a need?

>> diff --git a/git-send-email.perl b/git-send-email.perl
>> index d2febbda1f..676dd83d89 100755
>> --- a/git-send-email.perl
>> +++ b/git-send-email.perl
>> @@ -88,8 +88,9 @@ sub usage {
>>
>>    Automating:
>>      --identity              <str>  * Use the sendemail.<id> options.
>> -    --to-cmd                <str>  * Email To: via `<str> \$patch_path`
>> -    --cc-cmd                <str>  * Email Cc: via `<str> \$patch_path`
>> +    --to-cmd                <str>  * Email To: via `<str> \$patch_path`.
>> +    --cc-cmd                <str>  * Email Cc: via `<str> \$patch_path`.
>> +    --header-cmd            <str>  * Add headers via `<str> \$patch_path`.
>>      --suppress-cc           <str>  * author, self, sob, cc, cccmd, body, bodycc, misc-by, all.
>>      --[no-]cc-cover                * Email Cc: addresses in the cover letter.
>>      --[no-]to-cover                * Email To: addresses in the cover letter.
>> @@ -270,7 +271,7 @@ sub do_edit {
>>  # Variables with corresponding config settings
>>  my ($suppress_from, $signed_off_by_cc);
>>  my ($cover_cc, $cover_to);
>> -my ($to_cmd, $cc_cmd);
>> +my ($to_cmd, $cc_cmd, $header_cmd);
>>  my ($smtp_server, $smtp_server_port, @smtp_server_options);
>>  my ($smtp_authuser, $smtp_encryption, $smtp_ssl_cert_path);
>>  my ($batch_size, $relogin_delay);
>> @@ -319,6 +320,7 @@ sub do_edit {
>>      "tocmd" => \$to_cmd,
>>      "cc" => \@config_cc,
>>      "cccmd" => \$cc_cmd,
>> +    "headercmd" => \$header_cmd,
>>      "aliasfiletype" => \$aliasfiletype,
>>      "bcc" => \@config_bcc,
>>      "suppresscc" => \@suppress_cc,
>> @@ -520,6 +522,7 @@ sub config_regexp {
>>  		    "compose" => \$compose,
>>  		    "quiet" => \$quiet,
>>  		    "cc-cmd=s" => \$cc_cmd,
>> +		    "header-cmd=s" => \$header_cmd,
>>  		    "suppress-from!" => \$suppress_from,
>>  		    "no-suppress-from" => sub {$suppress_from = 0},
>>  		    "suppress-cc=s" => \@suppress_cc,
>> @@ -1777,6 +1780,9 @@ sub process_file {
>>  			push(@header, $_);
>>  		}
>>  	}
>> +	# Add computed headers, if applicable.
>> +	push @header, execute_cmd("header-cmd", $header_cmd, $t)
>> +		if defined $header_cmd;
>
> While execute_cmd() may be a good enough interface to be used
> without much post-processing to read cc-cmd and to-cmd output (but
> notice that even there it needs post-processing), I do not think it
> is a good interface to directly use to read header lines without any
> postprocessing like patch [2/2] does.
>
> Its use in recipients_cmd() is OK primarily because it is about just
> reading bunch of values placed on Cc: or To: lines.  If you are going
> to use it in arbitrary sets of header lines, it is very likely that
> you would need to handle header folding (see what the loop before "#
> Now parse the header" is doing to preprocess <$fh>, which is not done
> for lines you read into @header in [2/2]).

I've extracted such postprocessing into fold_headers and applied
execute_cmd to it in new invoke_header_cmd subroutine.

v2 will follow shortly.

Thanks for the review!

--
Maxim

[PATCH v2 0/2] send-email: add --header-cmd option

[Maxim Cournoyer]

Hello,

This v2 addresses most of the review comments from Junio (thanks!).
It clarifies the documentation text and postprocesses the headers,
adding support for multiline headers.

Thanks,

Maxim Cournoyer (2):
  send-email: extract execute_cmd from recipients_cmd
  send-email: add --header-cmd option

 Documentation/config/sendemail.txt |  1 +
 Documentation/git-send-email.txt   |  8 ++++
 git-send-email.perl                | 74 +++++++++++++++++++++++-------
 t/t9001-send-email.sh              | 21 ++++++++-
 4 files changed, 85 insertions(+), 19 deletions(-)

Diff-intervalle contre v1 :
1:  216b40c1b8 = 1:  216b40c1b8 send-email: extract execute_cmd from recipients_cmd
2:  332d8ac4fe = 2:  332d8ac4fe send-email: add --header-cmd option

base-commit: 7580f92ffa970b9484ac214f7b53cec5e26ca4bc
-- 
2.39.2

[PATCH v2 1/2] send-email: extract execute_cmd from recipients_cmd

[Maxim Cournoyer]

This refactor is to pave the way for the addition of the new
'--header-cmd' option to the send-email command.

Signed-off-by: Maxim Cournoyer <maxim.cournoyer@gmail.com>
---
 git-send-email.perl | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index fd8cd0d46f..b8d77ad214 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -2,6 +2,7 @@
 #
 # Copyright 2002,2005 Greg Kroah-Hartman <greg@kroah.com>
 # Copyright 2005 Ryan Anderson <ryan@michonline.com>
+# Copyright 2023 Maxim Cournoyer <maxim.cournoyer@gmail.com>
 #
 # GPL v2 (See COPYING)
 #
@@ -2006,15 +2007,29 @@ sub process_file {
 	}
 }
 
+# Execute a command and return its output lines as an array.
+sub execute_cmd {
+	my ($prefix, $cmd, $file) = @_;
+	my @lines = ();
+	open my $fh, "-|", "$cmd \Q$file\E"
+		or die sprintf(__("(%s) Could not execute '%s'"), $prefix, $cmd);
+	while (my $line = <$fh>) {
+		last if $line =~ /^$/;
+		push @lines, $line;
+	}
+	close $fh
+	    or die sprintf(__("(%s) failed to close pipe to '%s'"), $prefix, $cmd);
+	return @lines;
+}
+
 # Execute a command (e.g. $to_cmd) to get a list of email addresses
 # and return a results array
 sub recipients_cmd {
 	my ($prefix, $what, $cmd, $file) = @_;
-
+	my @lines = ();
 	my @addresses = ();
-	open my $fh, "-|", "$cmd \Q$file\E"
-	    or die sprintf(__("(%s) Could not execute '%s'"), $prefix, $cmd);
-	while (my $address = <$fh>) {
+	@lines = execute_cmd($prefix, $cmd, $file);
+	for my $address (@lines) {
 		$address =~ s/^\s*//g;
 		$address =~ s/\s*$//g;
 		$address = sanitize_address($address);
@@ -2023,8 +2038,6 @@ sub recipients_cmd {
 		printf(__("(%s) Adding %s: %s from: '%s'\n"),
 		       $prefix, $what, $address, $cmd) unless $quiet;
 		}
-	close $fh
-	    or die sprintf(__("(%s) failed to close pipe to '%s'"), $prefix, $cmd);
 	return @addresses;
 }
 
-- 
2.39.2

[PATCH v2 2/2] send-email: add --header-cmd option

[Maxim Cournoyer]

Sometimes, adding a header different than CC or TO is desirable; for
example, when using Debbugs, it is best to use 'X-Debbugs-Cc' headers
to keep people in CC; this is an example use case enabled by the new
'--header-cmd' option.

The header folding logic is extracted to a subroutine so that it can
be reused.

Signed-off-by: Maxim Cournoyer <maxim.cournoyer@gmail.com>
---
 Documentation/config/sendemail.txt |  1 +
 Documentation/git-send-email.txt   |  8 +++++
 git-send-email.perl                | 49 +++++++++++++++++++++++-------
 t/t9001-send-email.sh              | 21 +++++++++++--
 4 files changed, 66 insertions(+), 13 deletions(-)

diff --git a/Documentation/config/sendemail.txt b/Documentation/config/sendemail.txt
index 51da7088a8..92a9ebe98c 100644
--- a/Documentation/config/sendemail.txt
+++ b/Documentation/config/sendemail.txt
@@ -61,6 +61,7 @@ sendemail.ccCmd::
 sendemail.chainReplyTo::
 sendemail.envelopeSender::
 sendemail.from::
+sendemail.headerCmd::
 sendemail.signedoffbycc::
 sendemail.smtpPass::
 sendemail.suppresscc::
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index b0f438ec99..295a3dd67c 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -320,6 +320,14 @@ Automating
 	Output of this command must be single email address per line.
 	Default is the value of `sendemail.ccCmd` configuration value.
 
+--header-cmd=<command>::
+	Specify a command that is executed once per outgoing message
+	and output RFC 2822 style header lines to be inserted into
+	them. When the `sendemail.headerCmd` configuration variable is
+	set, its value is always used. When --header-cmd is provided
+	at the command line, its value takes precedence over the
+	`sendemail.headerCmd` configuration variable.
+
 --[no-]chain-reply-to::
 	If this is set, each email will be sent as a reply to the previous
 	email sent.  If disabled with "--no-chain-reply-to", all emails after
diff --git a/git-send-email.perl b/git-send-email.perl
index b8d77ad214..906fc1964d 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -88,8 +88,9 @@ sub usage {
 
   Automating:
     --identity              <str>  * Use the sendemail.<id> options.
-    --to-cmd                <str>  * Email To: via `<str> \$patch_path`
-    --cc-cmd                <str>  * Email Cc: via `<str> \$patch_path`
+    --to-cmd                <str>  * Email To: via `<str> \$patch_path`.
+    --cc-cmd                <str>  * Email Cc: via `<str> \$patch_path`.
+    --header-cmd            <str>  * Add headers via `<str> \$patch_path`.
     --suppress-cc           <str>  * author, self, sob, cc, cccmd, body, bodycc, misc-by, all.
     --[no-]cc-cover                * Email Cc: addresses in the cover letter.
     --[no-]to-cover                * Email To: addresses in the cover letter.
@@ -270,7 +271,7 @@ sub do_edit {
 # Variables with corresponding config settings
 my ($suppress_from, $signed_off_by_cc);
 my ($cover_cc, $cover_to);
-my ($to_cmd, $cc_cmd);
+my ($to_cmd, $cc_cmd, $header_cmd);
 my ($smtp_server, $smtp_server_port, @smtp_server_options);
 my ($smtp_authuser, $smtp_encryption, $smtp_ssl_cert_path);
 my ($batch_size, $relogin_delay);
@@ -319,6 +320,7 @@ sub do_edit {
     "tocmd" => \$to_cmd,
     "cc" => \@config_cc,
     "cccmd" => \$cc_cmd,
+    "headercmd" => \$header_cmd,
     "aliasfiletype" => \$aliasfiletype,
     "bcc" => \@config_bcc,
     "suppresscc" => \@suppress_cc,
@@ -520,6 +522,7 @@ sub config_regexp {
 		    "compose" => \$compose,
 		    "quiet" => \$quiet,
 		    "cc-cmd=s" => \$cc_cmd,
+		    "header-cmd=s" => \$header_cmd,
 		    "suppress-from!" => \$suppress_from,
 		    "no-suppress-from" => sub {$suppress_from = 0},
 		    "suppress-cc=s" => \@suppress_cc,
@@ -1766,17 +1769,15 @@ sub process_file {
 	$subject = $initial_subject;
 	$message = "";
 	$message_num++;
-	# First unfold multiline header fields
+	# Retrieve and fold header fields.
+	my @header_lines = ();
 	while(<$fh>) {
 		last if /^\s*$/;
-		if (/^\s+\S/ and @header) {
-			chomp($header[$#header]);
-			s/^\s+/ /;
-			$header[$#header] .= $_;
-	    } else {
-			push(@header, $_);
-		}
+		push(@header_lines, $_);
 	}
+	@header = fold_headers(@header_lines);
+	# Add computed headers, if applicable.
+	push @header, invoke_header_cmd($header_cmd, $t) if defined $header_cmd;
 	# Now parse the header
 	foreach(@header) {
 		if (/^From /) {
@@ -2022,6 +2023,32 @@ sub execute_cmd {
 	return @lines;
 }
 
+# Process headers lines, folding multiline headers as defined by RFC
+# 2822.
+sub fold_headers {
+	my @headers;
+	foreach(@_) {
+		last if /^\s*$/;
+		if (/^\s+\S/ and @headers) {
+			chomp($headers[$#headers]);
+			s/^\s+/ /;
+			$headers[$#headers] .= $_;
+		} else {
+			push(@headers, $_);
+		}
+	}
+	return @headers;
+}
+
+# Invoke the provided CMD with FILE as an argument, which should
+# output RFC 2822 email headers. Fold multiline headers and return the
+# headers as an array.
+sub invoke_header_cmd {
+	my ($cmd, $file) = @_;
+	my @lines = execute_cmd("header-cmd", $header_cmd, $file);
+	return fold_headers(@lines);
+}
+
 # Execute a command (e.g. $to_cmd) to get a list of email addresses
 # and return a results array
 sub recipients_cmd {
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 0de83b5d2b..3393725107 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -374,13 +374,16 @@ test_expect_success $PREREQ,!AUTOIDENT 'broken implicit ident aborts send-email'
 	)
 '
 
-test_expect_success $PREREQ 'setup tocmd and cccmd scripts' '
+test_expect_success $PREREQ 'setup cmd scripts' '
 	write_script tocmd-sed <<-\EOF &&
 	sed -n -e "s/^tocmd--//p" "$1"
 	EOF
-	write_script cccmd-sed <<-\EOF
+	write_script cccmd-sed <<-\EOF &&
 	sed -n -e "s/^cccmd--//p" "$1"
 	EOF
+	write_script headercmd-sed <<-\EOF
+	sed -n -e "s/^headercmd--//p" "$1"
+	EOF
 '
 
 test_expect_success $PREREQ 'tocmd works' '
@@ -410,6 +413,20 @@ test_expect_success $PREREQ 'cccmd works' '
 	grep "^	cccmd@example.com" msgtxt1
 '
 
+test_expect_success $PREREQ 'headercmd works' '
+	clean_fake_sendmail &&
+	cp $patches headercmd.patch &&
+	echo "headercmd--X-Debbugs-CC: dummy@example.com" >>headercmd.patch &&
+	git send-email \
+		--from="Example <nobody@example.com>" \
+		--to=nobody@example.com \
+		--header-cmd=./headercmd-sed \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		headercmd.patch \
+		&&
+	grep "^X-Debbugs-CC: dummy@example.com" msgtxt1
+'
+
 test_expect_success $PREREQ 'reject long lines' '
 	z8=zzzzzzzz &&
 	z64=$z8$z8$z8$z8$z8$z8$z8$z8 &&
-- 
2.39.2

Re: [PATCH 2/2] send-email: add --header-cmd option

[Junio C Hamano]

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> I'm not too familiar with the email format; but I presume an empty line
> would signal the end of a message?  That'd be bad yes, but I think it
> cannot currently happen given the 'last if $line =~ /^$/;' guard at in
> execute_cmd around line 2023; it'd means headers following an empty line
> would be discarded though.  The expected use case is indeed for a user's
> script to produce RFC 2822 style headers to messages.

Yes, silently discarding the end-user input is what I meant by a
disaster.

> The former (a true default with no way to turn it off other than
> redefining it), which I believe is the same behavior as for --cc-cmd or
> --to-cmd.  There are no '--no-cc-cmd' or '--no-to-cmd' options, although
> their result can be filtered via the '--no-cc' and '--no-to' options.

Yup.

> Looking in the source, options supporting '--no-' always appear to be
> boolean toggles (on/off) though, so I'm not sure how a '--no-header-cmd'
> that take a value can currently be implemented.  Perhaps it could be
> added later if there is a need?

Perhaps we can do without a configuration variable first, and
perhaps the variable could be added later if there is a need and a
proper way to turn it off per invocation basis.

> I've extracted such postprocessing into fold_headers and applied
> execute_cmd to it in new invoke_header_cmd subroutine.

Sounds like a good approach (without looking the actual patch).

Thanks.

Re: [PATCH v2 1/2] send-email: extract execute_cmd from recipients_cmd

[Eric Sunshine]

On Tue, Apr 25, 2023 at 12:46 PM Maxim Cournoyer
<maxim.cournoyer@gmail.com> wrote:
> diff --git a/git-send-email.perl b/git-send-email.perl
> @@ -2,6 +2,7 @@
>  # Copyright 2002,2005 Greg Kroah-Hartman <greg@kroah.com>
>  # Copyright 2005 Ryan Anderson <ryan@michonline.com>
> +# Copyright 2023 Maxim Cournoyer <maxim.cournoyer@gmail.com>

Let's avoid this change, please. Many people have worked on this file
over the years -- often making changes far more substantial than those
made by this patch series -- who have not staked such a claim.

[PATCH v3 0/3] send-email: add --header-cmd option

[Maxim Cournoyer]

Hello,

This third revision adds some input validation, to detect invalid
output coming from user scripts used with 'git send-email'.  It also
adds a test to ensure multiline (folded) headers are correctly
supported.

Thanks,

Maxim Cournoyer (3):
  send-email: extract execute_cmd from recipients_cmd
  send-email: add --header-cmd option
  send-email: detect empty blank lines in command output

 Documentation/config/sendemail.txt |  1 +
 Documentation/git-send-email.txt   |  8 +++
 git-send-email.perl                | 82 +++++++++++++++++++++++-------
 t/t9001-send-email.sh              | 56 +++++++++++++++++++-
 4 files changed, 128 insertions(+), 19 deletions(-)


base-commit: 7580f92ffa970b9484ac214f7b53cec5e26ca4bc
-- 
2.39.2

[PATCH v3 1/3] send-email: extract execute_cmd from recipients_cmd

[Maxim Cournoyer]

This refactor is to pave the way for the addition of the new
'--header-cmd' option to the send-email command.

Signed-off-by: Maxim Cournoyer <maxim.cournoyer@gmail.com>
---
 git-send-email.perl | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index fd8cd0d46f..b8d77ad214 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -2,6 +2,7 @@
 #
 # Copyright 2002,2005 Greg Kroah-Hartman <greg@kroah.com>
 # Copyright 2005 Ryan Anderson <ryan@michonline.com>
+# Copyright 2023 Maxim Cournoyer <maxim.cournoyer@gmail.com>
 #
 # GPL v2 (See COPYING)
 #
@@ -2006,15 +2007,29 @@ sub process_file {
 	}
 }
 
+# Execute a command and return its output lines as an array.
+sub execute_cmd {
+	my ($prefix, $cmd, $file) = @_;
+	my @lines = ();
+	open my $fh, "-|", "$cmd \Q$file\E"
+		or die sprintf(__("(%s) Could not execute '%s'"), $prefix, $cmd);
+	while (my $line = <$fh>) {
+		last if $line =~ /^$/;
+		push @lines, $line;
+	}
+	close $fh
+	    or die sprintf(__("(%s) failed to close pipe to '%s'"), $prefix, $cmd);
+	return @lines;
+}
+
 # Execute a command (e.g. $to_cmd) to get a list of email addresses
 # and return a results array
 sub recipients_cmd {
 	my ($prefix, $what, $cmd, $file) = @_;
-
+	my @lines = ();
 	my @addresses = ();
-	open my $fh, "-|", "$cmd \Q$file\E"
-	    or die sprintf(__("(%s) Could not execute '%s'"), $prefix, $cmd);
-	while (my $address = <$fh>) {
+	@lines = execute_cmd($prefix, $cmd, $file);
+	for my $address (@lines) {
 		$address =~ s/^\s*//g;
 		$address =~ s/\s*$//g;
 		$address = sanitize_address($address);
@@ -2023,8 +2038,6 @@ sub recipients_cmd {
 		printf(__("(%s) Adding %s: %s from: '%s'\n"),
 		       $prefix, $what, $address, $cmd) unless $quiet;
 		}
-	close $fh
-	    or die sprintf(__("(%s) failed to close pipe to '%s'"), $prefix, $cmd);
 	return @addresses;
 }
 
-- 
2.39.2

[PATCH v3 2/3] send-email: add --header-cmd option

[Maxim Cournoyer]

Sometimes, adding a header different than CC or TO is desirable; for
example, when using Debbugs, it is best to use 'X-Debbugs-Cc' headers
to keep people in CC; this is an example use case enabled by the new
'--header-cmd' option.

The header unfolding logic is extracted to a subroutine so that it can
be reused; a test is added for coverage.
---
 Documentation/config/sendemail.txt |  1 +
 Documentation/git-send-email.txt   |  8 +++++
 git-send-email.perl                | 49 +++++++++++++++++++++++-------
 t/t9001-send-email.sh              | 39 ++++++++++++++++++++++--
 4 files changed, 84 insertions(+), 13 deletions(-)

diff --git a/Documentation/config/sendemail.txt b/Documentation/config/sendemail.txt
index 51da7088a8..92a9ebe98c 100644
--- a/Documentation/config/sendemail.txt
+++ b/Documentation/config/sendemail.txt
@@ -61,6 +61,7 @@ sendemail.ccCmd::
 sendemail.chainReplyTo::
 sendemail.envelopeSender::
 sendemail.from::
+sendemail.headerCmd::
 sendemail.signedoffbycc::
 sendemail.smtpPass::
 sendemail.suppresscc::
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index b0f438ec99..295a3dd67c 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -320,6 +320,14 @@ Automating
 	Output of this command must be single email address per line.
 	Default is the value of `sendemail.ccCmd` configuration value.
 
+--header-cmd=<command>::
+	Specify a command that is executed once per outgoing message
+	and output RFC 2822 style header lines to be inserted into
+	them. When the `sendemail.headerCmd` configuration variable is
+	set, its value is always used. When --header-cmd is provided
+	at the command line, its value takes precedence over the
+	`sendemail.headerCmd` configuration variable.
+
 --[no-]chain-reply-to::
 	If this is set, each email will be sent as a reply to the previous
 	email sent.  If disabled with "--no-chain-reply-to", all emails after
diff --git a/git-send-email.perl b/git-send-email.perl
index b8d77ad214..666b37adc9 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -88,8 +88,9 @@ sub usage {
 
   Automating:
     --identity              <str>  * Use the sendemail.<id> options.
-    --to-cmd                <str>  * Email To: via `<str> \$patch_path`
-    --cc-cmd                <str>  * Email Cc: via `<str> \$patch_path`
+    --to-cmd                <str>  * Email To: via `<str> \$patch_path`.
+    --cc-cmd                <str>  * Email Cc: via `<str> \$patch_path`.
+    --header-cmd            <str>  * Add headers via `<str> \$patch_path`.
     --suppress-cc           <str>  * author, self, sob, cc, cccmd, body, bodycc, misc-by, all.
     --[no-]cc-cover                * Email Cc: addresses in the cover letter.
     --[no-]to-cover                * Email To: addresses in the cover letter.
@@ -270,7 +271,7 @@ sub do_edit {
 # Variables with corresponding config settings
 my ($suppress_from, $signed_off_by_cc);
 my ($cover_cc, $cover_to);
-my ($to_cmd, $cc_cmd);
+my ($to_cmd, $cc_cmd, $header_cmd);
 my ($smtp_server, $smtp_server_port, @smtp_server_options);
 my ($smtp_authuser, $smtp_encryption, $smtp_ssl_cert_path);
 my ($batch_size, $relogin_delay);
@@ -319,6 +320,7 @@ sub do_edit {
     "tocmd" => \$to_cmd,
     "cc" => \@config_cc,
     "cccmd" => \$cc_cmd,
+    "headercmd" => \$header_cmd,
     "aliasfiletype" => \$aliasfiletype,
     "bcc" => \@config_bcc,
     "suppresscc" => \@suppress_cc,
@@ -520,6 +522,7 @@ sub config_regexp {
 		    "compose" => \$compose,
 		    "quiet" => \$quiet,
 		    "cc-cmd=s" => \$cc_cmd,
+		    "header-cmd=s" => \$header_cmd,
 		    "suppress-from!" => \$suppress_from,
 		    "no-suppress-from" => sub {$suppress_from = 0},
 		    "suppress-cc=s" => \@suppress_cc,
@@ -1766,17 +1769,15 @@ sub process_file {
 	$subject = $initial_subject;
 	$message = "";
 	$message_num++;
-	# First unfold multiline header fields
+	# Retrieve and unfold header fields.
+	my @header_lines = ();
 	while(<$fh>) {
 		last if /^\s*$/;
-		if (/^\s+\S/ and @header) {
-			chomp($header[$#header]);
-			s/^\s+/ /;
-			$header[$#header] .= $_;
-	    } else {
-			push(@header, $_);
-		}
+		push(@header_lines, $_);
 	}
+	@header = unfold_headers(@header_lines);
+	# Add computed headers, if applicable.
+	push @header, invoke_header_cmd($header_cmd, $t) if defined $header_cmd;
 	# Now parse the header
 	foreach(@header) {
 		if (/^From /) {
@@ -2022,6 +2023,32 @@ sub execute_cmd {
 	return @lines;
 }
 
+# Process headers lines, unfolding multiline headers as defined by RFC
+# 2822.
+sub unfold_headers {
+	my @headers;
+	foreach(@_) {
+		last if /^\s*$/;
+		if (/^\s+\S/ and @headers) {
+			chomp($headers[$#headers]);
+			s/^\s+/ /;
+			$headers[$#headers] .= $_;
+		} else {
+			push(@headers, $_);
+		}
+	}
+	return @headers;
+}
+
+# Invoke the provided CMD with FILE as an argument, which should
+# output RFC 2822 email headers. Fold multiline headers and return the
+# headers as an array.
+sub invoke_header_cmd {
+	my ($cmd, $file) = @_;
+	my @lines = execute_cmd("header-cmd", $header_cmd, $file);
+	return unfold_headers(@lines);
+}
+
 # Execute a command (e.g. $to_cmd) to get a list of email addresses
 # and return a results array
 sub recipients_cmd {
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 0de83b5d2b..230f436f23 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -374,13 +374,16 @@ test_expect_success $PREREQ,!AUTOIDENT 'broken implicit ident aborts send-email'
 	)
 '
 
-test_expect_success $PREREQ 'setup tocmd and cccmd scripts' '
+test_expect_success $PREREQ 'setup cmd scripts' '
 	write_script tocmd-sed <<-\EOF &&
 	sed -n -e "s/^tocmd--//p" "$1"
 	EOF
-	write_script cccmd-sed <<-\EOF
+	write_script cccmd-sed <<-\EOF &&
 	sed -n -e "s/^cccmd--//p" "$1"
 	EOF
+	write_script headercmd-sed <<-\EOF
+	sed -n -e "s/^headercmd--//p" "$1"
+	EOF
 '
 
 test_expect_success $PREREQ 'tocmd works' '
@@ -410,6 +413,38 @@ test_expect_success $PREREQ 'cccmd works' '
 	grep "^	cccmd@example.com" msgtxt1
 '
 
+test_expect_success $PREREQ 'headercmd works' '
+	clean_fake_sendmail &&
+	cp $patches headercmd.patch &&
+	echo "headercmd--X-Debbugs-CC: dummy@example.com" >>headercmd.patch &&
+	git send-email \
+		--from="Example <nobody@example.com>" \
+		--to=nobody@example.com \
+		--header-cmd=./headercmd-sed \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		headercmd.patch \
+		&&
+	grep "^X-Debbugs-CC: dummy@example.com" msgtxt1
+'
+
+test_expect_success $PREREQ 'multiline fields are correctly unfolded' '
+	clean_fake_sendmail &&
+	cp $patches headercmd.patch &&
+	write_script headercmd-multiline <<-\EOF &&
+	echo "X-Debbugs-CC: someone@example.com
+FoldedField: This is a tale
+ best told using
+ multiple lines."
+	EOF
+	git send-email \
+		--from="Example <nobody@example.com>" \
+		--to=nobody@example.com \
+		--header-cmd=./headercmd-multiline \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		headercmd.patch &&
+	grep "^FoldedField: This is a tale best told using multiple lines.$" msgtxt1
+'
+
 test_expect_success $PREREQ 'reject long lines' '
 	z8=zzzzzzzz &&
 	z64=$z8$z8$z8$z8$z8$z8$z8$z8 &&
-- 
2.39.2

[PATCH v3 3/3] send-email: detect empty blank lines in command output

[Maxim Cournoyer]

The email format does not allow blank lines in headers; detect such
input and report it as malformed and add a test for it.

Signed-off-by: Maxim Cournoyer <maxim.cournoyer@gmail.com>
---
 git-send-email.perl   | 12 ++++++++++--
 t/t9001-send-email.sh | 17 +++++++++++++++++
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 666b37adc9..5adcbbeb49 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -2008,14 +2008,22 @@ sub process_file {
 	}
 }
 
-# Execute a command and return its output lines as an array.
+# Execute a command and return its output lines as an array.  Blank
+# lines which do not appear at the end of the output are reported as
+# errors.
 sub execute_cmd {
 	my ($prefix, $cmd, $file) = @_;
 	my @lines = ();
+	my $seen_blank_line = 0;
 	open my $fh, "-|", "$cmd \Q$file\E"
 		or die sprintf(__("(%s) Could not execute '%s'"), $prefix, $cmd);
 	while (my $line = <$fh>) {
-		last if $line =~ /^$/;
+		die sprintf(__("(%s) Malformed output from '%s'"), $prefix, $cmd)
+		    if $seen_blank_line;
+		if ($line =~ /^$/) {
+			$seen_blank_line = $line =~ /^$/;
+			next;
+		}
 		push @lines, $line;
 	}
 	close $fh
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 230f436f23..6701dd8848 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -445,6 +445,23 @@ FoldedField: This is a tale
 	grep "^FoldedField: This is a tale best told using multiple lines.$" msgtxt1
 '
 
+# Blank lines in the middle of the output of a command are invalid.
+test_expect_success $PREREQ 'malform output reported on blank lines in command output' '
+	clean_fake_sendmail &&
+	cp $patches headercmd.patch &&
+	write_script headercmd-malformed-output <<-\EOF &&
+	echo "X-Debbugs-CC: someone@example.com
+
+SomeOtherField: someone-else@example.com"
+	EOF
+	! git send-email \
+		--from="Example <nobody@example.com>" \
+		--to=nobody@example.com \
+		--header-cmd=./headercmd-malformed-output \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		headercmd.patch
+'
+
 test_expect_success $PREREQ 'reject long lines' '
 	z8=zzzzzzzz &&
 	z64=$z8$z8$z8$z8$z8$z8$z8$z8 &&
-- 
2.39.2

Re: [PATCH 2/2] send-email: add --header-cmd option

[Maxim Cournoyer]

Hi Junio,

Junio C Hamano <gitster@pobox.com> writes:

> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>
>> I'm not too familiar with the email format; but I presume an empty line
>> would signal the end of a message?  That'd be bad yes, but I think it
>> cannot currently happen given the 'last if $line =~ /^$/;' guard at in
>> execute_cmd around line 2023; it'd means headers following an empty line
>> would be discarded though.  The expected use case is indeed for a user's
>> script to produce RFC 2822 style headers to messages.
>
> Yes, silently discarding the end-user input is what I meant by a
> disaster.

In v3 just sent, empty blank lines are now detected and reported as an
error.

>> The former (a true default with no way to turn it off other than
>> redefining it), which I believe is the same behavior as for --cc-cmd or
>> --to-cmd.  There are no '--no-cc-cmd' or '--no-to-cmd' options, although
>> their result can be filtered via the '--no-cc' and '--no-to' options.
>
> Yup.
>
>> Looking in the source, options supporting '--no-' always appear to be
>> boolean toggles (on/off) though, so I'm not sure how a '--no-header-cmd'
>> that take a value can currently be implemented.  Perhaps it could be
>> added later if there is a need?
>
> Perhaps we can do without a configuration variable first, and
> perhaps the variable could be added later if there is a need and a
> proper way to turn it off per invocation basis.

I'd like to preserve the sendemail.headerCmd configuration variable; as
it properly enables the use case that motivated this change :-).  If it
means I need to add some ad-hoc --no-header-cmd, I can do so; let me
know!

>> I've extracted such postprocessing into fold_headers and applied
>> execute_cmd to it in new invoke_header_cmd subroutine.
>
> Sounds like a good approach (without looking the actual patch).

OK.  Make sure to look at the latest revision, v3.

Thanks again!

-- 
Maxim

Re: [PATCH v2 1/2] send-email: extract execute_cmd from recipients_cmd

[Maxim Cournoyer]

Hi Eric,

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Tue, Apr 25, 2023 at 12:46 PM Maxim Cournoyer
> <maxim.cournoyer@gmail.com> wrote:
>> diff --git a/git-send-email.perl b/git-send-email.perl
>> @@ -2,6 +2,7 @@
>>  # Copyright 2002,2005 Greg Kroah-Hartman <greg@kroah.com>
>>  # Copyright 2005 Ryan Anderson <ryan@michonline.com>
>> +# Copyright 2023 Maxim Cournoyer <maxim.cournoyer@gmail.com>
>
> Let's avoid this change, please. Many people have worked on this file
> over the years -- often making changes far more substantial than those
> made by this patch series -- who have not staked such a claim.

I don't mind to drop this hunk if it's unwelcome/not current practice.
I didn't mean to try to steal anyone's spotlight :-).  I know this
contribution is tiny; I barely write Perl (it's still enough of a change
to be protected by copyright though, but I don't mind too much).

Cheers,

-- 
Maxim

[PATCH v4 0/3] send-email: add --header-cmd option

[Maxim Cournoyer]

Hello,

This v4 drops the copyright hunk as suggested by Eric, and adds a
 --no-header-cmd as suggested by Junio.

Thanks you!

Maxim Cournoyer (3):
  send-email: extract execute_cmd from recipients_cmd
  send-email: add --header-cmd, --no-header-cmd options
  send-email: detect empty blank lines in command output

 Documentation/config/sendemail.txt |  1 +
 Documentation/git-send-email.txt   | 11 ++++
 git-send-email.perl                | 87 +++++++++++++++++++++++-------
 t/t9001-send-email.sh              | 71 +++++++++++++++++++++++-
 4 files changed, 150 insertions(+), 20 deletions(-)

Diff-intervalle contre v3 :
-:  ---------- > 1:  4bc38e69bd send-email: extract execute_cmd from recipients_cmd
-:  ---------- > 2:  60606e8d21 send-email: add --header-cmd, --no-header-cmd options.
-:  ---------- > 3:  2ed6a3f965 send-email: detect empty blank lines in command output

base-commit: 48d89b51b3bb8a60580c36731b96a7206ce1e5b9
-- 
2.39.2

[PATCH v4 1/3] send-email: extract execute_cmd from recipients_cmd

[Maxim Cournoyer]

This refactor is to pave the way for the addition of the new
'--header-cmd' option to the send-email command.

Signed-off-by: Maxim Cournoyer <maxim.cournoyer@gmail.com>
---
New in v4:
- Drop copyright line.

 git-send-email.perl | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 66c9171109..04503e3c3c 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -2021,15 +2021,29 @@ sub process_file {
 	}
 }
 
+# Execute a command and return its output lines as an array.
+sub execute_cmd {
+	my ($prefix, $cmd, $file) = @_;
+	my @lines = ();
+	open my $fh, "-|", "$cmd \Q$file\E"
+		or die sprintf(__("(%s) Could not execute '%s'"), $prefix, $cmd);
+	while (my $line = <$fh>) {
+		last if $line =~ /^$/;
+		push @lines, $line;
+	}
+	close $fh
+	    or die sprintf(__("(%s) failed to close pipe to '%s'"), $prefix, $cmd);
+	return @lines;
+}
+
 # Execute a command (e.g. $to_cmd) to get a list of email addresses
 # and return a results array
 sub recipients_cmd {
 	my ($prefix, $what, $cmd, $file) = @_;
-
+	my @lines = ();
 	my @addresses = ();
-	open my $fh, "-|", "$cmd \Q$file\E"
-	    or die sprintf(__("(%s) Could not execute '%s'"), $prefix, $cmd);
-	while (my $address = <$fh>) {
+	@lines = execute_cmd($prefix, $cmd, $file);
+	for my $address (@lines) {
 		$address =~ s/^\s*//g;
 		$address =~ s/\s*$//g;
 		$address = sanitize_address($address);
@@ -2038,8 +2052,6 @@ sub recipients_cmd {
 		printf(__("(%s) Adding %s: %s from: '%s'\n"),
 		       $prefix, $what, $address, $cmd) unless $quiet;
 		}
-	close $fh
-	    or die sprintf(__("(%s) failed to close pipe to '%s'"), $prefix, $cmd);
 	return @addresses;
 }
 
-- 
2.39.2

[PATCH v4 2/3] send-email: add --header-cmd, --no-header-cmd options

[Maxim Cournoyer]

Sometimes, adding a header different than CC or TO is desirable; for
example, when using Debbugs, it is best to use 'X-Debbugs-Cc' headers
to keep people in CC; this is an example use case enabled by the new
'--header-cmd' option.

The header unfolding logic is extracted to a subroutine so that it can
be reused; a test is added for coverage.

Signed-off-by: Maxim Cournoyer <maxim.cournoyer@gmail.com>
---
New in v4:
- Add a --no-header-cmd.

 Documentation/config/sendemail.txt |  1 +
 Documentation/git-send-email.txt   | 11 ++++++
 git-send-email.perl                | 55 +++++++++++++++++++++++-------
 t/t9001-send-email.sh              | 54 +++++++++++++++++++++++++++--
 4 files changed, 107 insertions(+), 14 deletions(-)

diff --git a/Documentation/config/sendemail.txt b/Documentation/config/sendemail.txt
index 51da7088a8..92a9ebe98c 100644
--- a/Documentation/config/sendemail.txt
+++ b/Documentation/config/sendemail.txt
@@ -61,6 +61,7 @@ sendemail.ccCmd::
 sendemail.chainReplyTo::
 sendemail.envelopeSender::
 sendemail.from::
+sendemail.headerCmd::
 sendemail.signedoffbycc::
 sendemail.smtpPass::
 sendemail.suppresscc::
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index b0f438ec99..4d2ae061f9 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -320,6 +320,17 @@ Automating
 	Output of this command must be single email address per line.
 	Default is the value of `sendemail.ccCmd` configuration value.
 
+--header-cmd=<command>::
+	Specify a command that is executed once per outgoing message
+	and output RFC 2822 style header lines to be inserted into
+	them. When the `sendemail.headerCmd` configuration variable is
+	set, its value is always used. When --header-cmd is provided
+	at the command line, its value takes precedence over the
+	`sendemail.headerCmd` configuration variable.
+
+--no-header-cmd::
+	Disable any header command in use.
+
 --[no-]chain-reply-to::
 	If this is set, each email will be sent as a reply to the previous
 	email sent.  If disabled with "--no-chain-reply-to", all emails after
diff --git a/git-send-email.perl b/git-send-email.perl
index 04503e3c3c..32febe9af3 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -87,8 +87,10 @@ sub usage {
 
   Automating:
     --identity              <str>  * Use the sendemail.<id> options.
-    --to-cmd                <str>  * Email To: via `<str> \$patch_path`
-    --cc-cmd                <str>  * Email Cc: via `<str> \$patch_path`
+    --to-cmd                <str>  * Email To: via `<str> \$patch_path`.
+    --cc-cmd                <str>  * Email Cc: via `<str> \$patch_path`.
+    --header-cmd            <str>  * Add headers via `<str> \$patch_path`.
+    --no-header-cmd                * Disable any header command in use.
     --suppress-cc           <str>  * author, self, sob, cc, cccmd, body, bodycc, misc-by, all.
     --[no-]cc-cover                * Email Cc: addresses in the cover letter.
     --[no-]to-cover                * Email To: addresses in the cover letter.
@@ -202,7 +204,7 @@ sub format_2822_time {
 	$author,$sender,$smtp_authpass,$annotate,$compose,$time);
 # Things we either get from config, *or* are overridden on the
 # command-line.
-my ($no_cc, $no_to, $no_bcc, $no_identity);
+my ($no_cc, $no_to, $no_bcc, $no_identity, $no_header_cmd);
 my (@config_to, @getopt_to);
 my (@config_cc, @getopt_cc);
 my (@config_bcc, @getopt_bcc);
@@ -269,7 +271,7 @@ sub do_edit {
 # Variables with corresponding config settings
 my ($suppress_from, $signed_off_by_cc);
 my ($cover_cc, $cover_to);
-my ($to_cmd, $cc_cmd);
+my ($to_cmd, $cc_cmd, $header_cmd);
 my ($smtp_server, $smtp_server_port, @smtp_server_options);
 my ($smtp_authuser, $smtp_encryption, $smtp_ssl_cert_path);
 my ($batch_size, $relogin_delay);
@@ -318,6 +320,7 @@ sub do_edit {
     "tocmd" => \$to_cmd,
     "cc" => \@config_cc,
     "cccmd" => \$cc_cmd,
+    "headercmd" => \$header_cmd,
     "aliasfiletype" => \$aliasfiletype,
     "bcc" => \@config_bcc,
     "suppresscc" => \@suppress_cc,
@@ -519,6 +522,8 @@ sub config_regexp {
 		    "compose" => \$compose,
 		    "quiet" => \$quiet,
 		    "cc-cmd=s" => \$cc_cmd,
+		    "header-cmd=s" => \$header_cmd,
+		    "no-header-cmd" => \$no_header_cmd,
 		    "suppress-from!" => \$suppress_from,
 		    "no-suppress-from" => sub {$suppress_from = 0},
 		    "suppress-cc=s" => \@suppress_cc,
@@ -1780,16 +1785,16 @@ sub process_file {
 	$subject = $initial_subject;
 	$message = "";
 	$message_num++;
-	# First unfold multiline header fields
+	# Retrieve and unfold header fields.
+	my @header_lines = ();
 	while(<$fh>) {
 		last if /^\s*$/;
-		if (/^\s+\S/ and @header) {
-			chomp($header[$#header]);
-			s/^\s+/ /;
-			$header[$#header] .= $_;
-	    } else {
-			push(@header, $_);
-		}
+		push(@header_lines, $_);
+	}
+	@header = unfold_headers(@header_lines);
+	# Add computed headers, if applicable.
+	unless ($no_header_cmd || ! $header_cmd) {
+		push @header, invoke_header_cmd($header_cmd, $t);
 	}
 	# Now parse the header
 	foreach(@header) {
@@ -2036,6 +2041,32 @@ sub execute_cmd {
 	return @lines;
 }
 
+# Process headers lines, unfolding multiline headers as defined by RFC
+# 2822.
+sub unfold_headers {
+	my @headers;
+	foreach(@_) {
+		last if /^\s*$/;
+		if (/^\s+\S/ and @headers) {
+			chomp($headers[$#headers]);
+			s/^\s+/ /;
+			$headers[$#headers] .= $_;
+		} else {
+			push(@headers, $_);
+		}
+	}
+	return @headers;
+}
+
+# Invoke the provided CMD with FILE as an argument, which should
+# output RFC 2822 email headers. Fold multiline headers and return the
+# headers as an array.
+sub invoke_header_cmd {
+	my ($cmd, $file) = @_;
+	my @lines = execute_cmd("header-cmd", $header_cmd, $file);
+	return unfold_headers(@lines);
+}
+
 # Execute a command (e.g. $to_cmd) to get a list of email addresses
 # and return a results array
 sub recipients_cmd {
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 6520346246..dfc5be581f 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -374,13 +374,16 @@ test_expect_success $PREREQ,!AUTOIDENT 'broken implicit ident aborts send-email'
 	)
 '
 
-test_expect_success $PREREQ 'setup tocmd and cccmd scripts' '
+test_expect_success $PREREQ 'setup cmd scripts' '
 	write_script tocmd-sed <<-\EOF &&
 	sed -n -e "s/^tocmd--//p" "$1"
 	EOF
-	write_script cccmd-sed <<-\EOF
+	write_script cccmd-sed <<-\EOF &&
 	sed -n -e "s/^cccmd--//p" "$1"
 	EOF
+	write_script headercmd-sed <<-\EOF
+	sed -n -e "s/^headercmd--//p" "$1"
+	EOF
 '
 
 test_expect_success $PREREQ 'tocmd works' '
@@ -410,6 +413,53 @@ test_expect_success $PREREQ 'cccmd works' '
 	grep "^	cccmd@example.com" msgtxt1
 '
 
+test_expect_success $PREREQ 'headercmd works' '
+	clean_fake_sendmail &&
+	cp $patches headercmd.patch &&
+	echo "headercmd--X-Debbugs-CC: dummy@example.com" >>headercmd.patch &&
+	git send-email \
+		--from="Example <nobody@example.com>" \
+		--to=nobody@example.com \
+		--header-cmd=./headercmd-sed \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		headercmd.patch \
+		&&
+	grep "^X-Debbugs-CC: dummy@example.com" msgtxt1
+'
+
+test_expect_success $PREREQ '--no-header-cmd works' '
+	clean_fake_sendmail &&
+	cp $patches headercmd.patch &&
+	echo "headercmd--X-Debbugs-CC: dummy@example.com" >>headercmd.patch &&
+	git send-email \
+		--from="Example <nobody@example.com>" \
+		--to=nobody@example.com \
+		--header-cmd=./headercmd-sed \
+                --no-header-cmd \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		headercmd.patch \
+		&&
+	! grep "^X-Debbugs-CC: dummy@example.com" msgtxt1
+'
+
+test_expect_success $PREREQ 'multiline fields are correctly unfolded' '
+	clean_fake_sendmail &&
+	cp $patches headercmd.patch &&
+	write_script headercmd-multiline <<-\EOF &&
+	echo "X-Debbugs-CC: someone@example.com
+FoldedField: This is a tale
+ best told using
+ multiple lines."
+	EOF
+	git send-email \
+		--from="Example <nobody@example.com>" \
+		--to=nobody@example.com \
+		--header-cmd=./headercmd-multiline \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		headercmd.patch &&
+	grep "^FoldedField: This is a tale best told using multiple lines.$" msgtxt1
+'
+
 test_expect_success $PREREQ 'reject long lines' '
 	z8=zzzzzzzz &&
 	z64=$z8$z8$z8$z8$z8$z8$z8$z8 &&
-- 
2.39.2

[PATCH v4 3/3] send-email: detect empty blank lines in command output

[Maxim Cournoyer]

The email format does not allow blank lines in headers; detect such
input and report it as malformed and add a test for it.

Signed-off-by: Maxim Cournoyer <maxim.cournoyer@gmail.com>
---
 git-send-email.perl   | 12 ++++++++++--
 t/t9001-send-email.sh | 17 +++++++++++++++++
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 32febe9af3..22a64e608f 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -2026,14 +2026,22 @@ sub process_file {
 	}
 }
 
-# Execute a command and return its output lines as an array.
+# Execute a command and return its output lines as an array.  Blank
+# lines which do not appear at the end of the output are reported as
+# errors.
 sub execute_cmd {
 	my ($prefix, $cmd, $file) = @_;
 	my @lines = ();
+	my $seen_blank_line = 0;
 	open my $fh, "-|", "$cmd \Q$file\E"
 		or die sprintf(__("(%s) Could not execute '%s'"), $prefix, $cmd);
 	while (my $line = <$fh>) {
-		last if $line =~ /^$/;
+		die sprintf(__("(%s) Malformed output from '%s'"), $prefix, $cmd)
+		    if $seen_blank_line;
+		if ($line =~ /^$/) {
+			$seen_blank_line = $line =~ /^$/;
+			next;
+		}
 		push @lines, $line;
 	}
 	close $fh
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index dfc5be581f..6519eea1ed 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -460,6 +460,23 @@ FoldedField: This is a tale
 	grep "^FoldedField: This is a tale best told using multiple lines.$" msgtxt1
 '
 
+# Blank lines in the middle of the output of a command are invalid.
+test_expect_success $PREREQ 'malform output reported on blank lines in command output' '
+	clean_fake_sendmail &&
+	cp $patches headercmd.patch &&
+	write_script headercmd-malformed-output <<-\EOF &&
+	echo "X-Debbugs-CC: someone@example.com
+
+SomeOtherField: someone-else@example.com"
+	EOF
+	! git send-email \
+		--from="Example <nobody@example.com>" \
+		--to=nobody@example.com \
+		--header-cmd=./headercmd-malformed-output \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		headercmd.patch
+'
+
 test_expect_success $PREREQ 'reject long lines' '
 	z8=zzzzzzzz &&
 	z64=$z8$z8$z8$z8$z8$z8$z8$z8 &&
-- 
2.39.2

Re: [PATCH v2 1/2] send-email: extract execute_cmd from recipients_cmd

[Felipe Contreras]

Maxim Cournoyer wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
> 
> > On Tue, Apr 25, 2023 at 12:46 PM Maxim Cournoyer
> > <maxim.cournoyer@gmail.com> wrote:
> >> diff --git a/git-send-email.perl b/git-send-email.perl
> >> @@ -2,6 +2,7 @@
> >>  # Copyright 2002,2005 Greg Kroah-Hartman <greg@kroah.com>
> >>  # Copyright 2005 Ryan Anderson <ryan@michonline.com>
> >> +# Copyright 2023 Maxim Cournoyer <maxim.cournoyer@gmail.com>
> >
> > Let's avoid this change, please. Many people have worked on this file
> > over the years -- often making changes far more substantial than those
> > made by this patch series -- who have not staked such a claim.
> 
> I don't mind to drop this hunk if it's unwelcome/not current practice.

In most open source projects the practice is that only the top one or two
contributors are mentioned.

> it's still enough of a change to be protected by copyright though, but
> I don't mind too much.

My understanding is that your work is protected by copyright laws
regardless of whether or not a copyright notice exists. Not that it
would matter much in practice though, because the cases where copyright
matters in open source projects is very fringe.

Cheers.

-- 
Felipe Contreras

Re: [PATCH v2 1/2] send-email: extract execute_cmd from recipients_cmd

[Maxim Cournoyer]

Hi,

Felipe Contreras <felipe.contreras@gmail.com> writes:

> Maxim Cournoyer wrote:
>> Eric Sunshine <sunshine@sunshineco.com> writes:
>> 
>> > On Tue, Apr 25, 2023 at 12:46 PM Maxim Cournoyer
>> > <maxim.cournoyer@gmail.com> wrote:
>> >> diff --git a/git-send-email.perl b/git-send-email.perl
>> >> @@ -2,6 +2,7 @@
>> >>  # Copyright 2002,2005 Greg Kroah-Hartman <greg@kroah.com>
>> >>  # Copyright 2005 Ryan Anderson <ryan@michonline.com>
>> >> +# Copyright 2023 Maxim Cournoyer <maxim.cournoyer@gmail.com>
>> >
>> > Let's avoid this change, please. Many people have worked on this file
>> > over the years -- often making changes far more substantial than those
>> > made by this patch series -- who have not staked such a claim.
>> 
>> I don't mind to drop this hunk if it's unwelcome/not current practice.
>
> In most open source projects the practice is that only the top one or two
> contributors are mentioned.

I see.  I got used adding copyright lines from contributing to GNU Guix,
which retains everyone's minimally substantial changes copyright notices
(if they wish), but that's probably not too common, given even the GNU
maintainer's manual says [0]:

   But if contributors are not all assigning their copyrights to a
   single copyright holder, it can easily happen that one file has
   several copyright holders. Each contributor of nontrivial text is a
   copyright holder.

   In that case, you should always include a copyright notice in the
   name of main copyright holder of the file. You can also include
   copyright notices for other copyright holders as well, and this is a
   good idea for those who have contributed a large amount and for those
   who specifically ask for notices in their names. (Sometimes the
   license on code that you copy in may require preserving certain
   copyright notices.) But you don’t have to include a notice for
   everyone who contributed to the file (which would be rather
   inconvenient).

[0]  https://www.gnu.org/prep/maintain/html_node/Copyright-Notices.html

>> it's still enough of a change to be protected by copyright though, but
>> I don't mind too much.
>
> My understanding is that your work is protected by copyright laws
> regardless of whether or not a copyright notice exists. Not that it
> would matter much in practice though, because the cases where copyright
> matters in open source projects is very fringe.

You are right; written works are automatically protected by copyright.
I think copyright ownership would matter in case the copyright holders
want to intent legal action against an entity violating the license of
the Git project (GPL v2).  Hopefully that'll never be necessary.

-- 
Thanks,
Maxim

Re: [PATCH v2 1/2] send-email: extract execute_cmd from recipients_cmd

[Felipe Contreras]

Maxim Cournoyer wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> > Maxim Cournoyer wrote:
> >> Eric Sunshine <sunshine@sunshineco.com> writes:
> >> 
> >> > On Tue, Apr 25, 2023 at 12:46 PM Maxim Cournoyer
> >> > <maxim.cournoyer@gmail.com> wrote:
> >> >> diff --git a/git-send-email.perl b/git-send-email.perl
> >> >> @@ -2,6 +2,7 @@
> >> >>  # Copyright 2002,2005 Greg Kroah-Hartman <greg@kroah.com>
> >> >>  # Copyright 2005 Ryan Anderson <ryan@michonline.com>
> >> >> +# Copyright 2023 Maxim Cournoyer <maxim.cournoyer@gmail.com>
> >> >
> >> > Let's avoid this change, please. Many people have worked on this file
> >> > over the years -- often making changes far more substantial than those
> >> > made by this patch series -- who have not staked such a claim.
> >> 
> >> I don't mind to drop this hunk if it's unwelcome/not current practice.
> >
> > In most open source projects the practice is that only the top one or two
> > contributors are mentioned.
> 
> I see.  I got used adding copyright lines from contributing to GNU Guix,
> which retains everyone's minimally substantial changes copyright notices
> (if they wish), but that's probably not too common, given even the GNU
> maintainer's manual says [0]:

I would say GNU practices are not what most OSS projects follow.

> >> it's still enough of a change to be protected by copyright though, but
> >> I don't mind too much.
> >
> > My understanding is that your work is protected by copyright laws
> > regardless of whether or not a copyright notice exists. Not that it
> > would matter much in practice though, because the cases where copyright
> > matters in open source projects is very fringe.
> 
> You are right; written works are automatically protected by copyright.
> I think copyright ownership would matter in case the copyright holders
> want to intent legal action against an entity violating the license of
> the Git project (GPL v2).  Hopefully that'll never be necessary.

Yes, that is one instance, but it only matters if your wishes contradict those
of the other copyright holders, that is: they want to sue, and you don't, or
you don't want to sue, and they do. As long as your wishes align the those of
the other developers in the Git community, it doesn't matter.

Cheers.

-- 
Felipe Contreras

Re: [PATCH v4 0/3] send-email: add --header-cmd option

[Maxim Cournoyer]

Hi,

Just checking if everything is OK with this submission?  If not, let me
know.

Thanks you,

-- 
Maxim

Re: [PATCH v4 0/3] send-email: add --header-cmd option

[Eric Sunshine]

On Mon, May 8, 2023 at 11:07 AM Maxim Cournoyer
<maxim.cournoyer@gmail.com> wrote:
> Just checking if everything is OK with this submission?  If not, let me
> know.

According to Junio's most recent "What's Cooking"[1], this patch
series is slated to be merged to his "next" branch, which means there
is nothing else you need to do. Eventually it should make it into
"master" and (presumably) into an actual release. You can monitor the
"What's Cooking" emails to see how the patch series progresses.

[1]: https://lore.kernel.org/git/xmqqmt2ibcq2.fsf@gitster.g/

Re: [PATCH v4 0/3] send-email: add --header-cmd option

[Maxim Cournoyer]

Hi Eric,

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Mon, May 8, 2023 at 11:07 AM Maxim Cournoyer
> <maxim.cournoyer@gmail.com> wrote:
>> Just checking if everything is OK with this submission?  If not, let me
>> know.
>
> According to Junio's most recent "What's Cooking"[1], this patch
> series is slated to be merged to his "next" branch, which means there
> is nothing else you need to do. Eventually it should make it into
> "master" and (presumably) into an actual release. You can monitor the
> "What's Cooking" emails to see how the patch series progresses.
>
> [1]: https://lore.kernel.org/git/xmqqmt2ibcq2.fsf@gitster.g/

OK, great!  Thanks for the update and link.

-- 
Maxim