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

From: Victoria Dye <vdye@github.com>
Date: Apr 20, 2023, 15:26 UTC
Message-ID: <069a53ef-63b8-c1e3-7502-6728bda50665@github.com>
In-Reply-To: <CAMO4yUF1P1Sv1aVJ1aw9US-QeNYD-GfaS7ndr=bwp-dgvOyexA@mail.gmail.com>

Shuqi Liang wrote:

> Hi Victoria,
> 
> Sorry for the late reply. I'm still in the middle of my final exams period.

No problem at all, thanks for following up!

Show 9 quoted lines

> It's true that in the case of 'diff-files', expanding the sparse directory to
> match a pathspec to its contents might not be necessary. If we don't use
> pathspec expansion in this case. It could optimize for performance better.
> 
> However, there could be some edge cases. if a user manually modifies the
> contents of a SKIP_WORKTREE file in the working tree, the diff between
> the working tree and the index would no longer be empty. So I think, In this
> case, expanding the sparse directory might still be necessary to ensure the
> correct behavior of the 'diff-files' command.

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/