Re: [PATCH v5 3/3] builtin/grep.c: walking tree instead of expanding index with --sparse

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

Derrick Stolee <derrickstolee@github.com> writes:

> On 9/8/2022 1:59 PM, Junio C Hamano wrote:
>> Shaoxuan Yuan <shaoxuan.yuan02@gmail.com> writes:
>> 
>>> +
>>> +	/*
>>> +	 * NEEDSWORK: when reading a submodule, the sparsity settings in the
>>> +	 * superproject are incorrectly forgotten or misused. For example:
>>> +	 *
>>> +	 * 1. "command_requires_full_index"
>>> +	 * 	When this setting is turned on for `grep`, only the superproject
>>> +	 *	knows it. All the submodules are read with their own configs
>>> +	 *	and get prepare_repo_settings()'d. Therefore, these submodules
>>> +	 *	"forget" the sparse-index feature switch. As a result, the index
>>> +	 *	of these submodules are expanded unexpectedly.
>>  ...
> I think this comment is misplaced. It should either be contained in
> the commit message or placed closer to this diff hunk:

OK, so given what you wrote below, except for such a minor
shuffling, the current series is ready to go?

Thanks.

> ...
> Shaoxuan's comment is attempting to list the reasons why submodules
> do not currently work with sparse-index, and specifically that we
> can add tests that _should_ exercise this code in a meaningful way,
> but because of the current limitations of the codebase, the code
> isn't actually exercised in that scenario.
>
> In order to actually create a test that demonstrates how submodules
> and sparse-checkout work with this logic, we need to do some serious
> refactoring of the sparse-checkout logic to care about the repository
> struct, along with some other concerns specifically around the sparse
> index. This doesn't seem appropriate for the GSoC timeline or even for
> just this topic.