git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johan Herland <johan@herland.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Jens Lehmann <Jens.Lehmann@web.de>,
	Heiko Voigt <hvoigt@hvoigt.net>
Subject: Re: [WIP PATCH 0/3] implement merge strategy for submodule links
Date: Tue, 22 Jun 2010 00:35:30 +0200	[thread overview]
Message-ID: <201006220035.31166.johan@herland.net> (raw)
In-Reply-To: <7vlja8if5r.fsf@alter.siamese.dyndns.org>

On Monday 21 June 2010, Junio C Hamano wrote:
> Johan Herland <johan@herland.net> writes:
> > I still don't like this, as IMHO it's too subtle, and possibly
> > conflicts with explicitly tracking submodule branches (which, to me,
> > is a more important feature).
> 
> If you mean, by "explicitly tracking", to say "I don't care which commit
> from the submodule appears at this path, as long as it is at the tip of
> this branch", I still don't think it makes much sense, but what I
> outlined is not _incompatible_ with such a scheme.  In fact I think it
> would rather fit naturally as a sanity/safety measure.

I'll first try to explain where I'm coming from, to hopefully eliminate any 
confusion about my position:

IMHO, there should be 2 primary modes for submodules in Git:

A. Explicitly tracking submodule commits. This is the existing submodule 
behaviour. The superproject refers directly (in its tree) to a submodule 
commit. The .gitmodules file contains associated information 
(submodule.<name>.path, .url and .update).

B. Explicitly tracking submodule branches. An extra setting 
(submodule.<name>.branch) is added to the .gitmodules file, to determine 
which submodule branch to checkout. This setting overrides whatever 
submodule commit, if any, is stored in the superproject tree. There are two 
sub-modes for this mode:

B.1. There is no submodule entry at all in the superproject tree. This 
indicates that you are not at all interested in tracking the history of the 
submodule relative to the superproject. You are always interested in 
checking out the tip of submodule.<name>.branch in the submodule, even when 
digging into the superproject's history.

B.2. There is a submodule entry in the superproject tree. This "hybrid" 
approach indicates that although you primarily want to track a branch in the 
submodule (i.e. you mostly want the latest version of the submodule's 
branch), you still want a record of where your submodule has been pointing, 
in your superproject's history. Exactly when (or how often) the 
superproject's submodule entry should be updated is yet TBD. So is when to 
check out according to .branch, and when to use the recorded submodule 
commit. In the end, it'll probably be a policy decision for the projects 
that choose this approach.

Ok, that hopefully explains the basic idea of tracking submodule commits (A) 
vs. tracking submodule branches (B). Now, how would this apply to merging 
submodules?

In case of B, I'd argue that the submodule merging should _only_ look at the 
value of submodule.<name>.branch from the superproject's .gitmodules. If 
this setting is ambiguous (because of merge conflicts in .gitmodules), Git 
should not touch the submodule at all (until .gitmodules is resolved). 
Otherwise, Git's only task is to checkout whatever branch is specified by 
.gitmodules. If the commit that is checked out does not descend from all of 
the merge alternatives, a warning should be printed.

With that in mind, I enter this discussion because it might provide insight 
on how to solve the problem of merging submodules in scenario A.

In mode A there is no submodule.<name>.branch setting, and I would not like 
to add an additional setting (let's call it submodule.<name>.merge_branch 
for now) that is "weaker" than submodule.<name>.branch (meaning that it does 
not trigger the transition from mode A to mode B). There are two major 
reasons for this:

1. submodule.<name>.merge_branch would add semantics to the case of merging 
submodules that would be similar in spirit to what submodule.<name>.branch 
does (the "spirit" here is the special relationship to a submodule branch 
that we're establishing), but still the .merge_branch setting would be 
different in practice, by (a) only applying to the case of merging 
submodules (while .branch changes the semantics of almost all submodule 
operations), and (b) not even in the case of merging submodules would the 
options do the same thing. I fear the semantics of the .merge_branch option 
would be too complicated for an average user, and that its similarity to 
.branch would cause confusion.

2. What would happen if you enabled _both_ .merge_branch and .branch? In the 
case of merging submodules which setting will "win"? Even worse, if you set 
.merge_branch to "foo", and .branch to "bar", what will then happen?

Ok, so if I oppose adding .merge_branch, what do I propose instead?

Currently, not much, I'm afraid. But I have a gut feeling that the use case 
presented by Heiko and Jens is best solved EITHER by having no special 
branch relationships between the superproject and submodule (which AFAICS is 
what we currently agree on in this thread, and Heiko has already submitted a 
patch to this effect), OR by employing a conservative version of mode B.2 in 
which we use .branch to track a submodule branch, but still keep a close 
look at the recorded submodule commit, and, if necessary, maybe introduce 
some other options to tell Git when to use the recorded commit, and when to 
use the branch tip.

In other words, I think we should explore the .branch direction before we 
add complexity and potential confusion by prematurely adding another option 
that it somewhat similar to .branch in some contexts.

> I presume that in your "explicitly tracked" world, if the user tries to
> commit at the superproject level with a submodule commit that is
> inconsistent with that "explicitly tracked" branch (e.g. the commit is
> not reachable from the tip of that branch), you would issue a warning of
> some sort, using that knowledge.

Yes.

> What I outlined uses the exact same
> knowledge of which branch in the submodule the superproject branch is
> tied to to reject irrelevant existing merges as resolution candidates.

True, but as I've argued above, I'm not sure that adding another setting 
(aka. .merge_branch) for this special/limited kind of branch tracking is 
worth it.

> Of course, this ".gitmodule in superproject can tell you which branch of
> submodule it follows" is optional; the user needs to take responsibility
> of picking the right one among I, E and G, of course, if the information
> does not exist or is not available.

Yes, of course. And this corresponds to what I've proposed for scenario A, 
when there is no branch-related setting specified for the submodule.


Hope this helps,

...Johan

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

  reply	other threads:[~2010-06-21 22:35 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
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 [this message]
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=201006220035.31166.johan@herland.net \
    --to=johan@herland.net \
    --cc=Jens.Lehmann@web.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --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).