git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johan Herland <johan@herland.net>
To: Jens Lehmann <Jens.Lehmann@web.de>
Cc: git@vger.kernel.org, Heiko Voigt <hvoigt@hvoigt.net>
Subject: Re: [WIP PATCH 0/3] implement merge strategy for submodule links
Date: Wed, 16 Jun 2010 23:32:56 +0200	[thread overview]
Message-ID: <201006162332.56700.johan@herland.net> (raw)
In-Reply-To: <4C1906FA.7010906@web.de>

On Wednesday 16 June 2010, Jens Lehmann wrote:
> 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.

> > 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

  reply	other threads:[~2010-06-16 21:33 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-06-11 12:23 [WIP PATCH 0/3] implement merge strategy for submodule links Heiko Voigt
2010-06-11 12:23 ` [WIP PATCH 1/3] extend ref iteration for submodules Heiko Voigt
2010-06-11 12:23 ` [WIP PATCH 2/3] add missing && to submodule-merge testcase Heiko Voigt
2010-06-11 12:23 ` [WIP PATCH 3/3] implement automatic fast forward merge for submodules Heiko Voigt
2010-06-12 10:12 ` [WIP PATCH 0/3] implement merge strategy for submodule links Johan Herland
2010-06-12 12:06   ` Heiko Voigt
2010-06-13 17:59     ` Johan Herland
2010-06-14 17:02       ` Heiko Voigt
2010-06-14 23:59         ` Johan Herland
2010-06-15 17:37           ` Jens Lehmann
2010-06-16  0:05             ` Johan Herland
2010-06-16 17:16               ` Jens Lehmann
2010-06-16 21:32                 ` Johan Herland [this message]
2010-06-16 22:11                   ` Junio C Hamano
2010-06-17  0:39                     ` Johan Herland
2010-06-17 21:13                       ` Jens Lehmann
2010-06-18  9:40                         ` Johan Herland
2010-06-18 13:55                           ` Jens Lehmann
2010-06-19  9:43                             ` Heiko Voigt
2010-06-19 15:54                               ` Jens Lehmann
2010-06-19 10:17                           ` Heiko Voigt
2010-06-19 13:15                             ` Johan Herland
2010-06-19 15:52                               ` [WIP PATCH 3/3] implement automatic fast forward merge for submodules Heiko Voigt
2010-06-20 18:04                           ` [WIP PATCH 0/3] implement merge strategy for submodule links Junio C Hamano
2010-06-20 23:06                             ` Johan Herland
2010-06-21  0:03                               ` Junio C Hamano
2010-06-21 10:19                                 ` Johan Herland
2010-06-21 15:22                                   ` Junio C Hamano
2010-06-21 22:35                                     ` Johan Herland
2010-06-22  4:04                                       ` Junio C Hamano
2010-06-22 10:48                                         ` Johan Herland
2010-06-23  7:38                                   ` Finn Arne Gangstad

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201006162332.56700.johan@herland.net \
    --to=johan@herland.net \
    --cc=Jens.Lehmann@web.de \
    --cc=git@vger.kernel.org \
    --cc=hvoigt@hvoigt.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).