Re: [PATCH v4 0/4] git-submodule add: Add --local-branch option

> On Mon, Nov 26, 2012 at 04:00:15PM -0500, W. Trevor King wrote:
> > From: "W. Trevor King" <wking@tremily.us>
> > 
> > On Fri, Nov 23, 2012 at 12:54:02PM -0500, W. Trevor King wrote:
> > > We could add
> > >
> > >   $ git submodule update --branch
> > >
> > > to checkout the gitlinked SHA1 as submodule.<name>.branch in each of
> > > the submodules, leaving the submodules on the .gitmodules-configured
> > > branch.  Effectively (for each submodule):
> > >
> > >   $ git branch -f $branch $sha1
> > >   $ git checkout $branch
> > 
> > I haven't gotten any feedback on this as an idea, but perhaps someone
> > will comment on it as a patch series ;).
> 
> I am not sure I understand you correctly. You are suggesting that the
> branch option as an alias for the registered SHA1 in the superproject?
> 
> I though the goal of your series was that you want to track submodules
> branch which come from the remote side?

> I would think more of some convention like:
> 
> 	$ git checkout -t origin/$branch
> 
> when first initialising the submodule with e.g.
> 
> 	$ git submodule update --init --branch
> 
> Then later calls of
> 
> 	$ git submodule update --branch
> 
> would have a branch configured to pull from. I imagine that results in
> a similar behavior gerrit is doing on the server side?

> > Changes since v3:
> > 
> > * --record=??? is now --local-branch=???
> > * Dropped patches 2 ($submodule_ export) and 3 (motivating documentation)
> > * Added local git-config overrides of .gitmodules' submodule.<name>.branch
> > * Added `submodule update --branch`
> 
> I would prefer if we could squash all these commits together into one
> since it seems to me one logical step, using the new variable for update
> belongs together with its configuration on initialization.
> 
> How about reusing the -b|--branch option for add? Since we only change
> the behavior when submodule.$name.update is set to branch it seems
> reasonable to me. Opinions?

> > Because you need to recurse through submodules for `update --branch`
> > even if "$subsha1" == "$sha1", I had to amend the conditional
> > controlling that block.  This broke one of the existing tests, which I
> > "fixed" in patch 4.  I think a proper fix would involve rewriting
> > 
> >   (clear_local_git_env; cd "$sm_path" &&
> >    ( (rev=$(git rev-list -n 1 $sha1 --not --all 2>/dev/null) &&
> >     test -z "$rev") || git-fetch)) ||
> >   die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")"
> > 
> > but I'm not familiar enough with rev-list to want to dig into that
> > yet.  If feedback for the earlier three patches is positive, I'll work
> > up a clean fix and resubmit.
> 
> You probably need to separate your handling here. The comparison of the
> currently checked out sha1 and the recorded sha1 is an optimization
> which skips unnecessary fetching in case the submodules commits are
> already correct. This code snippet checks whether the to be checked out
> sha1 is already local and also skips the fetch if it is. We should not
> break that.