* Regression in git send-email parsing sendemail.* config values
@ 2021-09-05 23:01 Eli Schwartz
2021-09-06 0:04 ` Ævar Arnfjörð Bjarmason
0 siblings, 1 reply; 7+ messages in thread
From: Eli Schwartz @ 2021-09-05 23:01 UTC (permalink / raw)
To: git
I recently noticed that git send-email was attempting to send emails
using the wrong email address. I have a global email configuration in
XDG_CONFIG_HOME, and a specific one set in the {repo}/.git/config of
some repos... this was trying to use the global configuration.
`git config -l | grep ^sendemail.smtpserver=` reports two emails
`git config --get sendemail.smtpserver` reports only the second,
repo-specific one
I bisected the issue to commit c95e3a3f0b8107b5dc7eac9dfdb9e5238280c9fb
send-email: move trivial config handling to Perl
Using this commit, git-send-email disagrees with git config --get on
which email to use.
Using commit f4dc9432fd287bde9100488943baf3c6a04d90d1 immediately
preceding this commit, git send-email agrees with git config --get.
--
Eli Schwartz
Arch Linux Bug Wrangler and Trusted User
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Regression in git send-email parsing sendemail.* config values
2021-09-05 23:01 Regression in git send-email parsing sendemail.* config values Eli Schwartz
@ 2021-09-06 0:04 ` Ævar Arnfjörð Bjarmason
2021-09-06 1:44 ` Eli Schwartz
0 siblings, 1 reply; 7+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-09-06 0:04 UTC (permalink / raw)
To: Eli Schwartz; +Cc: git
On Sun, Sep 05 2021, Eli Schwartz wrote:
> I recently noticed that git send-email was attempting to send emails
> using the wrong email address. I have a global email configuration in
> XDG_CONFIG_HOME, and a specific one set in the {repo}/.git/config of
> some repos... this was trying to use the global configuration.
>
> `git config -l | grep ^sendemail.smtpserver=` reports two emails
Don't you mean s/emails/servers/, ditto "wrong email address" should be
"the wrong server", right?
> `git config --get sendemail.smtpserver` reports only the second,
> repo-specific one
>
>
> I bisected the issue to commit c95e3a3f0b8107b5dc7eac9dfdb9e5238280c9fb
>
> send-email: move trivial config handling to Perl
>
>
> Using this commit, git-send-email disagrees with git config --get on
> which email to use.
>
> Using commit f4dc9432fd287bde9100488943baf3c6a04d90d1 immediately
> preceding this commit, git send-email agrees with git config --get.
That's a pretty bad bug, sorry about that. I believe that the following
patch should fix it (needs tests obviously). I.e. when we had N config
keys we'd previously pick the normal "last key wins", which my
c95e3a3f0b8107b5dc7eac9dfdb9e5238280c9fb changed to "first wins":
diff --git a/git-send-email.perl b/git-send-email.perl
index e65d969d0bb..6c7ab3d2e91 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -376,7 +376,7 @@ sub read_config {
@$target = @values;
}
else {
- my $v = $known_keys->{$key}->[0];
+ my $v = $known_keys->{$key}->[-1];
next unless defined $v;
next if $configured->{$setting}++;
$$target = $v;
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: Regression in git send-email parsing sendemail.* config values
2021-09-06 0:04 ` Ævar Arnfjörð Bjarmason
@ 2021-09-06 1:44 ` Eli Schwartz
2021-09-06 7:33 ` [PATCH] send-email: fix a "first config key wins" regression in v2.33.0 Ævar Arnfjörð Bjarmason
0 siblings, 1 reply; 7+ messages in thread
From: Eli Schwartz @ 2021-09-06 1:44 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason; +Cc: git
[-- Attachment #1.1: Type: text/plain, Size: 2364 bytes --]
On 9/5/21 8:04 PM, Ævar Arnfjörð Bjarmason wrote:
>
> On Sun, Sep 05 2021, Eli Schwartz wrote:
>
>> I recently noticed that git send-email was attempting to send emails
>> using the wrong email address. I have a global email configuration in
>> XDG_CONFIG_HOME, and a specific one set in the {repo}/.git/config of
>> some repos... this was trying to use the global configuration.
>>
>> `git config -l | grep ^sendemail.smtpserver=` reports two emails
>
> Don't you mean s/emails/servers/, ditto "wrong email address" should be
> "the wrong server", right?
Considering the time of technically-it-is-day-now I wrote this email, I
suppose I should have proofread it before sending it the next day.
I'll claim some sort of self-defense in that the email username
(smtpuser) was different, in addition to the smtpserver. Yeah, that was it!
(also I am used to thinking of email servers as a subcomponent of email
addresses even though this is technically not true, shhh)
>> `git config --get sendemail.smtpserver` reports only the second,
>> repo-specific one
>>
>>
>> I bisected the issue to commit c95e3a3f0b8107b5dc7eac9dfdb9e5238280c9fb
>>
>> send-email: move trivial config handling to Perl
>>
>>
>> Using this commit, git-send-email disagrees with git config --get on
>> which email to use.
>>
>> Using commit f4dc9432fd287bde9100488943baf3c6a04d90d1 immediately
>> preceding this commit, git send-email agrees with git config --get.
>
> That's a pretty bad bug, sorry about that. I believe that the following
> patch should fix it (needs tests obviously). I.e. when we had N config
> keys we'd previously pick the normal "last key wins", which my
> c95e3a3f0b8107b5dc7eac9dfdb9e5238280c9fb changed to "first wins":
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index e65d969d0bb..6c7ab3d2e91 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -376,7 +376,7 @@ sub read_config {
> @$target = @values;
> }
> else {
> - my $v = $known_keys->{$key}->[0];
> + my $v = $known_keys->{$key}->[-1];
> next unless defined $v;
> next if $configured->{$setting}++;
> $$target = $v;
>
Thanks, this worked for me and fixed my problem! Feel free to add my
tested-by.
--
Eli Schwartz
Arch Linux Bug Wrangler and Trusted User
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] send-email: fix a "first config key wins" regression in v2.33.0
2021-09-06 1:44 ` Eli Schwartz
@ 2021-09-06 7:33 ` Ævar Arnfjörð Bjarmason
2021-09-06 8:32 ` Carlo Arenas
2021-09-06 11:05 ` Bagas Sanjaya
0 siblings, 2 replies; 7+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-09-06 7:33 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Eli Schwartz, Jeff King, Eric Sunshine,
Elijah Newren, Ævar Arnfjörð Bjarmason
Fix a regression in my c95e3a3f0b8 (send-email: move trivial config
handling to Perl, 2021-05-28) where we'd pick the first config key out
of multiple defined ones, instead of using the normal "last key wins"
semantics of "git config --get".
This broke e.g. cases where a .git/config would have a different
sendemail.smtpServer than ~/.gitconfig. We'd pick the ~/.gitconfig
over .git/config, instead of preferring the repository-local
version. The same would go for /etc/gitconfig etc.
The full list of impacted config keys (the %config_settings values
which are references to scalars, not arrays) is:
sendemail.smtpencryption
sendemail.smtpserver
sendemail.smtpserverport
sendemail.smtpuser
sendemail.smtppass
sendemail.smtpdomain
sendemail.smtpauth
sendemail.smtpbatchsize
sendemail.smtprelogindelay
sendemail.tocmd
sendemail.cccmd
sendemail.aliasfiletype
sendemail.envelopesender
sendemail.confirm
sendemail.from
sendemail.assume8bitencoding
sendemail.composeencoding
sendemail.transferencoding
sendemail.sendmailcmd
I.e. having any of these set in say ~/.gitconfig and in-repo
.git/config regressed in v2.33.0 to prefer the --global one over the
--local.
To test this add a test of config priority to one of these config
variables, most don't have tests at all, but there was an existing one
for sendemail.8bitEncoding.
The "git config" (instead of "test_config") is somewhat of an
anti-pattern, but follows established conventions in
t9001-send-email.sh, likewise with any other pattern or idiom in this
test.
The populating of home/.gitconfig and setting of HOME= is copied from
a test in t0017-env-helper.sh added in 1ff750b128e (tests: make
GIT_TEST_GETTEXT_POISON a boolean, 2019-06-21). This test fails
without this bugfix, but not works.
Reported-by: Eli Schwartz <eschwartz@archlinux.org>
Tested-by: Eli Schwartz <eschwartz@archlinux.org>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
On Sun, Sep 05 2021, Eli Schwartz wrote:
> [[PGP Signed Part:Undecided]]
> On 9/5/21 8:04 PM, Ævar Arnfjörð Bjarmason wrote:
>>
>> On Sun, Sep 05 2021, Eli Schwartz wrote:
>>
>>> I recently noticed that git send-email was attempting to send emails
>>> using the wrong email address. I have a global email configuration in
>>> XDG_CONFIG_HOME, and a specific one set in the {repo}/.git/config of
>>> some repos... this was trying to use the global configuration.
>>>
>>> `git config -l | grep ^sendemail.smtpserver=` reports two emails
[...]
>>> `git config --get sendemail.smtpserver` reports only the second,
>>> repo-specific one
>>>
>>>
>>> I bisected the issue to commit c95e3a3f0b8107b5dc7eac9dfdb9e5238280c9fb
>>>
>>> send-email: move trivial config handling to Perl
>>>
>>>
>>> Using this commit, git-send-email disagrees with git config --get on
>>> which email to use.
>>>
>>> Using commit f4dc9432fd287bde9100488943baf3c6a04d90d1 immediately
>>> preceding this commit, git send-email agrees with git config --get.
>>
>> That's a pretty bad bug, sorry about that. I believe that the following
>> patch should fix it (needs tests obviously). I.e. when we had N config
>> keys we'd previously pick the normal "last key wins", which my
>> c95e3a3f0b8107b5dc7eac9dfdb9e5238280c9fb changed to "first wins":
>>
>> diff --git a/git-send-email.perl b/git-send-email.perl
>> index e65d969d0bb..6c7ab3d2e91 100755
>> --- a/git-send-email.perl
>> +++ b/git-send-email.perl
>> @@ -376,7 +376,7 @@ sub read_config {
>> @$target = @values;
>> }
>> else {
>> - my $v = $known_keys->{$key}->[0];
>> + my $v = $known_keys->{$key}->[-1];
>> next unless defined $v;
>> next if $configured->{$setting}++;
>> $$target = $v;
>>
>
>
>
> Thanks, this worked for me and fixed my problem! Feel free to add my
> tested-by.
Added, and here's a properly formatted patch with a regression test.
git-send-email.perl | 2 +-
t/t9001-send-email.sh | 15 +++++++++++++++
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index e65d969d0bb..6c7ab3d2e91 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -376,7 +376,7 @@ sub read_config {
@$target = @values;
}
else {
- my $v = $known_keys->{$key}->[0];
+ my $v = $known_keys->{$key}->[-1];
next unless defined $v;
next if $configured->{$setting}++;
$$target = $v;
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 57fc10e7f82..eae172e0a05 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1533,6 +1533,21 @@ test_expect_success $PREREQ 'sendemail.8bitEncoding works' '
test_cmp content-type-decl actual
'
+test_expect_success $PREREQ 'sendemail.8bitEncoding in .git/config overrides --global .gitconfig' '
+ clean_fake_sendmail &&
+ git config sendemail.assume8bitEncoding UTF-8 &&
+ test_when_finished "rm -rf home" &&
+ mkdir home &&
+ git config -f home/.gitconfig sendemail.assume8bitEncoding "bogus too" &&
+ echo bogus |
+ env HOME="$(pwd)/home" DEBUG=1 \
+ git send-email --from=author@example.com --to=nobody@example.com \
+ --smtp-server="$(pwd)/fake.sendmail" \
+ email-using-8bit >stdout &&
+ egrep "Content|MIME" msgtxt1 >actual &&
+ test_cmp content-type-decl actual
+'
+
test_expect_success $PREREQ '--8bit-encoding overrides sendemail.8bitEncoding' '
clean_fake_sendmail &&
git config sendemail.assume8bitEncoding "bogus too" &&
--
2.33.0.821.gfd4106eadbd
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] send-email: fix a "first config key wins" regression in v2.33.0
2021-09-06 7:33 ` [PATCH] send-email: fix a "first config key wins" regression in v2.33.0 Ævar Arnfjörð Bjarmason
@ 2021-09-06 8:32 ` Carlo Arenas
2021-09-06 11:05 ` Bagas Sanjaya
1 sibling, 0 replies; 7+ messages in thread
From: Carlo Arenas @ 2021-09-06 8:32 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, Junio C Hamano, Eli Schwartz, Jeff King, Eric Sunshine,
Elijah Newren
On Mon, Sep 6, 2021 at 12:35 AM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> This test fails without this bugfix, but not works.
s/not/now
Carlo
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] send-email: fix a "first config key wins" regression in v2.33.0
2021-09-06 7:33 ` [PATCH] send-email: fix a "first config key wins" regression in v2.33.0 Ævar Arnfjörð Bjarmason
2021-09-06 8:32 ` Carlo Arenas
@ 2021-09-06 11:05 ` Bagas Sanjaya
2021-09-07 9:15 ` Ævar Arnfjörð Bjarmason
1 sibling, 1 reply; 7+ messages in thread
From: Bagas Sanjaya @ 2021-09-06 11:05 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason, git
Cc: Junio C Hamano, Eli Schwartz, Jeff King, Eric Sunshine, Elijah Newren
On 06/09/21 14.33, Ævar Arnfjörð Bjarmason wrote:
> +test_expect_success $PREREQ 'sendemail.8bitEncoding in .git/config overrides --global .gitconfig' '
> + clean_fake_sendmail &&
> + git config sendemail.assume8bitEncoding UTF-8 &&
> + test_when_finished "rm -rf home" &&
> + mkdir home &&
> + git config -f home/.gitconfig sendemail.assume8bitEncoding "bogus too" &&
> + echo bogus |
> + env HOME="$(pwd)/home" DEBUG=1 \
> + git send-email --from=author@example.com --to=nobody@example.com \
> + --smtp-server="$(pwd)/fake.sendmail" \
> + email-using-8bit >stdout &&
> + egrep "Content|MIME" msgtxt1 >actual &&
> + test_cmp content-type-decl actual
> +'
> +
Did you mean overrides global .gitconfig (s/--global/global/)?
Anyway, compiled and tested successfully (test suite passed).
Tested-by: Bagas Sanjaya <bagasdotme@gmail.com>
--
An old man doll... just what I always wanted! - Clara
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] send-email: fix a "first config key wins" regression in v2.33.0
2021-09-06 11:05 ` Bagas Sanjaya
@ 2021-09-07 9:15 ` Ævar Arnfjörð Bjarmason
0 siblings, 0 replies; 7+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-09-07 9:15 UTC (permalink / raw)
To: Bagas Sanjaya
Cc: git, Junio C Hamano, Eli Schwartz, Jeff King, Eric Sunshine,
Elijah Newren
On Mon, Sep 06 2021, Bagas Sanjaya wrote:
> On 06/09/21 14.33, Ævar Arnfjörð Bjarmason wrote:
>> +test_expect_success $PREREQ 'sendemail.8bitEncoding in .git/config overrides --global .gitconfig' '
>> + clean_fake_sendmail &&
>> + git config sendemail.assume8bitEncoding UTF-8 &&
>> + test_when_finished "rm -rf home" &&
>> + mkdir home &&
>> + git config -f home/.gitconfig sendemail.assume8bitEncoding "bogus too" &&
>> + echo bogus |
>> + env HOME="$(pwd)/home" DEBUG=1 \
>> + git send-email --from=author@example.com --to=nobody@example.com \
>> + --smtp-server="$(pwd)/fake.sendmail" \
>> + email-using-8bit >stdout &&
>> + egrep "Content|MIME" msgtxt1 >actual &&
>> + test_cmp content-type-decl actual
>> +'
>> +
>
> Did you mean overrides global .gitconfig (s/--global/global/)?
I mean config set with the equivalent of "git config --global", but yeah
this is probably an awkward way to phrase that.
> Anyway, compiled and tested successfully (test suite passed).
>
> Tested-by: Bagas Sanjaya <bagasdotme@gmail.com>
Thanks, FWIW the test suite passing doesn't mean much in this case, the
only test that stresses this is the above. The issue being that the
regression wasn't caught by any existing test...
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-09-07 9:16 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-05 23:01 Regression in git send-email parsing sendemail.* config values Eli Schwartz
2021-09-06 0:04 ` Ævar Arnfjörð Bjarmason
2021-09-06 1:44 ` Eli Schwartz
2021-09-06 7:33 ` [PATCH] send-email: fix a "first config key wins" regression in v2.33.0 Ævar Arnfjörð Bjarmason
2021-09-06 8:32 ` Carlo Arenas
2021-09-06 11:05 ` Bagas Sanjaya
2021-09-07 9:15 ` Ævar Arnfjörð Bjarmason
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 NNTP newsgroup(s).