On Fri, Nov 18 2022, Taylor Blau wrote: > On Fri, Nov 18, 2022 at 03:15:08PM -0800, Junio C Hamano wrote: >> Taylor Blau <me@xxxxxxxxxxxx> 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/221114.86pmdplbs5.gmgdl@xxxxxxxxxxxxxxxxxxx/
