Re: [RFC/PATCH 2/4] Submodules: Add the lib-submodule-update.sh test library

[Jens Lehmann <Jens.Lehmann@web.de>]

Am 17.04.2014 18:41, schrieb W. Trevor King:
> On Tue, Mar 25, 2014 at 06:05:05PM +0100, Jens Lehmann wrote:
>> *) When a submodule is replaced with a tracked file of the same name
>>    the submodule work tree including any local modifications (and
>>    even the whole history if it uses a .git directory instead of a
>>    gitfile!) is simply removed.
>> …
>> I think the first bug really needs to be fixed, as that behavior is
>> extremely nasty. We should always protect work tree modifications
>> (unless forced) and *never* remove a .git directory (even when
>> forced).
> 
> I think this should be covered by the usual “don't allow checkouts
> from dirty workdirs unless the dirty-ing changes are easily applied to
> the target tree”.

Nope, the target tree will be removed completely and everything in
it is silently nuked. It should be allowed with '-f', but only if
the submodule contains a gitfile, and never if it contains a .git
directory (which is just what we do for rm too).

> Are we waiting to land this series (or a successor) before starting on
> a fix for this issue?

I think so, as this bug is there for a long time (so I see no urge
to fix it very soon) and my test harness is intended to document
this current bug (and then soon its fix).

>> *) Forced work tree updates happily manipulate files in the
>>    directory of a submodule that has just been removed in the
>>    superproject (but is of course still present in the work tree due
>>    to the way submodules are currently handled). This becomes
>>    dangerous when files in the submodule directory are overwritten
>>    by files from the new superproject commit, as any modifications
>>    to the submodule files will be lost) and is expected to also
>>    destroy history in the - admittedly unlikely case - the new
>>    commit adds a file named ".git" to the submodule directory.
>> …
>> I'm not so sure about the second one. Even though I believe the
>> current behavior is not correct (switching commits should never mess
>> around in a submodule directory)
> 
> This should also be covered by the usual “don't allow checkouts from
> dirty workdirs unless the dirty-ing changes are easily applied to the
> target tree”.  We don't implement this yet, but I'd like to force
> users to move any about-to-be-clobbered state from their submodule
> into .git/modules/<name>/ (via commits or stashes) before allowing
> them to begin the checkout.  Once we've ensured that the state is
> preserved out-of-tree, then clobber away ;).

I'm intending to fix this in the recursive checkout series, as I'm
a) not sure if any users currently depend on that for a submodule
to directory transition and b) recursive checkout is the place to
consistently care about submodule modifications (the submodule
script doesn't do that and it is impossible to change that without
causing trouble to a lot of users.