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

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

[-- Attachment #1: Type: text/plain, Size: 2589 bytes --]

Hi Junio,

On Wed, 7 Sep 2022, Junio C Hamano wrote:

> René Scharfe <l.s.r@web.de> writes:
>
> > "git diff --no-index - -" also doesn't complain, by the way.
>
> True, but in this case hopefully it is worth to call it out, as both
> this code that uses "diff --no-index" and "diff --no-index" itself
> came from the same author ;-)
>
> I think "git diff --no-index - -" should just exit 0 after slurping
> all its input (i.e. allow it to be placed downstream of a pipe
> without blocking the upstream), but it is also fine to exit with 0
> without reading a single byte from the standard input.  Of course
> the latter is easier to implement ;-)
>
> >>> But otherwise the idea is sound.  We compare them line by line,
> >>> using strbuf_getline() to ignore differences in CRLF and LF that
> >>> originates at 4d715ac0 (Windows: a test_cmp that is agnostic to
> >>> random LF <> CRLF conversions, 2013-10-26).  Only when we find the
> >>> input different, we use "git diff --no-index" to make the difference
> >>> (and unfortunately more, as it does not ignore CRLF <> LF
> >>> differences) visible.
> >
> > Why not use "git diff --no-index --ignore-cr-at-eol"?  Do you even need
> > to wrap it?
>
> Hmph.  That surely sounds sensible if it works, and I offhand do not
> see why it shouldn't work.

Is this a reversal of the stance you took in your reply in
https://lore.kernel.org/git/7vps7xrfxa.fsf@assigned-by-dhcp.cox.net/ to my
suggestion to replace `cmp` by `diff --no-index` (in that mail referred to
as patch [7/8])?

If I recall correctly, you clarified outside of that thread that "I do not
think it is a good enough reason to make the tests slower" was you being
concerned about employing the entire diff machinery instead of doing a
simple byte-for-byte comparison.

And while it is no longer _that_ simple a comparison (it now
special-handles Carriage Returns), the speed and simplicity concern is
still valid: `test-tool text-cmp` is vastly simpler (and provably faster)
than `diff --no-index`.

Just because it is easier to review a one-liner to switch from essentially
`cmp` to `git diff --no-index --ignore-cr-at-eol` does not mean that it is
reasonable: it would cause us to blast out that much more CO2, just for
our one-time convenience.

Or for that matter, it would willfully slow down the `windows-test` jobs
that already is mired by sloooow performance (mostly due to running a
shell script-based test suite).

Can you please let me make the Windows situation better rather than worse?

Thank you,
Dscho