Subject: Re: [PATCH v2 09/19] ls-files.c: use index api

From: Thomas Gummerer <[email-masked]>
Date: Wed, 17 Jul 2013 10:07:07 +0200

Duy Nguyen <[email-masked]> writes:

> On Sat, Jul 13, 2013 at 12:26 AM, Thomas Gummerer <[email-masked]> wrote:
>> +       if (!with_tree) {
>> +               memset(opts, 0, sizeof(*opts));
>> +               opts->pathspec = &pathspec_struct;
>> +               opts->read_staged = 1;
>> +               if (show_resolve_undo)
>> +                       opts->read_resolve_undo = 1;
>> +               read_cache_filtered(opts);
>
> So you load partial index here.
>
>> +       } else {
>> +               read_cache();
>> +       }
>> +       /* be nice with submodule paths ending in a slash */
>> +       if (pathspec)
>> +               strip_trailing_slash_from_submodules();
>
> Then strip_trailing_slash_from_submodules will attempt to convert
> pathspec "foo/" to "foo" if "foo" exists in the index and is a
> gitlink. But becaues "foo/" is used to load the partial index, "foo"
> is not loaded (is it?) and this could become incorrect no-op. I
> suggest you go through the pathspec once checking for ones ending with
> '/'. If so strip_trailing_... may potentially update pathspec, just
> load full index. If no pathspec ends with '/', strip_trail... is no-op
> and we can do partial loading safely.

It was loaded, because the adjusted_pathspec algorithm stripped the
trailing slash and then everything until the next slash.  That's
overkill except when the trailing slash had to be stripped.

I'll make the adjusted_pathspec algorithm more restrictive, so the last
trailing slash is no longer stripped.  If a pathspec contains a trailing
slash I'll load the whole index, as you suggested.