Hi, On Fri, 29 Jul 2022, Johannes Schindelin via GitGitGadget wrote: > From: Johannes Schindelin <johannes.schindelin@xxxxxx> > > This helper is more performant than running the `mingw_test_cmp` code > with with MSYS2's Bash. And a lot more readable. Ooops... s/with with/with/ Ciao, Dscho > > 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@xxxxxx> > --- > 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@xxxxxxxxx/], > 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 > >
