Re: [PATCH v3 2/2] tests(mingw): avoid very slow `mingw_test_cmp`
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Nov 14, 2022, 09:53 UTC
- Message-ID
- <8ec8f3f5-0827-25a3-464d-638378b65ddb@dunelm.org.uk>
- In-Reply-To
- <a7f4265ceb26c6dd9d347ef4cbef2aac7d60bf13.1668290855.git.gitgitgadget@gmail.com>
Hi Dscho
On 12/11/2022 22:07, Johannes Schindelin via GitGitGadget wrote:
Show 5 quoted lines
> From: Johannes Schindelin <johannes.schindelin@gmx.de> > > It is more performant to run `git diff --no-index` than running the > `mingw_test_cmp` code with MSYS2's Bash, i.e. the Bash that Git for > Windows uses. And a lot more readable.
This makes me wonder about the implications for our diff tests. We have ~200 calls to `test_cmp` in t*-diff-*.sh. I had a look at t4053-diff-no-index.sh and non of the tests that use `test_cmp` look critical for functionality that is used by `test_cmp` and there are tests for the exit code. I suspect that if `diff --no-index` breaks we'll end up with confusing test failures rather than misleading passes.
A side effect of this change is that on windows `test_cmp` will now except directories as well as files but I don't think that matters.
Show 5 quoted lines
> Note: Earlier attempts at fixing this involved a test helper that avoids > the overhead of the diff machinery, in favor of implementing a behavior > that is more in line with what `mingw_test_cmp` does now, but that > attempt saw a lot of backlash and distractions during review and was > therefore abandoned.
Hopefully most of the files we feed into `test_cmp` are small enough that the absolute difference in run-time is not too big. There is an optimization for -U0 that trims the common tail from the files before calling xdl_diff() but that does not help here because we need to use --ignore-cr-at-eol (otherwise `git diff --no-index -U0 || git diff --no-index` might speed up the common case of matching files).
Best Wishes
Phillip
Show 96 quoted lines
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> t/test-lib-functions.sh | 66 -----------------------------------------
> t/test-lib.sh | 2 +-
> 2 files changed, 1 insertion(+), 67 deletions(-)
>
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index 8c44856eaec..452fe9bc8aa 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -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..f8c6205e08f 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="GIT_DIR=/dev/null git diff --no-index --ignore-cr-at-eol --"
> ;;
> *CYGWIN*)
> test_set_prereq POSIXPERM