Re: [PATCH] submodule helper: accept '.' for repositories with no submodules

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 22, 2016, 20:06 UTC
Message-ID: <xmqqoaa6gtuu.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAGZ79kaMa3CyJ-Y9Xn9QShzTUGythos+pmKZXQ7m-2c0bF0u9A@mail.gmail.com>

Stefan Beller <sbeller@google.com> writes:

> Maybe we can also special case the "force" argument only here for
> Cedars use case.  ("git submodule deinit ." complains because
> there are no further submodules, but -f solves the complaint?)

I think that would have been the most sensible thing to do when we were adding "git submodule deinit", and it would be the most sensible behaviour in the longer term.

> So I have 2 goals in mind:

> * Git suggested to use '.' explicitly, so it should just work
> --even for a completely empty repository (see the test for it)

I actually view this as either a low-prio goal or even a non-goal, as long as we have a sensible migration plan to stop suggesting '.'.

> * Eventually -- not in this patch, but a later patch targeted
> at master -- we want to remove the recommendation to use '.',
> and allow no arguments or a different argument for "all
> submodules".

> git add uses '.' for it though, so '.' seems right and valid.
> git add '*' is also valid.

Perhaps I am misunderstanding what you want. Do you want a regular file that happens to match pathspec to prevent module_list from noticing "pathspec did not match" situation and giving the user an error? That would be the "match-pathspec first and then mode-check" behaviour, that is the same as v2.6.x series [*1*].

Show 13 quoted lines

> Maybe combine the second idea with a slight refactoring of
> parse_pathspec, such that you can pass a callback function to
> parse_pathspec which can decided on each file if it is a file
> to inspect. (i.e. for our usecase we'd check for ce_mode to be
> GITLINK, another hypothetical use case would be using
> parse_pathspec for finding all files with a certain property,
> e.g. finding all files ending in .c or files written in all
> capital letters or such)
>
> Then you could do a conditional parse_pathspec over the partial
> repository which matched
> the additional filtering function.
>

I do not think that buys us much. You have already shown how to implement "filter first and then pathspec match" if a caller wants to (which turned out to be a regression in this case, but that is besides the point).

[Footnote]

*1* If not, then similaritly with "git add" does not have much
    to do with what module_list() should do.  "git add $pathspec"
    fails if there is nothing that matches the pathspec, but
    "module_list" wants to complain if $pathspec does not match
    any submodule.  And it is an accident that "git add ." in
    an empty directory does not complain (indeed "git add '*'"
    does complain because it sidesteps the accident).