{"thread":{"id":"37537","subject":"Diffs for submodule conflicts during rebase usually empty","startedAt":"2014-09-11T17:50:57Z","lastAt":"2014-09-13T11:07:35Z","messageCount":4,"participants":["ezyang","Jens Lehmann","Edward Z. Yang"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"249242","messageId":"20140911135057.o7j9bwlnz4okgwsw@webmail.mit.edu","threadId":"37537","inReplyTo":null,"subject":"Diffs for submodule conflicts during rebase usually empty","fromName":"ezyang","fromEmail":"ezyang@mit.edu","sentAt":"2014-09-11T17:50:57Z","receivedAt":"2014-09-11T17:50:57Z","isPatch":false,"sender":{"key":"ezyang@mit.edu","avatar":"https://gravatar.com/avatar/6aaa9d10a82c2cf3d676f1f9397c2ae05ee2534182eda446128c0fe7c04494ba?d=mp&s=160"},"body":"Hello all,\n\nIn many situations, if you have a submodule conflict during a rebase,\nand you type 'git diff' to get a summary of the situation, you will get\nan empty diff.  Here's a simple transcript for one such case (I'm sorry\nI can't make it much shorter), tested on git version 2.0.3.693.g996b0fd:\n\n    git init\n    mkdir b\n    cd b\n    git init\n    git commit --allow-empty -m \"submodule initial\"\n    cd ..\n    git submodule add ./b\n    git commit -am \"parent initial\"\n    git branch dev\n    cd b\n    touch a\n    git add a\n    git commit -m \"submodule master\"\n    cd ..\n    git commit -am \"parent master\"\n    git checkout dev\n    git submodule update\n    cd b\n    touch b\n    git add b\n    git commit -m \"submodule dev\"\n    cd ..\n    git commit -am \"parent dev\"\n    git rebase master\n    git diff b\n\nThe last output is:\n\n    diff --cc b\n    index 4b1b6c6,c423df2..0000000\n    --- a/b\n    +++ b/b\n\nAs it turns out, this behavior is logical in a perverse sort of way.\n\n    - The rebase operation doesn't go about updating your submodule\n      checkouts, so whatever is in the file is what the submodule\n      was pointing to before your initiated the rebase.\n\n    - By default, 'git diff' on a merge conflict (implicitly\n      'git diff --cc') only will report if the submodule's HEAD\n      differs from all of the merge heads.  So if you only had\n      one commit which changed the submodule, you're probably\n      on that commit, and so the \"current state\" of the submodule\n\nHowever, just because behavior is logical, doesn't mean it is user\nfriendly.  There are a few problems here:\n\n    1. Git is treating the lagging submodule HEAD as if it were\n    actually a resolution that you might want for the conflict.\n    Actually, it's basically almost always wrong (in the example\n    above, if you commit it you'll be discarding commits made on\n    master.)  There is a sorter of wider UI issue here where Git\n    can't tell if you've legitimately changed the HEAD pointer\n    of a submodule, or if you checked out a new revision with different\n    submodule pointers and forgot to run 'git submodule update'.\n    (But by the way, you can't even do that here, because this is\n    a merge!)\n\n    2. The behavior of not reporting the diff when the diff for one\n    branch is non-empty is illogical: for submodules (whose \"file\n    contents\" are so short), you basically always want some hashes,\n    and not an empty diff.  Doubly so when the \"resolution\" is\n    bogus (c.f. (1)).\n\nOf course, changing behavior in a backwards-incompatible way is never a\ngood way, so it's not exactly obvious what should be done here. I would\nrecommend tweaking the default combined diff behavior for submodules and\nadding an admonition to the user that the submodules have not been\nupdated in the rebase message (I can submit a patch for this if people\nagree if it's a good idea), but maybe that's too much of a behavior\nchange.\n\nBy the way, the difference between 'git diff -c' and 'git diff --cc'\ndoes not seem to be documented anywhere, except for an oblique comment\nin diff-format.txt \"Note that 'combined diff' lists only files which\nwere modified from all parents.\" -- the user expected, of course, to\nfigure out that 'combined diff' here refers to --cc, but not -c.\n\nCheers,\nEdward\n"},{"id":"249250","messageId":"5411F818.6030701@web.de","threadId":"37537","inReplyTo":"20140911135057.o7j9bwlnz4okgwsw@webmail.mit.edu","subject":"Re: Diffs for submodule conflicts during rebase usually empty","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2014-09-11T19:29:28Z","receivedAt":"2014-09-11T19:29:28Z","isPatch":false,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 11.09.2014 um 19:50 schrieb ezyang:\n> Hello all,\n>\n> In many situations, if you have a submodule conflict during a rebase,\n> and you type 'git diff' to get a summary of the situation, you will get\n> an empty diff.  Here's a simple transcript for one such case (I'm sorry\n> I can't make it much shorter), tested on git version 2.0.3.693.g996b0fd:\n>\n>      git init\n>      mkdir b\n>      cd b\n>      git init\n>      git commit --allow-empty -m \"submodule initial\"\n>      cd ..\n>      git submodule add ./b\n>      git commit -am \"parent initial\"\n>      git branch dev\n>      cd b\n>      touch a\n>      git add a\n>      git commit -m \"submodule master\"\n>      cd ..\n>      git commit -am \"parent master\"\n>      git checkout dev\n>      git submodule update\n>      cd b\n>      touch b\n>      git add b\n>      git commit -m \"submodule dev\"\n>      cd ..\n>      git commit -am \"parent dev\"\n>      git rebase master\n>      git diff b\n>\n> The last output is:\n>\n>      diff --cc b\n>      index 4b1b6c6,c423df2..0000000\n>      --- a/b\n>      +++ b/b\n\nThanks for providing a simple way to reproduce what you are seeing.\n\n> As it turns out, this behavior is logical in a perverse sort of way.\n>\n>      - The rebase operation doesn't go about updating your submodule\n>        checkouts, so whatever is in the file is what the submodule\n>        was pointing to before your initiated the rebase.\n>\n>      - By default, 'git diff' on a merge conflict (implicitly\n>        'git diff --cc') only will report if the submodule's HEAD\n>        differs from all of the merge heads.  So if you only had\n>        one commit which changed the submodule, you're probably\n>        on that commit, and so the \"current state\" of the submodule\n>\n> However, just because behavior is logical, doesn't mean it is user\n> friendly.  There are a few problems here:\n>\n>      1. Git is treating the lagging submodule HEAD as if it were\n>      actually a resolution that you might want for the conflict.\n>      Actually, it's basically almost always wrong (in the example\n>      above, if you commit it you'll be discarding commits made on\n>      master.)  There is a sorter of wider UI issue here where Git\n>      can't tell if you've legitimately changed the HEAD pointer\n>      of a submodule, or if you checked out a new revision with different\n>      submodule pointers and forgot to run 'git submodule update'.\n>      (But by the way, you can't even do that here, because this is\n>      a merge!)\n>\n>      2. The behavior of not reporting the diff when the diff for one\n>      branch is non-empty is illogical: for submodules (whose \"file\n>      contents\" are so short), you basically always want some hashes,\n>      and not an empty diff.  Doubly so when the \"resolution\" is\n>      bogus (c.f. (1)).\n>\n> Of course, changing behavior in a backwards-incompatible way is never a\n> good way, so it's not exactly obvious what should be done here. I would\n> recommend tweaking the default combined diff behavior for submodules and\n> adding an admonition to the user that the submodules have not been\n> updated in the rebase message (I can submit a patch for this if people\n> agree if it's a good idea), but maybe that's too much of a behavior\n> change.\n>\n> By the way, the difference between 'git diff -c' and 'git diff --cc'\n> does not seem to be documented anywhere, except for an oblique comment\n> in diff-format.txt \"Note that 'combined diff' lists only files which\n> were modified from all parents.\" -- the user expected, of course, to\n> figure out that 'combined diff' here refers to --cc, but not -c.\n\nIt looks to me like your confusion is because current Git isn't\nterribly good at displaying merge conflicts in submodules. While\ndiff produces rather confusing output:\n\n\t$ git diff\n\tdiff --cc b\n\tindex fc12d34,33d9fa9..0000000\n\t--- a/b\n\t+++ b/b\n\nGit does know what's going on, just fails to display it properly\nin the diff, as the output of ls-files shows:\n\n\t$git ls-files -u\n\t160000 6a6e215138b7f343fba67ba1b6ffc152019c6085 1\tb\n\t160000 fc12d3455b120916ec508c3ccd04f23957c08ea5 2\tb\n\t160000 33d9fa9f9e25de2a85f84993d8f6c752f84c769a 3\tb\n\nI agree that this needs to be improved, but am currently lacking\nthe time to do it myself. But I believe this will get important\nrather soonish when we recursively update submodules too ...\n"},{"id":"249301","messageId":"1410526589-sup-2306@sabre","threadId":"37537","inReplyTo":"5411F818.6030701@web.de","subject":"Re: Diffs for submodule conflicts during rebase usually empty","fromName":"Edward Z. Yang","fromEmail":"ezyang@mit.edu","sentAt":"2014-09-12T13:03:34Z","receivedAt":"2014-09-12T13:03:34Z","isPatch":false,"sender":{"key":"ezyang@mit.edu","avatar":"https://gravatar.com/avatar/6aaa9d10a82c2cf3d676f1f9397c2ae05ee2534182eda446128c0fe7c04494ba?d=mp&s=160"},"body":"Hello Jens,\n\nExcerpts from Jens Lehmann's message of 2014-09-11 15:29:28 -0400:\n> Git does know what's going on, just fails to display it properly\n> in the diff, as the output of ls-files shows:\n> \n>     $git ls-files -u\n>     160000 6a6e215138b7f343fba67ba1b6ffc152019c6085 1    b\n>     160000 fc12d3455b120916ec508c3ccd04f23957c08ea5 2    b\n>     160000 33d9fa9f9e25de2a85f84993d8f6c752f84c769a 3    b\n\nRight. But I'd also add that even though Git knows what's going\non, even if we reported /that/ it wouldn't be user friendly:\nnamely, because submodules are not updated automatically so the\nfirst line would always be what the submodule was pointed to\nbefore we started rebasing.  That's not so useful either...\n\n> I agree that this needs to be improved, but am currently lacking\n> the time to do it myself. But I believe this will get important\n> rather soonish when we recursively update submodules too ...\n\nAs I've said, I'm happy to contribute a patch, if we can agree\nwhat the right resolution is...\n\nCheers,\nEdward\n"},{"id":"249344","messageId":"54142577.3080104@web.de","threadId":"37537","inReplyTo":"1410526589-sup-2306@sabre","subject":"Re: Diffs for submodule conflicts during rebase usually empty","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2014-09-13T11:07:35Z","receivedAt":"2014-09-13T11:07:35Z","isPatch":false,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 12.09.2014 um 15:03 schrieb Edward Z. Yang:\n> Hello Jens,\n>\n> Excerpts from Jens Lehmann's message of 2014-09-11 15:29:28 -0400:\n>> Git does know what's going on, just fails to display it properly\n>> in the diff, as the output of ls-files shows:\n>>\n>>      $git ls-files -u\n>>      160000 6a6e215138b7f343fba67ba1b6ffc152019c6085 1    b\n>>      160000 fc12d3455b120916ec508c3ccd04f23957c08ea5 2    b\n>>      160000 33d9fa9f9e25de2a85f84993d8f6c752f84c769a 3    b\n>\n> Right. But I'd also add that even though Git knows what's going\n> on, even if we reported /that/ it wouldn't be user friendly:\n> namely, because submodules are not updated automatically so the\n> first line would always be what the submodule was pointed to\n> before we started rebasing.  That's not so useful either...\n>\n>> I agree that this needs to be improved, but am currently lacking\n>> the time to do it myself. But I believe this will get important\n>> rather soonish when we recursively update submodules too ...\n>\n> As I've said, I'm happy to contribute a patch, if we can agree\n> what the right resolution is...\n\nMe thinks the next step would be that \"git diff --submodule\"\nshould learn to not only show 2-way diffs but also 3-way diffs.\nThen we'll be able to display submodule merge results in a human\nreadable way. After that we would have to find a way to display\nsubmodule merge conflicts in a human readable way, similar to\nwhat we do with conflict markers for regular files.\n"}]}