{"thread":{"id":"62046","subject":"Bug in \"git diff\" exit code with submodule and `--submodule=log`","startedAt":"2024-09-03T14:39:02Z","lastAt":"2024-09-03T22:14:11Z","messageCount":2,"participants":["David Hull","René Scharfe"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"502038","messageId":"CALpyDwE12kHV--NkbcSgxLacsYw_w1wdBhzM21-JO1yYKW2PwA@mail.gmail.com","threadId":"62046","inReplyTo":null,"subject":"Bug in \"git diff\" exit code with submodule and `--submodule=log`","fromName":"David Hull","fromEmail":"david.hull@friendbuy.com","sentAt":"2024-09-03T14:38:51Z","receivedAt":"2024-09-03T14:39:02Z","isPatch":false,"sender":{"key":"david.hull@friendbuy.com","avatar":null},"body":"What did you do before the bug happened? (Steps to reproduce your issue)\n\nCreate a git repository that has a submodule.  The git repository\nitself should be clean but the submodule should have changes that have\nbeen staged in the main repository.  Then run:\n`git diff --exit-code --cached --submodule=log --; echo $?`\n\nThis shell script will reproduce the bug:\n\n```shell\n#! /bin/sh\n\nset -x\n\n(mkdir sm; cd sm; git init .; git commit --allow-empty -m 'initial')\nmkdir test\ncd test\ngit init .\ngit commit --allow-empty -m 'initial'\ngit submodule init\ngit -c protocol.file.allow=always submodule add ../sm\ngit add .gitmodules sm\ngit commit -m 'add submodule sm'\n(cd sm; echo \"hello\" >greeting; git add greeting; git commit -m 'add greeting')\ngit add sm\n# This exit code is correct:\ngit diff --quiet --cached --submodule=short --; echo $?\n# This exit code is wrong:\ngit diff --quiet --cached --submodule=log --; echo $?\n```\n\nWhat did you expect to happen? (Expected behavior)\n\nThe output of this \"git diff\" command should be `1`:\n```\ngit diff --quiet --cached --submodule=log --; echo $?\n```\n\nWhat happened instead? (Actual behavior)\n\nThe output is `0`.  The output is correct if the `--submodule=log`\noption is omitted.\n\n```\n% git diff --quiet --cached --submodule=log --; echo $?\n0\n% git diff --quiet --cached --submodule=short --; echo $?\n1\n```\n\nAnything else you want to add:\n\nI believe this bug was introduced in git 2.46.0 or shortly before.\n\n[System Info]\ngit version:\ngit version 2.46.0\ncpu: x86_64\nno commit associated with this build\nsizeof-long: 8\nsizeof-size_t: 8\nshell-path: /bin/sh\nfeature: fsmonitor--daemon\nlibcurl: 8.4.0\nzlib: 1.2.12\nuname: Darwin 23.6.0 Darwin Kernel Version 23.6.0: Mon Jul 29 21:13:00\nPDT 2024; root:xnu-10063.141.2~1/RELEASE_X86_64 x86_64\ncompiler info: clang: 15.0.0 (clang-1500.3.9.4)\nlibc info: no libc information available\n$SHELL (typically, interactive shell): /bin/zsh\n\n\n[Enabled Hooks]\n"},{"id":"502076","messageId":"73fc18f3-9d2f-43fe-b0a1-2f3680f09ce6@web.de","threadId":"62046","inReplyTo":"CALpyDwE12kHV--NkbcSgxLacsYw_w1wdBhzM21-JO1yYKW2PwA@mail.gmail.com","subject":"Re: Bug in \"git diff\" exit code with submodule and `--submodule=log`","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2024-09-03T22:14:09Z","receivedAt":"2024-09-03T22:14:11Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 03.09.24 um 16:38 schrieb David Hull:\n> What did you do before the bug happened? (Steps to reproduce your issue)\n>\n> Create a git repository that has a submodule.  The git repository\n> itself should be clean but the submodule should have changes that have\n> been staged in the main repository.  Then run:\n> `git diff --exit-code --cached --submodule=log --; echo $?`\n>\n> This shell script will reproduce the bug:\n>\n> ```shell\n> #! /bin/sh\n>\n> set -x\n>\n> (mkdir sm; cd sm; git init .; git commit --allow-empty -m 'initial')\n> mkdir test\n> cd test\n> git init .\n> git commit --allow-empty -m 'initial'\n> git submodule init\n> git -c protocol.file.allow=always submodule add ../sm\n> git add .gitmodules sm\n> git commit -m 'add submodule sm'\n> (cd sm; echo \"hello\" >greeting; git add greeting; git commit -m 'add greeting')\n> git add sm\n> # This exit code is correct:\n> git diff --quiet --cached --submodule=short --; echo $?\n> # This exit code is wrong:\n> git diff --quiet --cached --submodule=log --; echo $?\n> ```\n\nVery helpful, thank you!\n\nThe misbehavior occurs with --exit-code instead of --quiet.  And with\n--submodule=diff.\n\n> What did you expect to happen? (Expected behavior)\n>\n> The output of this \"git diff\" command should be `1`:\n> ```\n> git diff --quiet --cached --submodule=log --; echo $?\n> ```\n>\n> What happened instead? (Actual behavior)\n>\n> The output is `0`.  The output is correct if the `--submodule=log`\n> option is omitted.\n>\n> ```\n> % git diff --quiet --cached --submodule=log --; echo $?\n> 0\n> % git diff --quiet --cached --submodule=short --; echo $?\n> 1\n> ```\n>\n> Anything else you want to add:\n>\n> I believe this bug was introduced in git 2.46.0 or shortly before.\n\nBisects to d7b97b7185 (diff: let external diffs report that changes are\nuninteresting, 2024-06-09).\n\nSo enabling diff_from_contents breaks the exit code of submodule diffs?\nThen the options -b/-w/... and --ignore-matching-lines breaks it as\nwell, as they set this option, too.\n\nI guess we have to do something like this, plus add a ton of tests to\ncover the option combinations:\n\n---\n diff.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/diff.c b/diff.c\nindex 4035a9374d..bc2e559ce9 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3565,6 +3565,7 @@ static void builtin_diff(const char *name_a,\n \t\tshow_submodule_diff_summary(o, one->path ? one->path : two->path,\n \t\t\t\t&one->oid, &two->oid,\n \t\t\t\ttwo->dirty_submodule);\n+\t\to->found_changes = 1;\n \t\treturn;\n \t} else if (o->submodule_format == DIFF_SUBMODULE_INLINE_DIFF &&\n \t\t   (!one->mode || S_ISGITLINK(one->mode)) &&\n@@ -3573,6 +3574,7 @@ static void builtin_diff(const char *name_a,\n \t\tshow_submodule_inline_diff(o, one->path ? one->path : two->path,\n \t\t\t\t&one->oid, &two->oid,\n \t\t\t\ttwo->dirty_submodule);\n+\t\to->found_changes = 1;\n \t\treturn;\n \t}\n\n--\n2.46.0\n\n"}]}