[PATCH v4 0/2] tests(mingw): avoid super-slow mingw_test_cmp

["Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>]

A few months ago, directly after 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 got curious to see whether Git for Windows had the same issue. And it did
not: 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 explanation is of course that Git for Windows uses a test helper for
test_cmp that is written in C, instead of the Bash function. And C is much
faster than a Bash function, especially on Windows. This is especially sad
when said Bash code is only used on Windows. So I originally had pulled out
this helper from the years-long effort to let Git for Windows use BusyBox'
ash to run the test suite. The result was a single-patch contribution of a
change that had been in Git for Windows since June 2018. Unfortunately, this
tried-and-tested code was rejected by the Git maintainer.

Let's fall back to the next-best solution: git diff --no-index, which the
Git maintainer seems to like. The downside is that the diff machinery does a
lot more than a simple cmp clone, and therefore a lot more things can go
wrong that might make it look like a test case is failing when the fault is
somewhere else entirely. There is one way to find out whether this is a
valid concern.

Changes since v3:

 * Fixed the subject of the cover letter (which should have been adjusted in
   v3)
 * Elaborated the paragraph about the historical context of this patch

Changes since v2:

 * Dropped the test helper, using diff --no-index instead.

Changes since v1:

 * Fixed double "with" in the commit message.
 * Renamed the test helper to text-cmp.
 * Made the diff --no-index call more robust by using a double-dash
   separator.

Johannes Schindelin (2):
  t0021: use Windows-friendly `pwd`
  tests(mingw): avoid very slow `mingw_test_cmp`

 t/t0021-conversion.sh   |  4 +--
 t/test-lib-functions.sh | 66 -----------------------------------------
 t/test-lib.sh           |  2 +-
 3 files changed, 3 insertions(+), 69 deletions(-)


base-commit: 23b219f8e3f2adfb0441e135f0a880e6124f766c
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1309%2Fdscho%2Fmingw-test-cmp-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1309/dscho/mingw-test-cmp-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/1309

Range-diff vs v3:

 1:  b38b8fb5a85 = 1:  b38b8fb5a85 t0021: use Windows-friendly `pwd`
 2:  a7f4265ceb2 ! 2:  128b1f348d8 tests(mingw): avoid very slow `mingw_test_cmp`
     @@ Commit message
          `mingw_test_cmp` code with MSYS2's Bash, i.e. the Bash that Git for
          Windows uses. And a lot more readable.
      
     -    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.
     +    The original reason why Git's test suite needs the `mingw_test_cmp`
     +    function at all (and why `cmp` is not good enough) is that Git's test
     +    suite is not actually trying to compare binary files when it calls
     +    `test_cmp`, but it compares text files. And those text files can contain
     +    CR/LF line endings depending on the circumstances.
     +
     +    Note: The original fix in the Git for Windows project implemented 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. This was done to minimize the risk in using something as
     +    complex as the diff machinery to perform something as simple as
     +    determining whether text output is identical to the expected output or
     +    not. This approach has served Git for Windows well for years, but the
     +    attempt to upstream this saw a lot of backlash and distractions during
     +    the review, was disliked by the Git maintainer and was therefore
     +    abandoned. For full details, see the thread at
     +    https://lore.kernel.org/git/pull.1309.git.1659106382128.gitgitgadget@gmail.com/t
      
          Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
      

-- 
gitgitgadget