[PATCH 1/4] t4014: check for empty files from git format-patch --stdout

[PATCH 1/4] t4014: check for empty files from git format-patch --stdout

[Thomas Rast]

Most kinds of failure in 'git format-patch --stdout >output' will
result in an empty 'output'.  This slips past checks that only verify
absence of output, such as the '! grep ...' that are quite prevalent
in t4014.

Introduce a helper check_patch() that checks that at least From, Date
and Subject are present, thus making sure it looks vaguely like a
patch (or cover letter) email.  Then insert calls to it in all tests
that do have positive checks for content.

This makes two of the tests fail.  Mark them as such; they'll be
fixed in a moment.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

I noticed "fatal: unrecognized argument: --no-add-headers" in the
slow-moving 'make valgrind' output, and unearthed some more
problems...  The other three patches are a corollary of this.

 t/t4014-format-patch.sh |   21 +++++++++++++++++++--
 1 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 92248d2..b4d4207 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -179,12 +179,21 @@ test_expect_success 'configuration To: header' '
 	grep "^To: R. E. Cipient <rcipient@example.com>\$" patch9
 '
 
+# check_patch <patch>: Verify that <patch> looks like a half-sane
+# patch email to avoid a false positive with !grep
+check_patch () {
+	grep -e "^From:" "$1" &&
+	grep -e "^Date:" "$1" &&
+	grep -e "^Subject:" "$1"
+}
+
 test_expect_success '--no-to overrides config.to' '
 
 	git config --replace-all format.to \
 		"R. E. Cipient <rcipient@example.com>" &&
 	git format-patch --no-to --stdout master..side |
 	sed -e "/^\$/q" >patch10 &&
+	check_patch patch10 &&
 	! grep "^To: R. E. Cipient <rcipient@example.com>\$" patch10
 '
 
@@ -195,6 +204,7 @@ test_expect_success '--no-to and --to replaces config.to' '
 	git format-patch --no-to --to="Someone Else <else@out.there>" \
 		--stdout master..side |
 	sed -e "/^\$/q" >patch11 &&
+	check_patch patch11 &&
 	! grep "^To: Someone <someone@out.there>\$" patch11 &&
 	grep "^To: Someone Else <else@out.there>\$" patch11
 '
@@ -205,15 +215,17 @@ test_expect_success '--no-cc overrides config.cc' '
 		"C. E. Cipient <rcipient@example.com>" &&
 	git format-patch --no-cc --stdout master..side |
 	sed -e "/^\$/q" >patch12 &&
+	check_patch patch12 &&
 	! grep "^Cc: C. E. Cipient <rcipient@example.com>\$" patch12
 '
 
-test_expect_success '--no-add-headers overrides config.headers' '
+test_expect_failure '--no-add-headers overrides config.headers' '
 
 	git config --replace-all format.headers \
 		"Header1: B. E. Cipient <rcipient@example.com>" &&
 	git format-patch --no-add-headers --stdout master..side |
 	sed -e "/^\$/q" >patch13 &&
+	check_patch patch13 &&
 	! grep "^Header1: B. E. Cipient <rcipient@example.com>\$" patch13
 '
 
@@ -480,6 +492,7 @@ test_expect_success 'cover-letter inherits diff options' '
 	git mv file foo &&
 	git commit -m foo &&
 	git format-patch --cover-letter -1 &&
+	check_patch 0000-cover-letter.patch &&
 	! grep "file => foo .* 0 *\$" 0000-cover-letter.patch &&
 	git format-patch --cover-letter -1 -M &&
 	grep "file => foo .* 0 *\$" 0000-cover-letter.patch
@@ -657,6 +670,7 @@ test_expect_success 'format-patch --no-signature ignores format.signature' '
 	git config format.signature "config sig" &&
 	git format-patch --stdout --signature="my sig" --no-signature \
 		-1 >output &&
+	check_patch output &&
 	! grep "config sig" output &&
 	! grep "my sig" output &&
 	! grep "^-- \$" output
@@ -673,17 +687,20 @@ test_expect_success 'format-patch --signature --cover-letter' '
 test_expect_success 'format.signature="" supresses signatures' '
 	git config format.signature "" &&
 	git format-patch --stdout -1 >output &&
+	check_patch output &&
 	! grep "^-- \$" output
 '
 
 test_expect_success 'format-patch --no-signature supresses signatures' '
 	git config --unset-all format.signature &&
 	git format-patch --stdout --no-signature -1 >output &&
