Re: git submodule deinit resulting in BUG: builtin/submodule--helper.c:1045: module_list_compute should not choke on empty pathspec

[Stefan Beller <sbeller@google.com>]

On Wed, Mar 28, 2018 at 12:37 PM, Peter Oberndorfer <kumbayo84@arcor.de> wrote:

>>> 2) Should "git submodule deinit" work on submodules that were removed by
>> upstream already?
>>
>> To answer the question "Is this a submodule that upstream removed
>> (recently)?"
>> we'd have to put in some effort, essentially checking if that was ever a
>> submodule
>> (and not a directory or file).
>>
>
> Hmm, yeah looks a bit more complicated than I initially imagined
> since submodules can have a name that's different from their path.
> And after the rebase, the name <-> path mapping via .gitmodules is not available anymore.
>
> Naively I think it could work the following way:
> * Either iterate over all submodules in .git/modules/ and check their config
>   has a worktree = "../../path" that resolves to the submodule path we want to remove.

This would work but scales linearly with the number of submodules.


> * Or check the "gitlink:" path in submodule/.git if it points to our .git/modules/
> Then if .git/config contains a [submodule "name"] entry
> we should have a pretty good idea if this folder contains a stale submodule.

If you move a submodule a directory up or down, the relative path is not exact
any more, we'd need to check for the last part to loosely match.


>> When using "git pull --recurse-submodules" the submodule ought to be removed
>> automatically.
>>
>> When doing a fetch && merge manually, we may want to teach merge to remove
>> a submodule that we have locally upon merge, too.
>>
>
> Yeah that would be nice :-)
> In my case I updated the repository via a rebase, so that would also have to be covered.

Oh rebase itself has not yet learned about recursion into submodules.
("git pull --rebase --recurse-submodules" is a thing though)

>> I view the git-submodule command as a bare bones plumbing helper, that we'd
>> want
>> to deprecate eventually as all other higher level commands will know how to
>> deal
>> with submodules.
>>
>> So I think we do not want to teach "git submodule deinit" to remove dormant
>> repositories, that were submodules removed by upstream already.
>>
>
> My gut feeling makes me expect the following:
> * It would be nice if such stale submodules showed up in "git submodule status" or "git status"
>   Now "git submodule" shows nothing related to this stale submodule

That has currently only two ways "+" or "-" for there/not there.
Maybe we'd need to add some characters similar to "git status --porcelain"
such as "?"

>   Now "git status" shows  Untracked files: src/rt which is a bit confusing as the actual submodule is in src/rt/hoedown
>   Now "Git gui" shows src/rt/hoedown as untracked git repository

hm. The current state of affairs doesn't sound intriguing.
Though, I think we'd want to step back one more step and rather want
to ask how a dormant submodule comes into existence, instead of
just improving the reporting. Reportingthem is of course also important,
but in the long run I'd rather want to have situations like these happen
less often. When upstream deletes a file, they are also not required to be
deleted manually, but merge/checkout would take care of them.

> * There should be an easy(and safe) way for the user to deinit such a submodule
>   if if the automatic submodule updating during a merge/rebase was not enabled or somehow failed.
> (Minus the problem of somebody having to actually do the work...)
>
>>> ~/src/rust/rust$ git submodule status
>> ...
>>>   b87873eaceb75cf9342d5273f01ba2c020f61ca8 src/tools/lld ((null))
>>
>>> -> strangely I get (null) for the current branch/commit in some
>> submodules?
>>
>> This sounds like (3). Looking into that.
>
> Sorry, what do you mean by (3)?

I meant the ((null)) issue is another third thought that we can
discuss separately,
slightly unrelated to the others (that you marked as (1) and (2))

Thanks,
Stefan