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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jan 09, 2014 at 09:31:13AM +0100, Jens Lehmann wrote:
> Am 09.01.2014 02:09, schrieb Francesco Pretto:
> > 2014/1/9 W. Trevor King <wking@xxxxxxxxxx>:
> >>
> >> 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.

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

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

> 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

Attachment: signature.asc
Description: OpenPGP digital signature


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]