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

From: Derrick Stolee <derrickstolee@github.com>
Date: Sep 9, 2022, 12:49 UTC
Message-ID: <2badb772-2aa2-460c-818b-5aab8497000c@github.com>
In-Reply-To: <xmqqo7vpoaan.fsf@gitster.g>

On 9/8/2022 4:56 PM, Junio C Hamano wrote:

Show 18 quoted lines

> Derrick Stolee <derrickstolee@github.com> writes:
> 
>> HOWEVER: it "doesn't matter" because the sparse index doesn't work
>> at all within a submodule. Specifically, if a super-repo does not
>> enable sparse-checkout, but the submodule _does_, then we don't
>> know how Git will behave currently. His reasonings go on to explain
>> why the situation is fraught:
>>
>> * command_requires_full_index is set in a builtin only for the
>>   top-level project, so when we traverse into a submodule, we don't
>>   re-check if the current builtin has integrated with sparse index
>>   and expand a sparse index to a full one.
> 
> Correct.  
> 
> Is it sufficient to propagate the bit from the_repository->settings
> to repo->settings of the submodule, or is there more things needed
> to fix it?

Likely that would suffice, but before we do that, we need to add a
lot of tests to be sure our previous sparse index integrations do
the right thing when within submodules.

Show 8 quoted lines

>> * core_apply_sparse_checkout is a global not even associated with
>>   a repository struct. What happens when a super project is not
>>   sparse but a submodule is? Or vice-versa? I honestly don't know,
>>   and it will require testing to find out.
> 
> Naïvely, I would think that we should just treat a non-sparse case
> as a mere specialization where the sparse cone covers everything,
> but there may be pitfalls.

I worry about how this works if the super-project and the submodule differ in the core.sparseCheckout config, but both have sparse-checkout files. Will one or the other cause the sparse-checkout patterns to be enabled despite the repo-local config? I honestly have no idea, and I don't think we have tests that protect this scenario. That's the kind of direction I would start in this investigation.

Thanks, -Stolee