Am 06.09.22 um 15:10 schrieb Johannes Schindelin:
> Hi Junio,
>
> On Fri, 29 Jul 2022, Junio C Hamano wrote:
>
>> "Johannes Schindelin via GitGitGadget" <gitgitgadget@xxxxxxxxx>
>> writes:
>>
>>> + const char *argv[] = {
>>> + "diff", "--no-index", NULL, NULL, NULL
>>> + };
>>
>> Don't we want to have "--" before the two paths?
>
> Yes!
>
>>> + if (!(f0 = !strcmp(argv[1], "-") ? stdin : fopen(argv[1], "r")))
>>> + return error_errno("could not open '%s'", argv[1]);
>>> + if (!(f1 = !strcmp(argv[2], "-") ? stdin : fopen(argv[2], "r"))) {
>>> + fclose(f0);
>>> + return error_errno("could not open '%s'", argv[2]);
>>> + }
>>
>> It is tricky that you need to take "-" and treat it as the standard
>> input stream in either argv[1] or argv[2] (but not both). If would
>> be a different story in an end-user facing program, but because this
>> is a test helper, feeding wrong input is developer's fault, and I do
>> not mind lack of attention to detail of error checking to make sure
>> we avoid comparing alternating lines of the standard input.
"git diff --no-index - -" also doesn't complain, by the way.
> No, you're right, I've added a guard that prevents `test-tool cmp - -`
> from failing in obscure ways.
>
>>> + for (;;) {
>>> + int r0 = strbuf_getline(&b0, f0);
>>> + int r1 = strbuf_getline(&b1, f1);
>>> +
>>> + if (r0 == EOF) {
>>> + fclose(f0);
>>> + fclose(f1);
>>> + strbuf_release(&b0);
>>> + strbuf_release(&b1);
>>> + if (r1 == EOF)
>>> + return 0;
>>
>> If both hit the EOF at the same time, we know they are the same, OK.
>>
>>> +cmp_failed:
>>> + if (!run_diff(argv[1], argv[2]))
>>
>> If one of argv[] was "-", then this wouldn't work correctly, as the
>> other file is read from the beginning but the "-" side have consumed
>> the initial part of the input and we cannot unseek it. This bug
>> needs to be fixed only if we expect a useful and reliable output
>> from the helper.
>
> Right. I've added a clause that says that we cannot show the diff because
> `stdin` has been consumed already.
>
>> 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?
René
