git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [RFC PATCH 3/3] grep: add option to ignore sparsity patterns

From
Elijah Newren <newren@gmail.com>
Date
Mar 31, 2020, 19:12 UTC
Message-ID
<CABPp-BE6zW0nJSStcVU=_DoDBnPgLqOR8pkTXK3dW11=T01OhA@mail.gmail.com>
In-Reply-To
<CAHd-oW5qq=dtaSmqcSyxoHd3HPipxjfaiCXtdBQwk9yNVe9okA@mail.gmail.com>

// adding Jonathan Tan to cc based on the fact that we keep bringing up partial clones and how it relates...

On Sun, Mar 29, 2020 at 8:23 PM Matheus Tavares Bernardino <matheus.bernardino@usp.br> wrote:

Show 41 quoted lines
>
> On Tue, Mar 24, 2020 at 3:30 PM Junio C Hamano <gitster@pobox.com> wrote:
> >
> > Elijah Newren <newren@gmail.com> writes:
> >
> > > On Mon, Mar 23, 2020 at 11:13 PM Matheus Tavares
> > > <matheus.bernardino@usp.br> wrote:
> > >>
> > >> In the last commit, git-grep learned to honor sparsity patterns. For
> > >> some use cases, however, it may be desirable to search outside the
> > >> sparse checkout. So add the '--ignore-sparsity' option, which restores
> > >> the old behavior. Also add the grep.ignoreSparsity configuration, to
> > >> allow setting this behavior by default.
> > >
> > > Should `--ignore-sparsity` be a global git option rather than a
> > > grep-specific one?  Also, should grep.ignoreSparsity rather be
> > > core.ignoreSparsity or core.searchOutsideSparsePaths or something?
> >
> > Great question.  I think "git diff" with various options would also
> > want to optionally be able to be confined within the sparse cone, or
> > checking the entire world by lazily fetching outside the sparsity.
> [...]
> > Regardless of the choice of the default, it would be a good
> > idea to make the subcommands consistently offer the same default and
> > allow the non-default views with the same UI.
>
> Yeah, it seems like a sensible path. Regarding implementation, there
> is the question that Elijah raised, of whether to use a global git
> option or separate but consistent options for each subcommand. I don't
> have much experience with sparse checkout to argument for one or
> another, so I would like to hear what others have to say about it.
>
> A question that comes to my mind regarding the global git option is:
> will --ignore-sparsity (or whichever name we choose for it [1]) be
> sufficient for all subcommands? Or may some of them require additional
> options for command-specific behaviors concerning sparsity patterns?
> Also, would it be OK if we just ignored the option in commands that do
> not operate differently in sparse checkouts (maybe, fetch, branch and
> send-email, for example)? And would it make sense to allow
> constructions such as `git --ignore-sparsity checkout` or even `git
> --ignore-sparsity sparse-checkout ...`?

I think the same option would probably be sufficient for all subcommands, though I have a minor question about the merge machinery (below). And generally, I think it would be unusual for people to pass the command line flag; I suspect most would set a config option for most cases and then only occasionally override it on the command line. Since that config option would always be set, I'd expect commands that are unaffected to just ignore it (much like both "git -c merge.detectRenames=true fetch" and "git --work-tree=othertree fetch" will both ignore the irrelevant options rather than trying to detect that they were specified and error out).

Show 5 quoted lines
> [1]: Does anyone have suggestions for the option/config name? The best
> I could come up with so far (without being too verbose) is
> --no-sparsity-constraints. But I fear this might sound generic. As
> Elijah already mentioned, --ignore-sparsity is not good either, as it
> introduces double negatives in code...

Does verbosity matter that much? I think people would set it in config, and tab completion would make it pretty easy to complete in any event.

Anyway, maybe it will help if I provide a very rough first draft of what changes we could introduce to Documentation/config/core.txt, and then ask a bunch of my own questions about it below:

"""
core.restrictToSparsePaths::
        Only meaningful in conjuntion with core.sparseCheckoutCone.
        This option extends sparse checkouts (which limit which paths
        are written to the worktree), so that output and operations
        are also limited to the sparsity paths where possible and
        implemented.  The purpose of this option is to (1) focus
        output for the user on the portion of the repository that is
        of interest to them, and (2) enable potentially dramatic
        performance improvements, especially in conjunction with
        partial clones.
+
When this option is true, git commands such as log, diff, and grep may
limit their output to the directories specified by the sparse cone, or
to the intersection of those paths and any (like `*.c) that the user
might also specify on the command line.  (Note that this limit for
diff and grep only becomes relevant with --cached or when specifying a
REVISION, since a search of the working tree will automatically be
limited to the sparse paths that are present.)  Also, commands like
bisect may only select commits which modify paths within the sparsity
cone.  The merge machinery may use the sparse paths as a heuristic to
avoid trying to detect renames from within the sparsity cone to
outside the sparsity cone when at least one side of history only
touches paths within the sparsity cone (this can make the merge
machinery faster, but may risk modify/delete conflicts since upstream
can rename a file within the sparsity paths to a location outside
them).  Commands which export, integrity check, or create history will
always operate on full trees (e.g. fast-export, format-patch, fsck,
commit, etc.), unaffected by any sparsity patterns.
"""
Several questions here, of course:
  * do people like or hate the name?  indifferent?  have alternate ideas?
  * should we restrict this to core.sparseCheckoutCone as I suggested
