Bug in "git diff" exit code with submodule and `--submodule=log`

Bug in "git diff" exit code with submodule and `--submodule=log`

[David Hull]

What did you do before the bug happened? (Steps to reproduce your issue)

Create a git repository that has a submodule.  The git repository
itself should be clean but the submodule should have changes that have
been staged in the main repository.  Then run:
`git diff --exit-code --cached --submodule=log --; echo $?`

This shell script will reproduce the bug:

```shell
#! /bin/sh

set -x

(mkdir sm; cd sm; git init .; git commit --allow-empty -m 'initial')
mkdir test
cd test
git init .
git commit --allow-empty -m 'initial'
git submodule init
git -c protocol.file.allow=always submodule add ../sm
git add .gitmodules sm
git commit -m 'add submodule sm'
(cd sm; echo "hello" >greeting; git add greeting; git commit -m 'add greeting')
git add sm
# This exit code is correct:
git diff --quiet --cached --submodule=short --; echo $?
# This exit code is wrong:
git diff --quiet --cached --submodule=log --; echo $?
```

What did you expect to happen? (Expected behavior)

The output of this "git diff" command should be `1`:
```
git diff --quiet --cached --submodule=log --; echo $?
```

What happened instead? (Actual behavior)

The output is `0`.  The output is correct if the `--submodule=log`
option is omitted.

```
% git diff --quiet --cached --submodule=log --; echo $?
0
% git diff --quiet --cached --submodule=short --; echo $?
1
```

Anything else you want to add:

I believe this bug was introduced in git 2.46.0 or shortly before.

[System Info]
git version:
git version 2.46.0
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
feature: fsmonitor--daemon
libcurl: 8.4.0
zlib: 1.2.12
uname: Darwin 23.6.0 Darwin Kernel Version 23.6.0: Mon Jul 29 21:13:00
PDT 2024; root:xnu-10063.141.2~1/RELEASE_X86_64 x86_64
compiler info: clang: 15.0.0 (clang-1500.3.9.4)
libc info: no libc information available
$SHELL (typically, interactive shell): /bin/zsh


[Enabled Hooks]

Re: Bug in "git diff" exit code with submodule and `--submodule=log`

[René Scharfe]

Am 03.09.24 um 16:38 schrieb David Hull:
> What did you do before the bug happened? (Steps to reproduce your issue)
>
> Create a git repository that has a submodule.  The git repository
> itself should be clean but the submodule should have changes that have
> been staged in the main repository.  Then run:
> `git diff --exit-code --cached --submodule=log --; echo $?`
>
> This shell script will reproduce the bug:
>
> ```shell
> #! /bin/sh
>
> set -x
>
> (mkdir sm; cd sm; git init .; git commit --allow-empty -m 'initial')
> mkdir test
> cd test
> git init .
> git commit --allow-empty -m 'initial'
> git submodule init
> git -c protocol.file.allow=always submodule add ../sm
> git add .gitmodules sm
> git commit -m 'add submodule sm'
> (cd sm; echo "hello" >greeting; git add greeting; git commit -m 'add greeting')
> git add sm
> # This exit code is correct:
> git diff --quiet --cached --submodule=short --; echo $?
> # This exit code is wrong:
> git diff --quiet --cached --submodule=log --; echo $?
> ```

Very helpful, thank you!

The misbehavior occurs with --exit-code instead of --quiet.  And with
--submodule=diff.

> What did you expect to happen? (Expected behavior)
>
> The output of this "git diff" command should be `1`:
> ```
> git diff --quiet --cached --submodule=log --; echo $?
> ```
>
> What happened instead? (Actual behavior)
>
> The output is `0`.  The output is correct if the `--submodule=log`
> option is omitted.
>
> ```
> % git diff --quiet --cached --submodule=log --; echo $?
> 0
> % git diff --quiet --cached --submodule=short --; echo $?
> 1
> ```
>
> Anything else you want to add:
>
> I believe this bug was introduced in git 2.46.0 or shortly before.

Bisects to d7b97b7185 (diff: let external diffs report that changes are
uninteresting, 2024-06-09).

So enabling diff_from_contents breaks the exit code of submodule diffs?
Then the options -b/-w/... and --ignore-matching-lines breaks it as
well, as they set this option, too.

I guess we have to do something like this, plus add a ton of tests to
cover the option combinations:

---
 diff.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/diff.c b/diff.c
index 4035a9374d..bc2e559ce9 100644
--- a/diff.c
+++ b/diff.c
@@ -3565,6 +3565,7 @@ static void builtin_diff(const char *name_a,
 		show_submodule_diff_summary(o, one->path ? one->path : two->path,
 				&one->oid, &two->oid,
 				two->dirty_submodule);
+		o->found_changes = 1;
 		return;
 	} else if (o->submodule_format == DIFF_SUBMODULE_INLINE_DIFF &&
 		   (!one->mode || S_ISGITLINK(one->mode)) &&
@@ -3573,6 +3574,7 @@ static void builtin_diff(const char *name_a,
 		show_submodule_inline_diff(o, one->path ? one->path : two->path,
 				&one->oid, &two->oid,
 				two->dirty_submodule);
+		o->found_changes = 1;
 		return;
 	}

--
2.46.0