Am 06.09.22 um 15:10 schrieb Johannes Schindelin:

Hi Junio,

On Fri, 29 Jul 2022, Junio C Hamano wrote:

quoted

"Johannes Schindelin via GitGitGadget" [off-list ref]
writes:

quoted

+	const char *argv[] = {
+		"diff", "--no-index", NULL, NULL, NULL
+	};

Don't we want to have "--" before the two paths?

Yes!

quoted

quoted

+	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.

quoted

quoted

+	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.

quoted

+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.

quoted

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é