Re: [PATCH v7 2/2] diff-files: integrate with sparse index

[Victoria Dye <vdye@github.com>]

Shuqi Liang wrote:
> Hi Victoria,
> 
>> If a user manually modifies a SKIP_WORKTREE file, SKIP_WORKTREE will be
>> removed from the file and the index expanded automatically [1]. If that
>> mechanism is working properly, there would be no need to manually check the
>> pathspec and expand the index.
>>
>> Have you tried removing the 'pathspec_needs_expanded_index()' and running
>> the tests? If so, is 'diff-files' producing incorrect results?
>>
>> [1] https://lore.kernel.org/git/11d46a399d26c913787b704d2b7169cafc28d639.1642175983.git.gitgitgadget@gmail.com/
> 
> As per your suggestion, I tried removing pathspec_needs_expanded_index()
> from the code, and 'diff-files pathspec expands index when necessary'
> test failed.
> 
> So, I'm thinking about keeping it to ensure everything works properly.
> I'd like to know your thoughts on this. Should we keep it or explore
> other options?

Did the test fail because the index wasn't expanded in a case where you
previously expended it to be expanded? Or because of the returned results of
'diff-files' are invalid?

Only the latter represents incorrect behavior. If we're aren't expanding the
index for a case that was causing index expansion before *and* the
user-facing behavior is as-expected, that's the best-case scenario for a
sparse index integration!

Taking a step back, it's important to remember that the overarching goal of
the project is not just to switch 'command_requires_full_index' to '0'
everywhere, but to find all of the places where Git is working with the
index and make sure that work can be done on a sparse directory.

In most cases, it's possible to adapt an index-related operation to work
with sparse directories (albeit with varying levels of complexity). The use
of 'ensure_full_index()' is reserved for cases where it is _impossible_ to
make Git perform a given action on a sparse directory - expanding the index
completely eliminates the performance gains had by using a sparse index, so
it should be avoided at all costs.

I hope that helps!

> 
> Thanks,
> Shuqi