* [PATCH 1/1] send-email: fix transferencoding config option @ 2019-04-09 19:27 Heinrich Schuchardt 2019-04-09 21:58 ` Jonathan Nieder 2019-04-09 22:55 ` [PATCH 1/1] send-email: fix transferencoding config option brian m. carlson 0 siblings, 2 replies; 39+ messages in thread From: Heinrich Schuchardt @ 2019-04-09 19:27 UTC (permalink / raw) To: Junio C Hamano; +Cc: Brian M Carlson, git, Heinrich Schuchardt Since e67a228cd8a ("send-email: automatically determine transfer-encoding") the value of sendmail.transferencoding is ignored because when parsing the configuration $target_xfer_encoding is not initial anymore. Instead of initializing variable $target_xfer_encoding on definition we have to set it to the default value of 'auto' if is initial after parsing the configuration files. The documentation erroneously mentions the option as sendmail.transferEncoding. Fix that typo. Fixes: e67a228cd8a ("send-email: automatically determine transfer-encoding") Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> --- Documentation/git-send-email.txt | 2 +- git-send-email.perl | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt index 1afe9fc858..884e776add 100644 --- a/Documentation/git-send-email.txt +++ b/Documentation/git-send-email.txt @@ -146,7 +146,7 @@ Note that no attempts whatsoever are made to validate the encoding. even more opaque. auto will use 8bit when possible, and quoted-printable otherwise. + -Default is the value of the `sendemail.transferEncoding` configuration +Default is the value of the `sendemail.transferencoding` configuration value; if that is unspecified, default to `auto`. --xmailer:: diff --git a/git-send-email.perl b/git-send-email.perl index 8200d58cdc..0e23193939 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -239,7 +239,7 @@ sub do_edit { my (@suppress_cc); my ($auto_8bit_encoding); my ($compose_encoding); -my $target_xfer_encoding = 'auto'; +my ($target_xfer_encoding); my ($debug_net_smtp) = 0; # Net::SMTP, see send_message() @@ -446,6 +446,8 @@ sub read_config { $smtp_encryption = 'ssl'; } } + + $target_xfer_encoding = 'auto' unless (defined $target_xfer_encoding); } # read configuration from [sendemail "$identity"], fall back on [sendemail] -- 2.20.1 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH 1/1] send-email: fix transferencoding config option 2019-04-09 19:27 [PATCH 1/1] send-email: fix transferencoding config option Heinrich Schuchardt @ 2019-04-09 21:58 ` Jonathan Nieder 2019-04-09 23:39 ` Heinrich Schuchardt 2019-04-10 3:48 ` Junio C Hamano 2019-04-09 22:55 ` [PATCH 1/1] send-email: fix transferencoding config option brian m. carlson 1 sibling, 2 replies; 39+ messages in thread From: Jonathan Nieder @ 2019-04-09 21:58 UTC (permalink / raw) To: Heinrich Schuchardt; +Cc: Junio C Hamano, Brian M Carlson, git (thanks for cc-ing bmc!) Hi, Heinrich Schuchardt wrote: > Subject: send-email: fix transferencoding config option nit: "fix" doesn't tell me what was broken and what you improved about it. Here, I think you mean "respect transferencoding config option". > Since e67a228cd8a ("send-email: automatically determine transfer-encoding") > the value of sendmail.transferencoding is ignored because when parsing > the configuration $target_xfer_encoding is not initial anymore. nit: I was confused when first reading this, since I read "the configuration $target_xfer_encoding" as a single phrase. A comma after "configuration" might help. > Instead of initializing variable $target_xfer_encoding on definition we > have to set it to the default value of 'auto' if is initial after parsing > the configuration files. run-on sentence. I'm having trouble parsing this part. Can you start from the beginning and describe again what this does? In other words, tell me - What is the user-facing effect of the change? What workflow is it part of? - Any risks or complications? - Any technical details that might be interesting to the later reader? - What does this allow me to do that I couldn't do before? The code can speak for itself, so this should primarily focus on the intention behind the change. [...] > --- a/Documentation/git-send-email.txt > +++ b/Documentation/git-send-email.txt > @@ -146,7 +146,7 @@ Note that no attempts whatsoever are made to validate the encoding. > even more opaque. auto will use 8bit when possible, and quoted-printable > otherwise. > + > -Default is the value of the `sendemail.transferEncoding` configuration > +Default is the value of the `sendemail.transferencoding` configuration Unrelated change? [...] > --- a/git-send-email.perl > +++ b/git-send-email.perl > @@ -239,7 +239,7 @@ sub do_edit { > my (@suppress_cc); > my ($auto_8bit_encoding); > my ($compose_encoding); > -my $target_xfer_encoding = 'auto'; > +my ($target_xfer_encoding); > > my ($debug_net_smtp) = 0; # Net::SMTP, see send_message() > > @@ -446,6 +446,8 @@ sub read_config { > $smtp_encryption = 'ssl'; > } > } > + > + $target_xfer_encoding = 'auto' unless (defined $target_xfer_encoding); Makes sense. Is there a way to cover this in tests (t/t9001-send-email.sh) so we can avoid regressing again? The rest looks good. Thanks for noticing, and hope that helps, Jonathan ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/1] send-email: fix transferencoding config option 2019-04-09 21:58 ` Jonathan Nieder @ 2019-04-09 23:39 ` Heinrich Schuchardt 2019-04-10 3:48 ` Junio C Hamano 1 sibling, 0 replies; 39+ messages in thread From: Heinrich Schuchardt @ 2019-04-09 23:39 UTC (permalink / raw) To: Jonathan Nieder; +Cc: Junio C Hamano, Brian M Carlson, git On 4/9/19 11:58 PM, Jonathan Nieder wrote: > (thanks for cc-ing bmc!) > Hi, > > Heinrich Schuchardt wrote: > >> Subject: send-email: fix transferencoding config option > > nit: "fix" doesn't tell me what was broken and what you improved about > it. Here, I think you mean "respect transferencoding config option". > >> Since e67a228cd8a ("send-email: automatically determine transfer-encoding") >> the value of sendmail.transferencoding is ignored because when parsing >> the configuration $target_xfer_encoding is not initial anymore. > > nit: I was confused when first reading this, since I read "the > configuration $target_xfer_encoding" as a single phrase. A comma > after "configuration" might help. > >> Instead of initializing variable $target_xfer_encoding on definition we >> have to set it to the default value of 'auto' if is initial after parsing >> the configuration files. > > run-on sentence. I'm having trouble parsing this part. > > Can you start from the beginning and describe again what this does? > In other words, tell me > > - What is the user-facing effect of the change? What workflow is it > part of? I am working with a repository which uses CRLF line endings. So when sending patches I should use an appropriate encoding. There should be two ways to do it: - call git-send-email with --transfer-encoding base64 - git config --global sendmail.transferencoding base64 Unfortunately the latter method did not show the expected result. The setting was simply ignored. > > - Any risks or complications? None that I am aware of. > > - Any technical details that might be interesting to the later reader? As I tried to explain above the setting is ignored because a variable is initialized too early. > > - What does this allow me to do that I couldn't do before? You can use a global setting for the transfer encoding. > > The code can speak for itself, so this should primarily focus on the > intention behind the change. > > [...] >> --- a/Documentation/git-send-email.txt >> +++ b/Documentation/git-send-email.txt >> @@ -146,7 +146,7 @@ Note that no attempts whatsoever are made to validate the encoding. >> even more opaque. auto will use 8bit when possible, and quoted-printable >> otherwise. >> + >> -Default is the value of the `sendemail.transferEncoding` configuration >> +Default is the value of the `sendemail.transferencoding` configuration > > Unrelated change? > > [...] >> --- a/git-send-email.perl >> +++ b/git-send-email.perl >> @@ -239,7 +239,7 @@ sub do_edit { >> my (@suppress_cc); >> my ($auto_8bit_encoding); >> my ($compose_encoding); >> -my $target_xfer_encoding = 'auto'; >> +my ($target_xfer_encoding); >> >> my ($debug_net_smtp) = 0; # Net::SMTP, see send_message() >> >> @@ -446,6 +446,8 @@ sub read_config { >> $smtp_encryption = 'ssl'; >> } >> } >> + >> + $target_xfer_encoding = 'auto' unless (defined $target_xfer_encoding); > > Makes sense. > > Is there a way to cover this in tests (t/t9001-send-email.sh) so we > can avoid regressing again? I will give it a try. > > The rest looks good. > > Thanks for noticing, and hope that helps, > Jonathan > Thanks for reviewing. Best regards Heinrich ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/1] send-email: fix transferencoding config option 2019-04-09 21:58 ` Jonathan Nieder 2019-04-09 23:39 ` Heinrich Schuchardt @ 2019-04-10 3:48 ` Junio C Hamano 2019-04-10 20:40 ` Heinrich Schuchardt 2019-05-08 8:18 ` Re* " Junio C Hamano 1 sibling, 2 replies; 39+ messages in thread From: Junio C Hamano @ 2019-04-10 3:48 UTC (permalink / raw) To: Jonathan Nieder; +Cc: Heinrich Schuchardt, Brian M Carlson, git Jonathan Nieder <jrnieder@gmail.com> writes: > nit: I was confused when first reading this, since I read "the > configuration $target_xfer_encoding" as a single phrase. A comma > after "configuration" might help. > ... > run-on sentence. I'm having trouble parsing this part. I had the same issue with the wording. Without addressing other parts of the suggestions in the thread (like describing the motivating use case, and protecting this with the test), here is what I have tentatively queued. As all the $scalar variables that are referenced by %config_settings etc. all potentially share this issue, I wonder if it makes sense to have a validation at the very beginning of the read_config sub, something along the lines of.... sub read_config { my ($prefix) = @_; while (my ($k, $v) = each %config_bool_settings) { if (defined $$v) { die "BUG: \%config_bool_settings{$k} is not undef\n"; } } ... similarly for %config_path_settings and %config_settings ... ... then the original code ... foreach my $setting (keys %config_bool_settings) { ... } By the way, if we look more closely to the two callsites of read_config(), however, we realize that Heinrich's patch is a wrong solution to the problem. What happens when "sendemail.<ident>.xferencoding" is not set, but "sendemail.xferencoding" is, with the updated code? The "ah, the configuration file did not define the xfer-encoding, so let's set it to auto" at the end of read_config is done still too early. After checking "sendemail.<ident>.*", the code added by the patch under review assigns 'auto' to $target_xfer_encoding and this assignment causes "sendemail.xferencoding" to be ignored, just like BMC's bug. In other words, the patch is reproducing the same bug it is attempting to fix; a quick-and-dirty and obvious band-aid is to move the assignment of 'auto' further down, outside the read_config() sub, after two calls to the sub is made by the caller, but singling this single variable out is very unsatisfactory. I wonder if we can follow the pattern used by the code to handle the fallback for %config_bool_settings we can see immediately after these two calls to read_config()? That is, each of the element in the %config_* hash is not merely a pointer to where the value is stored, but also knows what the default fallback value should be, and a loop _in the caller of_ read_config(), after it finishes making calls to the read_config function, fills in the missing default? -- >8 -- From: Heinrich Schuchardt <xypron.glpk@gmx.de> Date: Tue, 9 Apr 2019 21:27:33 +0200 Subject: [PATCH] send-email: honor transferencoding config option again Since e67a228cd8a ("send-email: automatically determine transfer-encoding"), the value of sendmail.transferencoding in the configuration file is ignored, because $target_xfer_encoding is already defined read_config sub parses the configuration file. Instead of initializing variable $target_xfer_encoding to 'auto' on definition, we have to set it to the default value of 'auto' if is undefined after parsing the configuration files. Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- git-send-email.perl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/git-send-email.perl b/git-send-email.perl index f4c07908d2..db32cddbde 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -231,7 +231,7 @@ sub do_edit { my (@suppress_cc); my ($auto_8bit_encoding); my ($compose_encoding); -my $target_xfer_encoding = 'auto'; +my ($target_xfer_encoding); my ($debug_net_smtp) = 0; # Net::SMTP, see send_message() @@ -434,6 +434,8 @@ sub read_config { $smtp_encryption = 'ssl'; } } + + $target_xfer_encoding = 'auto' unless (defined $target_xfer_encoding); } # read configuration from [sendemail "$identity"], fall back on [sendemail] -- 2.21.0-313-ge35b8cb8e2 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH 1/1] send-email: fix transferencoding config option 2019-04-10 3:48 ` Junio C Hamano @ 2019-04-10 20:40 ` Heinrich Schuchardt 2019-04-10 22:42 ` brian m. carlson 2019-05-08 8:18 ` Re* " Junio C Hamano 1 sibling, 1 reply; 39+ messages in thread From: Heinrich Schuchardt @ 2019-04-10 20:40 UTC (permalink / raw) To: Junio C Hamano, Jonathan Nieder; +Cc: Brian M Carlson, git On 4/10/19 5:48 AM, Junio C Hamano wrote: > Jonathan Nieder <jrnieder@gmail.com> writes: > >> nit: I was confused when first reading this, since I read "the >> configuration $target_xfer_encoding" as a single phrase. A comma >> after "configuration" might help. >> ... >> run-on sentence. I'm having trouble parsing this part. > > I had the same issue with the wording. Without addressing other > parts of the suggestions in the thread (like describing the > motivating use case, and protecting this with the test), here is > what I have tentatively queued. > > As all the $scalar variables that are referenced by %config_settings > etc. all potentially share this issue, I wonder if it makes sense to > have a validation at the very beginning of the read_config sub, > something along the lines of.... > > sub read_config { > my ($prefix) = @_; > > while (my ($k, $v) = each %config_bool_settings) { > if (defined $$v) { > die "BUG: \%config_bool_settings{$k} is not undef\n"; > } > } > ... similarly for %config_path_settings and %config_settings ... > > ... then the original code ... > foreach my $setting (keys %config_bool_settings) { > ... > } > > By the way, if we look more closely to the two callsites of > read_config(), however, we realize that Heinrich's patch is a wrong > solution to the problem. > > What happens when "sendemail.<ident>.xferencoding" is not set, but > "sendemail.xferencoding" is, with the updated code? The "ah, the > configuration file did not define the xfer-encoding, so let's set it > to auto" at the end of read_config is done still too early. After > checking "sendemail.<ident>.*", the code added by the patch under > review assigns 'auto' to $target_xfer_encoding and this assignment > causes "sendemail.xferencoding" to be ignored, just like BMC's bug. > > In other words, the patch is reproducing the same bug it is > attempting to fix; a quick-and-dirty and obvious band-aid is to move > the assignment of 'auto' further down, outside the read_config() > sub, after two calls to the sub is made by the caller, but singling > this single variable out is very unsatisfactory. > > I wonder if we can follow the pattern used by the code to handle the > fallback for %config_bool_settings we can see immediately after > these two calls to read_config()? That is, each of the element in > the %config_* hash is not merely a pointer to where the value is > stored, but also knows what the default fallback value should be, > and a loop _in the caller of_ read_config(), after it finishes > making calls to the read_config function, fills in the missing > default? Sounds reasonable. But including the tests requested nothing I could easily shoulder. Just a quite different thought: 'auto' should discover a safe transfer encoding. Why does 'auto' not discover that a patch contains carriage returns and should be base64 encoded (see subroutine apply_transfer_encoding())? Best regards Heinrich > > -- >8 -- > From: Heinrich Schuchardt <xypron.glpk@gmx.de> > Date: Tue, 9 Apr 2019 21:27:33 +0200 > Subject: [PATCH] send-email: honor transferencoding config option again > > Since e67a228cd8a ("send-email: automatically determine > transfer-encoding"), the value of sendmail.transferencoding in the > configuration file is ignored, because $target_xfer_encoding is > already defined read_config sub parses the configuration file. > > Instead of initializing variable $target_xfer_encoding to 'auto' on > definition, we have to set it to the default value of 'auto' if is > undefined after parsing the configuration files. > > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > Signed-off-by: Junio C Hamano <gitster@pobox.com> > --- > git-send-email.perl | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/git-send-email.perl b/git-send-email.perl > index f4c07908d2..db32cddbde 100755 > --- a/git-send-email.perl > +++ b/git-send-email.perl > @@ -231,7 +231,7 @@ sub do_edit { > my (@suppress_cc); > my ($auto_8bit_encoding); > my ($compose_encoding); > -my $target_xfer_encoding = 'auto'; > +my ($target_xfer_encoding); > > my ($debug_net_smtp) = 0; # Net::SMTP, see send_message() > > @@ -434,6 +434,8 @@ sub read_config { > $smtp_encryption = 'ssl'; > } > } > + > + $target_xfer_encoding = 'auto' unless (defined $target_xfer_encoding); > } > > # read configuration from [sendemail "$identity"], fall back on [sendemail] > ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/1] send-email: fix transferencoding config option 2019-04-10 20:40 ` Heinrich Schuchardt @ 2019-04-10 22:42 ` brian m. carlson 0 siblings, 0 replies; 39+ messages in thread From: brian m. carlson @ 2019-04-10 22:42 UTC (permalink / raw) To: Heinrich Schuchardt; +Cc: Junio C Hamano, Jonathan Nieder, git [-- Attachment #1: Type: text/plain, Size: 995 bytes --] On Wed, Apr 10, 2019 at 10:40:51PM +0200, Heinrich Schuchardt wrote: > Sounds reasonable. But including the tests requested nothing I could > easily shoulder. > > Just a quite different thought: > > 'auto' should discover a safe transfer encoding. Why does 'auto' not > discover that a patch contains carriage returns and should be base64 > encoded (see subroutine apply_transfer_encoding())? Because nobody thought of it. The original rationale for "auto" was to replace "8bit" with something sane that worked due to long lines as an alternative to forcing people to choose it themselves. And as the commit message says: Choose quoted-printable instead of base64, since base64-encoded plain text is treated as suspicious by some spam filters. I'll try to see if I can write something up to handle this better. If quoted-printable works, I'll pick that; otherwise, I'll choose base64. -- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 868 bytes --] ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re* [PATCH 1/1] send-email: fix transferencoding config option 2019-04-10 3:48 ` Junio C Hamano 2019-04-10 20:40 ` Heinrich Schuchardt @ 2019-05-08 8:18 ` Junio C Hamano 2019-05-08 8:20 ` [PATCH 2/2] send-email: honor transferencoding config option again Junio C Hamano 2019-05-08 10:13 ` Re* [PATCH 1/1] send-email: fix transferencoding config option Junio C Hamano 1 sibling, 2 replies; 39+ messages in thread From: Junio C Hamano @ 2019-05-08 8:18 UTC (permalink / raw) To: Heinrich Schuchardt; +Cc: Brian M Carlson, git, Jonathan Nieder Junio C Hamano <gitster@pobox.com> writes: > I wonder if we can follow the pattern used by the code to handle the > fallback for %config_bool_settings we can see immediately after > these two calls to read_config()? That is, each of the element in > the %config_* hash is not merely a pointer to where the value is > stored, but also knows what the default fallback value should be, > and a loop _in the caller of_ read_config(), after it finishes > making calls to the read_config function, fills in the missing > default? So, here is a two-patch series that tries to do so, primarily done to gauge if there still is the level of interest needed to make it worth for us to pursue this topic. Here is the first one; I'll send the second one that takes advantage of this change separately (but it should be trivial to imagine what that step would involve). -- >8 -- Subject: [PATCH 1/2] send-email: update the mechanism to set default configuration values The program has a good mechanism to specify the fallback default values for boolean configuration variables after two invocations of read_config() for "sendmail.$ident.$var" and "sendemail.$var" have not found any configuration. Imitate it so that we can set the default values for non-boolean variables as well. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- git-send-email.perl | 51 ++++++++++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 22 deletions(-) diff --git a/git-send-email.perl b/git-send-email.perl index f4c07908d2..ca7faff094 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -250,28 +250,28 @@ sub do_edit { ); my %config_settings = ( - "smtpserver" => \$smtp_server, - "smtpserverport" => \$smtp_server_port, - "smtpserveroption" => \@smtp_server_options, - "smtpuser" => \$smtp_authuser, - "smtppass" => \$smtp_authpass, - "smtpdomain" => \$smtp_domain, - "smtpauth" => \$smtp_auth, - "smtpbatchsize" => \$batch_size, - "smtprelogindelay" => \$relogin_delay, - "to" => \@initial_to, - "tocmd" => \$to_cmd, - "cc" => \@initial_cc, - "cccmd" => \$cc_cmd, - "aliasfiletype" => \$aliasfiletype, - "bcc" => \@bcclist, - "suppresscc" => \@suppress_cc, - "envelopesender" => \$envelope_sender, - "confirm" => \$confirm, - "from" => \$sender, - "assume8bitencoding" => \$auto_8bit_encoding, - "composeencoding" => \$compose_encoding, - "transferencoding" => \$target_xfer_encoding, + "smtpserver" => [\$smtp_server], + "smtpserverport" => [\$smtp_server_port], + "smtpserveroption" => [\@smtp_server_options], + "smtpuser" => [\$smtp_authuser], + "smtppass" => [\$smtp_authpass], + "smtpdomain" => [\$smtp_domain], + "smtpauth" => [\$smtp_auth], + "smtpbatchsize" => [\$batch_size], + "smtprelogindelay" => [\$relogin_delay], + "to" => [\@initial_to], + "tocmd" => [\$to_cmd], + "cc" => [\@initial_cc], + "cccmd" => [\$cc_cmd], + "aliasfiletype" => [\$aliasfiletype], + "bcc" => [\@bcclist], + "suppresscc" => [\@suppress_cc], + "envelopesender" => [\$envelope_sender], + "confirm" => [\$confirm], + "from" => [\$sender], + "assume8bitencoding" => [\$auto_8bit_encoding], + "composeencoding" => [\$compose_encoding], + "transferencoding" => [\$target_xfer_encoding], ); my %config_path_settings = ( @@ -446,6 +446,13 @@ sub read_config { ${$setting->[0]} = $setting->[1] unless (defined (${$setting->[0]})); } +# fall back to builtin defaults +for my $setting (values %config_settings) { + if (@$setting == 2 && !defined (${$setting->[0]})) { + ${$setting->[0]} = $setting->[1]; + } +} + # 'default' encryption is none -- this only prevents a warning $smtp_encryption = '' unless (defined $smtp_encryption); -- 2.21.0-777-g83232e3864 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 2/2] send-email: honor transferencoding config option again 2019-05-08 8:18 ` Re* " Junio C Hamano @ 2019-05-08 8:20 ` Junio C Hamano 2019-05-08 10:13 ` Re* [PATCH 1/1] send-email: fix transferencoding config option Junio C Hamano 1 sibling, 0 replies; 39+ messages in thread From: Junio C Hamano @ 2019-05-08 8:20 UTC (permalink / raw) To: Heinrich Schuchardt; +Cc: Brian M Carlson, git, Jonathan Nieder Since e67a228cd8a ("send-email: automatically determine transfer-encoding"), the value of sendmail.transferencoding in the configuration file is ignored, because $target_xfer_encoding is already defined read_config sub parses the configuration file. Instead of initializing variable $target_xfer_encoding to 'auto' on definition, we have to set it to the default value of 'auto' if is undefined after calling read_config() twice to parsing the configuration files for "sendemail.transferencoding" and "sendemail.$ident.transferencoding". This is made trivial by the previous change. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- * And this is the second one. Totally untested, though. git-send-email.perl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/git-send-email.perl b/git-send-email.perl index ca7faff094..7458a0d1ef 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -231,7 +231,7 @@ sub do_edit { my (@suppress_cc); my ($auto_8bit_encoding); my ($compose_encoding); -my $target_xfer_encoding = 'auto'; +my ($target_xfer_encoding); my ($debug_net_smtp) = 0; # Net::SMTP, see send_message() @@ -271,7 +271,7 @@ sub do_edit { "from" => [\$sender], "assume8bitencoding" => [\$auto_8bit_encoding], "composeencoding" => [\$compose_encoding], - "transferencoding" => [\$target_xfer_encoding], + "transferencoding" => [\$target_xfer_encoding, 'auto'], ); my %config_path_settings = ( -- 2.21.0-777-g83232e3864 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: Re* [PATCH 1/1] send-email: fix transferencoding config option 2019-05-08 8:18 ` Re* " Junio C Hamano 2019-05-08 8:20 ` [PATCH 2/2] send-email: honor transferencoding config option again Junio C Hamano @ 2019-05-08 10:13 ` Junio C Hamano 2019-05-08 10:56 ` [PATCH v2 0/2] send-email: set xfer encoding correctly Junio C Hamano ` (2 more replies) 1 sibling, 3 replies; 39+ messages in thread From: Junio C Hamano @ 2019-05-08 10:13 UTC (permalink / raw) To: Heinrich Schuchardt; +Cc: Brian M Carlson, git, Jonathan Nieder Junio C Hamano <gitster@pobox.com> writes: > So, here is a two-patch series that tries to do so, primarily done > to gauge if there still is the level of interest needed to make it > worth for us to pursue this topic. Here is the first one; I'll send > the second one that takes advantage of this change separately (but > it should be trivial to imagine what that step would involve). > > -- >8 -- > Subject: [PATCH 1/2] send-email: update the mechanism to set default configuration values > > The program has a good mechanism to specify the fallback default > values for boolean configuration variables after two invocations of > read_config() for "sendmail.$ident.$var" and "sendemail.$var" have > not found any configuration. Imitate it so that we can set the > default values for non-boolean variables as well. > > Signed-off-by: Junio C Hamano <gitster@pobox.com> > --- > git-send-email.perl | 51 ++++++++++++++++++++++++++------------------- > 1 file changed, 29 insertions(+), 22 deletions(-) This one was embarrassingly buggy, and needs the following squashed in. Sorry about that. git-send-email.perl | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/git-send-email.perl b/git-send-email.perl index ca7faff094..831947c7ed 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -411,7 +411,7 @@ sub read_config { } foreach my $setting (keys %config_settings) { - my $target = $config_settings{$setting}; + my $target = $config_settings{$setting}->[0]; next if $setting eq "to" and defined $no_to; next if $setting eq "cc" and defined $no_cc; next if $setting eq "bcc" and defined $no_bcc; @@ -447,10 +447,13 @@ sub read_config { } # fall back to builtin defaults -for my $setting (values %config_settings) { - if (@$setting == 2 && !defined (${$setting->[0]})) { - ${$setting->[0]} = $setting->[1]; - } +while (my ($name, $setting) = each %config_settings) { + next unless @$setting == 2; + + my ($target, $default) = @$setting; + if (ref($target) eq "SCALAR") { + $$target = $default unless defined $target; + } # elsif ... for other types later. } # 'default' encryption is none -- this only prevents a warning ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v2 0/2] send-email: set xfer encoding correctly 2019-05-08 10:13 ` Re* [PATCH 1/1] send-email: fix transferencoding config option Junio C Hamano @ 2019-05-08 10:56 ` Junio C Hamano 2019-05-09 11:48 ` [PATCH v3 0/3] send-email: fix cli->config parsing crazyness Ævar Arnfjörð Bjarmason ` (3 more replies) 2019-05-08 10:56 ` [PATCH v2 1/2] send-email: update the mechanism to set default configuration values Junio C Hamano 2019-05-08 10:56 ` [PATCH v2 2/2] send-email: honor transferencoding config option again Junio C Hamano 2 siblings, 4 replies; 39+ messages in thread From: Junio C Hamano @ 2019-05-08 10:56 UTC (permalink / raw) To: git; +Cc: Heinrich Schuchardt, Brian M Carlson, Jonathan Nieder Earlier I sent a series of "oops that was wrong" which was rather embarrassing. This is what I have to replace Heinrich's patch that was not quite right, while making it harder to make the same mistake while defining fallback default values to configuration variables. Junio C Hamano (2): send-email: update the mechanism to set default configuration values send-email: honor transferencoding config option again git-send-email.perl | 58 ++++++++++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 24 deletions(-) -- 2.21.0-777-g83232e3864 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 0/3] send-email: fix cli->config parsing crazyness 2019-05-08 10:56 ` [PATCH v2 0/2] send-email: set xfer encoding correctly Junio C Hamano @ 2019-05-09 11:48 ` Ævar Arnfjörð Bjarmason 2019-05-10 13:50 ` Junio C Hamano 2019-05-09 11:48 ` [PATCH v3 1/3] send-email: move the read_config() function above getopts Ævar Arnfjörð Bjarmason ` (2 subsequent siblings) 3 siblings, 1 reply; 39+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2019-05-09 11:48 UTC (permalink / raw) To: git Cc: Junio C Hamano, Heinrich Schuchardt, Brian M Carlson, Jonathan Nieder, Eric Sunshine, Ævar Arnfjörð Bjarmason This is a proposed replacement for Junio's version of the sendemail.transferencoding bugfix. As explained in 3/3 I think the root cause is that we're needlessly doing the config->cli parsing in the wrong order, so let's just fix that. It fixes the bug at hand, and makes the coge less fragile for future maintenance. Ævar Arnfjörð Bjarmason (3): send-email: move the read_config() function above getopts send-email: rename the @bcclist variable for consistency send-email: do defaults -> config -> getopt in that order git-send-email.perl | 179 ++++++++++++++++++++++-------------------- t/t9001-send-email.sh | 13 ++- 2 files changed, 105 insertions(+), 87 deletions(-) -- 2.21.0.1020.gf2820cf01a ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 0/3] send-email: fix cli->config parsing crazyness 2019-05-09 11:48 ` [PATCH v3 0/3] send-email: fix cli->config parsing crazyness Ævar Arnfjörð Bjarmason @ 2019-05-10 13:50 ` Junio C Hamano 0 siblings, 0 replies; 39+ messages in thread From: Junio C Hamano @ 2019-05-10 13:50 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: git, Heinrich Schuchardt, Brian M Carlson, Jonathan Nieder, Eric Sunshine Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > This is a proposed replacement for Junio's version of the > ... the > root cause is that we're needlessly doing the config->cli parsing in > the wrong order, so let's just fix that. Yup, that's absolutely the right approach. I just wanted to avoid restructuring that heavily, but I have no problem as long as it is somebody else, and somebody who is more competent in writing Perl than I am, doing the heavy lifting ;-) ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 1/3] send-email: move the read_config() function above getopts 2019-05-08 10:56 ` [PATCH v2 0/2] send-email: set xfer encoding correctly Junio C Hamano 2019-05-09 11:48 ` [PATCH v3 0/3] send-email: fix cli->config parsing crazyness Ævar Arnfjörð Bjarmason @ 2019-05-09 11:48 ` Ævar Arnfjörð Bjarmason 2019-05-09 11:48 ` [PATCH v3 2/3] send-email: rename the @bcclist variable for consistency Ævar Arnfjörð Bjarmason 2019-05-09 11:48 ` [PATCH v3 3/3] send-email: do defaults -> config -> getopt in that order Ævar Arnfjörð Bjarmason 3 siblings, 0 replies; 39+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2019-05-09 11:48 UTC (permalink / raw) To: git Cc: Junio C Hamano, Heinrich Schuchardt, Brian M Carlson, Jonathan Nieder, Eric Sunshine, Ævar Arnfjörð Bjarmason This is in preparation for a later change where we'll read the config first before parsing command-line options. As the move detection will show no lines (except one line of comment) is changed here, just moved around. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- git-send-email.perl | 97 ++++++++++++++++++++++----------------------- 1 file changed, 48 insertions(+), 49 deletions(-) diff --git a/git-send-email.perl b/git-send-email.perl index 24859a7bc3..0d87ed2b5d 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -315,6 +315,54 @@ sub signal_handler { $SIG{TERM} = \&signal_handler; $SIG{INT} = \&signal_handler; +# Read our sendemail.* config +sub read_config { + my ($prefix) = @_; + + foreach my $setting (keys %config_bool_settings) { + my $target = $config_bool_settings{$setting}->[0]; + $$target = Git::config_bool(@repo, "$prefix.$setting") unless (defined $$target); + } + + foreach my $setting (keys %config_path_settings) { + my $target = $config_path_settings{$setting}; + if (ref($target) eq "ARRAY") { + unless (@$target) { + my @values = Git::config_path(@repo, "$prefix.$setting"); + @$target = @values if (@values && defined $values[0]); + } + } + else { + $$target = Git::config_path(@repo, "$prefix.$setting") unless (defined $$target); + } + } + + foreach my $setting (keys %config_settings) { + my $target = $config_settings{$setting}; + next if $setting eq "to" and defined $no_to; + next if $setting eq "cc" and defined $no_cc; + next if $setting eq "bcc" and defined $no_bcc; + if (ref($target) eq "ARRAY") { + unless (@$target) { + my @values = Git::config(@repo, "$prefix.$setting"); + @$target = @values if (@values && defined $values[0]); + } + } + else { + $$target = Git::config(@repo, "$prefix.$setting") unless (defined $$target); + } + } + + if (!defined $smtp_encryption) { + my $enc = Git::config(@repo, "$prefix.smtpencryption"); + if (defined $enc) { + $smtp_encryption = $enc; + } elsif (Git::config_bool(@repo, "$prefix.smtpssl")) { + $smtp_encryption = 'ssl'; + } + } +} + # Begin by accumulating all the variables (defined above), that we will end up # needing, first, from the command line: @@ -399,55 +447,6 @@ sub signal_handler { "(via command-line or configuration option)\n") if defined $relogin_delay and not defined $batch_size; -# Now, let's fill any that aren't set in with defaults: - -sub read_config { - my ($prefix) = @_; - - foreach my $setting (keys %config_bool_settings) { - my $target = $config_bool_settings{$setting}->[0]; - $$target = Git::config_bool(@repo, "$prefix.$setting") unless (defined $$target); - } - - foreach my $setting (keys %config_path_settings) { - my $target = $config_path_settings{$setting}; - if (ref($target) eq "ARRAY") { - unless (@$target) { - my @values = Git::config_path(@repo, "$prefix.$setting"); - @$target = @values if (@values && defined $values[0]); - } - } - else { - $$target = Git::config_path(@repo, "$prefix.$setting") unless (defined $$target); - } - } - - foreach my $setting (keys %config_settings) { - my $target = $config_settings{$setting}; - next if $setting eq "to" and defined $no_to; - next if $setting eq "cc" and defined $no_cc; - next if $setting eq "bcc" and defined $no_bcc; - if (ref($target) eq "ARRAY") { - unless (@$target) { - my @values = Git::config(@repo, "$prefix.$setting"); - @$target = @values if (@values && defined $values[0]); - } - } - else { - $$target = Git::config(@repo, "$prefix.$setting") unless (defined $$target); - } - } - - if (!defined $smtp_encryption) { - my $enc = Git::config(@repo, "$prefix.smtpencryption"); - if (defined $enc) { - $smtp_encryption = $enc; - } elsif (Git::config_bool(@repo, "$prefix.smtpssl")) { - $smtp_encryption = 'ssl'; - } - } -} - # read configuration from [sendemail "$identity"], fall back on [sendemail] $identity = Git::config(@repo, "sendemail.identity") unless (defined $identity); read_config("sendemail.$identity") if (defined $identity); -- 2.21.0.1020.gf2820cf01a ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v3 2/3] send-email: rename the @bcclist variable for consistency 2019-05-08 10:56 ` [PATCH v2 0/2] send-email: set xfer encoding correctly Junio C Hamano 2019-05-09 11:48 ` [PATCH v3 0/3] send-email: fix cli->config parsing crazyness Ævar Arnfjörð Bjarmason 2019-05-09 11:48 ` [PATCH v3 1/3] send-email: move the read_config() function above getopts Ævar Arnfjörð Bjarmason @ 2019-05-09 11:48 ` Ævar Arnfjörð Bjarmason 2019-05-09 11:48 ` [PATCH v3 3/3] send-email: do defaults -> config -> getopt in that order Ævar Arnfjörð Bjarmason 3 siblings, 0 replies; 39+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2019-05-09 11:48 UTC (permalink / raw) To: git Cc: Junio C Hamano, Heinrich Schuchardt, Brian M Carlson, Jonathan Nieder, Eric Sunshine, Ævar Arnfjörð Bjarmason The "to" and "cc" variables are named @initial_{to,cc}, let's rename this one to match them. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- git-send-email.perl | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/git-send-email.perl b/git-send-email.perl index 0d87ed2b5d..48ed18a85c 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -177,7 +177,7 @@ sub format_2822_time { my $re_encoded_word = qr/=\?($re_token)\?($re_token)\?($re_encoded_text)\?=/; # Variables we fill in automatically, or via prompting: -my (@to,$no_to,@initial_to,@cc,$no_cc,@initial_cc,@bcclist,$no_bcc,@xh, +my (@to,$no_to,@initial_to,@cc,$no_cc,@initial_cc,@initial_bcc,$no_bcc,@xh, $initial_in_reply_to,$reply_to,$initial_subject,@files, $author,$sender,$smtp_authpass,$annotate,$use_xmailer,$compose,$time); @@ -272,7 +272,7 @@ sub do_edit { "cc" => \@initial_cc, "cccmd" => \$cc_cmd, "aliasfiletype" => \$aliasfiletype, - "bcc" => \@bcclist, + "bcc" => \@initial_bcc, "suppresscc" => \@suppress_cc, "envelopesender" => \$envelope_sender, "confirm" => \$confirm, @@ -383,7 +383,7 @@ sub read_config { "no-to" => \$no_to, "cc=s" => \@initial_cc, "no-cc" => \$no_cc, - "bcc=s" => \@bcclist, + "bcc=s" => \@initial_bcc, "no-bcc" => \$no_bcc, "chain-reply-to!" => \$chain_reply_to, "no-chain-reply-to" => sub {$chain_reply_to = 0}, @@ -940,7 +940,7 @@ sub expand_one_alias { @initial_to = process_address_list(@initial_to); @initial_cc = process_address_list(@initial_cc); -@bcclist = process_address_list(@bcclist); +@initial_bcc = process_address_list(@initial_bcc); if ($thread && !defined $initial_in_reply_to && $prompting) { $initial_in_reply_to = ask( @@ -1363,7 +1363,7 @@ sub send_message { } @cc); my $to = join (",\n\t", @recipients); - @recipients = unique_email_list(@recipients,@cc,@bcclist); + @recipients = unique_email_list(@recipients,@cc,@initial_bcc); @recipients = (map { extract_valid_address_or_die($_) } @recipients); my $date = format_2822_time($time++); my $gitversion = '@@GIT_VERSION@@'; -- 2.21.0.1020.gf2820cf01a ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v3 3/3] send-email: do defaults -> config -> getopt in that order 2019-05-08 10:56 ` [PATCH v2 0/2] send-email: set xfer encoding correctly Junio C Hamano ` (2 preceding siblings ...) 2019-05-09 11:48 ` [PATCH v3 2/3] send-email: rename the @bcclist variable for consistency Ævar Arnfjörð Bjarmason @ 2019-05-09 11:48 ` Ævar Arnfjörð Bjarmason 2019-05-09 18:04 ` Eric Sunshine ` (3 more replies) 3 siblings, 4 replies; 39+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2019-05-09 11:48 UTC (permalink / raw) To: git Cc: Junio C Hamano, Heinrich Schuchardt, Brian M Carlson, Jonathan Nieder, Eric Sunshine, Ævar Arnfjörð Bjarmason Change the git-send-email command-line argument parsing and config reading code to parse those two in the right order. I.e. first we set our hardcoded defaults, then we read our config, and finally we read the command-line, with later sets overriding earlier sets. This fixes a bug introduced in e67a228cd8 ("send-email: automatically determine transfer-encoding", 2018-07-08). That change broke the broke the reading of sendmail.transferencoding because it wasn't careful to update our fragile code dealing with doing this in the previous "defaults -> getopt -> config" order.. But as we can see from the history for this file doing it this way was never what we actually wanted, it just something we grew organically as of 5483c71d7a ("git-send-email: make options easier to configure.", 2007-06-27) and have been dealing with the fallout since, e.g. in 463b0ea22b ("send-email: Fix %config_path_settings handling", 2011-10-14). As can be seen in this change the only place where we actually want to do something clever is with the to/cc/bcc variables, where setting them on the command-line (or using --no-{to,cc,bcc}) should clear out values we grab from the config. All the rest are things where the command-line should simply override the config values, and by reading the config first the config code doesn't need all this "let's not set it was on the command-line" special-casing, as [1] shows we'd otherwise need to care about the difference between whether something was a default or present in config to fix the bug in e67a228cd8. 1. https://public-inbox.org/git/20190508105607.178244-2-gitster@pobox.com/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- git-send-email.perl | 92 +++++++++++++++++++++++-------------------- t/t9001-send-email.sh | 13 +++++- 2 files changed, 62 insertions(+), 43 deletions(-) diff --git a/git-send-email.perl b/git-send-email.perl index 48ed18a85c..fab255249f 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -177,11 +177,15 @@ sub format_2822_time { my $re_encoded_word = qr/=\?($re_token)\?($re_token)\?($re_encoded_text)\?=/; # Variables we fill in automatically, or via prompting: -my (@to,$no_to,@initial_to,@cc,$no_cc,@initial_cc,@initial_bcc,$no_bcc,@xh, +my (@to,@cc,,@xh,$envelope_sender, $initial_in_reply_to,$reply_to,$initial_subject,@files, - $author,$sender,$smtp_authpass,$annotate,$use_xmailer,$compose,$time); - -my $envelope_sender; + $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); +my (@config_to, @getopt_to); +my (@config_cc, @getopt_cc); +my (@config_bcc, @getopt_bcc); # Example reply to: #$initial_in_reply_to = ''; #<20050203173208.GA23964@foobar.com>'; @@ -228,33 +232,37 @@ sub do_edit { } # Variables with corresponding config settings -my ($thread, $chain_reply_to, $suppress_from, $signed_off_by_cc); +my ($suppress_from, $signed_off_by_cc); my ($cover_cc, $cover_to); my ($to_cmd, $cc_cmd); my ($smtp_server, $smtp_server_port, @smtp_server_options); my ($smtp_authuser, $smtp_encryption, $smtp_ssl_cert_path); my ($batch_size, $relogin_delay); my ($identity, $aliasfiletype, @alias_files, $smtp_domain, $smtp_auth); -my ($validate, $confirm); +my ($confirm); my (@suppress_cc); my ($auto_8bit_encoding); my ($compose_encoding); -my $target_xfer_encoding = 'auto'; - +# Variables with corresponding config settings & hardcoded defaults my ($debug_net_smtp) = 0; # Net::SMTP, see send_message() +my $thread = 1; +my $chain_reply_to = 0; +my $use_xmailer = 1; +my $validate = 1; +my $target_xfer_encoding = 'auto'; my %config_bool_settings = ( - "thread" => [\$thread, 1], - "chainreplyto" => [\$chain_reply_to, 0], - "suppressfrom" => [\$suppress_from, undef], - "signedoffbycc" => [\$signed_off_by_cc, undef], - "cccover" => [\$cover_cc, undef], - "tocover" => [\$cover_to, undef], - "signedoffcc" => [\$signed_off_by_cc, undef], # Deprecated - "validate" => [\$validate, 1], - "multiedit" => [\$multiedit, undef], - "annotate" => [\$annotate, undef], - "xmailer" => [\$use_xmailer, 1] + "thread" => \$thread, + "chainreplyto" => \$chain_reply_to, + "suppressfrom" => \$suppress_from, + "signedoffbycc" => \$signed_off_by_cc, + "cccover" => \$cover_cc, + "tocover" => \$cover_to, + "signedoffcc" => \$signed_off_by_cc, + "validate" => \$validate, + "multiedit" => \$multiedit, + "annotate" => \$annotate, + "xmailer" => \$use_xmailer, ); my %config_settings = ( @@ -267,12 +275,12 @@ sub do_edit { "smtpauth" => \$smtp_auth, "smtpbatchsize" => \$batch_size, "smtprelogindelay" => \$relogin_delay, - "to" => \@initial_to, + "to" => \@config_to, "tocmd" => \$to_cmd, - "cc" => \@initial_cc, + "cc" => \@config_cc, "cccmd" => \$cc_cmd, "aliasfiletype" => \$aliasfiletype, - "bcc" => \@initial_bcc, + "bcc" => \@config_bcc, "suppresscc" => \@suppress_cc, "envelopesender" => \$envelope_sender, "confirm" => \$confirm, @@ -320,8 +328,9 @@ sub read_config { my ($prefix) = @_; foreach my $setting (keys %config_bool_settings) { - my $target = $config_bool_settings{$setting}->[0]; - $$target = Git::config_bool(@repo, "$prefix.$setting") unless (defined $$target); + my $target = $config_bool_settings{$setting}; + my $v = Git::config_bool(@repo, "$prefix.$setting"); + $$target = $v if defined $v; } foreach my $setting (keys %config_path_settings) { @@ -333,15 +342,13 @@ sub read_config { } } else { - $$target = Git::config_path(@repo, "$prefix.$setting") unless (defined $$target); + my $v = Git::config_path(@repo, "$prefix.$setting"); + $$target = $v if defined $v; } } foreach my $setting (keys %config_settings) { my $target = $config_settings{$setting}; - next if $setting eq "to" and defined $no_to; - next if $setting eq "cc" and defined $no_cc; - next if $setting eq "bcc" and defined $no_bcc; if (ref($target) eq "ARRAY") { unless (@$target) { my @values = Git::config(@repo, "$prefix.$setting"); @@ -349,7 +356,8 @@ sub read_config { } } else { - $$target = Git::config(@repo, "$prefix.$setting") unless (defined $$target); + my $v = Git::config(@repo, "$prefix.$setting"); + $$target = $v if defined $v; } } @@ -363,6 +371,11 @@ sub read_config { } } +$identity = Git::config(@repo, "sendemail.identity"); +read_config("sendemail.$identity") if defined $identity; +read_config("sendemail"); +read_config("sendemail"); + # Begin by accumulating all the variables (defined above), that we will end up # needing, first, from the command line: @@ -378,12 +391,12 @@ sub read_config { "in-reply-to=s" => \$initial_in_reply_to, "reply-to=s" => \$reply_to, "subject=s" => \$initial_subject, - "to=s" => \@initial_to, + "to=s" => \@getopt_to, "to-cmd=s" => \$to_cmd, "no-to" => \$no_to, - "cc=s" => \@initial_cc, + "cc=s" => \@getopt_cc, "no-cc" => \$no_cc, - "bcc=s" => \@initial_bcc, + "bcc=s" => \@getopt_bcc, "no-bcc" => \$no_bcc, "chain-reply-to!" => \$chain_reply_to, "no-chain-reply-to" => sub {$chain_reply_to = 0}, @@ -434,6 +447,11 @@ sub read_config { "git-completion-helper" => \$git_completion_helper, ); +# Munge any "either config or getopt, not both" variables +my @initial_to = @getopt_to ? @getopt_to : ($no_to ? () : @config_to); +my @initial_cc = @getopt_cc ? @getopt_cc : ($no_cc ? () : @config_cc); +my @initial_bcc = @getopt_bcc ? @getopt_bcc : ($no_bcc ? () : @config_bcc); + usage() if $help; completion_helper() if $git_completion_helper; unless ($rc) { @@ -447,16 +465,6 @@ sub read_config { "(via command-line or configuration option)\n") if defined $relogin_delay and not defined $batch_size; -# read configuration from [sendemail "$identity"], fall back on [sendemail] -$identity = Git::config(@repo, "sendemail.identity") unless (defined $identity); -read_config("sendemail.$identity") if (defined $identity); -read_config("sendemail"); - -# fall back on builtin bool defaults -foreach my $setting (values %config_bool_settings) { - ${$setting->[0]} = $setting->[1] unless (defined (${$setting->[0]})); -} - # 'default' encryption is none -- this only prevents a warning $smtp_encryption = '' unless (defined $smtp_encryption); diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 1e3ac3c384..1b18201ce2 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -1461,7 +1461,18 @@ test_expect_success $PREREQ '--transfer-encoding overrides sendemail.transferEnc test -z "$(ls msgtxt*)" ' -test_expect_success $PREREQ 'sendemail.transferencoding=8bit' ' +test_expect_success $PREREQ 'sendemail.transferencoding=8bit via config' ' + clean_fake_sendmail && + git -c sendemail.transferencoding=8bit send-email \ + --smtp-server="$(pwd)/fake.sendmail" \ + email-using-8bit \ + 2>errors >out && + sed '1,/^$/d' msgtxt1 >actual && + sed '1,/^$/d' email-using-8bit >expected && + test_cmp expected actual +' + +test_expect_success $PREREQ 'sendemail.transferencoding=8bit via cli' ' clean_fake_sendmail && git send-email \ --transfer-encoding=8bit \ -- 2.21.0.1020.gf2820cf01a ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH v3 3/3] send-email: do defaults -> config -> getopt in that order 2019-05-09 11:48 ` [PATCH v3 3/3] send-email: do defaults -> config -> getopt in that order Ævar Arnfjörð Bjarmason @ 2019-05-09 18:04 ` Eric Sunshine 2019-05-13 8:46 ` Junio C Hamano 2019-05-09 23:51 ` brian m. carlson ` (2 subsequent siblings) 3 siblings, 1 reply; 39+ messages in thread From: Eric Sunshine @ 2019-05-09 18:04 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: Git List, Junio C Hamano, Heinrich Schuchardt, Brian M Carlson, Jonathan Nieder On Thu, May 9, 2019 at 7:48 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > Change the git-send-email command-line argument parsing and config > reading code to parse those two in the right order. I.e. first we set > our hardcoded defaults, then we read our config, and finally we read > the command-line, with later sets overriding earlier sets. > > This fixes a bug introduced in e67a228cd8 ("send-email: automatically > determine transfer-encoding", 2018-07-08). That change broke the broke s/broke the broke/broke/ > the reading of sendmail.transferencoding because it wasn't careful to > update our fragile code dealing with doing this in the previous > "defaults -> getopt -> config" order.. s/\.\.$/./ > But as we can see from the history for this file doing it this way was > never what we actually wanted, it just something we grew organically s/it/it's/ > as of 5483c71d7a ("git-send-email: make options easier to configure.", > 2007-06-27) and have been dealing with the fallout since, e.g. in > 463b0ea22b ("send-email: Fix %config_path_settings handling", > 2011-10-14). > > As can be seen in this change the only place where we actually want to > do something clever is with the to/cc/bcc variables, where setting > them on the command-line (or using --no-{to,cc,bcc}) should clear out > values we grab from the config. > > All the rest are things where the command-line should simply override > the config values, and by reading the config first the config code > doesn't need all this "let's not set it was on the command-line" ECANTPARSE "let's not set it was on the command-line" > special-casing, as [1] shows we'd otherwise need to care about the > difference between whether something was a default or present in > config to fix the bug in e67a228cd8. > > 1. https://public-inbox.org/git/20190508105607.178244-2-gitster@pobox.com/ > > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 3/3] send-email: do defaults -> config -> getopt in that order 2019-05-09 18:04 ` Eric Sunshine @ 2019-05-13 8:46 ` Junio C Hamano 0 siblings, 0 replies; 39+ messages in thread From: Junio C Hamano @ 2019-05-13 8:46 UTC (permalink / raw) To: Eric Sunshine Cc: Ævar Arnfjörð Bjarmason, Git List, Heinrich Schuchardt, Brian M Carlson, Jonathan Nieder Eric Sunshine <sunshine@sunshineco.com> writes: >> All the rest are things where the command-line should simply override >> the config values, and by reading the config first the config code >> doesn't need all this "let's not set it was on the command-line" > > ECANTPARSE "let's not set it was on the command-line" I took it as "let's not set it, if it was on the command-line". ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 3/3] send-email: do defaults -> config -> getopt in that order 2019-05-09 11:48 ` [PATCH v3 3/3] send-email: do defaults -> config -> getopt in that order Ævar Arnfjörð Bjarmason 2019-05-09 18:04 ` Eric Sunshine @ 2019-05-09 23:51 ` brian m. carlson 2019-05-13 8:50 ` Junio C Hamano 2019-05-16 22:59 ` Stephen Boyd 3 siblings, 0 replies; 39+ messages in thread From: brian m. carlson @ 2019-05-09 23:51 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: git, Junio C Hamano, Heinrich Schuchardt, Jonathan Nieder, Eric Sunshine [-- Attachment #1: Type: text/plain, Size: 710 bytes --] On Thu, May 09, 2019 at 01:48:30PM +0200, Ævar Arnfjörð Bjarmason wrote: > @@ -363,6 +371,11 @@ sub read_config { > } > } > > +$identity = Git::config(@repo, "sendemail.identity"); > +read_config("sendemail.$identity") if defined $identity; > +read_config("sendemail"); > +read_config("sendemail"); Did we really want to call this function twice with the same argument? If so, perhaps a comment is appropriate explaining why. In general, I'm not opposed to this approach, either. It does make it easier to handle parsing in general. However, having said that, I'm fine with either your series or Junio's. -- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 868 bytes --] ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 3/3] send-email: do defaults -> config -> getopt in that order 2019-05-09 11:48 ` [PATCH v3 3/3] send-email: do defaults -> config -> getopt in that order Ævar Arnfjörð Bjarmason 2019-05-09 18:04 ` Eric Sunshine 2019-05-09 23:51 ` brian m. carlson @ 2019-05-13 8:50 ` Junio C Hamano 2019-05-13 21:13 ` Ævar Arnfjörð Bjarmason 2019-05-16 22:59 ` Stephen Boyd 3 siblings, 1 reply; 39+ messages in thread From: Junio C Hamano @ 2019-05-13 8:50 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: git, Heinrich Schuchardt, Brian M Carlson, Jonathan Nieder, Eric Sunshine Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > # Variables we fill in automatically, or via prompting: > -my (@to,$no_to,@initial_to,@cc,$no_cc,@initial_cc,@initial_bcc,$no_bcc,@xh, > +my (@to,@cc,,@xh,$envelope_sender, ,,??? ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 3/3] send-email: do defaults -> config -> getopt in that order 2019-05-13 8:50 ` Junio C Hamano @ 2019-05-13 21:13 ` Ævar Arnfjörð Bjarmason 0 siblings, 0 replies; 39+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2019-05-13 21:13 UTC (permalink / raw) To: Junio C Hamano Cc: git, Heinrich Schuchardt, Brian M Carlson, Jonathan Nieder, Eric Sunshine On Mon, May 13 2019, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > >> # Variables we fill in automatically, or via prompting: >> -my (@to,$no_to,@initial_to,@cc,$no_cc,@initial_cc,@initial_bcc,$no_bcc,@xh, >> +my (@to,@cc,,@xh,$envelope_sender, > > ,,??? Just a typo. I see you fixed this and the other issues noted in your push-out to "next", thanks! Sorry about not getting to this before that. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 3/3] send-email: do defaults -> config -> getopt in that order 2019-05-09 11:48 ` [PATCH v3 3/3] send-email: do defaults -> config -> getopt in that order Ævar Arnfjörð Bjarmason ` (2 preceding siblings ...) 2019-05-13 8:50 ` Junio C Hamano @ 2019-05-16 22:59 ` Stephen Boyd 2019-05-16 23:13 ` Junio C Hamano 3 siblings, 1 reply; 39+ messages in thread From: Stephen Boyd @ 2019-05-16 22:59 UTC (permalink / raw) To: avarab; +Cc: git, gitster, jrnieder, sandals, sunshine, xypron.glpk (Sorry for weird reply, I'm not subscribed and my MUA is not prepared for this) > Change the git-send-email command-line argument parsing and config > reading code to parse those two in the right order. I.e. first we set > our hardcoded defaults, then we read our config, and finally we read > the command-line, with later sets overriding earlier sets. > > This fixes a bug introduced in e67a228cd8 ("send-email: automatically > determine transfer-encoding", 2018-07-08). That change broke the broke > the reading of sendmail.transferencoding because it wasn't careful to > update our fragile code dealing with doing this in the previous > "defaults -> getopt -> config" order.. > > But as we can see from the history for this file doing it this way was > never what we actually wanted, it just something we grew organically > as of 5483c71d7a ("git-send-email: make options easier to configure.", > 2007-06-27) and have been dealing with the fallout since, e.g. in > 463b0ea22b ("send-email: Fix %config_path_settings handling", > 2011-10-14). > > As can be seen in this change the only place where we actually want to > do something clever is with the to/cc/bcc variables, where setting > them on the command-line (or using --no-{to,cc,bcc}) should clear out > values we grab from the config. > > All the rest are things where the command-line should simply override > the config values, and by reading the config first the config code > doesn't need all this "let's not set it was on the command-line" > special-casing, as [1] shows we'd otherwise need to care about the > difference between whether something was a default or present in > config to fix the bug in e67a228cd8. This broke my workflow. I specify --identity=<account> on the commandline and I want that to pick out my send-email config from my global .gitconfig file that corresponds to that identity. With this change, the config is parsed before the getopt part so --identity on the commandline is a nop and never looks into the config file to figure this out. So at least --identity is special in addition to --to,cc,bcc. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 3/3] send-email: do defaults -> config -> getopt in that order 2019-05-16 22:59 ` Stephen Boyd @ 2019-05-16 23:13 ` Junio C Hamano 2019-05-17 3:43 ` Junio C Hamano 0 siblings, 1 reply; 39+ messages in thread From: Junio C Hamano @ 2019-05-16 23:13 UTC (permalink / raw) To: Stephen Boyd; +Cc: avarab, git, jrnieder, sandals, sunshine, xypron.glpk Stephen Boyd <swboyd@chromium.org> writes: >> As can be seen in this change the only place where we actually want to >> do something clever is with the to/cc/bcc variables, where setting >> them on the command-line (or using --no-{to,cc,bcc}) should clear out >> values we grab from the config. >> >> All the rest are things where the command-line should simply override >> the config values, and by reading the config first the config code >> doesn't need all this "let's not set it was on the command-line" >> special-casing, as [1] shows we'd otherwise need to care about the >> difference between whether something was a default or present in >> config to fix the bug in e67a228cd8. > > This broke my workflow. > > I specify --identity=<account> on the commandline and I want that to > pick out my send-email config from my global .gitconfig file that > corresponds to that identity. With this change, the config is parsed > before the getopt part so --identity on the commandline is a nop and > never looks into the config file to figure this out. So at least > --identity is special in addition to --to,cc,bcc. Ah, sorry that nobody noticed that case, but you are right. Because the ident is used as a part of the key to find identity-specific configuration values, if the command line gives one, we must have an access to it before we start reading the configuration. In that sense, it is more fundamental to special-case the option. We are past -rc0, so I am inclined to revert the change (and perhaps replace it with the other "fix" that did not break the parsing order like these patches did), with an expectation that a clever fix will be found later, *unless* a simple and correct fix is found quickly. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 3/3] send-email: do defaults -> config -> getopt in that order 2019-05-16 23:13 ` Junio C Hamano @ 2019-05-17 3:43 ` Junio C Hamano 2019-05-17 19:55 ` [PATCH 0/5] ab/send-email-transferencoding-fix-for-the-fix Ævar Arnfjörð Bjarmason ` (5 more replies) 0 siblings, 6 replies; 39+ messages in thread From: Junio C Hamano @ 2019-05-17 3:43 UTC (permalink / raw) To: Stephen Boyd; +Cc: avarab, git, jrnieder, sandals, sunshine, xypron.glpk Junio C Hamano <gitster@pobox.com> writes: > Ah, sorry that nobody noticed that case, but you are right. Because > the ident is used as a part of the key to find identity-specific > configuration values, if the command line gives one, we must have an > access to it before we start reading the configuration. In that sense, > it is more fundamental to special-case the option. > > We are past -rc0, so I am inclined to revert the change (and perhaps > replace it with the other "fix" that did not break the parsing order > like these patches did), with an expectation that a clever fix will > be found later, *unless* a simple and correct fix is found quickly. Oops, spoke too soon. The topic hasn't escaped to 'master' yet, so I'll make sure it stays that way. Thanks for catching a regression before it gets too late. ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 0/5] ab/send-email-transferencoding-fix-for-the-fix 2019-05-17 3:43 ` Junio C Hamano @ 2019-05-17 19:55 ` Ævar Arnfjörð Bjarmason 2019-05-17 19:55 ` [PATCH 1/5] send-email: remove cargo-culted multi-patch pattern in tests Ævar Arnfjörð Bjarmason ` (4 subsequent siblings) 5 siblings, 0 replies; 39+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2019-05-17 19:55 UTC (permalink / raw) To: git Cc: Junio C Hamano, Jeff King, Stephen Boyd, jrnieder, sandals, sunshine, xypron.glpk, Paolo Bonzini, Ævar Arnfjörð Bjarmason This ab/send-email-transferencoding-fix-for-the-fix series goes on top of the ab/send-email-transferencoding-fix merged into "next". It fixes the sendemail.identity issue Stephen Boyd reported, and then some. Maybe it would be more understandable to rewind the existing topic out of next and produce a new rebased series, but I think this is actually easier to review, and documents some of the tricky edge cases for future contributors. I did make sure that the new tests I added for existing behavior were actually testing existing behavior, and not whatever bugs I got into "next" by running the test on top of "master", only the expected (now fixed) tests fail there, not any tests for existing sendemail.identity behavior. Ævar Arnfjörð Bjarmason (5): send-email: remove cargo-culted multi-patch pattern in tests send-email: fix broken transferEncoding tests send-email: document --no-[to|cc|bcc] send-email: fix regression in sendemail.identity parsing send-email: remove support for deprecated sendemail.smtpssl Documentation/config/sendemail.txt | 3 - Documentation/git-send-email.txt | 11 ++- git-send-email.perl | 78 ++++++++++-------- t/t9001-send-email.sh | 122 +++++++++++++++++++++-------- 4 files changed, 144 insertions(+), 70 deletions(-) -- 2.21.0.1020.gf2820cf01a ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 1/5] send-email: remove cargo-culted multi-patch pattern in tests 2019-05-17 3:43 ` Junio C Hamano 2019-05-17 19:55 ` [PATCH 0/5] ab/send-email-transferencoding-fix-for-the-fix Ævar Arnfjörð Bjarmason @ 2019-05-17 19:55 ` Ævar Arnfjörð Bjarmason 2019-05-17 19:55 ` [PATCH 2/5] send-email: fix broken transferEncoding tests Ævar Arnfjörð Bjarmason ` (3 subsequent siblings) 5 siblings, 0 replies; 39+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2019-05-17 19:55 UTC (permalink / raw) To: git Cc: Junio C Hamano, Jeff King, Stephen Boyd, jrnieder, sandals, sunshine, xypron.glpk, Paolo Bonzini, Ævar Arnfjörð Bjarmason Change test code added in f434c083a0 ("send-email: add --no-cc, --no-to, and --no-bcc", 2010-03-07) which blindly copied a pattern from an earlier test added in 32ae83194b ("add a test for git-send-email for non-threaded mails", 2009-06-12) where the "$patches" variable was supplied more than once. As it turns out we didn't need more than one "$patches" for the test added in 32ae83194b either. The only tests that actually needed this sort of invocation were the tests added in 54aae5e1a0 ("t9001: send-email interation with --in-reply-to and --chain-reply-to", 2010-10-19). Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- t/t9001-send-email.sh | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 1b18201ce2..13de44686b 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -1204,7 +1204,7 @@ test_expect_success $PREREQ 'no in-reply-to and no threading' ' --from="Example <nobody@example.com>" \ --to=nobody@example.com \ --no-thread \ - $patches $patches >stdout && + $patches >stdout && ! grep "In-Reply-To: " stdout ' @@ -1224,7 +1224,7 @@ test_expect_success $PREREQ 'sendemail.to works' ' git send-email \ --dry-run \ --from="Example <nobody@example.com>" \ - $patches $patches >stdout && + $patches >stdout && grep "To: Somebody <somebody@ex.com>" stdout ' @@ -1234,7 +1234,7 @@ test_expect_success $PREREQ '--no-to overrides sendemail.to' ' --from="Example <nobody@example.com>" \ --no-to \ --to=nobody@example.com \ - $patches $patches >stdout && + $patches >stdout && grep "To: nobody@example.com" stdout && ! grep "To: Somebody <somebody@ex.com>" stdout ' @@ -1245,7 +1245,7 @@ test_expect_success $PREREQ 'sendemail.cc works' ' --dry-run \ --from="Example <nobody@example.com>" \ --to=nobody@example.com \ - $patches $patches >stdout && + $patches >stdout && grep "Cc: Somebody <somebody@ex.com>" stdout ' @@ -1256,7 +1256,7 @@ test_expect_success $PREREQ '--no-cc overrides sendemail.cc' ' --no-cc \ --cc=bodies@example.com \ --to=nobody@example.com \ - $patches $patches >stdout && + $patches >stdout && grep "Cc: bodies@example.com" stdout && ! grep "Cc: Somebody <somebody@ex.com>" stdout ' @@ -1268,7 +1268,7 @@ test_expect_success $PREREQ 'sendemail.bcc works' ' --from="Example <nobody@example.com>" \ --to=nobody@example.com \ --smtp-server relay.example.com \ - $patches $patches >stdout && + $patches >stdout && grep "RCPT TO:<other@ex.com>" stdout ' @@ -1280,7 +1280,7 @@ test_expect_success $PREREQ '--no-bcc overrides sendemail.bcc' ' --bcc=bodies@example.com \ --to=nobody@example.com \ --smtp-server relay.example.com \ - $patches $patches >stdout && + $patches >stdout && grep "RCPT TO:<bodies@example.com>" stdout && ! grep "RCPT TO:<other@ex.com>" stdout ' -- 2.21.0.1020.gf2820cf01a ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 2/5] send-email: fix broken transferEncoding tests 2019-05-17 3:43 ` Junio C Hamano 2019-05-17 19:55 ` [PATCH 0/5] ab/send-email-transferencoding-fix-for-the-fix Ævar Arnfjörð Bjarmason 2019-05-17 19:55 ` [PATCH 1/5] send-email: remove cargo-culted multi-patch pattern in tests Ævar Arnfjörð Bjarmason @ 2019-05-17 19:55 ` Ævar Arnfjörð Bjarmason 2019-05-17 19:55 ` [PATCH 3/5] send-email: document --no-[to|cc|bcc] Ævar Arnfjörð Bjarmason ` (2 subsequent siblings) 5 siblings, 0 replies; 39+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2019-05-17 19:55 UTC (permalink / raw) To: git Cc: Junio C Hamano, Jeff King, Stephen Boyd, jrnieder, sandals, sunshine, xypron.glpk, Paolo Bonzini, Ævar Arnfjörð Bjarmason I fixed a bug that had broken the reading of sendmail.transferEncoding in 3494dfd3ee ("send-email: do defaults -> config -> getopt in that order", 2019-05-09), but the test I added in that commit did nothing to assert the bug had been fixed. That issue originates in 8d81408435 ("git-send-email: add --transfer-encoding option", 2014-11-25) which first added the "sendemail.transferencoding=8bit". That test has never done anything meaningful. It tested that the "--transfer-encoding=8bit" option would turn on the 8bit Transfer-Encoding, but that was the default at the time (and now). As checking out 8d81408435 and editing the test to remove that option will reveal, supplying it never did anything. So when I copied it thinking it would work in 3494dfd3ee I copied a previously broken test, although I was making sure it did the right thing via da-hoc debugger inspection, so the bug was fixed. So fix the test I added in 3494dfd3ee, as well as the long-standing test added in 8d81408435. To test if we're actually setting the Transfer-Encoding let's set it to 7bit, not 8bit, as 7bit will error out on "email-using-8bit". This means that we can remove the "sendemail.transferencoding=7bit fails on 8bit data" test, since it was redundant, we now have other tests that assert that that'll fail. While I'm at it convert "git config <key> <value>" in the test setup to just "-c <key>=<value>" on the command-line. Then we don't need to cleanup after these tests, and there's no sense in asserting where config values come from in these tests, we can take that as a given. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- t/t9001-send-email.sh | 35 +++++++++++------------------------ 1 file changed, 11 insertions(+), 24 deletions(-) diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 13de44686b..61d484d1a6 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -1437,10 +1437,10 @@ test_expect_success $PREREQ 'setup expect' ' EOF ' -test_expect_success $PREREQ 'sendemail.transferencoding=7bit fails on 8bit data' ' +test_expect_success $PREREQ '--transfer-encoding overrides sendemail.transferEncoding' ' clean_fake_sendmail && - git config sendemail.transferEncoding 7bit && - test_must_fail git send-email \ + test_must_fail git -c sendemail.transferEncoding=8bit \ + send-email \ --transfer-encoding=7bit \ --smtp-server="$(pwd)/fake.sendmail" \ email-using-8bit \ @@ -1449,11 +1449,10 @@ test_expect_success $PREREQ 'sendemail.transferencoding=7bit fails on 8bit data' test -z "$(ls msgtxt*)" ' -test_expect_success $PREREQ '--transfer-encoding overrides sendemail.transferEncoding' ' +test_expect_success $PREREQ 'sendemail.transferEncoding via config' ' clean_fake_sendmail && - git config sendemail.transferEncoding 8bit && - test_must_fail git send-email \ - --transfer-encoding=7bit \ + test_must_fail git -c sendemail.transferEncoding=7bit \ + send-email \ --smtp-server="$(pwd)/fake.sendmail" \ email-using-8bit \ 2>errors >out && @@ -1461,27 +1460,15 @@ test_expect_success $PREREQ '--transfer-encoding overrides sendemail.transferEnc test -z "$(ls msgtxt*)" ' -test_expect_success $PREREQ 'sendemail.transferencoding=8bit via config' ' +test_expect_success $PREREQ 'sendemail.transferEncoding via cli' ' clean_fake_sendmail && - git -c sendemail.transferencoding=8bit send-email \ - --smtp-server="$(pwd)/fake.sendmail" \ - email-using-8bit \ - 2>errors >out && - sed '1,/^$/d' msgtxt1 >actual && - sed '1,/^$/d' email-using-8bit >expected && - test_cmp expected actual -' - -test_expect_success $PREREQ 'sendemail.transferencoding=8bit via cli' ' - clean_fake_sendmail && - git send-email \ - --transfer-encoding=8bit \ + test_must_fail git send-email \ + --transfer-encoding=7bit \ --smtp-server="$(pwd)/fake.sendmail" \ email-using-8bit \ 2>errors >out && - sed '1,/^$/d' msgtxt1 >actual && - sed '1,/^$/d' email-using-8bit >expected && - test_cmp expected actual + grep "cannot send message as 7bit" errors && + test -z "$(ls msgtxt*)" ' test_expect_success $PREREQ 'setup expect' ' -- 2.21.0.1020.gf2820cf01a ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 3/5] send-email: document --no-[to|cc|bcc] 2019-05-17 3:43 ` Junio C Hamano ` (2 preceding siblings ...) 2019-05-17 19:55 ` [PATCH 2/5] send-email: fix broken transferEncoding tests Ævar Arnfjörð Bjarmason @ 2019-05-17 19:55 ` Ævar Arnfjörð Bjarmason 2019-05-17 19:55 ` [PATCH 4/5] send-email: fix regression in sendemail.identity parsing Ævar Arnfjörð Bjarmason 2019-05-17 19:55 ` [PATCH 5/5] send-email: remove support for deprecated sendemail.smtpssl Ævar Arnfjörð Bjarmason 5 siblings, 0 replies; 39+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2019-05-17 19:55 UTC (permalink / raw) To: git Cc: Junio C Hamano, Jeff King, Stephen Boyd, jrnieder, sandals, sunshine, xypron.glpk, Paolo Bonzini, Ævar Arnfjörð Bjarmason These options added in f434c083a0 ("send-email: add --no-cc, --no-to, and --no-bcc", 2010-03-07) were never documented. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- Documentation/git-send-email.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt index 1afe9fc858..8100ff4b0f 100644 --- a/Documentation/git-send-email.txt +++ b/Documentation/git-send-email.txt @@ -278,6 +278,10 @@ must be used for each option. Automating ~~~~~~~~~~ +--no-[to|cc|bcc]:: + Clears any list of "To:", "Cc:", "Bcc:" addresses previously + set via config. + --to-cmd=<command>:: Specify a command to execute once per patch file which should generate patch file specific "To:" entries. -- 2.21.0.1020.gf2820cf01a ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 4/5] send-email: fix regression in sendemail.identity parsing 2019-05-17 3:43 ` Junio C Hamano ` (3 preceding siblings ...) 2019-05-17 19:55 ` [PATCH 3/5] send-email: document --no-[to|cc|bcc] Ævar Arnfjörð Bjarmason @ 2019-05-17 19:55 ` Ævar Arnfjörð Bjarmason 2019-05-19 1:29 ` Junio C Hamano 2019-05-22 20:25 ` Johannes Schindelin 2019-05-17 19:55 ` [PATCH 5/5] send-email: remove support for deprecated sendemail.smtpssl Ævar Arnfjörð Bjarmason 5 siblings, 2 replies; 39+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2019-05-17 19:55 UTC (permalink / raw) To: git Cc: Junio C Hamano, Jeff King, Stephen Boyd, jrnieder, sandals, sunshine, xypron.glpk, Paolo Bonzini, Ævar Arnfjörð Bjarmason Fix a regression in my recent 3494dfd3ee ("send-email: do defaults -> config -> getopt in that order", 2019-05-09). I missed that the $identity variable needs to be extracted from the command-line before we do the config reading, as it determines which config variable we should read first. See [1] for the report. The sendemail.identity feature was added back in 34cc60ce2b ("send-email: Add support for SSL and SMTP-AUTH", 2007-09-03), there were no tests to assert that it worked properly. So let's fix both the regression, and add some tests to assert that this is being parsed properly. While I'm at it I'm adding a --no-identity option to go with --[to|cc|bcc] variable, since the semantics are similar. It's like to/cc/bcc except that unlike those we don't support multiple identities, but we could now easily add it support for it if anyone cares. In just fixing the --identity command-line parsing bug I discovered that a narrow fix to that wouldn't do. In read_config() we had a state machine that would only set config values if they weren't set already, and thus by proxy we wouldn't e.g. set "to" based on sendemail.to if we'd seen sendemail.gmail.to before, with --identity=gmail. I'd modified some of the relevant code in 3494dfd3ee, but just reverting to that wouldn't do, since it would bring back the regression fixed in that commit. Refactor read_config() do what we actually mean here. We don't want to set a given sendemail.VAR if a sendemail.$identity.VAR previously set it. The old code was conflating this desire with the hardcoded defaults for these variables, and as discussed in 3494dfd3ee that was never going to work. Instead pass along the state of whether an identity config set something before, as distinguished from the state of the default just being false, or the default being a non-bool or true (e.g. --transferencoding). I'm still not happy with the test coverage here, e.g. there's nothing testing sendemail.smtpEncryption, but I only have so much time to fix this code. 1. https://public-inbox.org/git/5cddeb61.1c69fb81.47ed4.e648@mx.google.com/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- Documentation/git-send-email.txt | 4 ++ git-send-email.perl | 60 ++++++++++++++++++++---------- t/t9001-send-email.sh | 64 ++++++++++++++++++++++++++++++++ 3 files changed, 108 insertions(+), 20 deletions(-) diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt index 8100ff4b0f..a861934c69 100644 --- a/Documentation/git-send-email.txt +++ b/Documentation/git-send-email.txt @@ -282,6 +282,10 @@ Automating Clears any list of "To:", "Cc:", "Bcc:" addresses previously set via config. +--no-identity:: + Clears the previously read value of `sendemail.identity` set + via config, if any. + --to-cmd=<command>:: Specify a command to execute once per patch file which should generate patch file specific "To:" entries. diff --git a/git-send-email.perl b/git-send-email.perl index b2c4a77671..80cbbfd2b8 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -182,7 +182,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); +my ($no_cc, $no_to, $no_bcc, $no_identity); my (@config_to, @getopt_to); my (@config_cc, @getopt_cc); my (@config_bcc, @getopt_bcc); @@ -325,44 +325,53 @@ sub signal_handler { # Read our sendemail.* config sub read_config { - my ($prefix) = @_; + my ($configured, $prefix) = @_; foreach my $setting (keys %config_bool_settings) { my $target = $config_bool_settings{$setting}; my $v = Git::config_bool(@repo, "$prefix.$setting"); - $$target = $v if defined $v; + next unless defined $v; + next if $configured->{$setting}++; + $$target = $v; } foreach my $setting (keys %config_path_settings) { my $target = $config_path_settings{$setting}; if (ref($target) eq "ARRAY") { - unless (@$target) { - my @values = Git::config_path(@repo, "$prefix.$setting"); - @$target = @values if (@values && defined $values[0]); - } + my @values = Git::config_path(@repo, "$prefix.$setting"); + next unless @values; + next if $configured->{$setting}++; + @$target = @values; } else { my $v = Git::config_path(@repo, "$prefix.$setting"); - $$target = $v if defined $v; + next unless defined $v; + next if $configured->{$setting}++; + $$target = $v; } } foreach my $setting (keys %config_settings) { my $target = $config_settings{$setting}; if (ref($target) eq "ARRAY") { - unless (@$target) { - my @values = Git::config(@repo, "$prefix.$setting"); - @$target = @values if (@values && defined $values[0]); - } + my @values = Git::config(@repo, "$prefix.$setting"); + next unless @values; + next if $configured->{$setting}++; + @$target = @values; } else { my $v = Git::config(@repo, "$prefix.$setting"); - $$target = $v if defined $v; + next unless defined $v; + next if $configured->{$setting}++; + $$target = $v; } } if (!defined $smtp_encryption) { - my $enc = Git::config(@repo, "$prefix.smtpencryption"); + my $setting = "$prefix.smtpencryption"; + my $enc = Git::config(@repo, $setting); + return unless defined $enc; + return if $configured->{$setting}++; if (defined $enc) { $smtp_encryption = $enc; } elsif (Git::config_bool(@repo, "$prefix.smtpssl")) { @@ -371,17 +380,30 @@ sub read_config { } } +# sendemail.identity yields to --identity. We must parse this +# special-case first before the rest of the config is read. $identity = Git::config(@repo, "sendemail.identity"); -read_config("sendemail.$identity") if defined $identity; -read_config("sendemail"); +my $rc = GetOptions( + "identity=s" => \$identity, + "no-identity" => \$no_identity, +); +usage() unless $rc; +undef $identity if $no_identity; + +# Now we know enough to read the config +{ + my %configured; + read_config(\%configured, "sendemail.$identity") if defined $identity; + read_config(\%configured, "sendemail"); +} # Begin by accumulating all the variables (defined above), that we will end up # needing, first, from the command line: my $help; my $git_completion_helper; -my $rc = GetOptions("h" => \$help, - "dump-aliases" => \$dump_aliases); +$rc = GetOptions("h" => \$help, + "dump-aliases" => \$dump_aliases); usage() unless $rc; die __("--dump-aliases incompatible with other options\n") if !$help and $dump_aliases and @ARGV; @@ -410,8 +432,6 @@ sub read_config { "smtp-debug:i" => \$debug_net_smtp, "smtp-domain:s" => \$smtp_domain, "smtp-auth=s" => \$smtp_auth, - "no-smtp-auth" => sub {$smtp_auth = 'none'}, - "identity=s" => \$identity, "annotate!" => \$annotate, "no-annotate" => sub {$annotate = 0}, "compose" => \$compose, diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 61d484d1a6..890e2874c3 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -1228,6 +1228,61 @@ test_expect_success $PREREQ 'sendemail.to works' ' grep "To: Somebody <somebody@ex.com>" stdout ' +test_expect_success $PREREQ 'setup sendemail.identity' ' + git config --replace-all sendemail.to "default@example.com" && + git config --replace-all sendemail.isp.to "isp@example.com" && + git config --replace-all sendemail.cloud.to "cloud@example.com" +' + +test_expect_success $PREREQ 'sendemail.identity: reads the correct identity config' ' + git -c sendemail.identity=cloud send-email \ + --dry-run \ + --from="nobody@example.com" \ + $patches >stdout && + grep "To: cloud@example.com" stdout +' + +test_expect_success $PREREQ 'sendemail.identity: identity overrides sendemail.identity' ' + git -c sendemail.identity=cloud send-email \ + --identity=isp \ + --dry-run \ + --from="nobody@example.com" \ + $patches >stdout && + grep "To: isp@example.com" stdout +' + +test_expect_success $PREREQ 'sendemail.identity: --no-identity clears previous identity' ' + git -c sendemail.identity=cloud send-email \ + --no-identity \ + --dry-run \ + --from="nobody@example.com" \ + $patches >stdout && + grep "To: default@example.com" stdout +' + +test_expect_success $PREREQ 'sendemail.identity: bool identity variable existance overrides' ' + git -c sendemail.identity=cloud \ + -c sendemail.xmailer=true \ + -c sendemail.cloud.xmailer=false \ + send-email \ + --dry-run \ + --from="nobody@example.com" \ + $patches >stdout && + grep "To: cloud@example.com" stdout && + ! grep "X-Mailer" stdout +' + +test_expect_success $PREREQ 'sendemail.identity: bool variable fallback' ' + git -c sendemail.identity=cloud \ + -c sendemail.xmailer=false \ + send-email \ + --dry-run \ + --from="nobody@example.com" \ + $patches >stdout && + grep "To: cloud@example.com" stdout && + ! grep "X-Mailer" stdout +' + test_expect_success $PREREQ '--no-to overrides sendemail.to' ' git send-email \ --dry-run \ @@ -1785,6 +1840,15 @@ test_expect_success '--dump-aliases must be used alone' ' test_must_fail git send-email --dump-aliases --to=janice@example.com -1 refs/heads/accounting ' +test_expect_success 'aliases and sendemail.identity' ' + test_must_fail git \ + -c sendemail.identity=cloud \ + -c sendemail.aliasesfile=default-aliases \ + -c sendemail.cloud.aliasesfile=cloud-aliases \ + send-email -1 2>stderr && + test_i18ngrep "cloud-aliases" stderr +' + test_sendmail_aliases () { msg="$1" && shift && expect="$@" && -- 2.21.0.1020.gf2820cf01a ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH 4/5] send-email: fix regression in sendemail.identity parsing 2019-05-17 19:55 ` [PATCH 4/5] send-email: fix regression in sendemail.identity parsing Ævar Arnfjörð Bjarmason @ 2019-05-19 1:29 ` Junio C Hamano 2019-05-22 20:25 ` Johannes Schindelin 1 sibling, 0 replies; 39+ messages in thread From: Junio C Hamano @ 2019-05-19 1:29 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: git, Jeff King, Stephen Boyd, jrnieder, sandals, sunshine, xypron.glpk, Paolo Bonzini Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > Fix a regression in my recent 3494dfd3ee ("send-email: do defaults -> > config -> getopt in that order", 2019-05-09). I missed that the > $identity variable needs to be extracted from the command-line before > we do the config reading, as it determines which config variable we > should read first. See [1] for the report. > ... > Refactor read_config() do what we actually mean here. We don't want to > set a given sendemail.VAR if a sendemail.$identity.VAR previously set > it. The old code was conflating this desire with the hardcoded > defaults for these variables, and as discussed in 3494dfd3ee that was > never going to work. I am not sure if the "never going to work" claim is a correct one. The "no hardcoded default in the variable, read command line, fill missing ones from the two config files and finally apply the default for the ones that are still missing" was cumbersome, error-prone without a table, but did work. It seems that no matter how we cut it, the cumbersomeness has to exist, as long as the command line --identity needs to be taken care of. Without that complication, I really liked the base series---the "set var to hardcoded default, overwrite with config and then overwrite with command line, without having to check if we already got value in an earlier step" was so much simpler and easy to explain X-<. > @@ -371,17 +380,30 @@ sub read_config { > ... > my $help; > my $git_completion_helper; > -my $rc = GetOptions("h" => \$help, > - "dump-aliases" => \$dump_aliases); > ... > @@ -410,8 +432,6 @@ sub read_config { > ... > "smtp-auth=s" => \$smtp_auth, > - "no-smtp-auth" => sub {$smtp_auth = 'none'}, > - "identity=s" => \$identity, You seem to be building on top of ab/send-email-transferencoding-fix and something else, and these two hunks did not apply. I think I managed to wiggle the patch in correctly, though. Thanks. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 4/5] send-email: fix regression in sendemail.identity parsing 2019-05-17 19:55 ` [PATCH 4/5] send-email: fix regression in sendemail.identity parsing Ævar Arnfjörð Bjarmason 2019-05-19 1:29 ` Junio C Hamano @ 2019-05-22 20:25 ` Johannes Schindelin 2019-05-29 9:10 ` Johannes Schindelin 1 sibling, 1 reply; 39+ messages in thread From: Johannes Schindelin @ 2019-05-22 20:25 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: git, Junio C Hamano, Jeff King, Stephen Boyd, jrnieder, sandals, sunshine, xypron.glpk, Paolo Bonzini [-- Attachment #1: Type: text/plain, Size: 3225 bytes --] Hi Ævar, On Fri, 17 May 2019, Ævar Arnfjörð Bjarmason wrote: > diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh > index 61d484d1a6..890e2874c3 100755 > --- a/t/t9001-send-email.sh > +++ b/t/t9001-send-email.sh > @@ -1228,6 +1228,61 @@ test_expect_success $PREREQ 'sendemail.to works' ' > grep "To: Somebody <somebody@ex.com>" stdout > ' > > +test_expect_success $PREREQ 'setup sendemail.identity' ' > + git config --replace-all sendemail.to "default@example.com" && > + git config --replace-all sendemail.isp.to "isp@example.com" && > + git config --replace-all sendemail.cloud.to "cloud@example.com" > +' > + > +test_expect_success $PREREQ 'sendemail.identity: reads the correct identity config' ' > + git -c sendemail.identity=cloud send-email \ > + --dry-run \ > + --from="nobody@example.com" \ > + $patches >stdout && > + grep "To: cloud@example.com" stdout > +' > + > +test_expect_success $PREREQ 'sendemail.identity: identity overrides sendemail.identity' ' > + git -c sendemail.identity=cloud send-email \ > + --identity=isp \ > + --dry-run \ > + --from="nobody@example.com" \ > + $patches >stdout && > + grep "To: isp@example.com" stdout > +' > + > +test_expect_success $PREREQ 'sendemail.identity: --no-identity clears previous identity' ' > + git -c sendemail.identity=cloud send-email \ > + --no-identity \ > + --dry-run \ > + --from="nobody@example.com" \ > + $patches >stdout && > + grep "To: default@example.com" stdout > +' > + > +test_expect_success $PREREQ 'sendemail.identity: bool identity variable existance overrides' ' > + git -c sendemail.identity=cloud \ > + -c sendemail.xmailer=true \ > + -c sendemail.cloud.xmailer=false \ > + send-email \ > + --dry-run \ > + --from="nobody@example.com" \ > + $patches >stdout && > + grep "To: cloud@example.com" stdout && > + ! grep "X-Mailer" stdout > +' > + > +test_expect_success $PREREQ 'sendemail.identity: bool variable fallback' ' > + git -c sendemail.identity=cloud \ > + -c sendemail.xmailer=false \ > + send-email \ > + --dry-run \ > + --from="nobody@example.com" \ > + $patches >stdout && > + grep "To: cloud@example.com" stdout && > + ! grep "X-Mailer" stdout > +' > + These test cases all diligently use the `$PREREQ` prerequisite, but... > test_expect_success $PREREQ '--no-to overrides sendemail.to' ' > git send-email \ > --dry-run \ > @@ -1785,6 +1840,15 @@ test_expect_success '--dump-aliases must be used alone' ' > test_must_fail git send-email --dump-aliases --to=janice@example.com -1 refs/heads/accounting > ' > > +test_expect_success 'aliases and sendemail.identity' ' > + test_must_fail git \ > + -c sendemail.identity=cloud \ > + -c sendemail.aliasesfile=default-aliases \ > + -c sendemail.cloud.aliasesfile=cloud-aliases \ > + send-email -1 2>stderr && > + test_i18ngrep "cloud-aliases" stderr > +' > + This one is missing it. That breaks the Windows job in our Azure Pipeline where we leave out all of the Perl bits (to accelerate the tests somewhat). Ciao, Dscho > test_sendmail_aliases () { > msg="$1" && shift && > expect="$@" && > -- > 2.21.0.1020.gf2820cf01a > > > ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 4/5] send-email: fix regression in sendemail.identity parsing 2019-05-22 20:25 ` Johannes Schindelin @ 2019-05-29 9:10 ` Johannes Schindelin 0 siblings, 0 replies; 39+ messages in thread From: Johannes Schindelin @ 2019-05-29 9:10 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: git, Junio C Hamano, Jeff King, Stephen Boyd, jrnieder, sandals, sunshine, xypron.glpk, Paolo Bonzini [-- Attachment #1: Type: text/plain, Size: 2572 bytes --] Hi Junio & Ævar, On Wed, 22 May 2019, Johannes Schindelin wrote: > On Fri, 17 May 2019, Ævar Arnfjörð Bjarmason wrote: > > > [...] > > +test_expect_success $PREREQ 'sendemail.identity: bool variable fallback' ' > > + git -c sendemail.identity=cloud \ > > + -c sendemail.xmailer=false \ > > + send-email \ > > + --dry-run \ > > + --from="nobody@example.com" \ > > + $patches >stdout && > > + grep "To: cloud@example.com" stdout && > > + ! grep "X-Mailer" stdout > > +' > > + > > These test cases all diligently use the `$PREREQ` prerequisite, but... > > > test_expect_success $PREREQ '--no-to overrides sendemail.to' ' > > git send-email \ > > --dry-run \ > > @@ -1785,6 +1840,15 @@ test_expect_success '--dump-aliases must be used alone' ' > > test_must_fail git send-email --dump-aliases --to=janice@example.com -1 refs/heads/accounting > > ' > > > > +test_expect_success 'aliases and sendemail.identity' ' > > + test_must_fail git \ > > + -c sendemail.identity=cloud \ > > + -c sendemail.aliasesfile=default-aliases \ > > + -c sendemail.cloud.aliasesfile=cloud-aliases \ > > + send-email -1 2>stderr && > > + test_i18ngrep "cloud-aliases" stderr > > +' > > + > > This one is missing it. That breaks the Windows job in our Azure Pipeline > where we leave out all of the Perl bits (to accelerate the tests somewhat). For the record, this is still breaking our Azure Pipeline build. Junio, would you terribly mind applying this on top (or fetching it from the `shears/pu` branch at https://github.com/git-for-windows/git)? -- snipsnap -- From 4c946f956d894676f68f5b130ab414d8ea75e97d Mon Sep 17 00:00:00 2001 From: Johannes Schindelin <johannes.schindelin@gmx.de> Date: Wed, 29 May 2019 11:09:23 +0200 Subject: [PATCH] SQUASH??? Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- t/t9001-send-email.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 7caab8160fcf..7c5ef114ac90 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -1840,7 +1840,7 @@ test_expect_success '--dump-aliases must be used alone' ' test_must_fail git send-email --dump-aliases --to=janice@example.com -1 refs/heads/accounting ' -test_expect_success 'aliases and sendemail.identity' ' +test_expect_success $PREREQ 'aliases and sendemail.identity' ' test_must_fail git \ -c sendemail.identity=cloud \ -c sendemail.aliasesfile=default-aliases \ -- 2.22.0.rc1.windows.1.19.g571a93d65ff3 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 5/5] send-email: remove support for deprecated sendemail.smtpssl 2019-05-17 3:43 ` Junio C Hamano ` (4 preceding siblings ...) 2019-05-17 19:55 ` [PATCH 4/5] send-email: fix regression in sendemail.identity parsing Ævar Arnfjörð Bjarmason @ 2019-05-17 19:55 ` Ævar Arnfjörð Bjarmason 5 siblings, 0 replies; 39+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2019-05-17 19:55 UTC (permalink / raw) To: git Cc: Junio C Hamano, Jeff King, Stephen Boyd, jrnieder, sandals, sunshine, xypron.glpk, Paolo Bonzini, Ævar Arnfjörð Bjarmason Remove the sendemail.smtpssl configuration variable and its associated --smtp-ssl command-line option. This has been documented as deprecated since f6bebd121a ("git-send-email: add support for TLS via Net::SMTP::SSL", 2008-06-25) and 65180c6618 ("List send-email config options in config.txt.", 2009-07-22), respectively. Waiting for 10 years should be enough. This allows us to fix a special case in read_config(). We couldn't just parse sendemail.smtpEncryption like everything else because we'd need to fall back on sendemail.smtpssl. Now that we don't need to do that we don't need this special case anymore. Let's still find out if someone's using this and die() with a helpful message if that's the case. Because of my recent improvements to the command-line and config parsing we can also revert fa835cd572 ("git-send-email: prevent undefined variable warnings if no encryption is set", 2008-06-26), since we now sensibly support setting defaults for these mixed config & command-line options. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- Documentation/config/sendemail.txt | 3 --- Documentation/git-send-email.txt | 3 --- git-send-email.perl | 26 +++++++++----------------- t/t9001-send-email.sh | 9 +++++++++ 4 files changed, 18 insertions(+), 23 deletions(-) diff --git a/Documentation/config/sendemail.txt b/Documentation/config/sendemail.txt index 0006faf800..d7855bff1f 100644 --- a/Documentation/config/sendemail.txt +++ b/Documentation/config/sendemail.txt @@ -8,9 +8,6 @@ sendemail.smtpEncryption:: See linkgit:git-send-email[1] for description. Note that this setting is not subject to the 'identity' mechanism. -sendemail.smtpssl (deprecated):: - Deprecated alias for 'sendemail.smtpEncryption = ssl'. - sendemail.smtpsslcertpath:: Path to ca-certificates (either a directory or a single file). Set it to an empty string to disable certificate verification. diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt index a861934c69..6cf5c32ce8 100644 --- a/Documentation/git-send-email.txt +++ b/Documentation/git-send-email.txt @@ -236,9 +236,6 @@ The --smtp-server-option option must be repeated for each option you want to pass to the server. Likewise, different lines in the configuration files must be used for each option. ---smtp-ssl:: - Legacy alias for '--smtp-encryption ssl'. - --smtp-ssl-cert-path:: Path to a store of trusted CA certificates for SMTP SSL/TLS certificate validation (either a directory that has been processed diff --git a/git-send-email.perl b/git-send-email.perl index 80cbbfd2b8..b1ed45b907 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -77,7 +77,6 @@ sub usage { --smtp-user <str> * Username for SMTP-AUTH. --smtp-pass <str> * Password for SMTP-AUTH; not necessary. --smtp-encryption <str> * tls or ssl; anything else disables. - --smtp-ssl * Deprecated. Use '--smtp-encryption ssl'. --smtp-ssl-cert-path <str> * Path to ca-certificates (either directory or file). Pass an empty string to disable certificate verification. @@ -236,7 +235,7 @@ sub do_edit { my ($cover_cc, $cover_to); my ($to_cmd, $cc_cmd); my ($smtp_server, $smtp_server_port, @smtp_server_options); -my ($smtp_authuser, $smtp_encryption, $smtp_ssl_cert_path); +my ($smtp_authuser, $smtp_ssl_cert_path); my ($batch_size, $relogin_delay); my ($identity, $aliasfiletype, @alias_files, $smtp_domain, $smtp_auth); my ($confirm); @@ -250,6 +249,9 @@ sub do_edit { my $use_xmailer = 1; my $validate = 1; my $target_xfer_encoding = 'auto'; +my $smtp_encryption = ''; +# Deprecated variables +my $deprecated_smtp_ssl; my %config_bool_settings = ( "thread" => \$thread, @@ -266,10 +268,12 @@ sub do_edit { ); my %config_settings = ( + "smtpencryption" => \$smtp_encryption, "smtpserver" => \$smtp_server, "smtpserverport" => \$smtp_server_port, "smtpserveroption" => \@smtp_server_options, "smtpuser" => \$smtp_authuser, + "smtpssl" => \$deprecated_smtp_ssl, "smtppass" => \$smtp_authpass, "smtpdomain" => \$smtp_domain, "smtpauth" => \$smtp_auth, @@ -366,18 +370,6 @@ sub read_config { $$target = $v; } } - - if (!defined $smtp_encryption) { - my $setting = "$prefix.smtpencryption"; - my $enc = Git::config(@repo, $setting); - return unless defined $enc; - return if $configured->{$setting}++; - if (defined $enc) { - $smtp_encryption = $enc; - } elsif (Git::config_bool(@repo, "$prefix.smtpssl")) { - $smtp_encryption = 'ssl'; - } - } } # sendemail.identity yields to --identity. We must parse this @@ -426,7 +418,7 @@ sub read_config { "smtp-server-port=s" => \$smtp_server_port, "smtp-user=s" => \$smtp_authuser, "smtp-pass:s" => \$smtp_authpass, - "smtp-ssl" => sub { $smtp_encryption = 'ssl' }, + "smtp-ssl" => sub { $deprecated_smtp_ssl = 1 }, "smtp-encryption=s" => \$smtp_encryption, "smtp-ssl-cert-path=s" => \$smtp_ssl_cert_path, "smtp-debug:i" => \$debug_net_smtp, @@ -484,8 +476,8 @@ sub read_config { "(via command-line or configuration option)\n") if defined $relogin_delay and not defined $batch_size; -# 'default' encryption is none -- this only prevents a warning -$smtp_encryption = '' unless (defined $smtp_encryption); +die __("Use of deprecated option --smtp-ssl (or smtp.smtpssl config), use --smtp-encryption=ssl (or sendemail.smtpEncryption=ssl) instead\n") + if $deprecated_smtp_ssl; # Set CC suppressions my(%suppress_cc); diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 890e2874c3..b5ccfa8737 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -1849,6 +1849,15 @@ test_expect_success 'aliases and sendemail.identity' ' test_i18ngrep "cloud-aliases" stderr ' +test_expect_success 'deprecated --smtp-ssl (or sendemail.smtpssl=true)' ' + test_must_fail git -c sendemail.smtpssl=true send-email -1 2>stderr && + test_i18ngrep "deprecated option.*or.*config" stderr && + test_must_fail git send-email --smtp-ssl -1 && + test_i18ngrep "deprecated option.*or.*config" stderr && + test_must_fail git -c sendemail.identity=test -c sendemail.test.smtpssl=true send-email -1 && + test_i18ngrep "deprecated option.*or.*config" stderr +' + test_sendmail_aliases () { msg="$1" && shift && expect="$@" && -- 2.21.0.1020.gf2820cf01a ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v2 1/2] send-email: update the mechanism to set default configuration values 2019-05-08 10:13 ` Re* [PATCH 1/1] send-email: fix transferencoding config option Junio C Hamano 2019-05-08 10:56 ` [PATCH v2 0/2] send-email: set xfer encoding correctly Junio C Hamano @ 2019-05-08 10:56 ` Junio C Hamano 2019-05-08 10:56 ` [PATCH v2 2/2] send-email: honor transferencoding config option again Junio C Hamano 2 siblings, 0 replies; 39+ messages in thread From: Junio C Hamano @ 2019-05-08 10:56 UTC (permalink / raw) To: git; +Cc: Heinrich Schuchardt, Brian M Carlson, Jonathan Nieder The program has a good mechanism to specify the fallback default values for boolean configuration variables after two invocations of read_config() for "sendmail.$ident.$var" and "sendemail.$var" have not found any configuration. Imitate it so that we can set the default values for non-boolean variables as well. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- git-send-email.perl | 56 ++++++++++++++++++++++++++------------------- 1 file changed, 33 insertions(+), 23 deletions(-) diff --git a/git-send-email.perl b/git-send-email.perl index f4c07908d2..98bc295c6e 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -250,28 +250,28 @@ sub do_edit { ); my %config_settings = ( - "smtpserver" => \$smtp_server, - "smtpserverport" => \$smtp_server_port, - "smtpserveroption" => \@smtp_server_options, - "smtpuser" => \$smtp_authuser, - "smtppass" => \$smtp_authpass, - "smtpdomain" => \$smtp_domain, - "smtpauth" => \$smtp_auth, - "smtpbatchsize" => \$batch_size, - "smtprelogindelay" => \$relogin_delay, - "to" => \@initial_to, - "tocmd" => \$to_cmd, - "cc" => \@initial_cc, - "cccmd" => \$cc_cmd, - "aliasfiletype" => \$aliasfiletype, - "bcc" => \@bcclist, - "suppresscc" => \@suppress_cc, - "envelopesender" => \$envelope_sender, - "confirm" => \$confirm, - "from" => \$sender, - "assume8bitencoding" => \$auto_8bit_encoding, - "composeencoding" => \$compose_encoding, - "transferencoding" => \$target_xfer_encoding, + "smtpserver" => [\$smtp_server], + "smtpserverport" => [\$smtp_server_port], + "smtpserveroption" => [\@smtp_server_options], + "smtpuser" => [\$smtp_authuser], + "smtppass" => [\$smtp_authpass], + "smtpdomain" => [\$smtp_domain], + "smtpauth" => [\$smtp_auth], + "smtpbatchsize" => [\$batch_size], + "smtprelogindelay" => [\$relogin_delay], + "to" => [\@initial_to], + "tocmd" => [\$to_cmd], + "cc" => [\@initial_cc], + "cccmd" => [\$cc_cmd], + "aliasfiletype" => [\$aliasfiletype], + "bcc" => [\@bcclist], + "suppresscc" => [\@suppress_cc], + "envelopesender" => [\$envelope_sender], + "confirm" => [\$confirm], + "from" => [\$sender], + "assume8bitencoding" => [\$auto_8bit_encoding], + "composeencoding" => [\$compose_encoding], + "transferencoding" => [\$target_xfer_encoding], ); my %config_path_settings = ( @@ -411,7 +411,7 @@ sub read_config { } foreach my $setting (keys %config_settings) { - my $target = $config_settings{$setting}; + my $target = $config_settings{$setting}->[0]; next if $setting eq "to" and defined $no_to; next if $setting eq "cc" and defined $no_cc; next if $setting eq "bcc" and defined $no_bcc; @@ -446,6 +446,16 @@ sub read_config { ${$setting->[0]} = $setting->[1] unless (defined (${$setting->[0]})); } +# fall back to builtin defaults +while (my ($name, $setting) = each %config_settings) { + next unless @$setting == 2; + + my ($target, $default) = @$setting; + if (ref($target) eq "SCALAR") { + $$target = $default unless defined $$target; + } # elsif ... for other types later. +} + # 'default' encryption is none -- this only prevents a warning $smtp_encryption = '' unless (defined $smtp_encryption); -- 2.21.0-777-g83232e3864 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v2 2/2] send-email: honor transferencoding config option again 2019-05-08 10:13 ` Re* [PATCH 1/1] send-email: fix transferencoding config option Junio C Hamano 2019-05-08 10:56 ` [PATCH v2 0/2] send-email: set xfer encoding correctly Junio C Hamano 2019-05-08 10:56 ` [PATCH v2 1/2] send-email: update the mechanism to set default configuration values Junio C Hamano @ 2019-05-08 10:56 ` Junio C Hamano 2019-05-08 21:24 ` Eric Sunshine 2019-05-08 23:12 ` brian m. carlson 2 siblings, 2 replies; 39+ messages in thread From: Junio C Hamano @ 2019-05-08 10:56 UTC (permalink / raw) To: git; +Cc: Heinrich Schuchardt, Brian M Carlson, Jonathan Nieder Since e67a228cd8a ("send-email: automatically determine transfer-encoding"), the value of sendmail.transferencoding in the configuration file is ignored, because $target_xfer_encoding is already defined read_config sub parses the configuration file. Instead of initializing variable $target_xfer_encoding to 'auto' on definition, we have to set it to the default value of 'auto' if is undefined after calling read_config() twice to parsing the configuration files for "sendemail.transferencoding" and "sendemail.$ident.transferencoding". It was made trivial to do so by the previous change. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- git-send-email.perl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/git-send-email.perl b/git-send-email.perl index 98bc295c6e..b9dd775b63 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -231,7 +231,7 @@ sub do_edit { my (@suppress_cc); my ($auto_8bit_encoding); my ($compose_encoding); -my $target_xfer_encoding = 'auto'; +my ($target_xfer_encoding); my ($debug_net_smtp) = 0; # Net::SMTP, see send_message() @@ -271,7 +271,7 @@ sub do_edit { "from" => [\$sender], "assume8bitencoding" => [\$auto_8bit_encoding], "composeencoding" => [\$compose_encoding], - "transferencoding" => [\$target_xfer_encoding], + "transferencoding" => [\$target_xfer_encoding, 'auto'], ); my %config_path_settings = ( -- 2.21.0-777-g83232e3864 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH v2 2/2] send-email: honor transferencoding config option again 2019-05-08 10:56 ` [PATCH v2 2/2] send-email: honor transferencoding config option again Junio C Hamano @ 2019-05-08 21:24 ` Eric Sunshine 2019-05-09 6:47 ` Junio C Hamano 2019-05-08 23:12 ` brian m. carlson 1 sibling, 1 reply; 39+ messages in thread From: Eric Sunshine @ 2019-05-08 21:24 UTC (permalink / raw) To: Junio C Hamano Cc: Git List, Heinrich Schuchardt, Brian M Carlson, Jonathan Nieder On Wed, May 8, 2019 at 6:56 AM Junio C Hamano <gitster@pobox.com> wrote: > Since e67a228cd8a ("send-email: automatically determine > transfer-encoding"), the value of sendmail.transferencoding in the > configuration file is ignored, because $target_xfer_encoding is s/,// > already defined read_config sub parses the configuration file. s/defined/& when/ --or -- s/defined/& by the time/ > Instead of initializing variable $target_xfer_encoding to 'auto' on > definition, we have to set it to the default value of 'auto' if is s/if is/if it is/ > undefined after calling read_config() twice to parsing the s/parsing/parse/ > configuration files for "sendemail.transferencoding" and s/files/file/ (I think) > "sendemail.$ident.transferencoding". > > It was made trivial to do so by the previous change. > > Signed-off-by: Junio C Hamano <gitster@pobox.com> ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v2 2/2] send-email: honor transferencoding config option again 2019-05-08 21:24 ` Eric Sunshine @ 2019-05-09 6:47 ` Junio C Hamano 0 siblings, 0 replies; 39+ messages in thread From: Junio C Hamano @ 2019-05-09 6:47 UTC (permalink / raw) To: Eric Sunshine Cc: Git List, Heinrich Schuchardt, Brian M Carlson, Jonathan Nieder Eric Sunshine <sunshine@sunshineco.com> writes: > On Wed, May 8, 2019 at 6:56 AM Junio C Hamano <gitster@pobox.com> wrote: >> Since e67a228cd8a ("send-email: automatically determine >> transfer-encoding"), the value of sendmail.transferencoding in the >> configuration file is ignored, because $target_xfer_encoding is > > s/,// (also all other typofixes are obviously correct) Thanks. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v2 2/2] send-email: honor transferencoding config option again 2019-05-08 10:56 ` [PATCH v2 2/2] send-email: honor transferencoding config option again Junio C Hamano 2019-05-08 21:24 ` Eric Sunshine @ 2019-05-08 23:12 ` brian m. carlson 1 sibling, 0 replies; 39+ messages in thread From: brian m. carlson @ 2019-05-08 23:12 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Heinrich Schuchardt, Jonathan Nieder [-- Attachment #1: Type: text/plain, Size: 1016 bytes --] On Wed, May 08, 2019 at 07:56:07PM +0900, Junio C Hamano wrote: > diff --git a/git-send-email.perl b/git-send-email.perl > index 98bc295c6e..b9dd775b63 100755 > --- a/git-send-email.perl > +++ b/git-send-email.perl > @@ -231,7 +231,7 @@ sub do_edit { > my (@suppress_cc); > my ($auto_8bit_encoding); > my ($compose_encoding); > -my $target_xfer_encoding = 'auto'; > +my ($target_xfer_encoding); > > my ($debug_net_smtp) = 0; # Net::SMTP, see send_message() > > @@ -271,7 +271,7 @@ sub do_edit { > "from" => [\$sender], > "assume8bitencoding" => [\$auto_8bit_encoding], > "composeencoding" => [\$compose_encoding], > - "transferencoding" => [\$target_xfer_encoding], > + "transferencoding" => [\$target_xfer_encoding, 'auto'], > ); These changes seem sane and the series makes the second one straightforward. It's also a nice way to add defaults in the future, so thanks for that. -- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 868 bytes --] ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/1] send-email: fix transferencoding config option 2019-04-09 19:27 [PATCH 1/1] send-email: fix transferencoding config option Heinrich Schuchardt 2019-04-09 21:58 ` Jonathan Nieder @ 2019-04-09 22:55 ` brian m. carlson 2019-04-09 23:06 ` Heinrich Schuchardt 1 sibling, 1 reply; 39+ messages in thread From: brian m. carlson @ 2019-04-09 22:55 UTC (permalink / raw) To: Heinrich Schuchardt; +Cc: Junio C Hamano, git [-- Attachment #1: Type: text/plain, Size: 1867 bytes --] On Tue, Apr 09, 2019 at 09:27:33PM +0200, Heinrich Schuchardt wrote: > diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt > index 1afe9fc858..884e776add 100644 > --- a/Documentation/git-send-email.txt > +++ b/Documentation/git-send-email.txt > @@ -146,7 +146,7 @@ Note that no attempts whatsoever are made to validate the encoding. > even more opaque. auto will use 8bit when possible, and quoted-printable > otherwise. > + > -Default is the value of the `sendemail.transferEncoding` configuration > +Default is the value of the `sendemail.transferencoding` configuration > value; if that is unspecified, default to `auto`. In Git, two-part config settings are case-insensitive. We traditionally write them in lower camel case because it's easier for people to read. Git will canonicalize the values when "git config" runs. So I don't think we should change this. > diff --git a/git-send-email.perl b/git-send-email.perl > index 8200d58cdc..0e23193939 100755 > --- a/git-send-email.perl > +++ b/git-send-email.perl > @@ -239,7 +239,7 @@ sub do_edit { > my (@suppress_cc); > my ($auto_8bit_encoding); > my ($compose_encoding); > -my $target_xfer_encoding = 'auto'; > +my ($target_xfer_encoding); > > my ($debug_net_smtp) = 0; # Net::SMTP, see send_message() > > @@ -446,6 +446,8 @@ sub read_config { > $smtp_encryption = 'ssl'; > } > } > + > + $target_xfer_encoding = 'auto' unless (defined $target_xfer_encoding); > } > > # read configuration from [sendemail "$identity"], fall back on [sendemail] Thanks for fixing this. I didn't realize that we only set values if the variable holding them is undef. Would you mind adding a test for this case so we won't regress it in the future? -- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 868 bytes --] ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/1] send-email: fix transferencoding config option 2019-04-09 22:55 ` [PATCH 1/1] send-email: fix transferencoding config option brian m. carlson @ 2019-04-09 23:06 ` Heinrich Schuchardt 0 siblings, 0 replies; 39+ messages in thread From: Heinrich Schuchardt @ 2019-04-09 23:06 UTC (permalink / raw) To: brian m. carlson, Junio C Hamano, git On 4/10/19 12:55 AM, brian m. carlson wrote: > On Tue, Apr 09, 2019 at 09:27:33PM +0200, Heinrich Schuchardt wrote: >> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt >> index 1afe9fc858..884e776add 100644 >> --- a/Documentation/git-send-email.txt >> +++ b/Documentation/git-send-email.txt >> @@ -146,7 +146,7 @@ Note that no attempts whatsoever are made to validate the encoding. >> even more opaque. auto will use 8bit when possible, and quoted-printable >> otherwise. >> + >> -Default is the value of the `sendemail.transferEncoding` configuration >> +Default is the value of the `sendemail.transferencoding` configuration >> value; if that is unspecified, default to `auto`. > > In Git, two-part config settings are case-insensitive. We traditionally > write them in lower camel case because it's easier for people to read. > Git will canonicalize the values when "git config" runs. > > So I don't think we should change this. Thanks for the hint. > >> diff --git a/git-send-email.perl b/git-send-email.perl >> index 8200d58cdc..0e23193939 100755 >> --- a/git-send-email.perl >> +++ b/git-send-email.perl >> @@ -239,7 +239,7 @@ sub do_edit { >> my (@suppress_cc); >> my ($auto_8bit_encoding); >> my ($compose_encoding); >> -my $target_xfer_encoding = 'auto'; >> +my ($target_xfer_encoding); >> >> my ($debug_net_smtp) = 0; # Net::SMTP, see send_message() >> >> @@ -446,6 +446,8 @@ sub read_config { >> $smtp_encryption = 'ssl'; >> } >> } >> + >> + $target_xfer_encoding = 'auto' unless (defined $target_xfer_encoding); >> } >> >> # read configuration from [sendemail "$identity"], fall back on [sendemail] > > Thanks for fixing this. I didn't realize that we only set values if the > variable holding them is undef. Would you mind adding a test for this > case so we won't regress it in the future? > Nice challenge for my first patch for git :) I will give it a try. Best regards Heinrich ^ permalink raw reply [flat|nested] 39+ messages in thread
end of thread, other threads:[~2019-05-29 9:11 UTC | newest] Thread overview: 39+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-04-09 19:27 [PATCH 1/1] send-email: fix transferencoding config option Heinrich Schuchardt 2019-04-09 21:58 ` Jonathan Nieder 2019-04-09 23:39 ` Heinrich Schuchardt 2019-04-10 3:48 ` Junio C Hamano 2019-04-10 20:40 ` Heinrich Schuchardt 2019-04-10 22:42 ` brian m. carlson 2019-05-08 8:18 ` Re* " Junio C Hamano 2019-05-08 8:20 ` [PATCH 2/2] send-email: honor transferencoding config option again Junio C Hamano 2019-05-08 10:13 ` Re* [PATCH 1/1] send-email: fix transferencoding config option Junio C Hamano 2019-05-08 10:56 ` [PATCH v2 0/2] send-email: set xfer encoding correctly Junio C Hamano 2019-05-09 11:48 ` [PATCH v3 0/3] send-email: fix cli->config parsing crazyness Ævar Arnfjörð Bjarmason 2019-05-10 13:50 ` Junio C Hamano 2019-05-09 11:48 ` [PATCH v3 1/3] send-email: move the read_config() function above getopts Ævar Arnfjörð Bjarmason 2019-05-09 11:48 ` [PATCH v3 2/3] send-email: rename the @bcclist variable for consistency Ævar Arnfjörð Bjarmason 2019-05-09 11:48 ` [PATCH v3 3/3] send-email: do defaults -> config -> getopt in that order Ævar Arnfjörð Bjarmason 2019-05-09 18:04 ` Eric Sunshine 2019-05-13 8:46 ` Junio C Hamano 2019-05-09 23:51 ` brian m. carlson 2019-05-13 8:50 ` Junio C Hamano 2019-05-13 21:13 ` Ævar Arnfjörð Bjarmason 2019-05-16 22:59 ` Stephen Boyd 2019-05-16 23:13 ` Junio C Hamano 2019-05-17 3:43 ` Junio C Hamano 2019-05-17 19:55 ` [PATCH 0/5] ab/send-email-transferencoding-fix-for-the-fix Ævar Arnfjörð Bjarmason 2019-05-17 19:55 ` [PATCH 1/5] send-email: remove cargo-culted multi-patch pattern in tests Ævar Arnfjörð Bjarmason 2019-05-17 19:55 ` [PATCH 2/5] send-email: fix broken transferEncoding tests Ævar Arnfjörð Bjarmason 2019-05-17 19:55 ` [PATCH 3/5] send-email: document --no-[to|cc|bcc] Ævar Arnfjörð Bjarmason 2019-05-17 19:55 ` [PATCH 4/5] send-email: fix regression in sendemail.identity parsing Ævar Arnfjörð Bjarmason 2019-05-19 1:29 ` Junio C Hamano 2019-05-22 20:25 ` Johannes Schindelin 2019-05-29 9:10 ` Johannes Schindelin 2019-05-17 19:55 ` [PATCH 5/5] send-email: remove support for deprecated sendemail.smtpssl Ævar Arnfjörð Bjarmason 2019-05-08 10:56 ` [PATCH v2 1/2] send-email: update the mechanism to set default configuration values Junio C Hamano 2019-05-08 10:56 ` [PATCH v2 2/2] send-email: honor transferencoding config option again Junio C Hamano 2019-05-08 21:24 ` Eric Sunshine 2019-05-09 6:47 ` Junio C Hamano 2019-05-08 23:12 ` brian m. carlson 2019-04-09 22:55 ` [PATCH 1/1] send-email: fix transferencoding config option brian m. carlson 2019-04-09 23:06 ` Heinrich Schuchardt
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git 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).