[PATCHv3 0/3] Fix patch detection for thunderbird saved mails

[PATCHv3 0/3] Fix patch detection for thunderbird saved mails

[Stephen Boyd]

The original patch for this is a bit old (~1 month) but I'd like to see it
make it in. The first two patches are cleanup before the original patch in
the third spot. Now the test in patch 3 uses append_cr() instead of a hand
crafted version.

Stephen Boyd (3):
  tests: consolidate CR removal/addition functions
  t0022: replace non-portable literal CR
  am: fix patch format detection for Thunderbird "Save As" emails

 git-am.sh                     |    3 +-
 t/t0020-crlf.sh               |   55 +++++++++++++++-------------------------
 t/t0022-crlf-rename.sh        |    2 +-
 t/t3400-rebase.sh             |    4 ---
 t/t4012-diff-binary.sh        |    4 ---
 t/t4150-am.sh                 |   15 +++++++++++
 t/t6033-merge-crlf.sh         |    8 ------
 t/t9401-git-cvsserver-crlf.sh |    8 ------
 t/test-lib.sh                 |   16 ++++++++++++
 9 files changed, 55 insertions(+), 60 deletions(-)

[PATCHv3 1/3] tests: consolidate CR removal/addition functions

[Stephen Boyd]

append_cr(), remove_cr(), q_to_nul() and q_to_cr() are defined in multiple
tests. Consolidate them into test-lib.sh so we can stop redefining them.
The use of remove_cr() in t0020 to test for a CR is replaced with a new
function has_cr() to accurately reflect what is intended (the output of
remove_cr() was being thrown away).

Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---
 t/t0020-crlf.sh               |   55 +++++++++++++++-------------------------
 t/t3400-rebase.sh             |    4 ---
 t/t4012-diff-binary.sh        |    4 ---
 t/t6033-merge-crlf.sh         |    8 ------
 t/t9401-git-cvsserver-crlf.sh |    8 ------
 t/test-lib.sh                 |   16 ++++++++++++
 6 files changed, 37 insertions(+), 58 deletions(-)

diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh
index 4e72b53..c3e7e32 100755
--- a/t/t0020-crlf.sh
+++ b/t/t0020-crlf.sh
@@ -4,21 +4,8 @@ test_description='CRLF conversion'
 
 . ./test-lib.sh
 
