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

[Heiko Voigt <hvoigt@hvoigt.net>]

Hi,

On Thu, Jan 09, 2014 at 02:18:40PM -0800, W. Trevor King wrote:
> On Thu, Jan 09, 2014 at 10:40:52PM +0100, Jens Lehmann wrote:
> > Am 09.01.2014 20:55, schrieb W. Trevor King:
> > > On Thu, Jan 09, 2014 at 08:23:07PM +0100, Jens Lehmann wrote:
> > >> Am 09.01.2014 18:32, schrieb W. Trevor King:
> > >>>> 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.
> > >>
> > >> Won't the user loose any modifications to his local branch here?
> > > 
> > > They just called for a checkout-style update, so yes.  If they
> > > want to keep local modifications, chose an integration mode that
> > > preserves local changes.
> > 
> > Hmm, as current "submodule updates" already makes it too easy to
> > loose commits, this does not look right to me. I'd prefer to stop at
> > that point and tell the user what he can do to solve the conflict.
> 
> Users who are worried about loosing local updates should not be using
> a checkout-style updates.  If they are using a checkout-style update,
> and they ask for an update, they're specifically requesting that we
> blow away their local work and checkout/reset to the new sha1.
> Solving update conflicts is the whole point of the non-checkout update
> modes.

But checkout is different from reset --hard in that it does not blow
away local uncommitted changes. Please see below.

> > > 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. Even with the current checkout-mode
updates you'll never loose local modifications (without force) that are
not committed. I think you have to distinguish between local
modifications in the worktree and the ones that are committed.

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

I think the whole recursive checkout topic is already complicated enough
as it is so we should currently not add anything from this discussion to
it until it is cleaned up and merged. We also need recursive fetch for
that which I am planning to work on as soon as we settle this
discussion. I will write another post about how I think we should/can
proceed.

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

> [1]: http://article.gmane.org/gmane.comp.version-control.git/240251
> [2]: http://article.gmane.org/gmane.comp.version-control.git/240248
> [3]: http://article.gmane.org/gmane.comp.version-control.git/240013

Cheers Heiko