{"thread":{"id":"34228","subject":"[PATCH v2 2/2] send-email: add test for duplicate utf8 name","startedAt":"2013-06-20T12:27:05Z","lastAt":"2013-06-20T19:20:34Z","messageCount":7,"participants":["Michael S. Tsirkin","Thomas Rast","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":2},"messages":[{"id":"221467","messageId":"1371731166-24015-2-git-send-email-mst@redhat.com","threadId":"34228","inReplyTo":"1371731166-24015-1-git-send-email-mst@redhat.com","subject":"[PATCH v2 2/2] send-email: add test for duplicate utf8 name","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2013-06-20T12:27:05Z","receivedAt":"2013-06-20T12:27:05Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"Verify that author name is not duplicated if it matches\nsender, even if it is in utf8.\n\nSigned-off-by: Michael S. Tsirkin \n---\n t/t9001-send-email.sh | 14 ++++++++++++++\n 1 file changed, 14 insertions(+)\n\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 9f46f22..020acc4 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -956,6 +956,20 @@ test_expect_success $PREREQ 'utf8 author is correctly passed on' '\n \tgrep \"^From: Füñný Nâmé \" msgtxt1\n '\n \n+test_expect_success $PREREQ 'utf8 sender is not duplicated' '\n+\tclean_fake_sendmail &&\n+\ttest_commit weird_sender &&\n+\ttest_when_finished \"git reset --hard HEAD^\" &&\n+\tgit commit --amend --author \"Füñný Nâmé \" &&\n+\tgit format-patch --stdout -1 >funny_name.patch &&\n+\tgit send-email --from=\"Füñný Nâmé \" \\\n+\t --to=nobody@example.com \\\n+\t --smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t funny_name.patch &&\n+\tgrep \"^From:\" msgtxt1 > msgfrom &&\n+\ttest_line_count = 1 msgfrom\n+'\n+\n test_expect_success $PREREQ 'sendemail.composeencoding works' '\n \tclean_fake_sendmail &&\n \tgit config sendemail.composeencoding iso-8859-1 &&\n-- \nMST\n"},{"id":"221468","messageId":"1371731166-24015-1-git-send-email-mst@redhat.com","threadId":"34228","inReplyTo":null,"subject":"[PATCH v2 1/2] send-email: sanitize author when writing From line","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2013-06-20T12:27:08Z","receivedAt":"2013-06-20T12:27:08Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"sender is now sanitized, but we didn't sanitize\nauthor when checking whether From: line is needed\nin the message body.\nAs a result git started writing duplicate From lines\nwhen author matched sender and has utf8 characters.\n\nReported-by: SZEDER Gábor \nTested-by: SZEDER Gábor \nSigned-off-by: Michael S. Tsirkin \n---\n git-send-email.perl | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 671762b..ecbf56f 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1259,6 +1259,7 @@ foreach my $t (@files) {\n \topen my $fh, \"<\", $t or die \"can't open file $t\";\n \n \tmy $author = undef;\n+\tmy $sauthor = undef;\n \tmy $author_encoding;\n \tmy $has_content_type;\n \tmy $body_encoding;\n@@ -1297,7 +1298,7 @@ foreach my $t (@files) {\n \t\t\t}\n \t\t\telsif (/^From:\\s+(.*)$/i) {\n \t\t\t\t($author, $author_encoding) = unquote_rfc2047($1);\n-\t\t\t\tmy $sauthor = sanitize_address($author);\n+\t\t\t\t$sauthor = sanitize_address($author);\n \t\t\t\tnext if $suppress_cc{'author'};\n \t\t\t\tnext if $suppress_cc{'self'} and $sauthor eq $sender;\n \t\t\t\tprintf(\"(mbox) Adding cc: %s from line '%s'\\n\",\n@@ -1393,7 +1394,7 @@ foreach my $t (@files) {\n \t\t$subject = quote_subject($subject, $auto_8bit_encoding);\n \t}\n \n-\tif (defined $author and $author ne $sender) {\n+\tif (defined $sauthor and $sauthor ne $sender) {\n \t\t$message = \"From: $author\\n\\n$message\";\n \t\tif (defined $author_encoding) {\n \t\t\tif ($has_content_type) {\n-- \nMST\n"},{"id":"221470","messageId":"87a9ml9cou.fsf@linux-k42r.v.cablecom.net","threadId":"34228","inReplyTo":"1371731166-24015-2-git-send-email-mst@redhat.com","subject":"Re: [PATCH v2 2/2] send-email: add test for duplicate utf8 name","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-06-20T12:41:37Z","receivedAt":"2013-06-20T12:41:37Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"\"Michael S. Tsirkin\" writes:\n\n> Verify that author name is not duplicated if it matches\n> sender, even if it is in utf8.\n\nSmall nit: if you make two patches out of it, add the tests first with\ntest_expect_failure. Then flip it to test_expect_success in the actual\ncode change. That makes it easy to verify that the test actually checks\nthe right thing, and that it was your code change that fixed it.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"221471","messageId":"20130620124502.GA24172@redhat.com","threadId":"34228","inReplyTo":"87a9ml9cou.fsf@linux-k42r.v.cablecom.net","subject":"Re: [PATCH v2 2/2] send-email: add test for duplicate utf8 name","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2013-06-20T12:45:02Z","receivedAt":"2013-06-20T12:45:02Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"On Thu, Jun 20, 2013 at 02:41:37PM +0200, Thomas Rast wrote:\n> \"Michael S. Tsirkin\" writes:\n> \n> > Verify that author name is not duplicated if it matches\n> > sender, even if it is in utf8.\n> \n> Small nit: if you make two patches out of it, add the tests first with\n> test_expect_failure. Then flip it to test_expect_success in the actual\n> code change. That makes it easy to verify that the test actually checks\n> the right thing, and that it was your code change that fixed it.\n\nI did this by reverting 1/2 and rerunning.\n\nBut applying in reverse order means bisect can give us\na setup where some tests fail, I thought it's a\ngood idea to avoid that.\n\n> -- \n> Thomas Rast\n> trast@{inf,student}.ethz.ch\n"},{"id":"221473","messageId":"87vc597xtc.fsf@linux-k42r.v.cablecom.net","threadId":"34228","inReplyTo":"20130620124502.GA24172@redhat.com","subject":"Re: [PATCH v2 2/2] send-email: add test for duplicate utf8 name","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-06-20T12:48:15Z","receivedAt":"2013-06-20T12:48:15Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"\"Michael S. Tsirkin\" writes:\n\n> On Thu, Jun 20, 2013 at 02:41:37PM +0200, Thomas Rast wrote:\n>> \"Michael S. Tsirkin\" writes:\n>> \n>> > Verify that author name is not duplicated if it matches\n>> > sender, even if it is in utf8.\n>> \n>> Small nit: if you make two patches out of it, add the tests first with\n>> test_expect_failure. Then flip it to test_expect_success in the actual\n>> code change. That makes it easy to verify that the test actually checks\n>> the right thing, and that it was your code change that fixed it.\n>\n> I did this by reverting 1/2 and rerunning.\n>\n> But applying in reverse order means bisect can give us\n> a setup where some tests fail, I thought it's a\n> good idea to avoid that.\n\nThat's why you need to test_expect_*failure* in the commit that adds the\ntests -- essentially saying \"I know this is broken!\".\n\nYes, it's a roundabout way. But splitting code and tests in the way you\njust posted is equally roundabout, while not having the benefit that one\ncan check out the commit at patch 1 and verify that it is indeed broken\n(showing up as \"still have known breakage\").\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"221474","messageId":"20130620125329.GA24236@redhat.com","threadId":"34228","inReplyTo":"87vc597xtc.fsf@linux-k42r.v.cablecom.net","subject":"Re: [PATCH v2 2/2] send-email: add test for duplicate utf8 name","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2013-06-20T12:53:29Z","receivedAt":"2013-06-20T12:53:29Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"On Thu, Jun 20, 2013 at 02:48:15PM +0200, Thomas Rast wrote:\n> \"Michael S. Tsirkin\" writes:\n> \n> > On Thu, Jun 20, 2013 at 02:41:37PM +0200, Thomas Rast wrote:\n> >> \"Michael S. Tsirkin\" writes:\n> >> \n> >> > Verify that author name is not duplicated if it matches\n> >> > sender, even if it is in utf8.\n> >> \n> >> Small nit: if you make two patches out of it, add the tests first with\n> >> test_expect_failure. Then flip it to test_expect_success in the actual\n> >> code change. That makes it easy to verify that the test actually checks\n> >> the right thing, and that it was your code change that fixed it.\n> >\n> > I did this by reverting 1/2 and rerunning.\n> >\n> > But applying in reverse order means bisect can give us\n> > a setup where some tests fail, I thought it's a\n> > good idea to avoid that.\n> \n> That's why you need to test_expect_*failure* in the commit that adds the\n> tests -- essentially saying \"I know this is broken!\".\n> \n> Yes, it's a roundabout way. But splitting code and tests in the way you\n> just posted is equally roundabout, while not having the benefit that one\n> can check out the commit at patch 1 and verify that it is indeed broken\n> (showing up as \"still have known breakage\").\n\nI'll try to do it like this next time, I hope for now Junio\ncan take it as is.\n\n\n\n> -- \n> Thomas Rast\n> trast@{inf,student}.ethz.ch\n"},{"id":"221504","messageId":"7vppvgtwql.fsf@alter.siamese.dyndns.org","threadId":"34228","inReplyTo":"1371731166-24015-2-git-send-email-mst@redhat.com","subject":"Re: [PATCH v2 2/2] send-email: add test for duplicate utf8 name","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-20T19:20:34Z","receivedAt":"2013-06-20T19:20:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Michael S. Tsirkin\" writes:\n\n> Verify that author name is not duplicated if it matches\n> sender, even if it is in utf8.\n>\n> Signed-off-by: Michael S. Tsirkin \n> ---\n\nHmph. There seems to be something wrong in the original message I\nreceived from you (look at Cc: line I have above, which was copied\nby my MUA from the message I am responding to).\n\nVisiting\n\n http://article.gmane.org/gmane.comp.version-control.git/228485/raw\n\nshows this gem:\n\n Cc: =?us-ascii?B?PT9VVEYtOD9xP1NaRURFUj0yMEc9QzM9QTFib3I/PQ==?= \n \n\nSomebody is wrapping what \"=?UTF-8?q?...\" which you had already\nwrapped into \"=?us-ascii?B...\"???\n\nIn any case, I think Thomas's \"first expect failure and then flip\nit, if you add test and fix as separate patches\" is a good idea, and\nthe change between the previous one and this seems to be only the\nlast part of this test, so I'll tweak what I already have from the\nprevious on 'pu' locally and push the result out. Please double\ncheck the result.\n\nThanks.\n\n> t/t9001-send-email.sh | 14 ++++++++++++++\n> 1 file changed, 14 insertions(+)\n>\n> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\n> index 9f46f22..020acc4 100755\n> --- a/t/t9001-send-email.sh\n> +++ b/t/t9001-send-email.sh\n> @@ -956,6 +956,20 @@ test_expect_success $PREREQ 'utf8 author is correctly passed on' '\n> \tgrep \"^From: Füñný Nâmé \" msgtxt1\n> '\n> \n> +test_expect_success $PREREQ 'utf8 sender is not duplicated' '\n> +\tclean_fake_sendmail &&\n> +\ttest_commit weird_sender &&\n> +\ttest_when_finished \"git reset --hard HEAD^\" &&\n> +\tgit commit --amend --author \"Füñný Nâmé \" &&\n> +\tgit format-patch --stdout -1 >funny_name.patch &&\n> +\tgit send-email --from=\"Füñný Nâmé \" \\\n> +\t --to=nobody@example.com \\\n> +\t --smtp-server=\"$(pwd)/fake.sendmail\" \\\n> +\t funny_name.patch &&\n> +\tgrep \"^From:\" msgtxt1 > msgfrom &&\n> +\ttest_line_count = 1 msgfrom\n> +'\n> +\n> test_expect_success $PREREQ 'sendemail.composeencoding works' '\n> \tclean_fake_sendmail &&\n> \tgit config sendemail.composeencoding iso-8859-1 &&\n"}]}