Re: [PATCH] submodule helper list: Respect correct path prefix

[Junio C Hamano <gitster@pobox.com>]

Stefan Beller <sbeller@google.com> writes:

> On Wed, Feb 24, 2016 at 1:21 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Stefan Beller <sbeller@google.com> writes:
>>
>>> This is a regression introduced by 74703a1e4d (submodule: rewrite
>>> `module_list` shell function in C, 2015-09-02).
>>>
>>> Add a test to ensure we list the right submodule when giving a specific
>>> path spec.
>>>
>>> Reported-By: Caleb Jorden <cjorden@gmail.com>
>>> Signed-off-by: Stefan Beller <sbeller@google.com>
>>> ---
>>>
>>>  I developed this on top of current origin/master, though I can backport it
>>>  to 2.7 as well if desired.
>>>
>>>  I do not remember the cause why we started to ignore a common prefix.
>>
>> The code you are removing with this patch is probably an
>> optimization you copied from builtin/ls-files.c.  When the
>> optimization is used, the original also limits the list of paths to
>> those that match the prefix by calling prune_cache(), but perhaps
>> you didn't have a corresponding code in your copy?
>
> I think that is a good explanation. So do we want to add the pruning
> or use this patch to fixup the regression and wait until someone complains
> about the speed penalty due to no optimization?

As I do not know offhand if the optimization, especially the pruning
part, applies to the context of this code the same way ls-files does
things (which treats the index read into core as a throw-away data),
we shouldn't even attempt to salvage the faulty half-optimization
until we understand what it involves to make it work.  So "disable
broken optimization and make simple way work correctly" is the good
first step, especially for a fix that is meant to go to 2.7.x
series.

We must first be sure that removing the faulty half-optimization is
the only thing we need to fix this breakage, though ;-)

Thanks.