[PATCH 0/3] Fix tests on Windows caused by random CRLF output

[PATCH 0/3] Fix tests on Windows caused by random CRLF output

[Johannes Sixt]

When git, the executable, invokes a bash script (hooks, scripted
git commands, ...), bash writes its output with CRLF. This causes
tests to fail because the expected output does not contain CR, only LF.

This fixes the tests by replacing test_cmp with a diff computation
that disregards the CR.

Patch 1 replaces test_cmp with cmp when binary data is compared.
Patch 2 is the actual fix.
Patch 3 removes some work-arounds that are not necessary anymore.

Johannes Sixt (3):
  t5300-pack-object: do not compare binary data using test_cmp
  Windows: a test_cmp that is agnostic to random LF <> CRLF conversions
  tests: undo special treatment of CRLF for Windows

 t/lib-credential.sh          |  4 ---
 t/t5300-pack-object.sh       | 10 +++----
 t/t7407-submodule-foreach.sh |  4 ---
 t/t9001-send-email.sh        |  1 -
 t/test-lib-functions.sh      | 66 ++++++++++++++++++++++++++++++++++++++++++++
 t/test-lib.sh                |  1 +
 6 files changed, 72 insertions(+), 14 deletions(-)

-- 
1.8.4.33.gd68f7e8

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[PATCH 1/3] t5300-pack-object: do not compare binary data using test_cmp

[Johannes Sixt]

Users may set test_cmp to a comparison tool of their liking. The intent is
that the tool performs comparison of line-oriented texts. However, t5300
uses it also to compare binary data. Change those tests to use 'cmp'.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 t/t5300-pack-object.sh | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
index a07c871..61e787d 100755
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -151,7 +151,7 @@ test_expect_success \
 	    git cat-file $t $object || return 1
 	 done <obj-list
     } >current &&
-    test_cmp expect current'
+    cmp expect current'
 
 test_expect_success \
     'use packed deltified (REF_DELTA) objects' \
@@ -166,7 +166,7 @@ test_expect_success \
 	    git cat-file $t $object || return 1
 	 done <obj-list
     } >current &&
-    test_cmp expect current'
+    cmp expect current'
 
 test_expect_success \
     'use packed deltified (OFS_DELTA) objects' \
@@ -181,7 +181,7 @@ test_expect_success \
 	    git cat-file $t $object || return 1
 	 done <obj-list
     } >current &&
-    test_cmp expect current'
+    cmp expect current'
 
 unset GIT_OBJECT_DIRECTORY
 
@@ -195,9 +195,9 @@ test_expect_success 'survive missing objects/pack directory' '
 		rm -fr $GOP &&
 		git index-pack --stdin --keep=test <../test-3-${packname_3}.pack &&
 		test -f $GOP/pack-${packname_3}.pack &&
-		test_cmp $GOP/pack-${packname_3}.pack ../test-3-${packname_3}.pack &&
+		cmp $GOP/pack-${packname_3}.pack ../test-3-${packname_3}.pack &&
 		test -f $GOP/pack-${packname_3}.idx &&
-		test_cmp $GOP/pack-${packname_3}.idx ../test-3-${packname_3}.idx &&
+		cmp $GOP/pack-${packname_3}.idx ../test-3-${packname_3}.idx &&
 		test -f $GOP/pack-${packname_3}.keep
 	)
 '
-- 
1.8.4.33.gd68f7e8

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[PATCH 2/3] Windows: a test_cmp that is agnostic to random LF <> CRLF conversions

[Johannes Sixt]

In a number of tests, output that was produced by a shell script is
compared to expected output using test_cmp. Unfortunately, the MSYS bash--
when invoked via git, such as in hooks--converts LF to CRLF on output
(as produced by echo and printf), which leads to many false positives.

Implements a diff tool that undoes the converted CRLF. To avoid that
sub-processes are spawned (which is very slow on Windows), the tool is
implemented as a shell function. Diff is invoked as usual only when a
difference is detected by the shell code.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 t/test-lib-functions.sh | 66 +++++++++++++++++++++++++++++++++++++++++++++++++
 t/test-lib.sh           |  1 +
 2 files changed, 67 insertions(+)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index a7e9aac..f987694 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -710,3 +710,69 @@ test_ln_s_add () {
 		git update-index --add --cacheinfo 120000 $ln_s_obj "$2"
 	fi
 }
