* [PATCH 0/2] send-email: add --header-cmd option
@ 2023-04-23 12:27 Maxim Cournoyer
2023-04-23 12:27 ` [PATCH 1/2] send-email: extract execute_cmd from recipients_cmd Maxim Cournoyer
` (2 more replies)
0 siblings, 3 replies; 30+ messages in thread
From: Maxim Cournoyer @ 2023-04-23 12:27 UTC (permalink / raw)
To: git; +Cc: 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
^ permalink raw reply [flat|nested] 30+ messages in thread* [PATCH 1/2] send-email: extract execute_cmd from recipients_cmd 2023-04-23 12:27 [PATCH 0/2] send-email: add --header-cmd option Maxim Cournoyer @ 2023-04-23 12:27 ` Maxim Cournoyer 2023-04-24 21:46 ` Junio C Hamano 2023-04-23 12:27 ` [PATCH 2/2] send-email: add --header-cmd option Maxim Cournoyer 2023-04-24 21:45 ` [PATCH 0/2] " Junio C Hamano 2 siblings, 1 reply; 30+ messages in thread From: Maxim Cournoyer @ 2023-04-23 12:27 UTC (permalink / raw) To: git; +Cc: 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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 1/2] send-email: extract execute_cmd from recipients_cmd 2023-04-23 12:27 ` [PATCH 1/2] send-email: extract execute_cmd from recipients_cmd Maxim Cournoyer @ 2023-04-24 21:46 ` Junio C Hamano 2023-04-25 1:55 ` Maxim Cournoyer 0 siblings, 1 reply; 30+ messages in thread From: Junio C Hamano @ 2023-04-24 21:46 UTC (permalink / raw) To: Maxim Cournoyer; +Cc: git 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; > } ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/2] send-email: extract execute_cmd from recipients_cmd 2023-04-24 21:46 ` Junio C Hamano @ 2023-04-25 1:55 ` Maxim Cournoyer 0 siblings, 0 replies; 30+ messages in thread From: Maxim Cournoyer @ 2023-04-25 1:55 UTC (permalink / raw) To: Junio C Hamano; +Cc: git 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 2/2] send-email: add --header-cmd option 2023-04-23 12:27 [PATCH 0/2] send-email: add --header-cmd option Maxim Cournoyer 2023-04-23 12:27 ` [PATCH 1/2] send-email: extract execute_cmd from recipients_cmd Maxim Cournoyer @ 2023-04-23 12:27 ` Maxim Cournoyer 2023-04-24 22:09 ` Junio C Hamano 2023-04-24 21:45 ` [PATCH 0/2] " Junio C Hamano 2 siblings, 1 reply; 30+ messages in thread From: Maxim Cournoyer @ 2023-04-23 12:27 UTC (permalink / raw) To: git; +Cc: 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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 2/2] send-email: add --header-cmd option 2023-04-23 12:27 ` [PATCH 2/2] send-email: add --header-cmd option Maxim Cournoyer @ 2023-04-24 22:09 ` Junio C Hamano 2023-04-25 16:22 ` Maxim Cournoyer 2023-04-25 16:26 ` [PATCH v2 0/2] " Maxim Cournoyer 0 siblings, 2 replies; 30+ messages in thread From: Junio C Hamano @ 2023-04-24 22:09 UTC (permalink / raw) To: Maxim Cournoyer; +Cc: git 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2/2] send-email: add --header-cmd option 2023-04-24 22:09 ` Junio C Hamano @ 2023-04-25 16:22 ` Maxim Cournoyer 2023-04-25 16:29 ` Junio C Hamano 2023-04-25 16:26 ` [PATCH v2 0/2] " Maxim Cournoyer 1 sibling, 1 reply; 30+ messages in thread From: Maxim Cournoyer @ 2023-04-25 16:22 UTC (permalink / raw) To: Juniog C Hamano; +Cc: git 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2/2] send-email: add --header-cmd option 2023-04-25 16:22 ` Maxim Cournoyer @ 2023-04-25 16:29 ` Junio C Hamano 2023-04-25 18:50 ` [PATCH v3 0/3] " Maxim Cournoyer ` (2 more replies) 0 siblings, 3 replies; 30+ messages in thread From: Junio C Hamano @ 2023-04-25 16:29 UTC (permalink / raw) To: Maxim Cournoyer; +Cc: git 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v3 0/3] send-email: add --header-cmd option 2023-04-25 16:29 ` Junio C Hamano @ 2023-04-25 18:50 ` Maxim Cournoyer 2023-04-25 18:50 ` [PATCH v3 1/3] send-email: extract execute_cmd from recipients_cmd Maxim Cournoyer ` (2 more replies) 2023-04-25 18:53 ` [PATCH 2/2] send-email: add --header-cmd option Maxim Cournoyer 2023-05-01 14:38 ` [PATCH v4 0/3] " Maxim Cournoyer 2 siblings, 3 replies; 30+ messages in thread From: Maxim Cournoyer @ 2023-04-25 18:50 UTC (permalink / raw) To: git; +Cc: gitster, 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v3 1/3] send-email: extract execute_cmd from recipients_cmd 2023-04-25 18:50 ` [PATCH v3 0/3] " Maxim Cournoyer @ 2023-04-25 18:50 ` Maxim Cournoyer 2023-04-25 18:50 ` [PATCH v3 2/3] send-email: add --header-cmd option Maxim Cournoyer 2023-04-25 18:50 ` [PATCH v3 3/3] send-email: detect empty blank lines in command output Maxim Cournoyer 2 siblings, 0 replies; 30+ messages in thread From: Maxim Cournoyer @ 2023-04-25 18:50 UTC (permalink / raw) To: git; +Cc: gitster, 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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v3 2/3] send-email: add --header-cmd option 2023-04-25 18:50 ` [PATCH v3 0/3] " Maxim Cournoyer 2023-04-25 18:50 ` [PATCH v3 1/3] send-email: extract execute_cmd from recipients_cmd Maxim Cournoyer @ 2023-04-25 18:50 ` Maxim Cournoyer 2023-04-25 18:50 ` [PATCH v3 3/3] send-email: detect empty blank lines in command output Maxim Cournoyer 2 siblings, 0 replies; 30+ messages in thread From: Maxim Cournoyer @ 2023-04-25 18:50 UTC (permalink / raw) To: git; +Cc: gitster, 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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v3 3/3] send-email: detect empty blank lines in command output 2023-04-25 18:50 ` [PATCH v3 0/3] " Maxim Cournoyer 2023-04-25 18:50 ` [PATCH v3 1/3] send-email: extract execute_cmd from recipients_cmd Maxim Cournoyer 2023-04-25 18:50 ` [PATCH v3 2/3] send-email: add --header-cmd option Maxim Cournoyer @ 2023-04-25 18:50 ` Maxim Cournoyer 2 siblings, 0 replies; 30+ messages in thread From: Maxim Cournoyer @ 2023-04-25 18:50 UTC (permalink / raw) To: git; +Cc: gitster, 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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 2/2] send-email: add --header-cmd option 2023-04-25 16:29 ` Junio C Hamano 2023-04-25 18:50 ` [PATCH v3 0/3] " Maxim Cournoyer @ 2023-04-25 18:53 ` Maxim Cournoyer 2023-05-01 14:38 ` [PATCH v4 0/3] " Maxim Cournoyer 2 siblings, 0 replies; 30+ messages in thread From: Maxim Cournoyer @ 2023-04-25 18:53 UTC (permalink / raw) To: Junio C Hamano; +Cc: git 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4 0/3] send-email: add --header-cmd option 2023-04-25 16:29 ` Junio C Hamano 2023-04-25 18:50 ` [PATCH v3 0/3] " Maxim Cournoyer 2023-04-25 18:53 ` [PATCH 2/2] send-email: add --header-cmd option Maxim Cournoyer @ 2023-05-01 14:38 ` Maxim Cournoyer 2023-05-01 14:38 ` [PATCH v4 1/3] send-email: extract execute_cmd from recipients_cmd Maxim Cournoyer ` (3 more replies) 2 siblings, 4 replies; 30+ messages in thread From: Maxim Cournoyer @ 2023-05-01 14:38 UTC (permalink / raw) To: git; +Cc: Maxim Cournoyer, Junio C Hamano, Eric Sunshine 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4 1/3] send-email: extract execute_cmd from recipients_cmd 2023-05-01 14:38 ` [PATCH v4 0/3] " Maxim Cournoyer @ 2023-05-01 14:38 ` Maxim Cournoyer 2023-05-01 14:38 ` [PATCH v4 2/3] send-email: add --header-cmd, --no-header-cmd options Maxim Cournoyer ` (2 subsequent siblings) 3 siblings, 0 replies; 30+ messages in thread From: Maxim Cournoyer @ 2023-05-01 14:38 UTC (permalink / raw) To: git; +Cc: Maxim Cournoyer, Junio C Hamano, Eric Sunshine 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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v4 2/3] send-email: add --header-cmd, --no-header-cmd options 2023-05-01 14:38 ` [PATCH v4 0/3] " Maxim Cournoyer 2023-05-01 14:38 ` [PATCH v4 1/3] send-email: extract execute_cmd from recipients_cmd Maxim Cournoyer @ 2023-05-01 14:38 ` Maxim Cournoyer 2023-05-01 14:38 ` [PATCH v4 3/3] send-email: detect empty blank lines in command output Maxim Cournoyer 2023-05-08 15:07 ` [PATCH v4 0/3] send-email: add --header-cmd option Maxim Cournoyer 3 siblings, 0 replies; 30+ messages in thread From: Maxim Cournoyer @ 2023-05-01 14:38 UTC (permalink / raw) To: git; +Cc: Maxim Cournoyer, Junio C Hamano, Eric Sunshine 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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v4 3/3] send-email: detect empty blank lines in command output 2023-05-01 14:38 ` [PATCH v4 0/3] " Maxim Cournoyer 2023-05-01 14:38 ` [PATCH v4 1/3] send-email: extract execute_cmd from recipients_cmd Maxim Cournoyer 2023-05-01 14:38 ` [PATCH v4 2/3] send-email: add --header-cmd, --no-header-cmd options Maxim Cournoyer @ 2023-05-01 14:38 ` Maxim Cournoyer 2023-05-08 15:07 ` [PATCH v4 0/3] send-email: add --header-cmd option Maxim Cournoyer 3 siblings, 0 replies; 30+ messages in thread From: Maxim Cournoyer @ 2023-05-01 14:38 UTC (permalink / raw) To: git; +Cc: Maxim Cournoyer, Junio C Hamano, Eric Sunshine 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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v4 0/3] send-email: add --header-cmd option 2023-05-01 14:38 ` [PATCH v4 0/3] " Maxim Cournoyer ` (2 preceding siblings ...) 2023-05-01 14:38 ` [PATCH v4 3/3] send-email: detect empty blank lines in command output Maxim Cournoyer @ 2023-05-08 15:07 ` Maxim Cournoyer 2023-05-08 16:59 ` Eric Sunshine 3 siblings, 1 reply; 30+ messages in thread From: Maxim Cournoyer @ 2023-05-08 15:07 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Eric Sunshine Hi, Just checking if everything is OK with this submission? If not, let me know. Thanks you, -- Maxim ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4 0/3] send-email: add --header-cmd option 2023-05-08 15:07 ` [PATCH v4 0/3] send-email: add --header-cmd option Maxim Cournoyer @ 2023-05-08 16:59 ` Eric Sunshine 2023-05-08 19:18 ` Maxim Cournoyer 0 siblings, 1 reply; 30+ messages in thread From: Eric Sunshine @ 2023-05-08 16:59 UTC (permalink / raw) To: Maxim Cournoyer; +Cc: git, Junio C Hamano 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/ ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4 0/3] send-email: add --header-cmd option 2023-05-08 16:59 ` Eric Sunshine @ 2023-05-08 19:18 ` Maxim Cournoyer 0 siblings, 0 replies; 30+ messages in thread From: Maxim Cournoyer @ 2023-05-08 19:18 UTC (permalink / raw) To: Eric Sunshine; +Cc: git, Junio C Hamano 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 0/2] send-email: add --header-cmd option 2023-04-24 22:09 ` Junio C Hamano 2023-04-25 16:22 ` Maxim Cournoyer @ 2023-04-25 16:26 ` Maxim Cournoyer 2023-04-25 16:26 ` [PATCH v2 1/2] send-email: extract execute_cmd from recipients_cmd Maxim Cournoyer 2023-04-25 16:26 ` [PATCH v2 2/2] send-email: add --header-cmd option Maxim Cournoyer 1 sibling, 2 replies; 30+ messages in thread From: Maxim Cournoyer @ 2023-04-25 16:26 UTC (permalink / raw) To: git; +Cc: gitster, 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 1/2] send-email: extract execute_cmd from recipients_cmd 2023-04-25 16:26 ` [PATCH v2 0/2] " Maxim Cournoyer @ 2023-04-25 16:26 ` Maxim Cournoyer 2023-04-25 17:04 ` Eric Sunshine 2023-04-25 16:26 ` [PATCH v2 2/2] send-email: add --header-cmd option Maxim Cournoyer 1 sibling, 1 reply; 30+ messages in thread From: Maxim Cournoyer @ 2023-04-25 16:26 UTC (permalink / raw) To: git; +Cc: gitster, 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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v2 1/2] send-email: extract execute_cmd from recipients_cmd 2023-04-25 16:26 ` [PATCH v2 1/2] send-email: extract execute_cmd from recipients_cmd Maxim Cournoyer @ 2023-04-25 17:04 ` Eric Sunshine 2023-04-25 19:09 ` Maxim Cournoyer 0 siblings, 1 reply; 30+ messages in thread From: Eric Sunshine @ 2023-04-25 17:04 UTC (permalink / raw) To: Maxim Cournoyer; +Cc: git, gitster 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 1/2] send-email: extract execute_cmd from recipients_cmd 2023-04-25 17:04 ` Eric Sunshine @ 2023-04-25 19:09 ` Maxim Cournoyer 2023-05-02 18:39 ` Felipe Contreras 0 siblings, 1 reply; 30+ messages in thread From: Maxim Cournoyer @ 2023-04-25 19:09 UTC (permalink / raw) To: Eric Sunshine; +Cc: git, gitster 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 1/2] send-email: extract execute_cmd from recipients_cmd 2023-04-25 19:09 ` Maxim Cournoyer @ 2023-05-02 18:39 ` Felipe Contreras 2023-05-02 20:46 ` Maxim Cournoyer 0 siblings, 1 reply; 30+ messages in thread From: Felipe Contreras @ 2023-05-02 18:39 UTC (permalink / raw) To: Maxim Cournoyer, Eric Sunshine; +Cc: git, gitster 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 1/2] send-email: extract execute_cmd from recipients_cmd 2023-05-02 18:39 ` Felipe Contreras @ 2023-05-02 20:46 ` Maxim Cournoyer 2023-05-02 21:50 ` Felipe Contreras 0 siblings, 1 reply; 30+ messages in thread From: Maxim Cournoyer @ 2023-05-02 20:46 UTC (permalink / raw) To: Felipe Contreras; +Cc: Eric Sunshine, git, gitster 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 1/2] send-email: extract execute_cmd from recipients_cmd 2023-05-02 20:46 ` Maxim Cournoyer @ 2023-05-02 21:50 ` Felipe Contreras 0 siblings, 0 replies; 30+ messages in thread From: Felipe Contreras @ 2023-05-02 21:50 UTC (permalink / raw) To: Maxim Cournoyer, Felipe Contreras; +Cc: Eric Sunshine, git, gitster 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 2/2] send-email: add --header-cmd option 2023-04-25 16:26 ` [PATCH v2 0/2] " Maxim Cournoyer 2023-04-25 16:26 ` [PATCH v2 1/2] send-email: extract execute_cmd from recipients_cmd Maxim Cournoyer @ 2023-04-25 16:26 ` Maxim Cournoyer 1 sibling, 0 replies; 30+ messages in thread From: Maxim Cournoyer @ 2023-04-25 16:26 UTC (permalink / raw) To: git; +Cc: gitster, 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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 0/2] send-email: add --header-cmd option 2023-04-23 12:27 [PATCH 0/2] send-email: add --header-cmd option Maxim Cournoyer 2023-04-23 12:27 ` [PATCH 1/2] send-email: extract execute_cmd from recipients_cmd Maxim Cournoyer 2023-04-23 12:27 ` [PATCH 2/2] send-email: add --header-cmd option Maxim Cournoyer @ 2023-04-24 21:45 ` Junio C Hamano 2023-04-25 1:50 ` Maxim Cournoyer 2 siblings, 1 reply; 30+ messages in thread From: Junio C Hamano @ 2023-04-24 21:45 UTC (permalink / raw) To: Maxim Cournoyer; +Cc: git 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 ;-). ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/2] send-email: add --header-cmd option 2023-04-24 21:45 ` [PATCH 0/2] " Junio C Hamano @ 2023-04-25 1:50 ` Maxim Cournoyer 0 siblings, 0 replies; 30+ messages in thread From: Maxim Cournoyer @ 2023-04-25 1:50 UTC (permalink / raw) To: Junio C Hamano; +Cc: git 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2023-05-08 19:19 UTC | newest] Thread overview: 30+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-04-23 12:27 [PATCH 0/2] send-email: add --header-cmd option Maxim Cournoyer 2023-04-23 12:27 ` [PATCH 1/2] send-email: extract execute_cmd from recipients_cmd Maxim Cournoyer 2023-04-24 21:46 ` Junio C Hamano 2023-04-25 1:55 ` Maxim Cournoyer 2023-04-23 12:27 ` [PATCH 2/2] send-email: add --header-cmd option Maxim Cournoyer 2023-04-24 22:09 ` Junio C Hamano 2023-04-25 16:22 ` Maxim Cournoyer 2023-04-25 16:29 ` Junio C Hamano 2023-04-25 18:50 ` [PATCH v3 0/3] " Maxim Cournoyer 2023-04-25 18:50 ` [PATCH v3 1/3] send-email: extract execute_cmd from recipients_cmd Maxim Cournoyer 2023-04-25 18:50 ` [PATCH v3 2/3] send-email: add --header-cmd option Maxim Cournoyer 2023-04-25 18:50 ` [PATCH v3 3/3] send-email: detect empty blank lines in command output Maxim Cournoyer 2023-04-25 18:53 ` [PATCH 2/2] send-email: add --header-cmd option Maxim Cournoyer 2023-05-01 14:38 ` [PATCH v4 0/3] " Maxim Cournoyer 2023-05-01 14:38 ` [PATCH v4 1/3] send-email: extract execute_cmd from recipients_cmd Maxim Cournoyer 2023-05-01 14:38 ` [PATCH v4 2/3] send-email: add --header-cmd, --no-header-cmd options Maxim Cournoyer 2023-05-01 14:38 ` [PATCH v4 3/3] send-email: detect empty blank lines in command output Maxim Cournoyer 2023-05-08 15:07 ` [PATCH v4 0/3] send-email: add --header-cmd option Maxim Cournoyer 2023-05-08 16:59 ` Eric Sunshine 2023-05-08 19:18 ` Maxim Cournoyer 2023-04-25 16:26 ` [PATCH v2 0/2] " Maxim Cournoyer 2023-04-25 16:26 ` [PATCH v2 1/2] send-email: extract execute_cmd from recipients_cmd Maxim Cournoyer 2023-04-25 17:04 ` Eric Sunshine 2023-04-25 19:09 ` Maxim Cournoyer 2023-05-02 18:39 ` Felipe Contreras 2023-05-02 20:46 ` Maxim Cournoyer 2023-05-02 21:50 ` Felipe Contreras 2023-04-25 16:26 ` [PATCH v2 2/2] send-email: add --header-cmd option Maxim Cournoyer 2023-04-24 21:45 ` [PATCH 0/2] " Junio C Hamano 2023-04-25 1:50 ` Maxim Cournoyer
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).