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

From: Johan Herland <johan@herland.net>
Date: Jun 16, 2010, 21:32 UTC
Message-ID: <201006162332.56700.johan@herland.net>
In-Reply-To: <4C1906FA.7010906@web.de>

On Wednesday 16 June 2010, Jens Lehmann wrote:

Show 11 quoted lines

> Am 16.06.2010 02:05, schrieb Johan Herland:
> > - 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).
> 
> That this is all but happening seldom for us is the reason for this WIP
> patch from Heiko. And other use cases won't be harmed by this change, no?
> And if some are, we can add a config option to .gitmodules to control
> that.

Ok. I'm still not sure I see how this can happen frequently in practice, but since you both probably use submodules more heavily than I do, I will not stand in the way of progress.

Show 30 quoted lines

> > 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.
> 
> Yes, I would like to see that fast-forward case silently handled by a
> merge in the superproject.
> 
> And if it is no fast-forward but you find a unique merge where both of
> these SHA1s are included, you could advertise it as a possible solution
> but not automagically add it to the index. So you give the maintainer of
> the superproject the opportunity to assess a possible solution but spare
> him the chore of trying to find the reason why the merge failed and what
> he can do about it by showing him the right direction. He might then
> decide to take a later commit of the submodule or resolve the whole
> issue differently, but that is up to him.

I still particularily don't like the added config variable for specifying which branch(es) are considered "stable". Would it be possible to instead search all submodule branches for the earliest commits that reconcile the two commits, and then inform the user that these may be interesting to look at when trying to find a resolution? Something like:

  Cannot auto-resolve conflict between $a_sha1 and $b_sha1 in submodule
  $foo. The following merge commits in submodule $foo may help you resolve
  this conflict:
    - $sha1 (present in branches $a, $b, $c)
    - $sha2 (present in branches $c, $d)
    - $sha3 (present in branches $e, $f)

Thus the user/maintainer gets the full picture, and are given as many alternatives as possible to help resolve the conflict, instead of automatically getting one (possibly wrong) resolution, just because the "stable" config was unset or incorrect.

...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net