{"thread":{"id":"58349","subject":"Bug in 'git log --remerge-diff' when used with '--find-object' and '--submodule=log|diff'","startedAt":"2022-08-22T23:58:52Z","lastAt":"2022-08-31T07:37:39Z","messageCount":5,"participants":["Philippe Blain","Elijah Newren"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"461820","messageId":"43cf2a1d-058a-fd79-befe-7d9bc62581ed@gmail.com","threadId":"58349","inReplyTo":null,"subject":"Bug in 'git log --remerge-diff' when used with '--find-object' and '--submodule=log|diff'","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2022-08-22T23:58:43Z","receivedAt":"2022-08-22T23:58:52Z","isPatch":false,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hi Elijah,\n\nI found two bugs in '--remerge-diff' when combined with both '--find-object' and\n'--submodule=log|diff'. I don't know if they have the same cause.\n\nWhen using these flags together, *all* 2-parents merge commits are shown (even in a repo with\nno submodule!)\n\nMoreover, for merges with conflicted paths, all conflicted paths are shown as \"(new submodule)\",\neven if they are not a submodule (in fact, even when no submodule is present\nin the repository!).\n\nThis artificial example reproduces the bug:\n\n```bash\n#!/bin/bash\n\nset -euEo pipefail\n\nrepo=repro\nrm -rf $repo\n\nTEST_AUTHOR_LOCALNAME=author\nTEST_AUTHOR_DOMAIN=example.com\nGIT_AUTHOR_EMAIL=${TEST_AUTHOR_LOCALNAME}@${TEST_AUTHOR_DOMAIN}\nGIT_AUTHOR_NAME='A U Thor'\nGIT_AUTHOR_DATE='1112354055 +0200'\nTEST_COMMITTER_LOCALNAME=committer\nTEST_COMMITTER_DOMAIN=example.com\nGIT_COMMITTER_EMAIL=${TEST_COMMITTER_LOCALNAME}@${TEST_COMMITTER_DOMAIN}\nGIT_COMMITTER_NAME='C O Mitter'\nGIT_COMMITTER_DATE='1112354055 +0200'\nexport GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME\nexport GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME\nexport GIT_COMMITTER_DATE GIT_AUTHOR_DATE\nexport HOME=\n\ngit -c init.defaultBranch=unimportant init $repo\ncd $repo\necho i>file\ngit add file\ngit commit -m file\ngit checkout -b side\necho s>>file2\ngit add file2\ngit commit -am side\ngit checkout -\necho m >>file\ngit commit -am main\ngit merge side -m clean\ngit checkout side\necho c>>file\ngit add file\ngit commit -am side2\ngit checkout -\necho cc>>file\ngit commit -am main2\ngit merge side || true\nprintf 'i\\nm' > file\ngit commit -am conflicted\n# look for a dummy object\ngit log --oneline --diff-merges=r --submodule=log --find-object=2c042ac4213768e55791098110d2ef2ef845881a\n# same output with --submodule=diff\n```\n\nThis is the output I get from the 'git log' call:\n\nb4f1be9 (HEAD -> unimportant) conflicted\nSubmodule file 0000000...0000000 (new submodule)\na4ef223 clean\n\nNotice both merges are shown despite none of them changing the number of occurences of \n2c042ac4213768e55791098110d2ef2ef845881a, which does not even exist in this repository,\nand also that the conflicted merge shows 'file' as \"new submodule\".\n\nCheers,\n\nPhilippe.\n"},{"id":"461890","messageId":"CABPp-BEkC8xEkNa+hyKFKhO=cbBZqNqGWehqxbRzE6-BVR27NQ@mail.gmail.com","threadId":"58349","inReplyTo":"43cf2a1d-058a-fd79-befe-7d9bc62581ed@gmail.com","subject":"Re: Bug in 'git log --remerge-diff' when used with '--find-object' and '--submodule=log|diff'","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-08-24T07:36:56Z","receivedAt":"2022-08-24T07:37:11Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi Philippe,\n\nOn Mon, Aug 22, 2022 at 4:58 PM Philippe Blain\n<levraiphilippeblain@gmail.com> wrote:\n>\n> Hi Elijah,\n>\n> I found two bugs in '--remerge-diff' when combined with both '--find-object' and\n> '--submodule=log|diff'. I don't know if they have the same cause.\n>\n> When using these flags together, *all* 2-parents merge commits are shown (even in a repo with\n> no submodule!)\n>\n> Moreover, for merges with conflicted paths, all conflicted paths are shown as \"(new submodule)\",\n> even if they are not a submodule (in fact, even when no submodule is present\n> in the repository!).\n>\n> This artificial example reproduces the bug:\n>\n> ```bash\n> #!/bin/bash\n>\n> set -euEo pipefail\n>\n> repo=repro\n> rm -rf $repo\n>\n> TEST_AUTHOR_LOCALNAME=author\n> TEST_AUTHOR_DOMAIN=example.com\n> GIT_AUTHOR_EMAIL=${TEST_AUTHOR_LOCALNAME}@${TEST_AUTHOR_DOMAIN}\n> GIT_AUTHOR_NAME='A U Thor'\n> GIT_AUTHOR_DATE='1112354055 +0200'\n> TEST_COMMITTER_LOCALNAME=committer\n> TEST_COMMITTER_DOMAIN=example.com\n> GIT_COMMITTER_EMAIL=${TEST_COMMITTER_LOCALNAME}@${TEST_COMMITTER_DOMAIN}\n> GIT_COMMITTER_NAME='C O Mitter'\n> GIT_COMMITTER_DATE='1112354055 +0200'\n> export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME\n> export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME\n> export GIT_COMMITTER_DATE GIT_AUTHOR_DATE\n> export HOME=\n>\n> git -c init.defaultBranch=unimportant init $repo\n> cd $repo\n> echo i>file\n> git add file\n> git commit -m file\n> git checkout -b side\n> echo s>>file2\n> git add file2\n> git commit -am side\n> git checkout -\n> echo m >>file\n> git commit -am main\n> git merge side -m clean\n> git checkout side\n> echo c>>file\n> git add file\n> git commit -am side2\n> git checkout -\n> echo cc>>file\n> git commit -am main2\n> git merge side || true\n> printf 'i\\nm' > file\n> git commit -am conflicted\n> # look for a dummy object\n> git log --oneline --diff-merges=r --submodule=log --find-object=2c042ac4213768e55791098110d2ef2ef845881a\n> # same output with --submodule=diff\n> ```\n>\n> This is the output I get from the 'git log' call:\n>\n> b4f1be9 (HEAD -> unimportant) conflicted\n> Submodule file 0000000...0000000 (new submodule)\n> a4ef223 clean\n>\n> Notice both merges are shown despite none of them changing the number of occurences of\n> 2c042ac4213768e55791098110d2ef2ef845881a, which does not even exist in this repository,\n> and also that the conflicted merge shows 'file' as \"new submodule\".\n\nThanks for the report, and the steps to reproduce.  Very helpful.\n\nAfter some digging, it appears the remerge-diff headers are\nmisinterpreted by the submodule code.  They transform what would have\nbeen the following output:\n\n    b4f1be9 (HEAD -> unimportant) conflicted\n    diff --git a/file b/file\n    remerge CONFLICT (content): Merge conflict in file\n    a4ef223 clean\n\ninto what you saw, namely\n\n    b4f1be9 (HEAD -> unimportant) conflicted\n    Submodule file 0000000...0000000 (new submodule)\n    a4ef223 clean\n\nWe can recover the intended remerge-diff header with the following simple patch:\n\ndiff --git a/diff.c b/diff.c\nindex 974626a621..be23f66057 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3429,14 +3429,16 @@ static void builtin_diff(const char *name_a,\n\n        if (o->submodule_format == DIFF_SUBMODULE_LOG &&\n            (!one->mode || S_ISGITLINK(one->mode)) &&\n-           (!two->mode || S_ISGITLINK(two->mode))) {\n+           (!two->mode || S_ISGITLINK(two->mode)) &&\n+           (one->mode || two->mode)) {\n                show_submodule_diff_summary(o, one->path ? one->path :\ntwo->path,\n                                &one->oid, &two->oid,\n                                two->dirty_submodule);\n                return;\n        } else if (o->submodule_format == DIFF_SUBMODULE_INLINE_DIFF &&\n                   (!one->mode || S_ISGITLINK(one->mode)) &&\n-                  (!two->mode || S_ISGITLINK(two->mode))) {\n+                  (!two->mode || S_ISGITLINK(two->mode)) &&\n+                  (one->mode || two->mode)) {\n                show_submodule_inline_diff(o, one->path ? one->path : two->path,\n                                &one->oid, &two->oid,\n                                two->dirty_submodule);\n\nIn other words, when we have information about something other than a\nsubmodule, don't attempt to treat it as information about a submodule.\n\nNow, whether the remerge-diff header should be shown is an interesting\none -- it's a carry on to the discussion in the thread at [1], and\nisn't simple to answer.\n[1] https://lore.kernel.org/git/CABPp-BHjU+wDXNnf-rsGy86GvOFWH6qVLPEfAA2JDfLFRU4WSA@mail.gmail.com/\n\nAs for the first bug, the showing of any 2-parent commits, yes that's\ntrue.  But it's not limited to remerge-diff; you can change\n--diff-merges=r to --diff-merges=c or --diff-merges=cc and see the\nsame thing.  I'm not sure if that means that my looking at the\ndo_diff_combined() logic when developing the do_remerge_diff()\nfunction meant I copied a bug, or if I independently came up with it\non my own.  But, for now, I need some sleep.  Just thought I'd give\nyou an update on what I have found so far, though.\n"},{"id":"462096","messageId":"b19c7090-109c-8988-56cf-4f8887de3845@gmail.com","threadId":"58349","inReplyTo":"CABPp-BEkC8xEkNa+hyKFKhO=cbBZqNqGWehqxbRzE6-BVR27NQ@mail.gmail.com","subject":"Re: Bug in 'git log --remerge-diff' when used with '--find-object' and '--submodule=log|diff'","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2022-08-29T16:36:20Z","receivedAt":"2022-08-29T16:36:20Z","isPatch":false,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hi Elijah,\n\nLe 2022-08-24 à 03:36, Elijah Newren a écrit :\n> Hi Philippe,\n> \n> On Mon, Aug 22, 2022 at 4:58 PM Philippe Blain\n> <levraiphilippeblain@gmail.com> wrote:\n>>\n>> Hi Elijah,\n>>\n>> I found two bugs in '--remerge-diff' when combined with both '--find-object' and\n>> '--submodule=log|diff'. I don't know if they have the same cause.\n>>\n>> When using these flags together, *all* 2-parents merge commits are shown (even in a repo with\n>> no submodule!)\n>>\n>> Moreover, for merges with conflicted paths, all conflicted paths are shown as \"(new submodule)\",\n>> even if they are not a submodule (in fact, even when no submodule is present\n>> in the repository!).\n>>\n>> This artificial example reproduces the bug:\n>>\n>> ```bash\n>> #!/bin/bash\n>>\n>> set -euEo pipefail\n>>\n>> repo=repro\n>> rm -rf $repo\n>>\n>> TEST_AUTHOR_LOCALNAME=author\n>> TEST_AUTHOR_DOMAIN=example.com\n>> GIT_AUTHOR_EMAIL=${TEST_AUTHOR_LOCALNAME}@${TEST_AUTHOR_DOMAIN}\n>> GIT_AUTHOR_NAME='A U Thor'\n>> GIT_AUTHOR_DATE='1112354055 +0200'\n>> TEST_COMMITTER_LOCALNAME=committer\n>> TEST_COMMITTER_DOMAIN=example.com\n>> GIT_COMMITTER_EMAIL=${TEST_COMMITTER_LOCALNAME}@${TEST_COMMITTER_DOMAIN}\n>> GIT_COMMITTER_NAME='C O Mitter'\n>> GIT_COMMITTER_DATE='1112354055 +0200'\n>> export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME\n>> export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME\n>> export GIT_COMMITTER_DATE GIT_AUTHOR_DATE\n>> export HOME=\n>>\n>> git -c init.defaultBranch=unimportant init $repo\n>> cd $repo\n>> echo i>file\n>> git add file\n>> git commit -m file\n>> git checkout -b side\n>> echo s>>file2\n>> git add file2\n>> git commit -am side\n>> git checkout -\n>> echo m >>file\n>> git commit -am main\n>> git merge side -m clean\n>> git checkout side\n>> echo c>>file\n>> git add file\n>> git commit -am side2\n>> git checkout -\n>> echo cc>>file\n>> git commit -am main2\n>> git merge side || true\n>> printf 'i\\nm' > file\n>> git commit -am conflicted\n>> # look for a dummy object\n>> git log --oneline --diff-merges=r --submodule=log --find-object=2c042ac4213768e55791098110d2ef2ef845881a\n>> # same output with --submodule=diff\n>> ```\n>>\n>> This is the output I get from the 'git log' call:\n>>\n>> b4f1be9 (HEAD -> unimportant) conflicted\n>> Submodule file 0000000...0000000 (new submodule)\n>> a4ef223 clean\n>>\n>> Notice both merges are shown despite none of them changing the number of occurences of\n>> 2c042ac4213768e55791098110d2ef2ef845881a, which does not even exist in this repository,\n>> and also that the conflicted merge shows 'file' as \"new submodule\".\n> \n> Thanks for the report, and the steps to reproduce.  Very helpful.\n> \n> After some digging, it appears the remerge-diff headers are\n> misinterpreted by the submodule code.  They transform what would have\n> been the following output:\n> \n>     b4f1be9 (HEAD -> unimportant) conflicted\n>     diff --git a/file b/file\n>     remerge CONFLICT (content): Merge conflict in file\n>     a4ef223 clean\n> \n> into what you saw, namely\n> \n>     b4f1be9 (HEAD -> unimportant) conflicted\n>     Submodule file 0000000...0000000 (new submodule)\n>     a4ef223 clean\n> \n> We can recover the intended remerge-diff header with the following simple patch:\n> \n> diff --git a/diff.c b/diff.c\n> index 974626a621..be23f66057 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -3429,14 +3429,16 @@ static void builtin_diff(const char *name_a,\n> \n>         if (o->submodule_format == DIFF_SUBMODULE_LOG &&\n>             (!one->mode || S_ISGITLINK(one->mode)) &&\n> -           (!two->mode || S_ISGITLINK(two->mode))) {\n> +           (!two->mode || S_ISGITLINK(two->mode)) &&\n> +           (one->mode || two->mode)) {\n>                 show_submodule_diff_summary(o, one->path ? one->path :\n> two->path,\n>                                 &one->oid, &two->oid,\n>                                 two->dirty_submodule);\n>                 return;\n>         } else if (o->submodule_format == DIFF_SUBMODULE_INLINE_DIFF &&\n>                    (!one->mode || S_ISGITLINK(one->mode)) &&\n> -                  (!two->mode || S_ISGITLINK(two->mode))) {\n> +                  (!two->mode || S_ISGITLINK(two->mode)) &&\n> +                  (one->mode || two->mode)) {\n>                 show_submodule_inline_diff(o, one->path ? one->path : two->path,\n>                                 &one->oid, &two->oid,\n>                                 two->dirty_submodule);\n> \n> In other words, when we have information about something other than a\n> submodule, don't attempt to treat it as information about a submodule.\n\nThanks for digging into this.\nFrom what I understand in the case of a remerge-diff, both modes are all-zero, and this is\nnot expected by the submodule diff code. Were you planning to submit a proper\npatch ? I could get to it eventually, but not before mid/end of September...\n\n> \n> Now, whether the remerge-diff header should be shown is an interesting\n> one -- it's a carry on to the discussion in the thread at [1], and\n> isn't simple to answer.\n> [1] https://lore.kernel.org/git/CABPp-BHjU+wDXNnf-rsGy86GvOFWH6qVLPEfAA2JDfLFRU4WSA@mail.gmail.com/\n\nThanks for a link to a very interesting discussion! However I'm not exactly sure \nwhat you meant here. Are you saying that:\n\n- we use '--remerge-diff', so we are interested in seeing \n  (re)merge diffs\n- we do not use '-p', so we are not interested in seeing diffs\n- to reconcile both of the above, the code shows only the remerge diff header ?\n\nBecause if I do add '-p', (in my reproducer) I still do not get any diff content. But if I remove\n'--find-object', then I do...\n\n> As for the first bug, the showing of any 2-parent commits, yes that's\n> true.  But it's not limited to remerge-diff; you can change\n> --diff-merges=r to --diff-merges=c or --diff-merges=cc and see the\n> same thing.  I'm not sure if that means that my looking at the\n> do_diff_combined() logic when developing the do_remerge_diff()\n> function meant I copied a bug, or if I independently came up with it\n> on my own.  But, for now, I need some sleep.  Just thought I'd give\n> you an update on what I have found so far, though.\n> \n\nYes, I had noticed that I had the same behaviour with --cc or -c, I forgot to mention it.\nWith 'separate' or 'first-parent', though, no commits are shown (as expected).\nTo be clear, this behaviour is independent of the use of '--submodule=short|log|diff'.\n\nThanks for your answer,\n\nPhilippe.\n"},{"id":"462122","messageId":"CABPp-BGxN=9f4ODeRsX5TMuKRujCuhmRxCm4br43HNqm3Vmanw@mail.gmail.com","threadId":"58349","inReplyTo":"b19c7090-109c-8988-56cf-4f8887de3845@gmail.com","subject":"Re: Bug in 'git log --remerge-diff' when used with '--find-object' and '--submodule=log|diff'","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-08-30T02:51:59Z","receivedAt":"2022-08-30T02:52:14Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Aug 29, 2022 at 9:36 AM Philippe Blain\n<levraiphilippeblain@gmail.com> wrote:\n>\n> Hi Elijah,\n>\n> Le 2022-08-24 à 03:36, Elijah Newren a écrit :\n> > Hi Philippe,\n> >\n> > On Mon, Aug 22, 2022 at 4:58 PM Philippe Blain\n> > <levraiphilippeblain@gmail.com> wrote:\n> >>\n> >> Hi Elijah,\n> >>\n> >> I found two bugs in '--remerge-diff' when combined with both '--find-object' and\n> >> '--submodule=log|diff'. I don't know if they have the same cause.\n> >>\n> >> When using these flags together, *all* 2-parents merge commits are shown (even in a repo with\n> >> no submodule!)\n> >>\n> >> Moreover, for merges with conflicted paths, all conflicted paths are shown as \"(new submodule)\",\n> >> even if they are not a submodule (in fact, even when no submodule is present\n> >> in the repository!).\n> >>\n> >> This artificial example reproduces the bug:\n> >>\n> >> ```bash\n> >> #!/bin/bash\n> >>\n> >> set -euEo pipefail\n> >>\n> >> repo=repro\n> >> rm -rf $repo\n> >>\n> >> TEST_AUTHOR_LOCALNAME=author\n> >> TEST_AUTHOR_DOMAIN=example.com\n> >> GIT_AUTHOR_EMAIL=${TEST_AUTHOR_LOCALNAME}@${TEST_AUTHOR_DOMAIN}\n> >> GIT_AUTHOR_NAME='A U Thor'\n> >> GIT_AUTHOR_DATE='1112354055 +0200'\n> >> TEST_COMMITTER_LOCALNAME=committer\n> >> TEST_COMMITTER_DOMAIN=example.com\n> >> GIT_COMMITTER_EMAIL=${TEST_COMMITTER_LOCALNAME}@${TEST_COMMITTER_DOMAIN}\n> >> GIT_COMMITTER_NAME='C O Mitter'\n> >> GIT_COMMITTER_DATE='1112354055 +0200'\n> >> export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME\n> >> export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME\n> >> export GIT_COMMITTER_DATE GIT_AUTHOR_DATE\n> >> export HOME=\n> >>\n> >> git -c init.defaultBranch=unimportant init $repo\n> >> cd $repo\n> >> echo i>file\n> >> git add file\n> >> git commit -m file\n> >> git checkout -b side\n> >> echo s>>file2\n> >> git add file2\n> >> git commit -am side\n> >> git checkout -\n> >> echo m >>file\n> >> git commit -am main\n> >> git merge side -m clean\n> >> git checkout side\n> >> echo c>>file\n> >> git add file\n> >> git commit -am side2\n> >> git checkout -\n> >> echo cc>>file\n> >> git commit -am main2\n> >> git merge side || true\n> >> printf 'i\\nm' > file\n> >> git commit -am conflicted\n> >> # look for a dummy object\n> >> git log --oneline --diff-merges=r --submodule=log --find-object=2c042ac4213768e55791098110d2ef2ef845881a\n> >> # same output with --submodule=diff\n> >> ```\n> >>\n> >> This is the output I get from the 'git log' call:\n> >>\n> >> b4f1be9 (HEAD -> unimportant) conflicted\n> >> Submodule file 0000000...0000000 (new submodule)\n> >> a4ef223 clean\n> >>\n> >> Notice both merges are shown despite none of them changing the number of occurences of\n> >> 2c042ac4213768e55791098110d2ef2ef845881a, which does not even exist in this repository,\n> >> and also that the conflicted merge shows 'file' as \"new submodule\".\n> >\n> > Thanks for the report, and the steps to reproduce.  Very helpful.\n> >\n> > After some digging, it appears the remerge-diff headers are\n> > misinterpreted by the submodule code.  They transform what would have\n> > been the following output:\n> >\n> >     b4f1be9 (HEAD -> unimportant) conflicted\n> >     diff --git a/file b/file\n> >     remerge CONFLICT (content): Merge conflict in file\n> >     a4ef223 clean\n> >\n> > into what you saw, namely\n> >\n> >     b4f1be9 (HEAD -> unimportant) conflicted\n> >     Submodule file 0000000...0000000 (new submodule)\n> >     a4ef223 clean\n> >\n> > We can recover the intended remerge-diff header with the following simple patch:\n> >\n> > diff --git a/diff.c b/diff.c\n> > index 974626a621..be23f66057 100644\n> > --- a/diff.c\n> > +++ b/diff.c\n> > @@ -3429,14 +3429,16 @@ static void builtin_diff(const char *name_a,\n> >\n> >         if (o->submodule_format == DIFF_SUBMODULE_LOG &&\n> >             (!one->mode || S_ISGITLINK(one->mode)) &&\n> > -           (!two->mode || S_ISGITLINK(two->mode))) {\n> > +           (!two->mode || S_ISGITLINK(two->mode)) &&\n> > +           (one->mode || two->mode)) {\n> >                 show_submodule_diff_summary(o, one->path ? one->path :\n> > two->path,\n> >                                 &one->oid, &two->oid,\n> >                                 two->dirty_submodule);\n> >                 return;\n> >         } else if (o->submodule_format == DIFF_SUBMODULE_INLINE_DIFF &&\n> >                    (!one->mode || S_ISGITLINK(one->mode)) &&\n> > -                  (!two->mode || S_ISGITLINK(two->mode))) {\n> > +                  (!two->mode || S_ISGITLINK(two->mode)) &&\n> > +                  (one->mode || two->mode)) {\n> >                 show_submodule_inline_diff(o, one->path ? one->path : two->path,\n> >                                 &one->oid, &two->oid,\n> >                                 two->dirty_submodule);\n> >\n> > In other words, when we have information about something other than a\n> > submodule, don't attempt to treat it as information about a submodule.\n>\n> Thanks for digging into this.\n> From what I understand in the case of a remerge-diff, both modes are all-zero, and this is\n> not expected by the submodule diff code. Were you planning to submit a proper\n> patch ? I could get to it eventually, but not before mid/end of September...\n\nI intend to submit a proper patch; I've just been busy.\n\n> >\n> > Now, whether the remerge-diff header should be shown is an interesting\n> > one -- it's a carry on to the discussion in the thread at [1], and\n> > isn't simple to answer.\n> > [1] https://lore.kernel.org/git/CABPp-BHjU+wDXNnf-rsGy86GvOFWH6qVLPEfAA2JDfLFRU4WSA@mail.gmail.com/\n>\n> Thanks for a link to a very interesting discussion! However I'm not exactly sure\n> what you meant here.\n\nI'm just saying there are special cases where it's difficult to\ndetermine whether to include the \"remerge CONFLICT\" diff headers.  For\nbasic cases, they are obviously wanted when running under\n\"--remerge-diff\" since that mode was designed to show that kind of\ninformation.  I think below you were trying to get at the fact that\nremerge headers will often be accompanied by one or more diff hunks\nwhich show content changes in the relevant file...and implying that if\nthere are no such diff hunks (because other flags caused those to be\nfiltered out), then the remerge header should also be omitted.\nUnfortunately such a strategy would only work if we could replace\n\"often\" with \"always\" in that last sentence.  We can't do so; there\nare cases where remerge headers are accompanied by zero diff hunks and\nthose headers need to be shown despite there being zero related diff\nhunks.  (A pretty common example: imagine a simple modify/delete\nconflict resolved in favor of keeping the modified file.  The\nresulting file recorded in the merge commit exactly matches the\nauto-merged one so there are no diff hunks to show, but a `git show\n--remerge-diff $MERGE_COMMIT` should definitely show the modify/delete\nconflict header since the merge was conflicted and the user did work\nto resolve it.)  In fact, my first implementation of \"git show\n--remerge-diff\" did only include the remerge headers when the diff for\nthe corresponding file was non-empty, but I almost immediately noted\nseveral examples in my initial testing where doing so was causing\nimportant information to be incorrectly filtered out.\n\n[...]\n> Because if I do add '-p', (in my reproducer) I still do not get any diff content. But if I remove\n> '--find-object', then I do...\n\n--remerge-diff turns on -p implicitly, so adding an explicit '-p' is\nnot a useful test here.\n\n--find-object does some filtering, but how is the portion of the code\nresponsible for adding the remerge headers supposed to know whether\nsuch filtering has happened and if it's relevant to any conflict\nheaders that it needs to add?  I'm not sure how much information about\nthe command line flags or what filtering happened is available at that\npoint in the code.  And, as noted above, the lack of diff hunks is not\na usable criteria for judging whether to include the header.\n\n> > As for the first bug, the showing of any 2-parent commits, yes that's\n> > true.  But it's not limited to remerge-diff; you can change\n> > --diff-merges=r to --diff-merges=c or --diff-merges=cc and see the\n> > same thing.  I'm not sure if that means that my looking at the\n> > do_diff_combined() logic when developing the do_remerge_diff()\n> > function meant I copied a bug, or if I independently came up with it\n> > on my own.  But, for now, I need some sleep.  Just thought I'd give\n> > you an update on what I have found so far, though.\n> >\n>\n> Yes, I had noticed that I had the same behaviour with --cc or -c, I forgot to mention it.\n> With 'separate' or 'first-parent', though, no commits are shown (as expected).\n> To be clear, this behaviour is independent of the use of '--submodule=short|log|diff'.\n\nYep, the modes that treat merges as actual merges instead of\npretending they are regular commits are precisely the modes which\nexhibit this behavior.  I'll have to dig into why.\n"},{"id":"462255","messageId":"CABPp-BGxPTz3tZtjGNH1MAVCKtHpjYzNVwtmux5WzuMXWphV5A@mail.gmail.com","threadId":"58349","inReplyTo":"CABPp-BGxN=9f4ODeRsX5TMuKRujCuhmRxCm4br43HNqm3Vmanw@mail.gmail.com","subject":"Re: Bug in 'git log --remerge-diff' when used with '--find-object' and '--submodule=log|diff'","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-08-31T07:37:23Z","receivedAt":"2022-08-31T07:37:39Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Aug 29, 2022 at 7:51 PM Elijah Newren <newren@gmail.com> wrote:\n>\n> On Mon, Aug 29, 2022 at 9:36 AM Philippe Blain\n> <levraiphilippeblain@gmail.com> wrote:\n> >\n> > Hi Elijah,\n> >\n> > Le 2022-08-24 à 03:36, Elijah Newren a écrit :\n> > > Hi Philippe,\n> > >\n> > > On Mon, Aug 22, 2022 at 4:58 PM Philippe Blain\n> > > <levraiphilippeblain@gmail.com> wrote:\n> > >>\n> > >> Hi Elijah,\n> > >>\n> > >> I found two bugs in '--remerge-diff' when combined with both '--find-object' and\n> > >> '--submodule=log|diff'. I don't know if they have the same cause.\n[...]\n> > >> This artificial example reproduces the bug:\n[...]\n> > > Thanks for the report, and the steps to reproduce.  Very helpful.\n> > >\n> > > After some digging, it appears the remerge-diff headers are\n> > > misinterpreted by the submodule code.\n[...]\n> >\n> > Thanks for digging into this.\n> > From what I understand in the case of a remerge-diff, both modes are all-zero, and this is\n> > not expected by the submodule diff code. Were you planning to submit a proper\n> > patch ? I could get to it eventually, but not before mid/end of September...\n>\n> I intend to submit a proper patch; I've just been busy.\n\nI have some patches that I think fix all the --remerge-diff issues;\nsee https://lore.kernel.org/git/pull.1342.git.1661926908.gitgitgadget@gmail.com/\n\nI looked briefly at the same issue affecting --cc and -c, but only\nenough to find it appears to have a different codepath and I don't see\nany obvious similar fixes for it.\n"}]}