git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH] tests: replace mingw_test_cmp with a helper in C

From
Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>
Date
Jul 29, 2022, 14:53 UTC
Message-ID
<pull.1309.git.1659106382128.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>

This helper is more performant than running the `mingw_test_cmp` code with with MSYS2's Bash. And a lot more readable.

To accommodate t1050, which wants to compare files weighing in with 3MB (falling outside of t1050's malloc limit of 1.5MB), we simply lift the allocation limit by setting the environment variable GIT_ALLOC_LIMIT to zero when calling the helper.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
    tests: replace mingw_test_cmp with a helper in C
    
    On the heels of sending a patch to fix a performance regression due to a
    mis-use of test_cmp
    [https://lore.kernel.org/git/b9203ea247776332e4b6f519aa27d541207adc2f.1659097724.git.gitgitgadget@gmail.com/],
    I was curious to see whether Git for Windows had the same issue. And it
    does not
    [https://github.com/git-for-windows/git/runs/7556381815?check_suite_focus=true#step:5:127]:
    it passes t5351 in 22 seconds, even while using test_cmp to compare pack
    files
    [https://github.com/git-for-windows/git/blob/3922f62f0d5991e9fe0a0817ebf89a91339c7705/t/t5351-unpack-large-objects.sh#L90].
    
    The answer is of course that a test helper written in C is much faster
    than writing the same in Bash, especially on Windows. This is especially
    sad when said Bash code is only used on Windows. So I pulled out this
    helper from the years-long effort to let Git for Windows use BusyBox'
    ash to run the test suite. The result is this patch, which has been in
    Git for Windows since June 2018.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1309%2Fdscho%2Fmingw-test-cmp-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1309/dscho/mingw-test-cmp-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1309
 Makefile                |  1 +
 t/helper/test-cmp.c     | 73 +++++++++++++++++++++++++++++++++++++++++
 t/helper/test-tool.c    |  1 +
 t/helper/test-tool.h    |  1 +
 t/test-lib-functions.sh | 68 +-------------------------------------
 t/test-lib.sh           |  2 +-
 6 files changed, 78 insertions(+), 68 deletions(-)
 create mode 100644 t/helper/test-cmp.c
diff --git a/Makefile b/Makefile
index 1624471badc..45f108e43a1 100644
--- a/Makefile
+++ b/Makefile
@@ -723,6 +723,7 @@ TEST_BUILTINS_OBJS += test-advise.o
 TEST_BUILTINS_OBJS += test-bitmap.o
 TEST_BUILTINS_OBJS += test-bloom.o
 TEST_BUILTINS_OBJS += test-chmtime.o
+TEST_BUILTINS_OBJS += test-cmp.o
 TEST_BUILTINS_OBJS += test-config.o
 TEST_BUILTINS_OBJS += test-crontab.o
 TEST_BUILTINS_OBJS += test-csprng.o
diff --git a/t/helper/test-cmp.c b/t/helper/test-cmp.c
new file mode 100644
index 00000000000..1c646a54bf6
--- /dev/null
+++ b/t/helper/test-cmp.c
@@ -0,0 +1,73 @@
+#include "test-tool.h"
+#include "git-compat-util.h"
+#include "strbuf.h"
+#include "gettext.h"
+#include "parse-options.h"
+#include "run-command.h"
+
+#ifdef WIN32
+#define NO_SUCH_DIR "\\\\.\\GLOBALROOT\\invalid"
+#else
+#define NO_SUCH_DIR "/dev/null"
+#endif
+
+static int run_diff(const char *path1, const char *path2)
+{
+	const char *argv[] = {
+		"diff", "--no-index", NULL, NULL, NULL
+	};
+	const char *env[] = {
+		"GIT_PAGER=cat",
+		"GIT_DIR=" NO_SUCH_DIR,
+		"HOME=" NO_SUCH_DIR,
+		NULL
+	};
+
+	argv[2] = path1;
+	argv[3] = path2;
+	return run_command_v_opt_cd_env(argv,
+					RUN_COMMAND_NO_STDIN | RUN_GIT_CMD,
+					NULL, env);
+}
+
+int cmd__cmp(int argc, const char **argv)
+{
+	FILE *f0, *f1;
+	struct strbuf b0 = STRBUF_INIT, b1 = STRBUF_INIT;
+
+	if (argc != 3)
+		die("Require exactly 2 arguments, got %d", argc);
+
+	if (!(f0 = !strcmp(argv[1], "-") ? stdin : fopen(argv[1], "r")))
+		return error_errno("could not open '%s'", argv[1]);
+	if (!(f1 = !strcmp(argv[2], "-") ? stdin : fopen(argv[2], "r"))) {
+		fclose(f0);
+		return error_errno("could not open '%s'", argv[2]);
+	}
+
+	for (;;) {
+		int r0 = strbuf_getline(&b0, f0);
+		int r1 = strbuf_getline(&b1, f1);
+
+		if (r0 == EOF) {
+			fclose(f0);
+			fclose(f1);
+			strbuf_release(&b0);
+			strbuf_release(&b1);
+			if (r1 == EOF)
+				return 0;
+cmp_failed:
+			if (!run_diff(argv[1], argv[2]))
+				die("Huh? 'diff --no-index %s %s' succeeded",
+				    argv[1], argv[2]);
+			return 1;
+		}
+		if (r1 == EOF || strbuf_cmp(&b0, &b1)) {
+			fclose(f0);
+			fclose(f1);
+			strbuf_release(&b0);
+			strbuf_release(&b1);
+			goto cmp_failed;
+		}
+	}
+}
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index 318fdbab0c3..3334de248a1 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -18,6 +18,7 @@ static struct test_cmd cmds[] = {
 	{ "bitmap", cmd__bitmap },
 	{ "bloom", cmd__bloom },
 	{ "chmtime", cmd__chmtime },
+	{ "cmp", cmd__cmp },
 	{ "config", cmd__config },
 	{ "crontab", cmd__crontab },
 	{ "csprng", cmd__csprng },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index bb799271631..e1104898cc3 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -8,6 +8,7 @@ int cmd__advise_if_enabled(int argc, const char **argv);
 int cmd__bitmap(int argc, const char **argv);
 int cmd__bloom(int argc, const char **argv);
 int cmd__chmtime(int argc, const char **argv);
+int cmd__cmp(int argc, const char **argv);
 int cmd__config(int argc, const char **argv);
 int cmd__crontab(int argc, const char **argv);
 int cmd__csprng(int argc, const char **argv);
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 8c44856eaec..28eddbc8e36 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -1240,7 +1240,7 @@ test_expect_code () {
 
 test_cmp () {
 	test "$#" -ne 2 && BUG "2 param"
-	eval "$GIT_TEST_CMP" '"$@"'
+	GIT_ALLOC_LIMIT=0 eval "$GIT_TEST_CMP" '"$@"'
 }
 
 # Check that the given config key has the expected value.
@@ -1541,72 +1541,6 @@ test_skip_or_die () {
 	error "$2"
 }
 
-# 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
-}
-
 # Like "env FOO=BAR some-program", but run inside a subshell, which means
 # it also works for shell functions (though those functions cannot impact
 # the environment outside of the test_env invocation).
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 7726d1da88a..220c259e796 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1546,7 +1546,7 @@ case $uname_s in
 	test_set_prereq SED_STRIPS_CR
 	test_set_prereq GREP_STRIPS_CR
 	test_set_prereq WINDOWS
-	GIT_TEST_CMP=mingw_test_cmp
+	GIT_TEST_CMP="test-tool cmp"
 	;;
 *CYGWIN*)
 	test_set_prereq POSIXPERM

base-commit: 23b219f8e3f2adfb0441e135f0a880e6124f766c

-- 
gitgitgadget
Next: Johannes Schindelin
Message 1 of 58 in “tests: replace mingw_test_cmp with a helper in C”
  1. tests: replace mingw_test_cmp with a helper in CJohannes Schindelin via GitGitGadget, Jul 29, 2022
  2. Johannes SchindelinJul 29, 2022
  3. Junio C HamanoJul 29, 2022
  4. Johannes SchindelinSep 6, 2022
  5. René ScharfeSep 7, 2022
  6. Junio C HamanoSep 7, 2022
  7. Re* [PATCH] tests: replace mingw_test_cmp with a helper in CJunio C Hamano, Sep 7, 2022
  8. René ScharfeSep 7, 2022
  9. Junio C HamanoSep 8, 2022
  10. René ScharfeSep 8, 2022
  11. Ævar Arnfjörð BjarmasonSep 8, 2022
  12. Johannes SchindelinSep 8, 2022
  13. Junio C HamanoSep 8, 2022
  14. 0/2 tests: replace mingw_test_cmp with a helper in CJohannes Schindelin via GitGitGadget, Sep 6, 2022
  15. 1/2 t0021: use Windows-friendly `pwd`Johannes Schindelin via GitGitGadget, Sep 6, 2022
  16. 2/2 tests: replace mingw_test_cmp with a helper in CJohannes Schindelin via GitGitGadget, Sep 6, 2022
  17. Ævar Arnfjörð BjarmasonSep 7, 2022
  18. Ævar Arnfjörð BjarmasonSep 7, 2022
  19. Junio C HamanoSep 7, 2022
  20. Johannes SchindelinSep 7, 2022
  21. 0/2 tests: replace mingw_test_cmp with a helper in CJohannes Schindelin via GitGitGadget, Nov 12, 2022
  22. 1/2 t0021: use Windows-friendly `pwd`Johannes Schindelin via GitGitGadget, Nov 12, 2022
  23. 2/2 tests(mingw): avoid very slow `mingw_test_cmp`Johannes Schindelin via GitGitGadget, Nov 12, 2022
  24. Taylor BlauNov 13, 2022
  25. Johannes SchindelinNov 14, 2022
  26. Junio C HamanoNov 18, 2022
  27. Taylor BlauNov 19, 2022
  28. Ævar Arnfjörð BjarmasonNov 19, 2022
  29. Johannes SixtNov 19, 2022
  30. René ScharfeNov 19, 2022
  31. Torsten BögershausenNov 20, 2022
  32. Johannes SixtNov 21, 2022
  33. Junio C HamanoNov 21, 2022
  34. Phillip WoodNov 14, 2022
  35. Johannes SchindelinNov 14, 2022
  36. Ævar Arnfjörð BjarmasonNov 14, 2022
  37. Johannes SchindelinNov 14, 2022
  38. Ævar Arnfjörð BjarmasonNov 14, 2022
  39. Junio C HamanoNov 18, 2022
  40. Taylor BlauNov 19, 2022
  41. Ævar Arnfjörð BjarmasonNov 19, 2022
  42. Junio C HamanoNov 21, 2022
  43. 0/2 tests(mingw): avoid super-slow mingw_test_cmpJohannes Schindelin via GitGitGadget, Nov 14, 2022
  44. 1/2 t0021: use Windows-friendly `pwd`Johannes Schindelin via GitGitGadget, Nov 14, 2022
  45. 2/2 tests(mingw): avoid very slow `mingw_test_cmp`Johannes Schindelin via GitGitGadget, Nov 14, 2022
  46. Taylor BlauNov 14, 2022
  47. Johannes SchindelinNov 18, 2022
  48. Taylor BlauNov 18, 2022
  49. Johannes SchindelinNov 20, 2022
  50. Taylor BlauNov 21, 2022
  51. 0/2 tests(mingw): avoid super-slow mingw_test_cmpJohannes Schindelin via GitGitGadget, Dec 6, 2022
  52. 2/2 tests(mingw): avoid very slow `mingw_test_cmp`Johannes Schindelin via GitGitGadget, Dec 6, 2022
  53. Ævar Arnfjörð BjarmasonDec 6, 2022
  54. Johannes SixtDec 6, 2022
  55. René ScharfeDec 6, 2022
  56. Junio C HamanoDec 7, 2022
  57. Taylor BlauDec 7, 2022
  58. 1/2 t0021: use Windows-friendly `pwd`Johannes Schindelin via GitGitGadget, Dec 6, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.