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

David Hull· Sep 3, 2024, 14:38 UTC · lore

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]

René Scharfe· Sep 3, 2024, 22:14 UTC · re: David Hull · lore

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

Am 03.09.24 um 16:38 schrieb David Hull:

Show 30 quoted lines

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

Show 22 quoted lines

> 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