On Mon, Jan 06, 2014 at 04:47:39PM +0100, Heiko Voigt wrote:
> On Sun, Jan 05, 2014 at 03:39:43PM -0800, W. Trevor King wrote:
> > On Sun, Jan 05, 2014 at 11:57:33PM +0100, Heiko Voigt wrote:
> > > On Sun, Jan 05, 2014 at 01:24:58PM -0800, W. Trevor King wrote:
> > > > Thinking through this more, perhaps the logic should be:
> > > > 
> > > > * If submodule.<name>.update (defaulting to checkout) is checkout,
> > > >   create a detached HEAD.
> > > > * Otherwise, create a new branch submodule.<name>.branch
> > > >   (defaulting to master).
> > > 
> > > Why not trigger the attached state with the
> > > submodule.<name>.branch configuration option? If there is a
> > > local branch available use that, if not the tracking branch (as
> > > it is currently). Then a developer can start working on the
> > > branch with:
> > > 
> > >   cd submodule; git checkout -t origin/<branchname>
> > > 
> > > assuming that submodule update learns some more support for this.
> > 
> > Isn't that already what 'git update --remote <submodule>' already
> > does?
> 
> Does it? As far as I understood (not using the branch option yet) it
> only does
> 
>       git checkout origin/<branchname>
> 
> so there is no local branch created that tracks the remote branch (-t).

That's right.  Anyone who wants to do local development in a submodule
should probably not be using checkout updates, hence my proposal above
to base local-branch creation on submodule.<name>.update.

> What I was thinking is that when submodule.<name>.branch is set a
> 
>       git submodule update
> 
> will:
> 
> 1. if no local branch with that name exists:
> 
>    checkout the remote/<branch>
> 
> 2. If a local branch with that name exists:
> 
>    checkout the local branch and possibly advance it according to
>    its setting.

This sounds too complicated to me ;).

> Thinking further: Maybe submodule.<name>.update = pull could denote
> that a user wants to have a branch ready for work in a submodule.

This sounds like my quoted realization above.  We both even preface
the idea with "thinking" ;).  However, I think merge, rebase,
!command, and all other non-checkout update schemes are already
signals that the developer is interested in local developent (and
therefore wants a branch), without the need to add an aditional 'pull'
(and then how to distinguish between rebase/merge?).

> submodule update will then
> 
> 1. if no local branch with that name exists:
> 
>    - automatically create the branch based on the referenced sha1
>    - set up that its tracking remote/<branch>

With my patch this happens with the initial clone-update (as of v2,
only when submodule.<name>.branch is set.  In a hypothetical v3, only
when submodule.<name>.update is not checkout).  I'm not sure we want
to do this if the user switches to non-checkout updates after the
initial cloning update though.  They may actually have work in that
detached HEAD that we'd be clobbering.

>    - issue a git pull in the submodule

I think that updating using the gitlinked sha1 (a local update) and
updating using the upstream origin/$branch (a --remote update) should
always be two distinct events.  Combining them in a single operation
just complicates the situation.

> 2. if a local branch with that name exists:
> 
>    - issue a git pull in the submodule

That's what we already have with submodule.<name>.update as 'merge'.
The merged object is either the gitlinked sha1 (a local update) or a
re-fetched upstream branch tip (a --remote update).

> > > > We are on a local branch at this point, but not neccessarily
> > > > pointing at the gitlinked sha1.  The reset here ensures that
> > > > the new local branch does indeed point at the gitlinked sha1.
> > > 
> > > But isn't this a fresh clone? Why should the branch point at
> > > anything else?
> > 
> > We don't pass $sha1 to module_clone().  Before my patch, we don't
> > even pass $branch to module_clone().  That means that
> > module_clone() will only checkout the gitlinked sha1 when the
> > upstream HEAD (or $branch with my patch) happens to point to the
> > gitlinked sha1.  For example, if Alice adds Charie's repo as a
> > submodule (gitlinking his current master d2dbd39), then Charlie
> > pushes a new commit d0de817 to his master, and then Bob clones
> > Alice's superproject.  Post-clone, Charlie's submodule will have
> > checked out Charlie's new d0de817, and we need update's
> > additional:
> > 
> >   git reset --hard -q d2dbd39
> > 
> > to rewind to Alice's gitlinked sha1.
> 
> Ah yeah, sorry I was confusing this with the checkout of
> remote/<branch> here again. Since I have done that twice already
> maybe we should be careful about not confusing users with this as
> well...
>
> After wrapping my head around the fact that you want to simply create a
> local branch on the referenced sha1 (and hopefully remembering it) I
> still would like to think a little more about it and let it settle a
> bit.

The way I keep this straight is:

1. Submodules are links to Git commits (that's how they're stored in
   the index).
2. There are two places to look if you want to update the linked
   commit:
   a. The superproject's tree (a local updates).
   b. The remote subproject's current submodule.<name>.branch tip (a
      --remote update).
3. There are a number of ways to integrate external updates with your
   local submodule development.
   a. checkout: blow away local development
   b. merge: merge the update into the local branch
   c. rebase: rebase the local branch onto the update
   d. !command: do something fancier

We currently have easy config and command-line switches to select
between 2.a and 2.b, which for me makes extending 1 to support
floating branches unnecessary.  Note that we're always updating to a
referenced sha1, and there are only two places we can look to find
that sha1.

We also have easy config and command-line switches to select between
3.a, 3.b, 3.c, and 3.d.

One thing that's a bit fuzzy is the definition of "local development".
We're currently treating it as "any commits leading up to HEAD that
are not in the update source", which works well until upstream starts
rebasing (or rolling back to an earlier release, etc.).  It's hard for
Git to handle that sort of thing automatically, for all the usual
"recovering from upstream rebase" reasons [1].  I'm fine if we leave
it up to users to resolve this sort of situation by hand.  Folks with
local work should know what was local, and folks without local work
can use a checkout update.

We're also not doing a great job of setting people up for local
development, but that's tricky if they may already have local work.
My "start them on a branch" patch is not saving anyone a lot of work,
but it's a step in that direction.  Doing anything after the initial
cloning update is going to be much harder, because there's not much
that would be unabmiguously helpful.  After all, the current submodule
implementation supports my workflows pretty well.

Cheers,
Trevor

[1]: See "RECOVERING FROM UPSTREAM REBASE" in git-rebase(1).

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

Reply via email to