Re: Diffs for submodule conflicts during rebase usually empty

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 11.09.2014 um 19:50 schrieb ezyang:
Hello all,

In many situations, if you have a submodule conflict during a rebase,
and you type 'git diff' to get a summary of the situation, you will get
an empty diff.  Here's a simple transcript for one such case (I'm sorry
I can't make it much shorter), tested on git version 2.0.3.693.g996b0fd:

     git init
     mkdir b
     cd b
     git init
     git commit --allow-empty -m "submodule initial"
     cd ..
     git submodule add ./b
     git commit -am "parent initial"
     git branch dev
     cd b
     touch a
     git add a
     git commit -m "submodule master"
     cd ..
     git commit -am "parent master"
     git checkout dev
     git submodule update
     cd b
     touch b
     git add b
     git commit -m "submodule dev"
     cd ..
     git commit -am "parent dev"
     git rebase master
     git diff b

The last output is:

     diff --cc b
     index 4b1b6c6,c423df2..0000000
     --- a/b
     +++ b/b

Thanks for providing a simple way to reproduce what you are seeing.

As it turns out, this behavior is logical in a perverse sort of way.

     - The rebase operation doesn't go about updating your submodule
       checkouts, so whatever is in the file is what the submodule
       was pointing to before your initiated the rebase.

     - By default, 'git diff' on a merge conflict (implicitly
       'git diff --cc') only will report if the submodule's HEAD
       differs from all of the merge heads.  So if you only had
       one commit which changed the submodule, you're probably
       on that commit, and so the "current state" of the submodule

However, just because behavior is logical, doesn't mean it is user
friendly.  There are a few problems here:

     1. Git is treating the lagging submodule HEAD as if it were
     actually a resolution that you might want for the conflict.
     Actually, it's basically almost always wrong (in the example
     above, if you commit it you'll be discarding commits made on
     master.)  There is a sorter of wider UI issue here where Git
     can't tell if you've legitimately changed the HEAD pointer
     of a submodule, or if you checked out a new revision with different
     submodule pointers and forgot to run 'git submodule update'.
     (But by the way, you can't even do that here, because this is
     a merge!)

     2. The behavior of not reporting the diff when the diff for one
     branch is non-empty is illogical: for submodules (whose "file
     contents" are so short), you basically always want some hashes,
     and not an empty diff.  Doubly so when the "resolution" is
     bogus (c.f. (1)).

Of course, changing behavior in a backwards-incompatible way is never a
good way, so it's not exactly obvious what should be done here. I would
recommend tweaking the default combined diff behavior for submodules and
adding an admonition to the user that the submodules have not been
updated in the rebase message (I can submit a patch for this if people
agree if it's a good idea), but maybe that's too much of a behavior
change.

By the way, the difference between 'git diff -c' and 'git diff --cc'
does not seem to be documented anywhere, except for an oblique comment
in diff-format.txt "Note that 'combined diff' lists only files which
were modified from all parents." -- the user expected, of course, to
figure out that 'combined diff' here refers to --cc, but not -c.

It looks to me like your confusion is because current Git isn't
terribly good at displaying merge conflicts in submodules. While
diff produces rather confusing output:

	$ git diff
	diff --cc b
	index fc12d34,33d9fa9..0000000
	--- a/b
	+++ b/b

Git does know what's going on, just fails to display it properly
in the diff, as the output of ls-files shows:

	$git ls-files -u
	160000 6a6e215138b7f343fba67ba1b6ffc152019c6085 1	b
	160000 fc12d3455b120916ec508c3ccd04f23957c08ea5 2	b
	160000 33d9fa9f9e25de2a85f84993d8f6c752f84c769a 3	b

I agree that this needs to be improved, but am currently lacking
the time to do it myself. But I believe this will get important
rather soonish when we recursively update submodules too ...
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]