Re: git-grep in sparse checkout

From: Matheus Tavares Bernardino <matheus.bernardino@usp.br>
Date: Oct 2, 2019, 00:06 UTC
Message-ID: <CAHd-oW6XUFO2mG_5kiqsTYuh998j95Sg_6zBBE4sXTtCJ3dPDA@mail.gmail.com>
In-Reply-To: <f3101c8f-9709-5bcb-35f0-39ffaf8aa809@gmail.com>

On Tue, Oct 1, 2019 at 3:29 PM Derrick Stolee <stolee@gmail.com> wrote:

Show 22 quoted lines

>
> On 10/1/2019 9:06 AM, Matheus Tavares Bernardino wrote:
> > 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.)

Show 8 quoted lines

> 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