threads / patch / 19835

patchTest cccmd in t9001-send-email.sh and fix two bugs

Subject: [PATCH resend] Test cccmd in t9001-send-email.sh and fix two bugs

## tl;dr

5 messages between Jun 17, 2009 and Jun 19, 2009. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Paolo Bonzini· Jun 17, 2009, 07:17 UTC · lore

For another patch series I'm working on I needed some tests for the cc-cmd feature of git-send-email.

This patch adds 3 tests for the feature, and fixes two bugs. The first bug is that the --suppress-cc option for `cccmd' was misspelled as `ccmd' in the code. The second bug, which is actually found only with my other series, is that the argument to the cccmd is never quoted, so the cccmd would fail with patch file names containing a space.

I also test the possibility to specify --suppress-cc multiple times.

Signed-Off-By: Paolo Bonzini  <bonzini@gnu.org>
---
 git-send-email.perl   |    4 +-
 t/t9001-send-email.sh |   80 +++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 77 insertions(+), 7 deletions(-)
	The patch was sent first a week ago with no remarks.
Show changes to 2 files +77 −7

git-send-email.perl, t/t9001-send-email.sh

diff --git a/git-send-email.perl b/git-send-email.perl
index 303e03a..7ae1f29 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -334,7 +334,7 @@ if (@suppress_cc) {
 }
 
 if ($suppress_cc{'all'}) {
-	foreach my $entry (qw (ccmd cc author self sob body bodycc)) {
+	foreach my $entry (qw (cccmd cc author self sob body bodyccc)) {
 		$suppress_cc{$entry} = 1;
 	}
 	delete $suppress_cc{'all'};
@@ -1104,7 +1104,7 @@ foreach my $t (@files) {
 	close F;
 
 	if (defined $cc_cmd && !$suppress_cc{'cccmd'}) {
-		open(F, "$cc_cmd $t |")
+		open(F, "$cc_cmd \Q$t\E |")
 			or die "(cc-cmd) Could not execute '$cc_cmd'";
 		while(<F>) {
 			my $c = $_;
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 9ce04fd..fb7d9f3 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -148,6 +148,22 @@ test_expect_success 'Prompting works' '
 		grep "^To: to@example.com$" msgtxt1
 '
 
+test_expect_success 'cccmd works' '
+	clean_fake_sendmail &&
+	cp $patches cccmd.patch &&
+	echo cccmd--cccmd@example.com >>cccmd.patch &&
+	echo sed -n s/^cccmd--//p \"\$1\" > cccmd-sed &&
+	chmod +x cccmd-sed &&
+	git send-email \
+		--from="Example <nobody@example.com>" \
+		--to=nobody@example.com \
+		--cc-cmd=./cccmd-sed \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		cccmd.patch \
+		&&
+	grep ^Cc:.*cccmd@example.com msgtxt1
+'
+
 z8=zzzzzzzz
 z64=$z8$z8$z8$z8$z8$z8$z8$z8
 z512=$z64$z64$z64$z64$z64$z64$z64$z64
@@ -274,7 +290,7 @@ EOF
 test_suppression () {
 	git send-email \
 		--dry-run \
-		--suppress-cc=$1 \
+		--suppress-cc=$1 ${2+"--suppress-cc=$2"} \
 		--from="Example <from@example.com>" \
 		--to=to@example.com \
 		--smtp-server relay.example.com \
@@ -282,8 +298,8 @@ test_suppression () {
 	sed	-e "s/^\(Date:\).*/\1 DATE-STRING/" \
 		-e "s/^\(Message-Id:\).*/\1 MESSAGE-ID-STRING/" \
 		-e "s/^\(X-Mailer:\).*/\1 X-MAILER-STRING/" \
-		>actual-suppress-$1 &&
-	test_cmp expected-suppress-$1 actual-suppress-$1
+		>actual-suppress-$1${2+"-$2"} &&
+	test_cmp expected-suppress-$1${2+"-$2"} actual-suppress-$1${2+"-$2"}
 }
 
 test_expect_success 'sendemail.cc set' '
@@ -316,6 +332,34 @@ test_expect_success 'sendemail.cc unset' '
 	test_suppression sob
 '
 
+cat >expected-suppress-cccmd <<\EOF
+0001-Second.patch
+(mbox) Adding cc: A <author@example.com> from line 'From: A <author@example.com>'
+(mbox) Adding cc: One <one@example.com> from line 'Cc: One <one@example.com>, two@example.com'
+(mbox) Adding cc: two@example.com from line 'Cc: One <one@example.com>, two@example.com'
+(body) Adding cc: C O Mitter <committer@example.com> from line 'Signed-off-by: C O Mitter <committer@example.com>'
+Dry-OK. Log says:
+Server: relay.example.com
+MAIL FROM:<from@example.com>
+RCPT TO:<to@example.com>,<author@example.com>,<one@example.com>,<two@example.com>,<committer@example.com>
+From: Example <from@example.com>
+To: to@example.com
+Cc: A <author@example.com>, One <one@example.com>, two@example.com, C O Mitter <committer@example.com>
+Subject: [PATCH 1/1] Second.
+Date: DATE-STRING
+Message-Id: MESSAGE-ID-STRING
+X-Mailer: X-MAILER-STRING
+
+Result: OK
+EOF
+
+test_expect_success 'sendemail.cccmd' '
+	echo echo cc-cmd@example.com > cccmd &&
+	chmod +x cccmd &&
+	git config sendemail.cccmd ./cccmd &&
+	test_suppression cccmd
+'
+
 cat >expected-suppress-all <<\EOF
 0001-Second.patch
 Dry-OK. Log says:
@@ -341,13 +385,14 @@ cat >expected-suppress-body <<\EOF
 (mbox) Adding cc: A <author@example.com> from line 'From: A <author@example.com>'
 (mbox) Adding cc: One <one@example.com> from line 'Cc: One <one@example.com>, two@example.com'
 (mbox) Adding cc: two@example.com from line 'Cc: One <one@example.com>, two@example.com'
+(cc-cmd) Adding cc: cc-cmd@example.com from: './cccmd'
 Dry-OK. Log says:
 Server: relay.example.com
 MAIL FROM:<from@example.com>
-RCPT TO:<to@example.com>,<author@example.com>,<one@example.com>,<two@example.com>
+RCPT TO:<to@example.com>,<author@example.com>,<one@example.com>,<two@example.com>,<cc-cmd@example.com>
 From: Example <from@example.com>
 To: to@example.com
-Cc: A <author@example.com>, One <one@example.com>, two@example.com
+Cc: A <author@example.com>, One <one@example.com>, two@example.com, cc-cmd@example.com
 Subject: [PATCH 1/1] Second.
 Date: DATE-STRING
 Message-Id: MESSAGE-ID-STRING
@@ -360,6 +405,30 @@ test_expect_success '--suppress-cc=body' '
 	test_suppression body
 '
 
+cat >expected-suppress-body-cccmd <<\EOF
+0001-Second.patch
+(mbox) Adding cc: A <author@example.com> from line 'From: A <author@example.com>'
+(mbox) Adding cc: One <one@example.com> from line 'Cc: One <one@example.com>, two@example.com'
+(mbox) Adding cc: two@example.com from line 'Cc: One <one@example.com>, two@example.com'
+Dry-OK. Log says:
+Server: relay.example.com
+MAIL FROM:<from@example.com>
+RCPT TO:<to@example.com>,<author@example.com>,<one@example.com>,<two@example.com>
+From: Example <from@example.com>
+To: to@example.com
+Cc: A <author@example.com>, One <one@example.com>, two@example.com
+Subject: [PATCH 1/1] Second.
+Date: DATE-STRING
+Message-Id: MESSAGE-ID-STRING
+X-Mailer: X-MAILER-STRING
+
+Result: OK
+EOF
+
+test_expect_success '--suppress-cc=body --suppress-cc=cccmd' '
+	test_suppression body cccmd
+'
+
 cat >expected-suppress-sob <<\EOF
 0001-Second.patch
 (mbox) Adding cc: A <author@example.com> from line 'From: A <author@example.com>'
@@ -381,6 +450,7 @@ Result: OK
 EOF
 
 test_expect_success '--suppress-cc=sob' '
+	git config --unset sendemail.cccmd
 	test_suppression sob
 '
 
-- 
1.6.0.3
Markus Heidelberg· Jun 18, 2009, 10:11 UTC · re: Paolo Bonzini · lore

Re: [PATCH resend] Test cccmd in t9001-send-email.sh and fix two bugs

Paolo Bonzini, 17.06.2009:
> The first bug is that the --suppress-cc option for `cccmd' was
> misspelled as `ccmd' in the code.
Show 7 quoted lines
> +++ b/git-send-email.perl
> @@ -334,7 +334,7 @@ if (@suppress_cc) {
>  }
>  
>  if ($suppress_cc{'all'}) {
> -	foreach my $entry (qw (ccmd cc author self sob body bodycc)) {
> +	foreach my $entry (qw (cccmd cc author self sob body bodyccc)) {
Additionally you incorrectly changed bodycc to bodyccc.

Then I noticed that the documentation mentions ccbody, but the code uses bodycc. The author seemed to intend using bodycc, since it is also used in the tests and the commit message (3531e2703d, send-email: --suppress-cc improvements, 2009-02-15), but besides the documentation, ccbody is used in the bash completion.

>  		$suppress_cc{$entry} = 1;
>  	}
>  	delete $suppress_cc{'all'};
Paolo Bonzini· Jun 18, 2009, 12:31 UTC · re: Markus Heidelberg · lore

[PATCH v2] Test cccmd in t9001-send-email.sh and fix some bugs

For another patch series I'm working on I needed some tests for the cc-cmd feature of git-send-email.

This patch adds 3 tests for the feature and for the possibility to specify --suppress-cc multiple times, and fixes two bugs. The first bug is that the --suppress-cc option for `cccmd' was misspelled as `ccmd' in the code. The second bug, which is actually found only with my other series, is that the argument to the cccmd is never quoted, so the cccmd would fail with patch file names containing a space.

A third bug I fix (in the docs) is that the bodycc argument was actually spelled ccbody in the documentation and bash completion.

Signed-off-by: Paolo Bonzini <bonzini@gnu.org>
Cc: Markus Heidelberg <markus.heidelberg@web.de>
---
 Documentation/git-send-email.txt       |    4 +-
 contrib/completion/git-completion.bash |    2 +-
 git-send-email.perl                    |    4 +-
 t/t9001-send-email.sh                  |   80 ++++++++++++++++++++++++++++++--
 4 files changed, 80 insertions(+), 10 deletions(-)
	Thanks Markus for noticing my erroneous change and
	the inconsistency in the docs.
Show changes to 4 files +80 −10

Documentation/git-send-email.txt, contrib/completion/git-completion.bash, git-send-email.perl, t/t9001-send-email.sh

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 9902da4..fbde2d3 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -193,12 +193,12 @@ Automating
 - 'self' will avoid including the sender
 - 'cc' will avoid including anyone mentioned in Cc lines in the patch header
   except for self (use 'self' for that).
-- 'ccbody' will avoid including anyone mentioned in Cc lines in the
+- 'bodycc' will avoid including anyone mentioned in Cc lines in the
   patch body (commit message) except for self (use 'self' for that).
 - 'sob' will avoid including anyone mentioned in Signed-off-by lines except
    for self (use 'self' for that).
 - 'cccmd' will avoid running the --cc-cmd.
-- 'body' is equivalent to 'sob' + 'ccbody'
+- 'body' is equivalent to 'sob' + 'bodycc'
 - 'all' will suppress all auto cc values.
 --
 +
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 80ab4e4..b60cb68 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1299,7 +1299,7 @@ _git_rebase ()
 }
 
 __git_send_email_confirm_options="always never auto cc compose"
-__git_send_email_suppresscc_options="author self cc ccbody sob cccmd body all"
+__git_send_email_suppresscc_options="author self cc bodycc sob cccmd body all"
 
 _git_send_email ()
 {
diff --git a/git-send-email.perl b/git-send-email.perl
index 303e03a..8ce6f1f 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -334,7 +334,7 @@ if (@suppress_cc) {
 }
 
 if ($suppress_cc{'all'}) {
-	foreach my $entry (qw (ccmd cc author self sob body bodycc)) {
+	foreach my $entry (qw (cccmd cc author self sob body bodycc)) {
 		$suppress_cc{$entry} = 1;
 	}
 	delete $suppress_cc{'all'};
@@ -1104,7 +1104,7 @@ foreach my $t (@files) {
 	close F;
 
 	if (defined $cc_cmd && !$suppress_cc{'cccmd'}) {
-		open(F, "$cc_cmd $t |")
+		open(F, "$cc_cmd \Q$t\E |")
 			or die "(cc-cmd) Could not execute '$cc_cmd'";
 		while(<F>) {
 			my $c = $_;
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 9ce04fd..fb7d9f3 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -148,6 +148,22 @@ test_expect_success 'Prompting works' '
 		grep "^To: to@example.com$" msgtxt1
 '
 
+test_expect_success 'cccmd works' '
+	clean_fake_sendmail &&
+	cp $patches cccmd.patch &&
+	echo cccmd--cccmd@example.com >>cccmd.patch &&
+	echo sed -n s/^cccmd--//p \"\$1\" > cccmd-sed &&
+	chmod +x cccmd-sed &&
+	git send-email \
+		--from="Example <nobody@example.com>" \
+		--to=nobody@example.com \
+		--cc-cmd=./cccmd-sed \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		cccmd.patch \
+		&&
+	grep ^Cc:.*cccmd@example.com msgtxt1
+'
+
 z8=zzzzzzzz
 z64=$z8$z8$z8$z8$z8$z8$z8$z8
 z512=$z64$z64$z64$z64$z64$z64$z64$z64
@@ -274,7 +290,7 @@ EOF
 test_suppression () {
 	git send-email \
 		--dry-run \
-		--suppress-cc=$1 \
+		--suppress-cc=$1 ${2+"--suppress-cc=$2"} \
 		--from="Example <from@example.com>" \
 		--to=to@example.com \
 		--smtp-server relay.example.com \
@@ -282,8 +298,8 @@ test_suppression () {
 	sed	-e "s/^\(Date:\).*/\1 DATE-STRING/" \
 		-e "s/^\(Message-Id:\).*/\1 MESSAGE-ID-STRING/" \
 		-e "s/^\(X-Mailer:\).*/\1 X-MAILER-STRING/" \
-		>actual-suppress-$1 &&
-	test_cmp expected-suppress-$1 actual-suppress-$1
+		>actual-suppress-$1${2+"-$2"} &&
+	test_cmp expected-suppress-$1${2+"-$2"} actual-suppress-$1${2+"-$2"}
 }
 
 test_expect_success 'sendemail.cc set' '
@@ -316,6 +332,34 @@ test_expect_success 'sendemail.cc unset' '
 	test_suppression sob
 '
 
+cat >expected-suppress-cccmd <<\EOF
+0001-Second.patch
+(mbox) Adding cc: A <author@example.com> from line 'From: A <author@example.com>'
+(mbox) Adding cc: One <one@example.com> from line 'Cc: One <one@example.com>, two@example.com'
+(mbox) Adding cc: two@example.com from line 'Cc: One <one@example.com>, two@example.com'
+(body) Adding cc: C O Mitter <committer@example.com> from line 'Signed-off-by: C O Mitter <committer@example.com>'
+Dry-OK. Log says:
+Server: relay.example.com
+MAIL FROM:<from@example.com>
+RCPT TO:<to@example.com>,<author@example.com>,<one@example.com>,<two@example.com>,<committer@example.com>
+From: Example <from@example.com>
+To: to@example.com
+Cc: A <author@example.com>, One <one@example.com>, two@example.com, C O Mitter <committer@example.com>
+Subject: [PATCH 1/1] Second.
+Date: DATE-STRING
+Message-Id: MESSAGE-ID-STRING
+X-Mailer: X-MAILER-STRING
+
+Result: OK
+EOF
+
+test_expect_success 'sendemail.cccmd' '
+	echo echo cc-cmd@example.com > cccmd &&
+	chmod +x cccmd &&
+	git config sendemail.cccmd ./cccmd &&
+	test_suppression cccmd
+'
+
 cat >expected-suppress-all <<\EOF
 0001-Second.patch
 Dry-OK. Log says:
@@ -341,13 +385,14 @@ cat >expected-suppress-body <<\EOF
 (mbox) Adding cc: A <author@example.com> from line 'From: A <author@example.com>'
 (mbox) Adding cc: One <one@example.com> from line 'Cc: One <one@example.com>, two@example.com'
 (mbox) Adding cc: two@example.com from line 'Cc: One <one@example.com>, two@example.com'
+(cc-cmd) Adding cc: cc-cmd@example.com from: './cccmd'
 Dry-OK. Log says:
 Server: relay.example.com
 MAIL FROM:<from@example.com>
-RCPT TO:<to@example.com>,<author@example.com>,<one@example.com>,<two@example.com>
+RCPT TO:<to@example.com>,<author@example.com>,<one@example.com>,<two@example.com>,<cc-cmd@example.com>
 From: Example <from@example.com>
 To: to@example.com
-Cc: A <author@example.com>, One <one@example.com>, two@example.com
+Cc: A <author@example.com>, One <one@example.com>, two@example.com, cc-cmd@example.com
 Subject: [PATCH 1/1] Second.
 Date: DATE-STRING
 Message-Id: MESSAGE-ID-STRING
@@ -360,6 +405,30 @@ test_expect_success '--suppress-cc=body' '
 	test_suppression body
 '
 
+cat >expected-suppress-body-cccmd <<\EOF
+0001-Second.patch
+(mbox) Adding cc: A <author@example.com> from line 'From: A <author@example.com>'
+(mbox) Adding cc: One <one@example.com> from line 'Cc: One <one@example.com>, two@example.com'
+(mbox) Adding cc: two@example.com from line 'Cc: One <one@example.com>, two@example.com'
+Dry-OK. Log says:
+Server: relay.example.com
+MAIL FROM:<from@example.com>
+RCPT TO:<to@example.com>,<author@example.com>,<one@example.com>,<two@example.com>
+From: Example <from@example.com>
+To: to@example.com
+Cc: A <author@example.com>, One <one@example.com>, two@example.com
+Subject: [PATCH 1/1] Second.
+Date: DATE-STRING
+Message-Id: MESSAGE-ID-STRING
+X-Mailer: X-MAILER-STRING
+
+Result: OK
+EOF
+
+test_expect_success '--suppress-cc=body --suppress-cc=cccmd' '
+	test_suppression body cccmd
+'
+
 cat >expected-suppress-sob <<\EOF
 0001-Second.patch
 (mbox) Adding cc: A <author@example.com> from line 'From: A <author@example.com>'
@@ -381,6 +450,7 @@ Result: OK
 EOF
 
 test_expect_success '--suppress-cc=sob' '
+	git config --unset sendemail.cccmd
 	test_suppression sob
 '
 
-- 
1.6.0.3
Markus Heidelberg· Jun 19, 2009, 10:06 UTC · re: Paolo Bonzini · lore

Re: [PATCH v2] Test cccmd in t9001-send-email.sh and fix some bugs

Paolo Bonzini, 18.06.2009:
> For another patch series I'm working on I needed some tests
> for the cc-cmd feature of git-send-email.
I think this is text for a cover-letter.
Show 10 quoted lines
> This patch adds 3 tests for the feature and for the possibility
> to specify --suppress-cc multiple times, and fixes two bugs.
> The first bug is that the --suppress-cc option for `cccmd' was
> misspelled as `ccmd' in the code.  The second bug, which is
> actually found only with my other series, is that the argument
> to the cccmd is never quoted, so the cccmd would fail with
> patch file names containing a space.
> 
> A third bug I fix (in the docs) is that the bodycc argument was
> actually spelled ccbody in the documentation and bash completion.

And these three unrelated bug fixes should rather go into three separate patches.

Show 11 quoted lines
> Signed-off-by: Paolo Bonzini <bonzini@gnu.org>
> Cc: Markus Heidelberg <markus.heidelberg@web.de>
> ---
>  Documentation/git-send-email.txt       |    4 +-
>  contrib/completion/git-completion.bash |    2 +-
>  git-send-email.perl                    |    4 +-
>  t/t9001-send-email.sh                  |   80 ++++++++++++++++++++++++++++++--
>  4 files changed, 80 insertions(+), 10 deletions(-)
> 
> 	Thanks Markus for noticing my erroneous change and
> 	the inconsistency in the docs.

This normally goes directly under --- I didn't notice it before the second look.

Jay Soffian· Jun 18, 2009, 12:49 UTC · re: Markus Heidelberg · lore

Re: [PATCH resend] Test cccmd in t9001-send-email.sh and fix two bugs

On Thu, Jun 18, 2009 at 6:11 AM, Markus Heidelberg<markus.heidelberg@web.de> wrote:

> Paolo Bonzini, 17.06.2009:
>> The first bug is that the --suppress-cc option for `cccmd' was
>> misspelled as `ccmd' in the code.

Good catch. That error has actually existed since --suppress-cc was introduced in 65648283. But nor did I (sheesh, aren't many eyes supposed to make all bugs shallow?) catch it in 3531e2703d.

Show 6 quoted lines
> Then I noticed that the documentation mentions ccbody, but the code uses
> bodycc.
> The author seemed to intend using bodycc, since it is also used in the
> tests and the commit message (3531e2703d, send-email: --suppress-cc
> improvements, 2009-02-15), but besides the documentation, ccbody is used
> in the bash completion.
Wow, I'm pretty sure I intended bodycc. And I'm not even dyslexic.
Thanks,
j.

← back to recent threads