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:

> 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?

> * 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.

> Shaoxuan's comment is attempting to list the reasons why submodules
> do not currently work with sparse-index,

"do not currently work" in a sense that it produces wrong result, or
it just expands in-core index unnecessarily before applying pathspec
to produce the right result in an inefficient way?

If it is "functionally broken", is there a simple way out to give us
correct result even if it becomes less efficient?  Like "we scan the
index and we see we have some submodules---so we disable the sparse
handling"?

> Victoria and I have noted this issue down and will try to find time
> to investigate further, with a target of being able to actually
> exercise this grep_tree() call within a sparse index in a submodule,
> giving us full confidence that name_base_len is the correct value to
> put in that parameter.

OK.