+
+# The following mingw_* functions obey POSIX shell syntax, but are actually
+# bash scripts, and are meant to be used only with bash on Windows.
+
+# A test_cmp function that treats LF and CRLF equal and avoids to fork
+# diff when possible.
+mingw_test_cmp () {
+	# Read text into shell variables and compare them. If the results
+	# are different, use regular diff to report the difference.
+	local test_cmp_a= test_cmp_b=
+
+	# When text came from stdin (one argument is '-') we must feed it
+	# to diff.
+	local stdin_for_diff=
+
+	# Since it is difficult to detect the difference between an
+	# empty input file and a failure to read the files, we go straight
+	# to diff if one of the inputs is empty.
+	if test -s "$1" && test -s "$2"
+	then
+		# regular case: both files non-empty
+		mingw_read_file_strip_cr_ test_cmp_a <"$1"
+		mingw_read_file_strip_cr_ test_cmp_b <"$2"
+	elif test -s "$1" && test "$2" = -
+	then
+		# read 2nd file from stdin
+		mingw_read_file_strip_cr_ test_cmp_a <"$1"
+		mingw_read_file_strip_cr_ test_cmp_b
+		stdin_for_diff='<<<"$test_cmp_b"'
+	elif test "$1" = - && test -s "$2"
+	then
+		# read 1st file from stdin
+		mingw_read_file_strip_cr_ test_cmp_a
+		mingw_read_file_strip_cr_ test_cmp_b <"$2"
+		stdin_for_diff='<<<"$test_cmp_a"'
+	fi
+	test -n "$test_cmp_a" &&
+	test -n "$test_cmp_b" &&
+	test "$test_cmp_a" = "$test_cmp_b" ||
+	eval "diff -u \"\$@\" $stdin_for_diff"
+}
+
+# $1 is the name of the shell variable to fill in
+mingw_read_file_strip_cr_ () {
+	# Read line-wise using LF as the line separator
+	# and use IFS to strip CR.
+	local line
+	while :
+	do
+		if IFS=$'\r' read -r -d $'\n' line
+		then
+			# good
+			line=$line$'\n'
+		else
+			# we get here at EOF, but also if the last line
+			# was not terminated by LF; in the latter case,
+			# some text was read
+			if test -z "$line"
+			then
+				# EOF, really
+				break
+			fi
+		fi
+		eval "$1=\$$1\$line"
+	done
+}
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 0fa7dfd..77e487b 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -817,6 +817,7 @@ case $(uname -s) in
 	test_set_prereq NOT_CYGWIN
 	test_set_prereq SED_STRIPS_CR
 	test_set_prereq GREP_STRIPS_CR
+	GIT_TEST_CMP=mingw_test_cmp
 	;;
 *CYGWIN*)
 	test_set_prereq POSIXPERM
-- 
1.8.4.33.gd68f7e8

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[PATCH 3/3] tests: undo special treatment of CRLF for Windows

[Johannes Sixt]

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 t/lib-credential.sh          | 4 ----
 t/t7407-submodule-foreach.sh | 4 ----
 t/t9001-send-email.sh        | 1 -
 3 files changed, 9 deletions(-)

diff --git a/t/lib-credential.sh b/t/lib-credential.sh
index 3c43ff1..957ae93 100755
--- a/t/lib-credential.sh
+++ b/t/lib-credential.sh
@@ -18,10 +18,6 @@ check() {
 		cat stderr &&
 		false
 	fi &&
-	if test_have_prereq MINGW
-	then
-		dos2unix -q stderr
-	fi &&
 	test_cmp expect-stdout stdout &&
 	test_cmp expect-stderr stderr
 }
diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh
index be93f10..b64c9ed 100755
--- a/t/t7407-submodule-foreach.sh
+++ b/t/t7407-submodule-foreach.sh
@@ -254,10 +254,6 @@ test_expect_success 'ensure "status --cached --recursive" preserves the --cached
 		) &&
 		git submodule status --cached --recursive -- nested1 > ../actual
 	) &&
