{"thread":{"id":"56446","subject":"Regression in git send-email parsing sendemail.* config values","startedAt":"2021-09-05T23:10:27Z","lastAt":"2021-09-07T09:16:36Z","messageCount":7,"participants":["Eli Schwartz","Ævar Arnfjörð Bjarmason","Carlo Arenas","Bagas Sanjaya"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"434739","messageId":"e86f64ff-0395-26f7-5d5b-7df2e6e63ff4@archlinux.org","threadId":"56446","inReplyTo":null,"subject":"Regression in git send-email parsing sendemail.* config values","fromName":"Eli Schwartz","fromEmail":"eschwartz@archlinux.org","sentAt":"2021-09-05T23:01:00Z","receivedAt":"2021-09-05T23:10:27Z","isPatch":false,"sender":{"key":"eschwartz@archlinux.org","avatar":"https://avatars.githubusercontent.com/u/6551424?v=4"},"body":"I recently noticed that git send-email was attempting to send emails\nusing the wrong email address. I have a global email configuration in\nXDG_CONFIG_HOME, and a specific one set in the {repo}/.git/config of\nsome repos... this was trying to use the global configuration.\n\n`git config -l | grep ^sendemail.smtpserver=` reports two emails\n\n`git config --get sendemail.smtpserver` reports only the second,\nrepo-specific one\n\n\nI bisected the issue to commit c95e3a3f0b8107b5dc7eac9dfdb9e5238280c9fb\n\n    send-email: move trivial config handling to Perl\n\n\nUsing this commit, git-send-email disagrees with git config --get on\nwhich email to use.\n\nUsing commit f4dc9432fd287bde9100488943baf3c6a04d90d1 immediately\npreceding this commit, git send-email agrees with git config --get.\n\n\n-- \nEli Schwartz\nArch Linux Bug Wrangler and Trusted User\n"},{"id":"434745","messageId":"87k0jupnxi.fsf@evledraar.gmail.com","threadId":"56446","inReplyTo":"e86f64ff-0395-26f7-5d5b-7df2e6e63ff4@archlinux.org","subject":"Re: Regression in git send-email parsing sendemail.* config values","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-06T00:04:44Z","receivedAt":"2021-09-06T00:45:16Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sun, Sep 05 2021, Eli Schwartz wrote:\n\n> I recently noticed that git send-email was attempting to send emails\n> using the wrong email address. I have a global email configuration in\n> XDG_CONFIG_HOME, and a specific one set in the {repo}/.git/config of\n> some repos... this was trying to use the global configuration.\n>\n> `git config -l | grep ^sendemail.smtpserver=` reports two emails\n\nDon't you mean s/emails/servers/, ditto \"wrong email address\" should be\n\"the wrong server\", right?\n\n> `git config --get sendemail.smtpserver` reports only the second,\n> repo-specific one\n>\n>\n> I bisected the issue to commit c95e3a3f0b8107b5dc7eac9dfdb9e5238280c9fb\n>\n>     send-email: move trivial config handling to Perl\n>\n>\n> Using this commit, git-send-email disagrees with git config --get on\n> which email to use.\n>\n> Using commit f4dc9432fd287bde9100488943baf3c6a04d90d1 immediately\n> preceding this commit, git send-email agrees with git config --get.\n\nThat's a pretty bad bug, sorry about that. I believe that the following\npatch should fix it (needs tests obviously). I.e. when we had N config\nkeys we'd previously pick the normal \"last key wins\", which my\nc95e3a3f0b8107b5dc7eac9dfdb9e5238280c9fb changed to \"first wins\":\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex e65d969d0bb..6c7ab3d2e91 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -376,7 +376,7 @@ sub read_config {\n \t\t\t@$target = @values;\n \t\t}\n \t\telse {\n-\t\t\tmy $v = $known_keys->{$key}->[0];\n+\t\t\tmy $v = $known_keys->{$key}->[-1];\n \t\t\tnext unless defined $v;\n \t\t\tnext if $configured->{$setting}++;\n \t\t\t$$target = $v;\n"},{"id":"434752","messageId":"96814e5e-54be-1eca-0d75-68be53b1be3d@archlinux.org","threadId":"56446","inReplyTo":"87k0jupnxi.fsf@evledraar.gmail.com","subject":"Re: Regression in git send-email parsing sendemail.* config values","fromName":"Eli Schwartz","fromEmail":"eschwartz@archlinux.org","sentAt":"2021-09-06T01:44:31Z","receivedAt":"2021-09-06T01:44:39Z","isPatch":false,"sender":{"key":"eschwartz@archlinux.org","avatar":"https://avatars.githubusercontent.com/u/6551424?v=4"},"body":"On 9/5/21 8:04 PM, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Sun, Sep 05 2021, Eli Schwartz wrote:\n> \n>> I recently noticed that git send-email was attempting to send emails\n>> using the wrong email address. I have a global email configuration in\n>> XDG_CONFIG_HOME, and a specific one set in the {repo}/.git/config of\n>> some repos... this was trying to use the global configuration.\n>>\n>> `git config -l | grep ^sendemail.smtpserver=` reports two emails\n> \n> Don't you mean s/emails/servers/, ditto \"wrong email address\" should be\n> \"the wrong server\", right?\n\n\nConsidering the time of technically-it-is-day-now I wrote this email, I\nsuppose I should have proofread it before sending it the next day.\n\nI'll claim some sort of self-defense in that the email username\n(smtpuser) was different, in addition to the smtpserver. Yeah, that was it!\n\n(also I am used to thinking of email servers as a subcomponent of email\naddresses even though this is technically not true, shhh)\n\n\n>> `git config --get sendemail.smtpserver` reports only the second,\n>> repo-specific one\n>>\n>>\n>> I bisected the issue to commit c95e3a3f0b8107b5dc7eac9dfdb9e5238280c9fb\n>>\n>>     send-email: move trivial config handling to Perl\n>>\n>>\n>> Using this commit, git-send-email disagrees with git config --get on\n>> which email to use.\n>>\n>> Using commit f4dc9432fd287bde9100488943baf3c6a04d90d1 immediately\n>> preceding this commit, git send-email agrees with git config --get.\n> \n> That's a pretty bad bug, sorry about that. I believe that the following\n> patch should fix it (needs tests obviously). I.e. when we had N config\n> keys we'd previously pick the normal \"last key wins\", which my\n> c95e3a3f0b8107b5dc7eac9dfdb9e5238280c9fb changed to \"first wins\":\n> \n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index e65d969d0bb..6c7ab3d2e91 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -376,7 +376,7 @@ sub read_config {\n>  \t\t\t@$target = @values;\n>  \t\t}\n>  \t\telse {\n> -\t\t\tmy $v = $known_keys->{$key}->[0];\n> +\t\t\tmy $v = $known_keys->{$key}->[-1];\n>  \t\t\tnext unless defined $v;\n>  \t\t\tnext if $configured->{$setting}++;\n>  \t\t\t$$target = $v;\n> \n\n\n\nThanks, this worked for me and fixed my problem! Feel free to add my\ntested-by.\n\n\n-- \nEli Schwartz\nArch Linux Bug Wrangler and Trusted User\n\n"},{"id":"434776","messageId":"patch-1.1-ae6ff9f77f1-20210906T073306Z-avarab@gmail.com","threadId":"56446","inReplyTo":"96814e5e-54be-1eca-0d75-68be53b1be3d@archlinux.org","subject":"[PATCH] send-email: fix a \"first config key wins\" regression in v2.33.0","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-06T07:33:29Z","receivedAt":"2021-09-06T07:33:35Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a regression in my c95e3a3f0b8 (send-email: move trivial config\nhandling to Perl, 2021-05-28) where we'd pick the first config key out\nof multiple defined ones, instead of using the normal \"last key wins\"\nsemantics of \"git config --get\".\n\nThis broke e.g. cases where a .git/config would have a different\nsendemail.smtpServer than ~/.gitconfig. We'd pick the ~/.gitconfig\nover .git/config, instead of preferring the repository-local\nversion. The same would go for /etc/gitconfig etc.\n\nThe full list of impacted config keys (the %config_settings values\nwhich are references to scalars, not arrays) is:\n\n    sendemail.smtpencryption\n    sendemail.smtpserver\n    sendemail.smtpserverport\n    sendemail.smtpuser\n    sendemail.smtppass\n    sendemail.smtpdomain\n    sendemail.smtpauth\n    sendemail.smtpbatchsize\n    sendemail.smtprelogindelay\n    sendemail.tocmd\n    sendemail.cccmd\n    sendemail.aliasfiletype\n    sendemail.envelopesender\n    sendemail.confirm\n    sendemail.from\n    sendemail.assume8bitencoding\n    sendemail.composeencoding\n    sendemail.transferencoding\n    sendemail.sendmailcmd\n\nI.e. having any of these set in say ~/.gitconfig and in-repo\n.git/config regressed in v2.33.0 to prefer the --global one over the\n--local.\n\nTo test this add a test of config priority to one of these config\nvariables, most don't have tests at all, but there was an existing one\nfor sendemail.8bitEncoding.\n\nThe \"git config\" (instead of \"test_config\") is somewhat of an\nanti-pattern, but follows established conventions in\nt9001-send-email.sh, likewise with any other pattern or idiom in this\ntest.\n\nThe populating of home/.gitconfig and setting of HOME= is copied from\na test in t0017-env-helper.sh added in 1ff750b128e (tests: make\nGIT_TEST_GETTEXT_POISON a boolean, 2019-06-21). This test fails\nwithout this bugfix, but not works.\n\nReported-by: Eli Schwartz <eschwartz@archlinux.org>\nTested-by: Eli Schwartz <eschwartz@archlinux.org>\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n\nOn Sun, Sep 05 2021, Eli Schwartz wrote:\n\n> [[PGP Signed Part:Undecided]]\n> On 9/5/21 8:04 PM, Ævar Arnfjörð Bjarmason wrote:\n>> \n>> On Sun, Sep 05 2021, Eli Schwartz wrote:\n>> \n>>> I recently noticed that git send-email was attempting to send emails\n>>> using the wrong email address. I have a global email configuration in\n>>> XDG_CONFIG_HOME, and a specific one set in the {repo}/.git/config of\n>>> some repos... this was trying to use the global configuration.\n>>>\n>>> `git config -l | grep ^sendemail.smtpserver=` reports two emails\n\n[...]\n\n>>> `git config --get sendemail.smtpserver` reports only the second,\n>>> repo-specific one\n>>>\n>>>\n>>> I bisected the issue to commit c95e3a3f0b8107b5dc7eac9dfdb9e5238280c9fb\n>>>\n>>>     send-email: move trivial config handling to Perl\n>>>\n>>>\n>>> Using this commit, git-send-email disagrees with git config --get on\n>>> which email to use.\n>>>\n>>> Using commit f4dc9432fd287bde9100488943baf3c6a04d90d1 immediately\n>>> preceding this commit, git send-email agrees with git config --get.\n>> \n>> That's a pretty bad bug, sorry about that. I believe that the following\n>> patch should fix it (needs tests obviously). I.e. when we had N config\n>> keys we'd previously pick the normal \"last key wins\", which my\n>> c95e3a3f0b8107b5dc7eac9dfdb9e5238280c9fb changed to \"first wins\":\n>> \n>> diff --git a/git-send-email.perl b/git-send-email.perl\n>> index e65d969d0bb..6c7ab3d2e91 100755\n>> --- a/git-send-email.perl\n>> +++ b/git-send-email.perl\n>> @@ -376,7 +376,7 @@ sub read_config {\n>>  \t\t\t@$target = @values;\n>>  \t\t}\n>>  \t\telse {\n>> -\t\t\tmy $v = $known_keys->{$key}->[0];\n>> +\t\t\tmy $v = $known_keys->{$key}->[-1];\n>>  \t\t\tnext unless defined $v;\n>>  \t\t\tnext if $configured->{$setting}++;\n>>  \t\t\t$$target = $v;\n>> \n>\n>\n>\n> Thanks, this worked for me and fixed my problem! Feel free to add my\n> tested-by.\n\nAdded, and here's a properly formatted patch with a regression test.\n\n git-send-email.perl   |  2 +-\n t/t9001-send-email.sh | 15 +++++++++++++++\n 2 files changed, 16 insertions(+), 1 deletion(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex e65d969d0bb..6c7ab3d2e91 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -376,7 +376,7 @@ sub read_config {\n \t\t\t@$target = @values;\n \t\t}\n \t\telse {\n-\t\t\tmy $v = $known_keys->{$key}->[0];\n+\t\t\tmy $v = $known_keys->{$key}->[-1];\n \t\t\tnext unless defined $v;\n \t\t\tnext if $configured->{$setting}++;\n \t\t\t$$target = $v;\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 57fc10e7f82..eae172e0a05 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -1533,6 +1533,21 @@ test_expect_success $PREREQ 'sendemail.8bitEncoding works' '\n \ttest_cmp content-type-decl actual\n '\n \n+test_expect_success $PREREQ 'sendemail.8bitEncoding in .git/config overrides --global .gitconfig' '\n+\tclean_fake_sendmail &&\n+\tgit config sendemail.assume8bitEncoding UTF-8 &&\n+\ttest_when_finished \"rm -rf home\" &&\n+\tmkdir home &&\n+\tgit config -f home/.gitconfig sendemail.assume8bitEncoding \"bogus too\" &&\n+\techo bogus |\n+\tenv HOME=\"$(pwd)/home\" DEBUG=1 \\\n+\tgit send-email --from=author@example.com --to=nobody@example.com \\\n+\t\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t\t\temail-using-8bit >stdout &&\n+\tegrep \"Content|MIME\" msgtxt1 >actual &&\n+\ttest_cmp content-type-decl actual\n+'\n+\n test_expect_success $PREREQ '--8bit-encoding overrides sendemail.8bitEncoding' '\n \tclean_fake_sendmail &&\n \tgit config sendemail.assume8bitEncoding \"bogus too\" &&\n-- \n2.33.0.821.gfd4106eadbd\n\n"},{"id":"434778","messageId":"CAPUEspicOHC5W0pPFcAjQ6BfmLmcS-M8neDdLYWJuNO7fvpDVA@mail.gmail.com","threadId":"56446","inReplyTo":"patch-1.1-ae6ff9f77f1-20210906T073306Z-avarab@gmail.com","subject":"Re: [PATCH] send-email: fix a \"first config key wins\" regression in v2.33.0","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2021-09-06T08:32:10Z","receivedAt":"2021-09-06T08:32:25Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Mon, Sep 6, 2021 at 12:35 AM Ævar Arnfjörð Bjarmason\n<avarab@gmail.com> wrote:\n> This test fails without this bugfix, but not works.\n\ns/not/now\n\nCarlo\n"},{"id":"434787","messageId":"ce194f13-2b1f-fb21-e224-80190f3b2fd6@gmail.com","threadId":"56446","inReplyTo":"patch-1.1-ae6ff9f77f1-20210906T073306Z-avarab@gmail.com","subject":"Re: [PATCH] send-email: fix a \"first config key wins\" regression in v2.33.0","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2021-09-06T11:05:25Z","receivedAt":"2021-09-06T11:05:36Z","isPatch":true,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"On 06/09/21 14.33, Ævar Arnfjörð Bjarmason wrote:\n> +test_expect_success $PREREQ 'sendemail.8bitEncoding in .git/config overrides --global .gitconfig' '\n> +\tclean_fake_sendmail &&\n> +\tgit config sendemail.assume8bitEncoding UTF-8 &&\n> +\ttest_when_finished \"rm -rf home\" &&\n> +\tmkdir home &&\n> +\tgit config -f home/.gitconfig sendemail.assume8bitEncoding \"bogus too\" &&\n> +\techo bogus |\n> +\tenv HOME=\"$(pwd)/home\" DEBUG=1 \\\n> +\tgit send-email --from=author@example.com --to=nobody@example.com \\\n> +\t\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n> +\t\t\temail-using-8bit >stdout &&\n> +\tegrep \"Content|MIME\" msgtxt1 >actual &&\n> +\ttest_cmp content-type-decl actual\n> +'\n> +\n\nDid you mean overrides global .gitconfig (s/--global/global/)?\n\nAnyway, compiled and tested successfully (test suite passed).\n\nTested-by: Bagas Sanjaya <bagasdotme@gmail.com>\n\n-- \nAn old man doll... just what I always wanted! - Clara\n"},{"id":"434821","messageId":"87mtoon3tc.fsf@evledraar.gmail.com","threadId":"56446","inReplyTo":"ce194f13-2b1f-fb21-e224-80190f3b2fd6@gmail.com","subject":"Re: [PATCH] send-email: fix a \"first config key wins\" regression in v2.33.0","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-07T09:15:11Z","receivedAt":"2021-09-07T09:16:36Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Sep 06 2021, Bagas Sanjaya wrote:\n\n> On 06/09/21 14.33, Ævar Arnfjörð Bjarmason wrote:\n>> +test_expect_success $PREREQ 'sendemail.8bitEncoding in .git/config overrides --global .gitconfig' '\n>> +\tclean_fake_sendmail &&\n>> +\tgit config sendemail.assume8bitEncoding UTF-8 &&\n>> +\ttest_when_finished \"rm -rf home\" &&\n>> +\tmkdir home &&\n>> +\tgit config -f home/.gitconfig sendemail.assume8bitEncoding \"bogus too\" &&\n>> +\techo bogus |\n>> +\tenv HOME=\"$(pwd)/home\" DEBUG=1 \\\n>> +\tgit send-email --from=author@example.com --to=nobody@example.com \\\n>> +\t\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n>> +\t\t\temail-using-8bit >stdout &&\n>> +\tegrep \"Content|MIME\" msgtxt1 >actual &&\n>> +\ttest_cmp content-type-decl actual\n>> +'\n>> +\n>\n> Did you mean overrides global .gitconfig (s/--global/global/)?\n\nI mean config set with the equivalent of \"git config --global\", but yeah\nthis is probably an awkward way to phrase that.\n\n> Anyway, compiled and tested successfully (test suite passed).\n>\n> Tested-by: Bagas Sanjaya <bagasdotme@gmail.com>\n\nThanks, FWIW the test suite passing doesn't mean much in this case, the\nonly test that stresses this is the above. The issue being that the\nregression wasn't caught by any existing test...\n"}]}