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

From: Jens Lehmann <[email-masked]>
Date: Thu, 17 Jun 2010 23:13:00 +0200

Am 17.06.2010 02:39, schrieb Johan Herland:
> But this is pure speculation, and as you say, I'd like to see what workflows 
> Jens and Heiko are actually using.

Ok, here we go. And as I have difficulties thinking about that when looking
at a single graph, I'll draw two: The upper for the superproject and the
lower for the submodule.

Superproject:
  -----2         [Alice's branch]
 /      \
1--3-----4---5   [master]
    \       /
     ------6     [Bob's branch]

       ^   ^
       |   |     [commits of the submodule committed in the superproject]

Submodule:
  ---B           [feature_a]
 /    \
A--C---D---E     [master]
    \     /
     ----F       [feature_b]

Alice hacks away on her feature branch and notices she has to make changes
to a submodule. She creates the "feature_a" branch there with commit 'B'
and asks the maintainer of the submodule to review and merge her change.
Our policy is to never commit submodule commits that are not merged yet, as
they could just vanish (e.g. by rebasing; imagine having git as a submodule
and committing a SHA1 from the "pu" branch in the superproject ... a later
bisect might get really frustrating). So the submodule maintainer merges 'B'
into 'D' and tells Alice that. She commits 'D' for the submodule in her '2'
commit and asks the maintainer of the superproject to review and merge that.
The moment he merges that into '4', 'D' gets recorded in the master branch
of the superproject for the submodule.

Meanwhile Bob also needs a change in the submodule for his work in the
superproject and adds commit 'F' on the "feature_b" branch there. He waits
for the submodule maintainer to merge that into 'E' so he can do commit '6'.

But now the submodule commit 'D' in the superproject commit '4' has become
an obstacle for him and the superprojects maintainer. Bob can't rebase or
cherrypick beyond or up to '4' because he will get a merge conflict. If he
asks to merge his branch into '5', the superprojects maintainer will get a
merge conflict and tells to him to resolve that.

This situation would disappear when git merge would do fast-forwards for
submodule commits. And I argue that this is The Right Thing, because just as
commit '5' contains /all/ changes from both branches to the files it should
also contain /all/ changes to the submodules files that happened during
these branches. And that means merge should resolve the submodule to commit
'E'.

This is somehow similar to merging binary files. But for submodules Git has
a chance to tell the combined version of both changes in the fast-forward
case, whereas it can't know that for binary files. And yes, merge conflicts
could happen for the same reasons they may happen to files: The changes in
Bob's branch could break something in Alice's branch. But that applies for
files just like it does for submodule commits, no?

And the non-fast-forward case happens e.g. when Alice and Bob do not wait
for the submodule maintainer to merge their changes:

Superproject:
  ---2         [Alice's branch]
 /    \
1--3---4---5   [master]
    \     /
     ----6     [Bob's branch]

     ^   ^
     |   |       [commits of the submodule committed in the superproject]

Submodule:
  ---B           [feature_a]
 /    \
A--C---D---E     [master]
    \     /
     ----F       [feature_b]

In this case submodule commit 'B' is recorded in '2' and thus '4', while
commit 'F' will be recorded in '6'. So when '4' and '6' are merged, a valid
guess for '5' would be to use submodule commit 'E', as it is the first one
based on both 'B' and 'F'.

But in this case it is not so clear that 'E' is the right commit, as there
might be other commits present in the paths 'B'->'E' and 'F'->'E'. So 'E'
is just a probable solution for the merge, but not one I would like to see
automatically merged. But it should be proposed to the person doing the
merge as a probable resolution of the conflict, so that she can decide if
that is the case.

And no 'special' branch is used here. But I think this approach will solve
a lot of the problems we - and maybe others - have with submodule merges
without doing any harm to other workflows.