Re: [PATCH v5 3/3] builtin/grep.c: walking tree instead of expanding index with --sparse
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 8, 2022, 20:56 UTC
- Message-ID
- <xmqqo7vpoaan.fsf@gitster.g>
- In-Reply-To
- <093827ae-41ef-5f7c-7829-647536ce1305@github.com>
Derrick Stolee <derrickstolee@github.com> writes:
Show 10 quoted lines
> 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"?
Show 5 quoted lines
> 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.