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

From: Heiko Voigt <hvoigt@hvoigt.net>
Date: Jan 14, 2014, 10:24 UTC
Message-ID: <20140114102445.GA27915@sandbox-ub>
In-Reply-To: <20140109221840.GW29954@odin.tremily.us>

Hi,

On Thu, Jan 09, 2014 at 02:18:40PM -0800, W. Trevor King wrote:

Show 27 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:
> > > 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.

Show 16 quoted lines

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

Show 7 quoted lines

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