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

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

Am 12.12.2012 20:32, schrieb Junio C Hamano:
> 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)

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

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