Re: git-grep in sparse checkout

On Tue, Oct 1, 2019 at 3:29 PM Derrick Stolee [off-list ref] wrote:

On 10/1/2019 9:06 AM, Matheus Tavares Bernardino wrote:

quoted

Hi,

During Git Summit it was mentioned that git-grep searches outside
sparsity pattern which is not aligned with user expectation. I took a
quick look at it and it seems the reason is
builtin/grep.c:grep_cache() (which also greps worktree) will grep the
object store when a given index entry has the CE_SKIP_WORKTREE bit
turned on.

From what I understand, this bit is used exactly for sparse checkouts
(as described in Documentation/technical/index-format.txt[1]). But
should we perhaps ignore it in git-grep to have the expected behavior?
I'll be happy to send the patch if so, but I wanted to check with you
first.

Is that the expected behavior? In a sparse-checkout, wouldn't you _want_
Git to report things outside the cone? You can already use external tools
to search for things in the sparse cone: they are on disk. You need "git
grep" for the objects reachable from the current tree but not already
on disk.

Hmm, we can use external tools to search in the sparse cone, but even
in this circumstance, I think we need git-grep for some usecases.
git-grep on disk can exclude files not being tracked and access other
git functionalities such as --textconv to read .gitattributes and
convert the files before grepping. So maybe people would want to grep
just the cone but also have these other options that would be
unavailable through external tools?

(Users could already simulate such behavior giving the sparse clone
patterns as pathspecs to git-grep, but that can get complicated for
more complex sparse patterns.)

I respect the goal to minimize the work "git grep" is doing, especially
in a sparse-checkout + partial-clone world, where we wouldn't expect to
have the blobs locally and this search would cause many blob downloads.
I just want to truly examine if this is the right behavior.

At minimum, I would expect a new option to have "git grep" go back to
the old behavior, so users who really want a tree-wide search can have
one.

Yes, I totally agree that this behavior change should come with such
an option. Maybe --ignore-sparsity-patterns, as Elijah suggested.

Thanks,
-Stolee