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

From: W. Trevor King <wking@tremily.us>
Date: Jan 5, 2014, 23:39 UTC
Message-ID: <20140105233943.GJ3156@odin.tremily.us>
In-Reply-To: <20140105225733.GB4660@book.hvoigt.net>

On Sun, Jan 05, 2014 at 11:57:33PM +0100, Heiko Voigt wrote:

Show 10 quoted lines

> On Sun, Jan 05, 2014 at 01:24:58PM -0800, W. Trevor King wrote:
> > If submodule.<name>.branch is set, it *always* creates a new local
> > branch of that name pointing to the exact sha1.  If
> > submodule.<name>.branch is not set, we still create a
> > detached-HEAD checkout of the exact sha1.
> 
> Thanks for this clarification. Since the usual usage with --remote
> is with a remote-tracking branch, I confused this here. I am not
> sure whether blindly creating a local branch from the recorded sha1
> is the right thing to do. In what situations would that be helpful?

In any situation where your going to develop the submodule locally, you're going to want a branch to develop in. Starting local-submodule developers off on a branch seems useful, even if we can only use submodule.<name>.branch to guess at their preferred local branch name. Sometimes (often?) the guess will be right. However, A detached HEAD will never be right for local development, so being right sometimes is still an improvement ;).

> At $dayjob we usually use feature branches for our work. So if
> someone wants to work in a submodule you simply create a branch at
> the current sha1 which you then send out for review.

I'm all for named feature branches for development, and in this case submodule.<name>.branch is likely to be the wrong choice. However, it's still safer to develop in that branch and then rename the branch to match your feature than it would be to develop your fix with a detached HEAD. If your developers have enough discipline to always checkout their feature branch before starting development, my patch won't affect them. However, I know a number of folks who go into fight-or-flight mode when they have a detached HEAD :p.

Show 6 quoted lines

> The reason why one would set a branch option here is to share the
> superproject branch with colleagues. He can make sure they can
> always fetch and checkout the submodule even though the branch there
> is still under cleanup and thus will be rebased often. The commit
> referenced by sha1 would not be available to a developer fetching
> after a rebase.

Yeah, floating gitlinks are something else. I'd be happy to have that functionality (gitlinks pointing to references) should be built into gitlinks themselves, not added as an additional layer in the submodule script. This "gitlinked sha1 rebased out of existence" scenario is the first I've heard where I think gitlinked references would be useful.

Show 15 quoted lines

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

Show 15 quoted lines

> > > > -				update_module= ;;
> > > > +				if test -n "$config_branch"; then
> > > > +					update_module="!git reset --hard -q"
> > > 
> > > If we get here the checkout has already been done. Shouldn't
> > > this rather specify a noop. I.E. like
> > > 
> > > 	update_module="!true"
> > 
> > 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.

Cheers, Trevor

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