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

From: Elijah Newren <newren@gmail.com>
Date: May 21, 2020, 07:26 UTC
Message-ID: <CABPp-BFiPO_Hg-F2edv9GVQSqJCNj6NvOaB5Ez8UUzAMPdTqUw@mail.gmail.com>
In-Reply-To: <xmqqsgg4wuwt.fsf@gitster.c.googlers.com>

On Tue, May 12, 2020 at 5:17 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 10 quoted lines

>
> Matheus Tavares Bernardino <matheus.bernardino@usp.br> writes:
>
> > The idea behind not skipping gitlinks here was to be compliant with
> > what we have in the working tree. In 4fd683b ("sparse-checkout:
> > document interactions with submodules"), we decided that, if the
> > sparse-checkout patterns exclude a submodule, the submodule would
> > still appear in the working tree. The purpose was to keep these
> > features (submodules and sparse-checkout) independent. Along the same
> > lines, I think we should always recurse into initialized submodules in

Sorry if I missed it in the code, but do you check whether the submodule is initialized before descending into it, or do you descend into it based on it just being a submodule?

Show 12 quoted lines

> > grep, and then load their own sparsity patterns, to decide what should
> > be grepped within.
>
> OK.
>
> I do not necessarily agree with the justification described in
> 4fd683b (e.g. "would easily cause problems." that is not
> substantiated is merely an opinion), but I do agree with you that
> the new code in "git grep" we are discussing here does behave in
> line with that design.
>
> Thanks.

I'm also a little worried by 4fd683b; are we headed towards a circular
reasoning of some sort?  In particular, sparse-checkout was written
assuming submodules might already be checked out.  I can see how
un-checking-out an existing submodule could raise fears of losing
untracked or ignored files within it, or stuff stored on other
branches, etc.  But that's not the only relevant case.  What if
someone runs:
   git clone --recurse-submodules --sparse=moduleA git.hosting.site:my/repo.git
In such a case, we don't have already checked out submodules.
Obviously, we should clone submodules that are within our sparsity
paths.  But should we automatically clone the submodules outside our
sparsity paths?  The the logic presented in 4fd683b makes this
completely ambiguous.  ("It will appear if it's initialized."  Okay,
but do we initialize it?)

You may say that clone doesn't have a --sparse= flag right now.  So
let me change the example slightly.  What if someone runs
   git checkout --recurse-submodules $otherBranch
and $otherBranch adds a new submodule somewhere deep under a directory
excluded by the sparsity patterns (i.e. deep within a directory we
aren't interested in and don't have checked out).  Should the
submodule be checked out, i.e. should it be initialized?  Commit
4fd683b only says it will appear if it's initialized, but my whole
question is should we initialize it?