-q_to_nul () {
-	perl -pe 'y/Q/\000/'
-}
-
-q_to_cr () {
-	tr Q '\015'
-}
-
-append_cr () {
-	sed -e 's/$/Q/' | tr Q '\015'
-}
-
-remove_cr () {
-	tr '\015' Q <"$1" | grep Q >/dev/null &&
-	tr '\015' Q <"$1" | sed -ne 's/Q$//p'
+has_cr() {
+	tr '\015' Q <"$1" | grep Q >/dev/null
 }
 
 test_expect_success setup '
@@ -156,7 +143,7 @@ test_expect_success 'checkout with autocrlf=true' '
 
 	for f in one dir/two
 	do
-		remove_cr "$f" >tmp && mv -f tmp $f &&
+		remove_cr <"$f" >tmp && mv -f tmp $f &&
 		git update-index -- $f || {
 			echo "Eh? $f"
 			false
@@ -180,7 +167,7 @@ test_expect_success 'checkout with autocrlf=input' '
 
 	for f in one dir/two
 	do
-		if remove_cr "$f" >/dev/null
+		if has_cr "$f"
 		then
 			echo "Eh? $f"
 			false
@@ -245,7 +232,7 @@ test_expect_success 'apply patch (autocrlf=true)' '
 	git read-tree --reset -u HEAD &&
 
 	git apply patch.file &&
-	test "$patched" = "`remove_cr one | git hash-object --stdin`" || {
+	test "$patched" = "`remove_cr <one | git hash-object --stdin`" || {
 		echo "Eh?  apply without index"
 		false
 	}
@@ -272,7 +259,7 @@ test_expect_success 'apply patch --index (autocrlf=true)' '
 
 	git apply --index patch.file &&
 	test "$patched" = `git rev-parse :one` &&
-	test "$patched" = "`remove_cr one | git hash-object --stdin`" || {
+	test "$patched" = "`remove_cr <one | git hash-object --stdin`" || {
 		echo "Eh?  apply with --index"
 		false
 	}
@@ -285,7 +272,7 @@ test_expect_success '.gitattributes says two is binary' '
 	git config core.autocrlf true &&
 	git read-tree --reset -u HEAD &&
 
-	if remove_cr dir/two >/dev/null
+	if has_cr dir/two
 	then
 		echo "Huh?"
 		false
@@ -293,7 +280,7 @@ test_expect_success '.gitattributes says two is binary' '
 		: happy
 	fi &&
 
-	if remove_cr one >/dev/null
+	if has_cr one
 	then
 		: happy
 	else
@@ -301,7 +288,7 @@ test_expect_success '.gitattributes says two is binary' '
 		false
 	fi &&
 
-	if remove_cr three >/dev/null
+	if has_cr three
 	then
 		echo "Huh?"
 		false
@@ -316,7 +303,7 @@ test_expect_success '.gitattributes says two is input' '
 	echo "two crlf=input" >.gitattributes &&
 	git read-tree --reset -u HEAD &&
 
-	if remove_cr dir/two >/dev/null
+	if has_cr dir/two
 	then
 		echo "Huh?"
 		false
@@ -331,7 +318,7 @@ test_expect_success '.gitattributes says two and three are text' '
 	echo "t* crlf" >.gitattributes &&
 	git read-tree --reset -u HEAD &&
 
-	if remove_cr dir/two >/dev/null
+	if has_cr dir/two
 	then
 		: happy
 	else
@@ -339,7 +326,7 @@ test_expect_success '.gitattributes says two and three are text' '
 		false
 	fi &&
 
-	if remove_cr three >/dev/null
+	if has_cr three
 	then
 		: happy
 	else
@@ -357,14 +344,14 @@ test_expect_success 'in-tree .gitattributes (1)' '
 	rm -rf tmp one dir .gitattributes patch.file three &&
 	git read-tree --reset -u HEAD &&
 
-	if remove_cr one >/dev/null
+	if has_cr one
 	then
 		echo "Eh? one should not have CRLF"
 		false
 	else
 		: happy
 	fi &&
-	remove_cr three >/dev/null || {
+	has_cr three || {
 		echo "Eh? three should still have CRLF"
 		false
 	}
@@ -376,14 +363,14 @@ test_expect_success 'in-tree .gitattributes (2)' '
 	git read-tree --reset HEAD &&
 	git checkout-index -f -q -u -a &&
 
-	if remove_cr one >/dev/null
+	if has_cr one
 	then
 		echo "Eh? one should not have CRLF"
 		false
 	else
 		: happy
 	fi &&
-	remove_cr three >/dev/null || {
+	has_cr three || {
 		echo "Eh? three should still have CRLF"
 		false
 	}
@@ -396,14 +383,14 @@ test_expect_success 'in-tree .gitattributes (3)' '
 	git checkout-index -u .gitattributes &&
 	git checkout-index -u one dir/two three &&
 
-	if remove_cr one >/dev/null
+	if has_cr one
 	then
 		echo "Eh? one should not have CRLF"
 		false
 	else
 		: happy
 	fi &&
-	remove_cr three >/dev/null || {
+	has_cr three || {
 		echo "Eh? three should still have CRLF"
 		false
 	}
@@ -416,14 +403,14 @@ test_expect_success 'in-tree .gitattributes (4)' '
 	git checkout-index -u one dir/two three &&
 	git checkout-index -u .gitattributes &&
 
-	if remove_cr one >/dev/null
+	if has_cr one
 	then
 		echo "Eh? one should not have CRLF"
 		false
 	else
 		: happy
 	fi &&
-	remove_cr three >/dev/null || {
+	has_cr three || {
 		echo "Eh? three should still have CRLF"
 		false
 	}
@@ -456,7 +443,7 @@ test_expect_success 'checkout when deleting .gitattributes' '
 
 	git checkout master~1 &&
 	git checkout master &&
-	remove_cr .file2 >/dev/null
+	has_cr .file2
 
 '
 
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 4e6a44b..4314ad2 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -134,10 +134,6 @@ test_expect_success 'rebase -q is quiet' '
      test ! -s output.out
 '
 
-q_to_cr () {
-	tr Q '\015'
-}
-
 test_expect_success 'Rebase a commit that sprinkles CRs in' '
 	(
 		echo "One"
diff --git a/t/t4012-diff-binary.sh b/t/t4012-diff-binary.sh
index f64aa48..bc46563 100755
--- a/t/t4012-diff-binary.sh
+++ b/t/t4012-diff-binary.sh
@@ -77,10 +77,6 @@ test_expect_success 'apply binary patch' \
 	 tree1=`git write-tree` &&
 	 test "$tree1" = "$tree0"'
 
-q_to_nul() {
-	perl -pe 'y/Q/\000/'
-}
-
 nul_to_q() {
 	perl -pe 'y/\000/Q/'
 }
diff --git a/t/t6033-merge-crlf.sh b/t/t6033-merge-crlf.sh
index 75d9602..e8d65ee 100755
--- a/t/t6033-merge-crlf.sh
+++ b/t/t6033-merge-crlf.sh
@@ -1,13 +1,5 @@
 #!/bin/sh
 
-append_cr () {
-	sed -e 's/$/Q/' | tr Q '\015'
-}
-
-remove_cr () {
-	tr '\015' Q | sed -e 's/Q$//'
-}
-
 test_description='merge conflict in crlf repo
 
 		b---M
diff --git a/t/t9401-git-cvsserver-crlf.sh b/t/t9401-git-cvsserver-crlf.sh
index 40637d6..ed7b513 100755
--- a/t/t9401-git-cvsserver-crlf.sh
+++ b/t/t9401-git-cvsserver-crlf.sh
@@ -11,14 +11,6 @@ repository using cvs CLI client via git-cvsserver server'
 
 . ./test-lib.sh
 
-q_to_nul () {
-    perl -pe 'y/Q/\000/'
-}
-
-q_to_cr () {
-    tr Q '\015'
-}
-
 marked_as () {
     foundEntry="$(grep "^/$2/" "$1/CVS/Entries")"
     if [ x"$foundEntry" = x"" ] ; then
diff --git a/t/test-lib.sh b/t/test-lib.sh
index baa4093..afd3053 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -230,6 +230,22 @@ test_decode_color () {
 		-e 's/.\[m/<RESET>/g'
 }
 
+q_to_nul () {
+	perl -pe 'y/Q/\000/'
+}
+
+q_to_cr () {
+	tr Q '\015'
+}
+
+append_cr () {
+	sed -e 's/$/Q/' | tr Q '\015'
+}
+
+remove_cr () {
+	tr '\015' Q | sed -e 's/Q$//'
+}
+
 test_tick () {
 	if test -z "${test_tick+set}"
 	then
-- 
1.7.0.rc0

[PATCHv3 2/3] t0022: replace non-portable literal CR

[Stephen Boyd]

We shouldn't have literal CR's in tests as they aren't portable.

Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---

Hopefully this isn't botched over the wire.

 t/t0022-crlf-rename.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/t/t0022-crlf-rename.sh b/t/t0022-crlf-rename.sh
index f1e1d48..7af3fbc 100755
--- a/t/t0022-crlf-rename.sh
+++ b/t/t0022-crlf-rename.sh
@@ -12,7 +12,7 @@ test_expect_success setup '
 	test_tick &&
 	git commit -m Initial &&
 
-	sed -e "s/\$/
/" "$TEST_DIRECTORY"/t0022-crlf-rename.sh >elpmas &&
+	append_cr <"$TEST_DIRECTORY"/t0022-crlf-rename.sh >elpmas &&
 	git add elpmas &&
 	rm -f sample &&
 
-- 
1.7.0.rc0

[PATCHv3 3/3] am: fix patch format detection for Thunderbird "Save As" emails

[Stephen Boyd]

The patch detection wants to inspect all the headers of a rfc2822 message
and ensure that they look like header fields. The headers are always
separated from the message body with a blank line. When Thunderbird saves
the message the blank line separating the headers from the body includes a
CR. The patch detection is failing because a CRLF doesn't match /^$/. Fix
this by allowing a CR to exist on the separating line.

Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---

I gave up trying to make it portable and now just use append_cr() from
test-lib. Hopefully this is a much better and simpler approach.

 git-am.sh     |    3 ++-
 t/t4150-am.sh |   15 +++++++++++++++
 2 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index 2f46fda..c8b9cbb 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -205,7 +205,8 @@ check_patch_format () {
 			# discarding the indented remainder of folded lines,
 			# and see if it looks like that they all begin with the
 			# header field names...
-			sed -n -e '/^$/q' -e '/^[ 	]/d' -e p "$1" |
+			tr -d '\015' <"$1" |
+			sed -n -e '/^$/q' -e '/^[ 	]/d' -e p |
 			sane_egrep -v '^[!-9;-~]+:' >/dev/null ||
 			patch_format=mbox
 		fi
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 8296605..a034918 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -83,6 +83,12 @@ test_expect_success setup '
 		echo "X-Fake-Field: Line Three" &&
 		git format-patch --stdout first | sed -e "1d"
 	} > patch1.eml &&
+	{
+		echo "X-Fake-Field: Line One" &&
+		echo "X-Fake-Field: Line Two" &&
+		echo "X-Fake-Field: Line Three" &&
+		git format-patch --stdout first | sed -e "1d"
+	} | append_cr >patch1-crlf.eml &&
 	sed -n -e "3,\$p" msg >file &&
 	git add file &&
 	test_tick &&
@@ -123,6 +129,15 @@ test_expect_success 'am applies patch e-mail not in a mbox' '
 	test "$(git rev-parse second^)" = "$(git rev-parse HEAD^)"
 '
 
+test_expect_success 'am applies patch e-mail not in a mbox with CRLF' '
+	git checkout first &&
+	git am patch1-crlf.eml &&
+	! test -d .git/rebase-apply &&
+	test -z "$(git diff second)" &&
+	test "$(git rev-parse second)" = "$(git rev-parse HEAD)" &&
+	test "$(git rev-parse second^)" = "$(git rev-parse HEAD^)"
+'
+
 GIT_AUTHOR_NAME="Another Thor"
 GIT_AUTHOR_EMAIL="a.thor@example.com"
 GIT_COMMITTER_NAME="Co M Miter"
-- 
1.7.0.rc0

Re: [PATCHv3 0/3] Fix patch detection for thunderbird saved mails

[Junio C Hamano]

Looks sensible; thanks.