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

["René Scharfe" <l.s.r@web.de>]

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