Re: [PATCH 2/2] diff: fix --exit-code with external diff

From: René Scharfe <l.s.r@web.de>
Date: May 12, 2024, 09:38 UTC
Message-ID: <e97d91db-cbb5-43f5-84d0-289681620558@web.de>
In-Reply-To: <f3454199-2eed-4ca1-82db-7f1419b3084e@web.de>

Am 11.05.24 um 22:32 schrieb René Scharfe:

Show 32 quoted lines

> Am 08.05.24 um 17:25 schrieb phillip.wood123@gmail.com:
>> Hi René
>>
>> On 06/05/2024 19:23, René Scharfe wrote:
>>> Am 05.05.24 um 17:25 schrieb Phillip Wood:
>>>> Merge strategies are expected to exit 0 on success, 1 when there are
>>>> conflicts and another non-zero value for other errors - it would be
>>>> nice to do something similar here where 1 means "there were
>>>> differences" but it is probably too late to do that without a config
>>>> value to indicate that we should trust the exit code.
>>> Right, such a diff command protocol v2 would not need to pipe the
>>> output through an inspection loop.  Sounds like a good idea.  It's
>>> unfortunate that it would increase the configuration surface, which is
>>> not in an acute need to expand.  We could advertise the new option when
>>> dying due to the unsupported combination of --exit-code and external
>>> diff, but that's in equal parts helpful and obnoxious, I feel.
>>
>> Yes, diff dying would be annoying but the message would be useful.
>
> Having poked at it a bit more, I wonder if we need to add some further
> nuance/trick to avoid having to reject certain combinations of options.
>
> Currently external diffs can't signal content equality.  That doesn't
> matter for trivial equality (where content and mode of the two files
> match), as this case is always handled by the diff machinery already.
> Only lossy comparisons (e.g. ignoring whitespace) even have the need to
> signal equality.
>
> If we (continue to) assume that external diffs are lossless then we
> don't need to change the code, just document that assumption.  And add a
> way to specify lossy external diffs that can signal whether they found
> interesting differences, to address the originally reported shortcoming.

One more step in that direction: If we continue to use exit code 0 to mean "notable changes present" and redefine 1 to mean "non-trivial equality" instead of "fatal error" then we don't need to add any config options.

A downside is that the exit codes of diff(1) have the opposite meaning. Which is weird in itself: You say "give me a diff" and it answers "true, I got nothing for you" or "false, I actually had to print that dang thing". Inherited from cmp(1), I guess. Which I further guess got it from bcmp(3)?

But we can't directly use diff(1) anyway because we pass either one or six parameters instead of the two that it would expect. Our external diff calling protocol is already special enough that we can freely chose the meaning of exit codes.

Any other downsides?  Am I missing something?

René