Re: [RFC v2] submodule: Respect requested branch on all clones

From: W. Trevor King <wking@tremily.us>
Date: Jan 9, 2014, 17:32 UTC
Message-ID: <20140109173218.GA8042@odin.tremily.us>
In-Reply-To: <52CE5E51.4060507@web.de>

On Thu, Jan 09, 2014 at 09:31:13AM +0100, Jens Lehmann wrote:

Show 25 quoted lines

> Am 09.01.2014 02:09, schrieb Francesco Pretto:
> > 2014/1/9 W. Trevor King <wking@tremily.us>:
> >>
> >> However, submodule.<name>.local-branch has nothing to do with remote
> >> repositories or tracking branches.
> > 
> > My bad: this means the feature is still not entirely clear to me.
> > 
> >>
> >>   [branch "my-feature"]
> >>         remote = origin
> >>         merge = refs/heads/my-feature
> >>         [submodule "submod"]
> >>             local-branch = "my-feature"
> >>
> >> and I don't think Git's config supports such nesting.
> >>
> > 
> > Aesthetically, It doesn't look very nice.
> 
> And I'm not sure we even need that. What's wrong with having the
> branch setting in the .gitmodules file of the my-feature branch?
> The only problem I can imagine is accidentally merging that into
> a branch where that isn't set, but that could be solved by a merge
> helper for the .gitmodules file.

.gitmodules is fine so long as the config can live in the versioned tree. Many (all?) .gitmodules settings can be overridden in .git/config. However, the local-branch setting needs to be both per-submodule and per-superproject-branch, so .git/config doesn't work very well. I think it's better to use something like my .git/modules/<submodule-name>/config implementation [1] to set this override.

This lack of per-superproject-branch overrides applies to all of the submodule.<name>.* settings, but you're unlikely to want an out-of-tree override for 'path' or a per-superproject-branch override for 'url', 'ignore', 'update', or 'chRecurseSubmodules'. Maybe folks would want per-superproject-branch overrides to 'branch', although I'd prefer we reuse branch.<name>.merge in the submodule's config for that [2].

On the other hand, maybe an in-tree .gitmodules is good enough, and folks who want a local override can just edit .gitmodules in their local branch? I've never felt the need to override .gitmodules myself (for any setting), so feedback from someone who has would be useful.

Show 21 quoted lines

> >> I can resuscitate that if folks want, but Heiko felt that my initial
> >> consolidation didn't go far enough [2].  If it turns out that we're ok
> >> with the current level of consolidation, would you be ok with
> >> "non-checkout submodule.<name>.update" as the trigger [3]?
> > 
> > For me it was ok with what you did:
> > -------------------------------------------------
> > if "just_cloned" and "config_branch"
> > then
> >      !git reset --hard -q"
> > fi
> > -------------------------------------------------
> > 
> > So yes: at the first clone 'checkout' keeps attached HEAD, while
> > 'merge' and 'rebase' attach to the branch.
> 
> It have the impression that attaching the head to the given branch
> for merge and rebase might be the sensible thing to do, but it
> would be great to hear from users of merge and rebase if that
> would break anything for them in their current use cases for these
> settings.

Which local branch would you attach to before merging? I think 'git submodule update' should always use the current submodule state (attached branch or detached HEAD) [3], and we should have a separate call that explicitly checked out the desired submodule branch [4].

Show 8 quoted lines

> > If it's not the first clone, you should take no action (and your
> > original patch was ok about this).
> 
> I'm not sure this is the right thing to do, after all you
> configured git to follow that branch so I'd expect it to be
> updated later too, no? Otherwise you might end up with an old
> version of your branch while upstream is a zillion commits
> ahead.

Non-clone updates should not change the submodule's *local* branch *name*. They should change the commit that that branch references, otherwise 'git submodule update' would be a no-op ;).

> First I'd like to see a real consensus about what exactly should
> happen when a branch is configured to be checked out (and if I
> missed such a summary in this thread, please point me to it ;-).

I don't think we have a consensus yet. A stand-alone outline of my current position is in my v3 RFC [5], but I don't have any buy-in yet ;).

> And we should contrast that to the exact checkout and floating
> branch use cases.

With my v3 series, there are no more detached HEADs. Folks using checkout updates get a local master branch. I do not change any of the exact checkout (superproject gitlinked sha1) vs. floating (subproject's remote submodule.<name>.branch via 'update --remote') logic, because that already works well. The problem is the local branch handling, not the update/integration logic.

> So what should happen on initial clone,

For 'add', clone the command line URL and create a new branch 'master' pointing at the commit referenced by the remote's HEAD (or other branch with --branch).

For 'update', do the same, except use a local-branch setting to determine the name for the local branch, falling back to 'master' if it is not set.

> later updates,

The same thing that currently happens, with the exception that checkout-style updates should use reset to update the currently-checked out branch (or detached-HEAD), instead of always detaching the HEAD.

> updates where the local and the remote branch diverged,

The same thing that currently happens. For local (non --remote) updates, the integrated branch is the superproject's gitlinked sha1. For --remote updates, the integrated branch is the remote subproject's submodule.<name>.branch. We integrate that with the currently-checked-out local branch (or detached HEAD) using the user's preferred submodule.<name>.update strategy.

> when superproject branches are merged (with and without conflicts),

I don't think this currently does anything to the submodule itself, and that makes sense to me (use 'submodule update' or my 'submodule checkout' if you want such effects). We should keep the current logic for updating the gitlinked $sha. In the case that the .gitmodule-configured local-branches disagree, we should give the usual conflict warning (and <<<===>>> markup) and let the user resolve the conflict in the usual way.

> on a rebase in the superproject and so on.

Same as the merge case. Why would configuring a preferred local-branch that only affects checkout (and the initial clone) have anything to do with rebases and merges?

> After that we can discuss about how to implement them (even though I
> believe we won't need a new submodule command at the end of this
> process, simply because if it isn't configurable we cannot teach git
> checkout and friends to do that automatically for us later).

I think it is configurable, and I have an implementation that works (modulo bugs) [5]. I think we should teach 'git checkout --recurse-submodules' this logic, and then we can drop my 'git submodule checkout' entirely.

Show 6 quoted lines

> And from reading this discussion I believe we need another value for
> the ignore option which only ignores changes to the SHA-1 but not to
> work tree modifications of a submodule work tree relative to its HEAD
> (or make that two: another one which ignores untracked files too and
> only shows modification of tracked files). Otherwise users of the
> floating or attached model can easily miss submodule modifications.

I am ignore-agnostic ;).  Do whatever you like here.

Cheers, Trevor

[1]: http://article.gmane.org/gmane.comp.version-control.git/240251 [2]: http://article.gmane.org/gmane.comp.version-control.git/240164 [3]: http://article.gmane.org/gmane.comp.version-control.git/240250 [4]: http://article.gmane.org/gmane.comp.version-control.git/240249 [5]: http://article.gmane.org/gmane.comp.version-control.git/240248

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy