git diff --exit-code misbehaving in 2.46.x

git diff --exit-code misbehaving in 2.46.x

[Jan Wendland]

Hey team,

git diff --exit-code in 2.46.x is unexpectedly returning a zero exit
code for files marked as binary in .gitattributes where 2.45.x would
correctly produce a non-zero exit code.

To reproduce:
mkdir -p git-exit-code/dist
cd git-exit-code
git init
echo "hi" > dist/main.js
echo "dist/** binary" > .gitattributes
git add --all
git commit -m "Initial commit"
echo "some change" >> dist/main.js
git diff --exit-code
echo $? # actual is 0, expected to be 1

Is this intended?

Kind regards
Jan

Re: git diff --exit-code misbehaving in 2.46.x

[René Scharfe]

Am 25.09.24 um 16:27 schrieb Jan Wendland:
> Hey team,
>
> git diff --exit-code in 2.46.x is unexpectedly returning a zero exit
> code for files marked as binary in .gitattributes where 2.45.x would
> correctly produce a non-zero exit code.
>
> To reproduce:
> mkdir -p git-exit-code/dist
> cd git-exit-code
> git init
> echo "hi" > dist/main.js
> echo "dist/** binary" > .gitattributes
> git add --all
> git commit -m "Initial commit"
> echo "some change" >> dist/main.js
> git diff --exit-code
> echo $? # actual is 0, expected to be 1
>
> Is this intended?

Thanks for the report!  This is a known bug.  The "next" branch contains
a fix, 9a41735af6 (diff: report modified binary files as changes in
builtin_diff(), 2024-09-21).

René

Re: git diff --exit-code misbehaving in 2.46.x

[Junio C Hamano]

René Scharfe <l.s.r@web.de> writes:

> Am 25.09.24 um 16:27 schrieb Jan Wendland:
>>
>> git diff --exit-code in 2.46.x is unexpectedly returning a zero exit
>> code for files marked as binary in .gitattributes where 2.45.x would
>> correctly produce a non-zero exit code.
>> ...
> Thanks for the report!  This is a known bug.  The "next" branch contains
> a fix, 9a41735af6 (diff: report modified binary files as changes in
> builtin_diff(), 2024-09-21).

I just noticed something curious.

9a41735a (diff: report modified binary files as changes in
builtin_diff(), 2024-09-21) explains that since 1aaf69e6 (diff:
shortcut for diff'ing two binary SHA-1 objects, 2014-08-16) added
binary comparison, the code path always used a quick hash-only
comparison.  But the above report claims it is a behaviour change
between 2.45 and 2.46.

It does seem to say things are different with 2.45 when binary
changes were checked with --quiet/--exit-code from my manual
testing, though.

Thanks.

Re: git diff --exit-code misbehaving in 2.46.x

[René Scharfe]

Am 25.09.24 um 18:58 schrieb Junio C Hamano:
> René Scharfe <l.s.r@web.de> writes:
>
>> Am 25.09.24 um 16:27 schrieb Jan Wendland:
>>>
>>> git diff --exit-code in 2.46.x is unexpectedly returning a zero exit
>>> code for files marked as binary in .gitattributes where 2.45.x would
>>> correctly produce a non-zero exit code.
>>> ...
>> Thanks for the report!  This is a known bug.  The "next" branch contains
>> a fix, 9a41735af6 (diff: report modified binary files as changes in
>> builtin_diff(), 2024-09-21).
>
> I just noticed something curious.
>
> 9a41735a (diff: report modified binary files as changes in
> builtin_diff(), 2024-09-21) explains that since 1aaf69e6 (diff:
> shortcut for diff'ing two binary SHA-1 objects, 2014-08-16) added
> binary comparison, the code path always used a quick hash-only
> comparison.  But the above report claims it is a behaviour change
> between 2.45 and 2.46.
>
> It does seem to say things are different with 2.45 when binary
> changes were checked with --quiet/--exit-code from my manual
> testing, though.

There are levels: If the flag diff_from_contents is disabled, diff just
compares hashes (and other meta data).  If diff_from_contents is
enabled, it compares file contents as well.  But not for binary files,
since 1aaf69e6 (diff: shortcut for diff'ing two binary SHA-1 objects,
2014-08-16).

The shortcut exempts binary files from diff_from_contents.  But it
forgot to report changes for exit code calculation.  So git diff with
an option that turns on diff_from_contents (e.g. "git diff -b -q")
has not been reporting a modified binary file since then.

d7b97b7185 (diff: let external diffs report that changes are
uninteresting, 2024-06-09) enabled diff_from_contents by default,
exposing the bug much more widely in 2.46.0.

René