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

From: Heiko Voigt <hvoigt@hvoigt.net>
Date: Jan 14, 2014, 20:58 UTC
Message-ID: <20140114205830.GA838@sandbox-ub>
In-Reply-To: <20140114165709.GH7078@odin.tremily.us>

On Tue, Jan 14, 2014 at 08:57:09AM -0800, W. Trevor King wrote:

Show 24 quoted lines

> > > On Thu, Jan 09, 2014 at 10:40:52PM +0100, Jens Lehmann wrote:
> > > > Am 09.01.2014 20:55, schrieb W. Trevor King:
> > > > > Maybe you meant "for checkout I can easily overwrite the local
> > > > > changes with the upstream branch", which is what I understand
> > > > > checkout to do.
> > > > 
> > > > But which I find really unfriendly and would not like to see in
> > > > a new feature. We should protect the user from loosing any local
> > > > changes, not simply throw them away. Recursive update makes sure
> > > > it won't overwrite any local modification before it checks out
> > > > anything and will abort before doing so (unless forced of
> > > > course).
> > > 
> > > If you want to get rid of checkout-mode updates, I'm fine with
> > > that.  However, I don't think it supports use-cases like Heiko's
> > > (implied) “I don't care what's happening upstream, I never touch
> > > that submodule, just checkout what the superproject maintainer
> > > says should be checked out for this branch.  Even if they have
> > > been rebasing or whatever” [3].
> > 
> > I never stated that in that post.
> 
> Sorry for misunderstanding.  I think I'm just unclear on your
> workflow?

Yes probably. We mostly use submodules for shared code and for tracking external libraries. For the shared code we want to make sure that the changes that come from one project do not break anything in another project that also uses that code so the submodules are maintained and reviewed separately and ideally contain tests for the expected functionality.

A typical workflow where a feature in a project needs some extension or change in a submodule goes like this:

1. The developer does his changes locally implementing everything
   needed. To commit he creates a local branch in the submodule and in
   the superproject (most of the times from the current HEAD that is
   checked out).

2. For convenience I usually commit the resulting commit sha1 of the
   submodule in the commit that needs the change. That way when I switch
   to a different branch and back I can simply say: git submodule update
   and get the correct code everywhere.

3. Once done with the whole feature I first do the review process
   (adding any missing tests to ensure my change does not break, ...)
   for the submodule to get the feature branch merged into a stable one.
   In our superproject only commits sha1 from a stable branch are
   allowed so the submodule change needs to be reviewed first.

4. Once the change is in a stable branch in the submodule I then update
   the commit sha1 link in the superproject that needs the change. That
   is usually done by rebasing the branch in the superproject and
   registering the new stable branch (typically master).

5. Then I proceed with the review process in the superproject as if it
   was a normal change without a submodule.

Thats our main use case.

Show 29 quoted lines

> > The recursive checkout Jens is working on is simply implementing the
> > current checkout-mode to the places where the superproject checks
> > out its files. That way submodules get checked out when the
> > superproject is checked out. If the submodule does not match the
> > sha1 registered in the superproject it either stays there (if the
> > checkout would not need to update the submodule) or (if checkout
> > would need to update) git will not do the checkout and bail out with
> > "you have local modifications to ... .
> 
> Sounds good to me as far as it goes.  I think it misses the “what
> should we do if the gitlinked hashes are different” case, because the
> checkout will always leave you with a detached HEAD.
> 
> > > > or be asked to resolve the conflict manually when "checkout" is
> > > > configured and the branches diverged.
> > > 
> > > I still think that checkout-mode updates should be destructive.
> > > See my paraphrased-version of Heiko's use case above.  How are
> > > they going to resolve this manually?  Merge or rebase?  Why
> > > weren't they using that update mode in the first place?
> > 
> > I strongly disagree. They should only be destructive in the sense
> > that another commit get checked out but never loose local
> > modifications.
> 
> I think the key I'm missing is an example workflow where a developer
> wants to make local submodule changes, but also wants to use
> checkout-mode updates instead of merge/rebase updates.  Can you sketch
> one out for me?

How about the use-case I sketched above? Is that what you are searching for? In that use-case we have to update to the new master after a submodule change was merged. That could be achieved by

	git submodule update --remote <submodule>

with the wanted stable branch configured. But in practise something along the lines of

	(cd <submodule> && git checkout origin/<stable>)

is usually used and simple enough.

We have a tool in our git gui configuration that does

	git submodule foreach 'git fetch && git checkout origin/master'

which can be used by the maintainer before a release to ensure that all submodules are up-to-date. But in practise it turn out that all submodules are fairly current since everyone is adding features to the quite frequently and thus pulling in the current version automatically.

I hope that draws a clear picture of how we use submodules.

Cheers Heiko