[PATCH v2 0/2] tests: replace mingw_test_cmp with a helper in C
- From
Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>
- Date
- Sep 6, 2022, 13:10 UTC
- Message-ID
- <pull.1309.v2.git.1662469859.gitgitgadget@gmail.com>
- In-Reply-To
- <pull.1309.git.1659106382128.gitgitgadget@gmail.com>
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.
Changes since v1:
* Fixed double "with" in the commit message.
Johannes Schindelin (2): t0021: use Windows-friendly `pwd` tests: replace mingw_test_cmp with a helper in C
Makefile | 1 + t/helper/test-text-cmp.c | 78 ++++++++++++++++++++++++++++++++++++++++ t/helper/test-tool.c | 1 + t/helper/test-tool.h | 1 + t/t0021-conversion.sh | 4 +-- t/test-lib-functions.sh | 68 +---------------------------------- t/test-lib.sh | 2 +- 7 files changed, 85 insertions(+), 70 deletions(-) create mode 100644 t/helper/test-text-cmp.c
base-commit: 23b219f8e3f2adfb0441e135f0a880e6124f766c Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1309%2Fdscho%2Fmingw-test-cmp-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1309/dscho/mingw-test-cmp-v2 Pull-Request: https://github.com/gitgitgadget/git/pull/1309
Range-diff vs v1:
-: ----------- > 1: ad7c41401ee t0021: use Windows-friendly `pwd`
1: eacbbc53700 ! 2: 1f5366f1379 tests: replace mingw_test_cmp with a helper in C
@@ Commit message
tests: replace mingw_test_cmp with a helper in C
This helper is more performant than running the `mingw_test_cmp` code
- with with MSYS2's Bash. And a lot more readable.
+ 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
@@ Commit message
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
## Makefile ##
-@@ Makefile: 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
+@@ Makefile: TEST_BUILTINS_OBJS += test-string-list.o
+ TEST_BUILTINS_OBJS += test-submodule-config.o
+ TEST_BUILTINS_OBJS += test-submodule-nested-repo-config.o
+ TEST_BUILTINS_OBJS += test-subprocess.o
++TEST_BUILTINS_OBJS += test-text-cmp.o
+ TEST_BUILTINS_OBJS += test-trace2.o
+ TEST_BUILTINS_OBJS += test-urlmatch-normalization.o
+ TEST_BUILTINS_OBJS += test-userdiff.o
- ## t/helper/test-cmp.c (new) ##
+ ## t/helper/test-text-cmp.c (new) ##
@@
+#include "test-tool.h"
+#include "git-compat-util.h"
@@ t/helper/test-cmp.c (new)
+static int run_diff(const char *path1, const char *path2)
+{
+ const char *argv[] = {
-+ "diff", "--no-index", NULL, NULL, NULL
++ "diff", "--no-index", "--", NULL, NULL, NULL
+ };
+ const char *env[] = {
+ "GIT_PAGER=cat",
@@ t/helper/test-cmp.c (new)
+ NULL
+ };
+
-+ argv[2] = path1;
-+ argv[3] = path2;
++ argv[3] = path1;
++ argv[4] = 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)
++int cmd__text_cmp(int argc, const char **argv)
+{
+ FILE *f0, *f1;
+ struct strbuf b0 = STRBUF_INIT, b1 = STRBUF_INIT;
@@ t/helper/test-cmp.c (new)
+ if (argc != 3)
+ die("Require exactly 2 arguments, got %d", argc);
+
++ if (!strcmp(argv[1], "-") && !strcmp(argv[2], "-"))
++ die("only one parameter can refer to `stdin` but not both");
++
+ 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"))) {
@@ t/helper/test-cmp.c (new)
+ if (r1 == EOF)
+ return 0;
+cmp_failed:
-+ if (!run_diff(argv[1], argv[2]))
++ if (!strcmp(argv[1], "-") || !strcmp(argv[2], "-"))
++ warning("cannot show diff because `stdin` was already consumed");
++ else if (!run_diff(argv[1], argv[2]))
+ die("Huh? 'diff --no-index %s %s' succeeded",
+ argv[1], argv[2]);
+ return 1;
@@ t/helper/test-cmp.c (new)
## t/helper/test-tool.c ##
@@ t/helper/test-tool.c: 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 },
+ { "submodule-config", cmd__submodule_config },
+ { "submodule-nested-repo-config", cmd__submodule_nested_repo_config },
+ { "subprocess", cmd__subprocess },
++ { "text-cmp", cmd__text_cmp },
+ { "trace2", cmd__trace2 },
+ { "userdiff", cmd__userdiff },
+ { "urlmatch-normalization", cmd__urlmatch_normalization },
## t/helper/test-tool.h ##
-@@ t/helper/test-tool.h: 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);
+@@ t/helper/test-tool.h: int cmd__string_list(int argc, const char **argv);
+ int cmd__submodule_config(int argc, const char **argv);
+ int cmd__submodule_nested_repo_config(int argc, const char **argv);
+ int cmd__subprocess(int argc, const char **argv);
++int cmd__text_cmp(int argc, const char **argv);
+ int cmd__trace2(int argc, const char **argv);
+ int cmd__userdiff(int argc, const char **argv);
+ int cmd__urlmatch_normalization(int argc, const char **argv);
## t/test-lib-functions.sh ##
@@ t/test-lib-functions.sh: test_expect_code () {
@@ t/test-lib.sh: case $uname_s in
test_set_prereq GREP_STRIPS_CR
test_set_prereq WINDOWS
- GIT_TEST_CMP=mingw_test_cmp
-+ GIT_TEST_CMP="test-tool cmp"
++ GIT_TEST_CMP="test-tool text-cmp"
;;
*CYGWIN*)
test_set_prereq POSIXPERM-- gitgitgadget