-	if test_have_prereq MINGW
-	then
-		dos2unix actual
-	fi &&
 	test_cmp expect actual
 '
 
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 2813aa9..3119c8c 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -23,7 +23,6 @@ test_expect_success $PREREQ \
       echo do
       echo "  echo \"!\$a!\""
       echo "done >commandline\$output"
-      test_have_prereq MINGW && echo "dos2unix commandline\$output"
       echo "cat > msgtxt\$output"
       ) >fake.sendmail &&
      chmod +x ./fake.sendmail &&
-- 
1.8.4.33.gd68f7e8

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [PATCH 2/3] Windows: a test_cmp that is agnostic to random LF <> CRLF conversions

[Sebastian Schuberth]

On 26.10.2013 21:17, Johannes Sixt wrote:

> In a number of tests, output that was produced by a shell script is
> compared to expected output using test_cmp. Unfortunately, the MSYS bash--
> when invoked via git, such as in hooks--converts LF to CRLF on output
> (as produced by echo and printf), which leads to many false positives.

As you correctly point out the LF vs. CRLF issues are caused by MSYS 
bash. Then why are the functions called mingw_* instead of msys_*? I'd 
prefer to make very clear that not MinGW but MSYS is the culprit 
concerning the LF vs. CRLF issues (and also the path mangling issues).

-- 
Sebastian Schuberth

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [PATCH 2/3] Windows: a test_cmp that is agnostic to random LF <> CRLF conversions

[Johannes Sixt]

Am 02.11.2013 21:33, schrieb Sebastian Schuberth:
> On 26.10.2013 21:17, Johannes Sixt wrote:
> 
>> In a number of tests, output that was produced by a shell script is
>> compared to expected output using test_cmp. Unfortunately, the MSYS
>> bash--
>> when invoked via git, such as in hooks--converts LF to CRLF on output
>> (as produced by echo and printf), which leads to many false positives.
> 
> As you correctly point out the LF vs. CRLF issues are caused by MSYS
> bash. Then why are the functions called mingw_* instead of msys_*? I'd
> prefer to make very clear that not MinGW but MSYS is the culprit
> concerning the LF vs. CRLF issues (and also the path mangling issues).

It's a historical accident. We already have the prerequisite MINGW to
work around various annoyances, most of which are caused by MSYS bash. I
didn't want to invent a new prefix.

-- Hannes

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [PATCH 2/3] Windows: a test_cmp that is agnostic to random LF <> CRLF conversions

[Sebastian Schuberth]

On Sat, Nov 2, 2013 at 9:40 PM, Johannes Sixt <j6t@kdbg.org> wrote:

>>> In a number of tests, output that was produced by a shell script is
>>> compared to expected output using test_cmp. Unfortunately, the MSYS
>>> bash--
>>> when invoked via git, such as in hooks--converts LF to CRLF on output
>>> (as produced by echo and printf), which leads to many false positives.
>>
>> As you correctly point out the LF vs. CRLF issues are caused by MSYS
>> bash. Then why are the functions called mingw_* instead of msys_*? I'd
>> prefer to make very clear that not MinGW but MSYS is the culprit
>> concerning the LF vs. CRLF issues (and also the path mangling issues).
>
> It's a historical accident. We already have the prerequisite MINGW to
> work around various annoyances, most of which are caused by MSYS bash. I
> didn't want to invent a new prefix.

So maybe it's a good point now to also change the MINGW prerequisite
name to MSYS as part of your patch, and then name the functions more
appropriately?

-- 
Sebastian Schuberth

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [PATCH 2/3] Windows: a test_cmp that is agnostic to random LF <> CRLF conversions

[Johannes Sixt]

Am 02.11.2013 21:47, schrieb Sebastian Schuberth:
> So maybe it's a good point now to also change the MINGW prerequisite
> name to MSYS as part of your patch, and then name the functions more
> appropriately?

It's too late. The series is already in master, and I see no point in a
follow-up patch of this kind as long as there is no additional benefit.

-- Hannes

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.