Subject: Re: [PATCH v6 1/1] builtin/grep.c: integrate with sparse index

From: Junio C Hamano <[email-masked]>
Date: Mon, 26 Sep 2022 10:28:13 -0700

Junio C Hamano <[email-masked]> writes:

> Shaoxuan Yuan <[email-masked]> writes:
>
>> +test_expect_success 'grep with and --cached' '
>
> "with and --cached"?  "with and without --cached" is probably a good
> thing to test but you may need to add tests for "with" case, too?

I meant "for WITHOUT case, too", but ...

>> +	init_repos &&
>> +
>> +	test_all_match git grep --cached a &&
>> +	test_all_match git grep --cached a -- "folder1/*"
>> +'
>
> The above is very relevant for the purpose of ...
>
>> -	/* TODO: audit for interaction with sparse-index. */
>> -	ensure_full_index(repo->index);
>
> ... auditing.  Run the command with a pathspec that specify areas
> inside and outside the sparse cone(s) and ensure the result match
> those in a non-sparse-index, with test_all_match().
>
> As to the lack of the tests WITHOUT "--cached", I suspect that it is
> omitted because there is no checked-out copies to grep in, but I
> suspect that it is papering over a buggy design.

... in light of the recent "sparse-checkout.txt: ... directions"
document patch by Elijah

  http://lore.kernel.org/git/[email-masked]/

I think I was quite mistaken.  The guiding principle should not be
to pretend that the paths stubbed out with sparse checkout mechanism
are unchanged from HEAD.  It should be to pretend that they do not
exist and they never existed.

So it is perfectly expected that the output with and without
"--cached" are different.  The former (without an option to ignore
paths outside the sparse checkout even for in-repository data)
should find stuff from in-tree, while the latter should look for
things only in the checked out files.