+	check_patch output &&
 	! grep "^-- \$" output
 '
 
-test_expect_success 'format-patch --signature="" supresses signatures' '
+test_expect_failure 'format-patch --signature="" supresses signatures' '
 	git format-patch --signature="" -1 >output &&
+	check_patch output &&
 	! grep "^-- \$" output
 '
 
-- 
1.7.7.rc0.370.gdcae57

[PATCH 2/4] t4014: invoke format-patch with --stdout where intended

[Thomas Rast]

The test wrote something along the lines of 0001-foo.patch to output,
which of course never contained a signature.  Luckily the tested
behaviour is actually present.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 t/t4014-format-patch.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index b4d4207..a45d4fb 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -698,8 +698,8 @@ test_expect_success 'format-patch --no-signature supresses signatures' '
 	! grep "^-- \$" output
 '
 
-test_expect_failure 'format-patch --signature="" supresses signatures' '
-	git format-patch --signature="" -1 >output &&
+test_expect_success 'format-patch --signature="" supresses signatures' '
+	git format-patch --stdout --signature="" -1 >output &&
 	check_patch output &&
 	! grep "^-- \$" output
 '
-- 
1.7.7.rc0.370.gdcae57

[PATCH 3/4] t4014: "no-add-headers" is actually called "no-add-header"

[Thomas Rast]

Since c426003 (format-patch: add --no-cc, --no-to, and
--no-add-headers, 2010-03-07) the tests have checked for an option
called --no-add-headers introduced by letting the user negate
--add-header.

However, the parseopt machinery does not automatically pluralize
anything, so it is in fact called --no-add-header.

Since the option never worked, is not documented anywhere, and
implementing an actual --no-add-headers would lead to silly code
complications, we just adapt the test to the code.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 t/t4014-format-patch.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index a45d4fb..5cbc066 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -219,11 +219,11 @@ test_expect_success '--no-cc overrides config.cc' '
 	! grep "^Cc: C. E. Cipient <rcipient@example.com>\$" patch12
 '
 
-test_expect_failure '--no-add-headers overrides config.headers' '
+test_expect_success '--no-add-header overrides config.headers' '
 
 	git config --replace-all format.headers \
 		"Header1: B. E. Cipient <rcipient@example.com>" &&
-	git format-patch --no-add-headers --stdout master..side |
+	git format-patch --no-add-header --stdout master..side |
 	sed -e "/^\$/q" >patch13 &&
 	check_patch patch13 &&
 	! grep "^Header1: B. E. Cipient <rcipient@example.com>\$" patch13
-- 
1.7.7.rc0.370.gdcae57

[PATCH 4/4] Document negated forms of format-patch --to --cc --add-headers

[Thomas Rast]

The negated forms introduced in c426003 (format-patch: add --no-cc,
--no-to, and --no-add-headers, 2010-03-07) were not documented
anywhere.  Add them to the descriptions of the positive forms.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

I can't help but notice that the options aren't symmetric:
--no-add-header clears more than just the options added by
--add-header.  It's probably too late to change though.


 Documentation/git-format-patch.txt |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index d13c9b2..6ea9be7 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -166,15 +166,22 @@ will want to ensure that threading is disabled for `git send-email`.
 --to=<email>::
 	Add a `To:` header to the email headers. This is in addition
 	to any configured headers, and may be used multiple times.
+	The negated form `--no-to` discards all `To:` headers added so
+	far (from config or command line).
 
 --cc=<email>::
 	Add a `Cc:` header to the email headers. This is in addition
 	to any configured headers, and may be used multiple times.
+	The negated form `--no-cc` discards all `Cc:` headers added so
+	far (from config or command line).
 
 --add-header=<header>::
 	Add an arbitrary header to the email headers.  This is in addition
 	to any configured headers, and may be used multiple times.
-	For example, `--add-header="Organization: git-foo"`
+	For example, `--add-header="Organization: git-foo"`.
+	The negated form `--no-add-header` discards *all* (`To:`,
+	`Cc:`, and custom) headers added so far from config or command
+	line.
 
 --cover-letter::
 	In addition to the patches, generate a cover letter file
-- 
1.7.7.rc0.370.gdcae57

Re: [PATCH 1/4] t4014: check for empty files from git format-patch --stdout

[Junio C Hamano]

Thanks.