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

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Sep 8, 2022, 15:26 UTC
Message-ID: <220908.861qsllw5x.gmgdl@evledraar.gmail.com>
In-Reply-To: <2d839209-21e2-b05c-125b-57a9c1dad1ea@web.de>

On Thu, Sep 08 2022, René Scharfe wrote:

Show 21 quoted lines

> Am 07.09.22 um 18:25 schrieb Junio C Hamano:
>> René Scharfe <l.s.r@web.de> writes:
>>
>>>>> 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.
>
> Using git diff in test_cmp and using test_cmp to check if git diff works
> would become a cyclical dependency.  Only doing that on one platform
> limits the potential blind spot to platform-specific bugs, though.
> Enough to go wrapper-less?  Not sure.

I don't see how being wrapper-less is less of a "cyclical dependency" than using "git diff" directly.

If we are to postulate some bug where "git diff" thwarts us for the use of "test_cmp" it's going to be *very* broken.

I don't see how such a "git diff" would pass the rest of the test suite (some of which involves comparing its exact output), but still be functional enough to work as a GIT_TEST_CMP.

Even one where it just returns 0 unconditionally wouldn't pass, as we rely on "! test_cmp" in some cases.

And any such breakage we imagine might just as well affect a wrapper for it, and I'd think that would be the more likely of two unlikely possibilities, as that wrapper would only be used for the test suite, whereas "git diff" is more widely tested.

In any case, as long as we preserve the ability to set a GIT_TEST_CMP=cmp we can sanity check any such wrapper or dogfodding of "git diff" with an external program.

Needing to deal just with "git diff" and "cmp" would be a step forward, as we'd be able to drop the current shellscript "mingw" wrapper, as well as the special support for a "diff" that doesn't understand "-u".