above or also allow people to do it with core.sparseCheckout without
the cone mode?  I think attempting to weld partial clones together
with core.sparseCheckout is crazy, so I'm tempted to just make it be
specific to cone mode and to push people to use it.  But I'm
interested in thoughts on the matter.
  * should worktrees be affected?  (I've been an advocate of new
worktrees inheriting the sparse patterns of the worktree in use at the
time the new worktree was created.  Junio once suggested he didn't
like that and that worktrees should start out dense.  That seems
problematic to me in big repos with partial clones and sparse chckouts
in use.  Perhaps dense new worktrees is the behavior you get when
core.restrictToSparsePaths is false?)
  * does my idea for the merge machinery make folks uncomfortable?
Should that be a different option?  Being able to do trivial *tree*
merges for the huge portion of the tree outside the sparsity paths
would be a huge win, especially with partial clones, but it certainly
is different.  Then again, microsoft has disabled rename detection
entirely based on it being too expensive, so perhaps the idea of
rename-detection-within-your-cone-if-you-really-didn't-modify-anything-outside-the-cone-on-your-side-of-history
is a reasonable middle ground between off and on for rename detection.
  * what should the default be?  Junio suggested elsewhere[1] that
sparse-checkouts and partial clones should probably be welded together
(with partial clones downloading just history in the sparsity paths by
default), in which case having this option be true would be useful.
But it may also be slightly weird because it'll probably take us a
while to implement this; while the big warning in
git-sparse-checkout.txt certainly allows this:
        THIS COMMAND IS EXPERIMENTAL. ITS BEHAVIOR, AND THE BEHAVIOR OF OTHER
        COMMANDS IN THE PRESENCE OF SPARSE-CHECKOUTS, WILL LIKELY CHANGE IN
        THE FUTURE.
It may still be slightly weird that the default behavior of commands
in the presence of sparse-checkouts changes release to release until
we get it all implemented.
[1] https://lore.kernel.org/git/xmqqh7ycw5lc.fsf@gitster.c.googlers.com/
Previous: Matheus Tavares BernardinoNext: Derrick Stolee
Message 23 of 62 in “grep: honor sparse checkout and add option to ignore it”
  1. 0/3 grep: honor sparse checkout and add option to ignore itMatheus Tavares, Mar 24, 2020
  2. 1/3 doc: grep: unify info on configuration variablesMatheus Tavares, Mar 24, 2020
  3. Elijah NewrenMar 24, 2020
  4. Junio C HamanoMar 24, 2020
  5. Matheus TavaresMar 24, 2020
  6. 2/3 grep: honor sparse checkout patternsMatheus Tavares, Mar 24, 2020
  7. Elijah NewrenMar 24, 2020
  8. Derrick StoleeMar 24, 2020
  9. Elijah NewrenMar 24, 2020
  10. Derrick StoleeMar 24, 2020
  11. Matheus Tavares BernardinoMar 24, 2020
  12. Matheus Tavares BernardinoMar 24, 2020
  13. Matheus Tavares BernardinoApr 21, 2020
  14. Elijah NewrenApr 21, 2020
  15. Derrick StoleeApr 22, 2020
  16. Matheus Tavares BernardinoApr 23, 2020
  17. 3/3 grep: add option to ignore sparsity patternsMatheus Tavares, Mar 24, 2020
  18. Elijah NewrenMar 24, 2020
  19. Junio C HamanoMar 24, 2020
  20. Elijah NewrenMar 24, 2020
  21. Junio C HamanoMar 25, 2020
  22. Matheus Tavares BernardinoMar 30, 2020
  23. Elijah NewrenMar 31, 2020
  24. Derrick StoleeMar 31, 2020
  25. Matheus Tavares BernardinoApr 27, 2020
  26. Elijah NewrenApr 29, 2020
  27. Elijah NewrenApr 29, 2020
  28. Matheus Tavares BernardinoMar 25, 2020
  29. Elijah NewrenMar 26, 2020
  30. Junio C HamanoMar 27, 2020
  31. Elijah NewrenMar 27, 2020
  32. Matheus Tavares BernardinoMar 30, 2020
  33. Elijah NewrenMar 31, 2020
  34. 0/4 grep: honor sparse checkout and add option to ignore itMatheus Tavares, May 10, 2020
  35. 1/4 doc: grep: unify info on configuration variablesMatheus Tavares, May 10, 2020
  36. 2/4 config: load the correct config.worktree fileMatheus Tavares, May 10, 2020
  37. Junio C HamanoMay 11, 2020
  38. Matheus Tavares BernardinoMay 12, 2020
  39. Junio C HamanoMay 12, 2020
  40. 3/4 grep: honor sparse checkout patternsMatheus Tavares, May 10, 2020
  41. Junio C HamanoMay 11, 2020
  42. Matheus Tavares BernardinoMay 13, 2020
  43. Junio C HamanoMay 13, 2020
  44. Elijah NewrenMay 21, 2020
  45. Matheus Tavares BernardinoMay 21, 2020
  46. Elijah NewrenMay 21, 2020
  47. Matheus Tavares BernardinoMay 22, 2020
  48. Elijah NewrenMay 22, 2020
  49. Elijah NewrenMay 22, 2020
  50. Matheus Tavares BernardinoMay 22, 2020
  51. Elijah NewrenMay 22, 2020
  52. Derrick StoleeJun 10, 2020
  53. Matheus Tavares BernardinoJun 10, 2020
  54. Derrick StoleeJun 10, 2020
  55. Matheus Tavares BernardinoJun 10, 2020
  56. Elijah NewrenJun 10, 2020
  57. Elijah NewrenJun 10, 2020
  58. Elijah NewrenMay 21, 2020
  59. 4/4 config: add setting to ignore sparsity patterns in some cmdsMatheus Tavares, May 10, 2020
  60. Matheus Tavares BernardinoMay 10, 2020
  61. Elijah NewrenMay 21, 2020
  62. Elijah NewrenMay 21, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.