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

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 24, 2016, 21:38 UTC
Message-ID: <xmqqpovlssar.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAGZ79kbbKnips12CU6KZX39rAZ_O-pYy20nsSGCCf+1w5LzgxA@mail.gmail.com>

Stefan Beller <sbeller@google.com> writes:

Show 27 quoted lines

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