Re: [RFC PATCH v2 3/4] grep: honor sparse checkout patterns

[Matheus Tavares Bernardino <matheus.bernardino@usp.br>]

Hi, Elijah

On Fri, May 22, 2020 at 12:36 PM Elijah Newren <newren@gmail.com> wrote:
>
> On Fri, May 22, 2020 at 7:26 AM Elijah Newren <newren@gmail.com> wrote:
> >
> > Hi Matheus,
> >
> > On Thu, May 21, 2020 at 10:49 PM Matheus Tavares Bernardino <matheus.bernardino@usp.br> wrote:
> > >
> > > On Thu, May 21, 2020 at 2:52 PM Elijah Newren <newren@gmail.com> wrote:
> > > >
> <snip>
> > > Does this seem like a good approach? Or is there another solution that
> > > I have not considered? Or even further, should we choose to skip the
> > > submodules in excluded paths? My only concern in this case is that it
> > > would be contrary to the design in git-sparse-checkout.txt. And the
> > > working tree grep and cached grep would differ even on a clean working
> > > tree.
> >
> <snip>
> > Anyway, the wording in that file seems to be really important, so
> > let's fix it.
> >
>
> Let me also try to give a concrete proposal for grep behavior for the
> edge cases we've discussed:

Thank you for this proposal and for the previous comments as well.

> git -c sparse.restrictCmds=true grep --recurse-submodules $PATTERN
>
> This goes through all the files in the index (i.e. all tracked files)
> which do not have the SKIP_WORKTREE bit set.  For each of these: If
> the file is a symlink, ignore it (like grep currently does).  If the
> file is a regular file and is present in the working copy, search it.
> If the file is a submodule and it is initialized, recurse into it.

Sounds good. And when sparse.restrictCmds=false, we also search the
present files and present initialized submodules that have the
SKIP_WORKTREE set, right?

> git -c sparse.restrictCmds=true grep --recurse-submodules --cached $PATTERN
>
> This goes through all the files in the index (i.e. all tracked files)
> which do not have the SKIP_WORKTREE bit set.  For each of these: Skip
> symlinks.  Search regular files.  Recurse into submodules if they are
> initialized.

OK.

> git -c sparse.restrictCmds=true grep --recurse-submodules $REVISION $PATTERN
>
> This goes through all the files in the given revision (i.e. all
> tracked files) which match the sparsity patterns (i.e. that would not
> have the SKIP_WORKTREE bit set if were we to checkout that commit).
> For each of these: Skip symlinks.  Search regular files.  Recurse into
> submodules if they are initialized.

OK.

> Further, for any of these, when recursing into submodules, make sure
> to load that submodules' core.sparseCheckout setting (and related
> settings) and the submodules' sparsity patterns, if any.
>
> Sound good?
>
> I think this addresses the edge cases we've discussed so far:
> interaction between submodules and sparsity patterns, and handling of
> files that are still present despite not matching the sparsity
> patterns. (Also note that files which are present-despite-the-rules
> are prone to be removed by the next `git sparse-checkout reapply` or
> anything that triggers a call to unpack_trees(); there's already
> multiple things that do and Stolee's proposed patches would add more).
> If I've missed edge cases, let me know.

Sounds great. This addresses all the edge cases we've mentioned
before. Thanks again for the detailed proposal, and for considering
case by case.