Re: [PATCH v2] submodule: add 'deinit' command

From: Jens Lehmann <jens.lehmann@web.de>
Date: Dec 12, 2012, 22:25 UTC
Message-ID: <50C90469.8080303@web.de>
In-Reply-To: <7vr4mv3w2x.fsf@alter.siamese.dyndns.org>

Am 12.12.2012 20:32, schrieb Junio C Hamano:

Show 7 quoted lines

> Jens Lehmann <Jens.Lehmann@web.de> writes:
> 
>> Especially as I suspect the number of submodule users having
>> customized those in .git/config is not that high ...
> 
> I thought the point of "deinit" was to say "I am not interested in
> having a checkout of these submodules in my working tree anymore".

Yes. (But I'm not sure users expect that command to also remove the work tree)

Show 6 quoted lines

> The user could do "rm -fr submodule && mkdir submodule" to remove it
> locally and keep "diff" and "status" from noticing the removal, but
> the primary reason the user needs an explicit "deinit" is because
> many subcommands of "git submodule" are documented to operate on all
> submodules that have been "init"ed when given no explicit submodule
> names [*1*].

The real reason we need deinit is that the next run of "submodule update" will otherwise happily recreate the submodule checkout you just removed as long as it finds the url setting in .git/config.

> Your "deinit" is documented not to actually remove the submodule
> checkout, but that very much goes against my intuition.  What is the
> justification behind that choice?

I thought it should match what "submodule init" does, which is to do nothing to the work tree until the next "submodule update" is run. (But I agree that analogy is somewhat flawed until we teach "update" to remove a deinitialized submodule - or maybe teach it the --deinit option which could do both). On the other hand with current git submodule work trees always stay around anyway until you remove them by hand (e.g. when you switch to a branch that doesn't have it), so I'm not sure what would surprise people more here. So I just left the work tree unchanged.

> "We'll remove the configuration,
> you remove the tree yourself" will invite the mistake of running
> "git rm" on it, which you wanted to avoid with the addition to the
> "git rm" documentation, no?

I think that'll happen only if git would remind them that they still have a populated work tree, which I believe it shouldn't.

Show 7 quoted lines

> [Footnote]
> 
> *1* In reality, the code looks at presense of .git in the submodule
> path to decide if it has been "init"ed (cf. cmd_update), but this
> implementation of "deinit" does not seem to cause that .git to be
> removed, leaving the submodule in "init"ed state from these other
> command's perspective.

Nope, cmd_update() checks first if the url is found in .git/config and skips the submodule if not. I rechecked and only "summary" and "foreach" still recurse into a deinitialized submodule, which they shouldn't. But a quick test shows that "git status" and "git diff" also still inspect a deinitialized submodule, so there's some work left to do to handle the case where the work tree is not removed.

So unless people agree that deinit should also remove the work tree I'll prepare some patches teaching all git commands to consistently ignore deinitialized submodules. Opinions?