Subject: Re: [PATCH v3 2/2] tests(mingw): avoid very slow `mingw_test_cmp`

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Sat, 19 Nov 2022 13:03:03 +0100

On Fri, Nov 18 2022, Taylor Blau wrote:

> On Fri, Nov 18, 2022 at 03:15:08PM -0800, Junio C Hamano wrote:
>> Taylor Blau <[email-masked]> writes:
>>
>> > One thing that the commit message doesn't allude to (that is covered in
>> > the earlier discussion) is why it is important to pass
>> > `--ignore-cr-at-eol`. I think that is worth mentioning here.
>>
>> Isn't it because Git on the platform is expected to use CRLF in
>> certain places, unlike on other platforms where LF is used, but the
>> platform port hasn't adjusted tests to match that expectation?  And
>> vice versa, where Git is expected to produce LF terminated text
>> everywhere but the expected output is not "ported" to force LF
>> termination and instead produces CRLF terminated text on platforms
>> whose native line ending is CRLF?
>
> Yes, I think that's right. My suggestion to Johannes was to (a) make
> sure that your and my understanding is correct, and (b) to memorialize
> that understanding in the commit message itself.

It's not right, but mostly right. I.e. the "--ignore-cr-at-eol" is
surely needed to avoid a deluge of errors on Windows, but as I noted in
[1] you'll also find that the tests don't pass on a *nix box without it.

Which is apparently because "--ignore-cr-at-eol" is also conflating "is
binary?" with "should I replace \r\n", or something.

>> Use of "ignore-cr-at-eol" may allow such tests that are not ported
>> correctly to prepare expected output with a "wrong" line ending and
>> still pass, and I do think it may be an expedite way to make tests
>> appear to pass.
>>
>> But I worry that it may not be a good thing for the health of the
>> Windows port in the longer term.
>
> I share your concerns, too.

I don't really care about that, and would tend to defer to one or more
Johannes in this thread on that question.

But as noted in [1] the upthread patch seems to be replacing some
existing newline munging with not-quite-the-same "diff" behavior.

Which I think is something that needs addressing, either by the commit
explaining why that's desired & OK.

Which is also a good reason to pick one of our *nix CI jobs and run it
with "git diff --no-index", but without "--ignore-cr-at-eol", as I
suggested. It would tease out exactly that difference, both for current
and future tests.

1. https://lore.kernel.org/git/[email-masked]/