Re: [PATCH] update-index: allow overwriting existing submodule index entries

From: Jens Lehmann <jens.lehmann@web.de>
Date: Jun 12, 2012, 21:18 UTC
Message-ID: <4FD7B20D.9080803@web.de>
In-Reply-To: <20120612203316.GA17053@book.hvoigt.net>

Am 12.06.2012 22:33, schrieb Heiko Voigt:

Show 25 quoted lines

> Here a quick idea of what we could do:
> 
> We could mark a path as transitioned from submodule (similar to the
> assume unchanged bit) if files were skipped due to removal of a
> submodule and have submodule update clear that flag. That way we could
> teach diff, status and so on to only show one entry for a submodule to
> be removed and replaced with something else.
> 
> Thinking further: We could actually prevent adding such an out-of-date
> submodule as a safeguard. Which in fact was something which happened by
> mistake to some of our users. The story is that when you see a *changed*
> submodule in a merge conflict it can be easily mistaken for needing
> resolution and added to the merge. Most times this rewinds your
> submodule to some old state
> 
> If we agree on how to handle the submodule => file(s) cases I think the
> implementation in the submodule script possibly requires less work than
> the fully fledged recursive checkout and could also be used for gaining
> some experience with such transitions.
> 
> So the first step would be to extend submodule update to support the
> removal of submodules. The next step is then that we finish the fully
> automatic recursive submodule checkout.
> 
> What do you think of such a two step plan?

Hmm, I suspect the real problem here is that "git submodule update" is run *after* the actual checkout, merge, reset or whatever. So if for example you want to replace a submodule with a plain directory containing the same files the submodule update would not only have to remove the now obsolete submodule but also has to remember to check out all files in the former submodule directory again. IMO this should be part of the initial unpack_trees(), not done in a later script. Imagine a submodule having local modifications which would be overwritten by the checkout, then the later submodule update would have to fail (when used without -f) to protect the users changes. The only sane thing to do would be to not allow such a checkout in the first place (when the user chose to automatically update submodules that is) but abort just like we do for changes to regular files right now telling the user to save his changes.

And I suspect you would have to teach at least status and diff to give meaningful output in that "submodule should be removed and replaced with something else but submodule update hasn't run yet" case, and apart from the change to add you mentioned above some other commands might need safeguards too.

So me thinks we should skip step one and implement recursive checkout right away. Adding much more complexity to the submodule script for an intermediate solution doesn't sound like a good idea to me.