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

>
> On 5/22/2020 10:26 AM, Elijah Newren wrote:
>
> Sorry I missed this patch. I was searching all over for patches with
> "sparse" or "submodule" in the _subject_. Thanks for calling out the
> need for review, Junio!
>
> > Subject: [PATCH] git-sparse-checkout: clarify interactions with submodules
> >
> > Ignoring the sparse-checkout feature momentarily, if one has a submodule and
> > creates local branches within it with unpushed changes and maybe adds some
> > untracked files to it, then we would want to avoid accidentally removing such
> > a submodule.  So, for example with git.git, if you run
> >    git checkout v2.13.0
> > then the sha1collisiondetection/ submodule is NOT removed even though it
> > did not exist as a submodule until v2.14.0.  Similarly, if you only had
> > v2.13.0 checked out previously and ran
> >    git checkout v2.14.0
> > the sha1collisiondetection/ submodule would NOT be automatically
> > initialized despite being part of v2.14.0.  In both cases, git requires
> > submodules to be initialized or deinitialized separately.  Further, we
> > also have special handling for submodules in other commands such as
> > clean, which requires two --force flags to delete untracked submodules,
> > and some commands have a --recurse-submodules flag.
> >
> > sparse-checkout is very similar to checkout, as evidenced by the similar
> > name -- it adds and removes files from the working copy.  However, for
> > the same avoid-data-loss reasons we do not want to remove a submodule
> > from the working copy with checkout, we do not want to do it with
> > sparse-checkout either.  So submodules need to be separately initialized
> > or deinitialized; changing sparse-checkout rules should not
> > automatically trigger the removal or vivification of submodules.
>
> This is a good summary of how submodules decide to be present or not.
>
> > I believe the previous wording in git-sparse-checkout.txt about
> > submodules was only about this particular issue.  Unfortunately, the
> > previous wording could be interpreted to imply that submodules should be
> > considered active regardless of sparsity patterns.  Update the wording
> > to avoid making such an implication.  It may be helpful to consider two
> > example situations where the differences in wording become important:
>
> You are correct, the wording was unclear. Worth fixing.
>
> > In the future, we want users to be able to run commands like
> >    git clone --sparse=moduleA --recurse-submodules $REPO_URL
> > and have sparsity paths automatically set up and have submodules *within
> > the sparsity paths* be automatically initialized.  We do not want all
> > submodules in any path to be automatically initialized with that
> > command.
>
> INTERESTING. You are correct that it would be nice to have one
> feature that describes "what should be present or not". The in-tree
> sparse-checkout feature (still in infancy) would benefit from a
> redesign with that in mind.
>
> I am interested as well in the idea that combining "--sparse[=X]"
> with "--recurse-submodules" might want to imply that the submodules
> themselves are initialized with sparse-checkout patterns.
>
> These ramblings are of course off-topic for the current patch.

> > Similarly, we want to be able to do things like
> >    git -c sparse.restrictCmds grep --recurse-submodules $REV $PATTERN
> > and search through $REV for $PATTERN within the recorded sparsity
> > patterns.  We want it to recurse into submodules within those sparsity
> > patterns, but do not want to recurse into directories that do not match
> > the sparsity patterns in search of a possible submodule.
>
> (snipping way the old paragraph and focusing on the new text)
>
> > +If your repository contains one or more submodules, then those submodules
> > +will appear based on which you initialized with the `git submodule`
> > +command.
>
> This sentence is awkward. Here is a potential replacement:
>
>   If your repository contains one or more submodules, then submodules are
>   populated based on interactions with the `git submodule` command.
>   Specifically, `git submodule init -- <path>` will ensure the submodule at
>   `<path>` is present while `git submodule deinit -- <path>` will remove the
>   files for the submodule at `<path>`. Similar to sparse-checkout, the
>   deinitialized submodules still exist in the index, but are not present in
>   the working directory.
>
> That got a lot longer as I was working on it. Perhaps add a paragraph break
> before the next bit.

> >  Submodules may have additional untracked files or code stored on
>
> To emphasize the importance of the following "to avoid data loss" statement,
> you could mention that when a submodule is removed from the working directory,
> then so is all of its Git data such as objects and branches. If that data was
> not pushed to another repository, then deinitializing a submodule can result
> in loss of important data. (Also: maybe I'm wrong about that?)
>
> > +other branches, so to avoid data loss, changing sparse inclusion/exclusion

> > +rules will not cause an already checked out submodule to be removed from
> > +the working copy.  Said another way, just as `checkout` will not cause
> > +submodules to be automatically removed or initialized even when switching
> > +between branches that remove or add submodules, using `sparse-checkout` to
> > +reduce or expand the scope of "interesting" files will not cause submodules
> > +to be automatically deinitialized or initialized either.  Adding or
> > +removing them must be done as a separate step with `git submodule init` or
> > +`git submodule deinit`.
>
> This final sentence may be redundant if you include reference to init/deinit
> earlier in the section.

> > +This may mean that even if your sparsity patterns include or exclude
> > +submodules, until you manually initialize or deinitialize them, commands
> > +like grep that work on tracked files in the working copy will ignore "not
> > +yet initialized" submodules and pay attention to "left behind" ones.
>
> I don't think that "left behind" is a good phrase here. It feels like
> they've been _dropped_ instead of _persisted despite sparse-checkout
> changes_.

> Perhaps:
>
>   commands like `git grep` that work on tracked files in the working copy
>   will pay attention only to initialized submodules, regardless of the
>   sparse-checkout definition.