Re: [PATCH] submodule helper: accept '.' for repositories with no submodules
- From
Stefan Beller <sbeller@google.com>
- Date
- Mar 22, 2016, 21:16 UTC
- Message-ID
- <CAGZ79ka=EJ-L+Zaqh0kWUdjmyUva3fEcbwJn18uxshe3R5ABBQ@mail.gmail.com>
- In-Reply-To
- <xmqqoaa6gtuu.fsf@gitster.mtv.corp.google.com>
On Tue, Mar 22, 2016 at 1:06 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 53 quoted lines
> 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*].
>
>> 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).And by including this filtering into the pathspec machine we can pass a flag DONT_BREAK_ON_NO_FILTER_RESULTS_WHEN_HAVING_OTHER_MATCHES (name for illustration purpose only ;) which is how I understand this regression?
Show 14 quoted lines
> > > > [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). > >