Re: [WIP PATCH 0/3] implement merge strategy for submodule links

On Tuesday 15 June 2010, Jens Lehmann wrote:

Ok, let's look at a sequence of events:

0. There is a master branch in the superproject which points to commit A (on 
the master branch) in the submodule.

1. Alice creates feature_a branch in both superproject and submodule, 
creates commits B, C & D in the submodule and updates the superproject to 
point to D

2. Someone creates E in the submodule, and updates the master branch in the 
superproject to point to E.

3. Bob creates feature_b branch in both superproject and submodule, creates 
commits H, I & J in the submodule and updates the superproject to point to J

4. Someone creates F in the submodule, and updates the master branch in the 
superproject to point to F.

5. Maintainer starts integrating feature_a into master in superproject, and 
discovers the conflict between A->D and A->F. Mantainer then descends into 
submodule to create the merge G. Maintainer can now 'git add' the submodule 
in the superproject to record A->G as the merge resolution of the A->D vs. 
A->F conflict.

(6. Same as step #5, but replace Alice/A/D/F/G with Bob/E/J/G/K)

I assume here that nobody has made the merge commit G before Git produces the A->D vs. A->F conflict in step #5 (which prompts Maintainer to make G in order to resolve the conflict). I believe this would be the most common case.

If the merge commit G for some reason _already_ exists in the submodule before step #5, the maintainer's job is to simply recognize it as the correct resolution of the conflict, and check it out (and finally 'git add' it to the superproject index). But I don't see this happening very often: For one, who has the incentive to create G before it is needed in step #5? Both Alice and Bob are content with pointing to their respective submodule branches, and the only person who cares about doing the submodule merges is Maintainer who has to tie everything together into a coherent whole.

However, we should not require clairvoyance from the Maintainer as to which submodules have been modified by each feature branch, and hence which of them require preparatory submodule merges to be performed before the main superproject merge can be started. To the contrary, I believe the typical Maintainer will start the superproject merge, and then respond to the submodule conflicts that Git produces by descending into the submodule and merging submodules (or whatever else is required to reach a satisfactory submodule state).

Thus, if the purpose of Heiko's patches is to simply recognize merges that have already happened before step #5, then I'm afraid they will seldom or never be useful in practice (since these merges typically happen _after_ the superproject merge has been started).

Ah, yes, sorry, I confused the concepts at this point. Still:

- If the purpose is to re-use existing submodule merges then I'm afraid (as 
I've argued above) that this would happen too seldom to be useful in 
practice (and even then you would already have had to set up the appropriate 
config for your branch, to enable Git to find this pre-existing merge at 
all).

- If the purpose is to create new submodule merges to resolve the conflicts 
(which, granted, the patches currently don't do, but that I'm afraid they 
would _have_ to do in order to be useful in practice), then there is too 
much cleverness/magic for my liking.

As I've argued above, I'm afraid this situation would seldom/never arise in practice.

Taking a step back and comparing the merging of submodules vs. the merging of regular files:

Git's rules are simple and straightforward for regular files: If both sides/branches have changed the same area of code (and the changes don't exactly coincide), you get a conflict. There's no magic/cleverness applied to try to figure out what a good resolution would look like; it's a conflict, and the user must resolve it. Simple as that.

I'd argue that the submodule case should be the same: If both sides/branches change the submodule (and the SHA1s don't exactly match), you get a conflict, and it's up to the user to resolve it.

We may to make an exception for the case where one SHA1 is a descendant of the other (i.e. a fast-forward situation), since that seems like a safe choice in most situations, but I don't feel safe doing much beyond that.

In that case you will have to modify Heiko's patches, because (I believe) they currently choose the _latest_ commit containing the others...

Cheers,

...Johan