On Saturday 12 June 2010, Heiko Voigt wrote: > On Sat, Jun 12, 2010 at 12:12:50PM +0200, Johan Herland wrote: > > On Friday 11 June 2010, Heiko Voigt wrote: > > > The following patch series is a work in progress. The idea is > > > whenever you need to merge two SHA1's of a submodule we search for a > > > ref in the submodule which already contains both. If one such ref > > > exists the resulting SHA1 is the one pointed at by that ref. > > > > I appreciate the effort to improve submodule handling, but I'm not sure > > I like this approach. Even though you try to apply it as > > conservatively as possible, it still smells a little like trying to > > make Git too clever for its own good. > > > > E.g. say we have the following commit history in the submodule: > > A---B---C---D <-- master > > > > Now, say that your merge conflict comes from one branch updating the > > submodule from B to C, while the other branch reverts the submodule > > from B to A. In your proposed scheme, Git would auto-resolve the > > conflict to D. > > You are right. I did forget to mention this in my topic letter: Both > changes need to point forward. This exact case is also tested in the > testcases and results in a merge conflict which needs to be resolved by > hand. Still doesn't solve one of the cases I gave in the last email: Say one branch updates the submodule from A to B, and the other updates from A to C. Your proposal resolves the merge by fast-forwarding to D, which seems irresponsible, since we have no concept of how well D is tested. Maybe it introduces another showstopper bug, and that is why neither branch has upgraded to it yet? A better solution would be, to put it generally: Given a submodule being part of a superproject conflict, if one of the candidate submodule SHA1s is is a descendant of _all_ the other submodule SHA1 candidates, then choose that SHA1 as the proposed resolution (but please leave the index entry "unmerged", so that the resolution must be confirmed by the user). This removes all the "stable" branch magic from your patch. All you need to look at are the candidate SHA1s and their relationship in the commit graph. No refs involved. In the A->B vs. A->C case above, we would see that C is a descendant of B, and we would therefore choose C as a suggested conflict resolution, which IMHO is a much better choice than D. I still don't want to add a lot of auto-resolving cleverness to Git, as it inevitably _will_ choose incorrectly sometimes, and in those situations it will be much more confusing than if it didn't choose at all. > > This whole idea is somewhat similar to branch-tracking submodules > > (recently discussed in another thread), except that it only applies on > > _merge_ in the superproject, and you don't get to choose _which_ > > branch it's tracking. That's _way_ too arbitrary for my tastes. > > The difference to branch-tracking submodules is, if I understand it > correctly, that with a merge you get an explicit SHA1 which is recorded. > Whereras with branch-tracking you never know on which revision on the > tracked branch the submodule was. Technically you may be right, but my point is that in your original proposal I don't get to _choose_ which submodule SHA1 is explicitly recorded for the merge resolution, but instead your patch chooses whatever SHA1 happens to be at the tip of some branch considered "stable". Although technically different, this is similar in _spirit_ to what branch-tracking submodules is about. > Thats why I only want to search through stable branches further down. I > mean stable in the git sense that they never get rewound and of course > should contain the most stable part of development. To ease the > configuration we would default to master which we could assume as > stable. But if we want to be on the safe side we could also say that > automatic submodule merging only works when the user has configured some > stable branches. Ok, so you can configure exactly which branch(es) you consider stable. I'd still much rather prefer the approach I outlined above, which does away with all the "stable" branch magic, and only considers the commit ancestry directly. Hope this helps, ...Johan -- Johan Herland, <johan@xxxxxxxxxxx> www.herland.net -- 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