git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Elijah Newren <newren@gmail.com>
To: Matheus Tavares Bernardino <matheus.bernardino@usp.br>
Cc: Git Mailing List <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>,
	Derrick Stolee <stolee@gmail.com>,
	Jonathan Tan <jonathantanmy@google.com>
Subject: Re: [PATCH v3 5/5] config: add setting to ignore sparsity patterns in some cmds
Date: Tue, 2 Jun 2020 19:39:53 -0700	[thread overview]
Message-ID: <CABPp-BH0d5WKz7bXLkFQGDBKXbd0tccS_+1_iNc1Wqr93h+KaQ@mail.gmail.com> (raw)
In-Reply-To: <CAHd-oW7b41hqfPXm_GL_sth+QRfx56ceMr4VM9B9xhj3_P9iGQ@mail.gmail.com>

On Sun, May 31, 2020 at 9:46 PM Matheus Tavares Bernardino
<matheus.bernardino@usp.br> wrote:
>
> On Sat, May 30, 2020 at 1:18 PM Elijah Newren <newren@gmail.com> wrote:
> >
> > On Wed, May 27, 2020 at 6:14 PM Matheus Tavares
> > <matheus.bernardino@usp.br> wrote:
> > > diff --git a/Documentation/config/sparse.txt b/Documentation/config/sparse.txt
> > > new file mode 100644
> > > index 0000000000..2a25b4b8ef
> > > --- /dev/null
> > > +++ b/Documentation/config/sparse.txt
> > > @@ -0,0 +1,24 @@
> > > +sparse.restrictCmds::
> > > +       Only meaningful in conjunction with core.sparseCheckout. This option
> > > +       extends sparse checkouts (which limit which paths are written to the
> > > +       working tree), 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 (default), some git commands may limit their behavior
> > > +to the paths specified by the sparsity patterns, or to the intersection of
> > > +those paths and any (like `*.c`) that the user might also specify on the
> > > +command line. When false, the affected commands will work on full trees,
> > > +ignoring the sparsity patterns. For now, only git-grep honors this setting. In
> > > +this command, the restriction takes effect in three cases: with --cached; when
> > > +a commit-ish is given; when searching a working tree where some paths excluded
> > > +by the sparsity patterns are present (e.g. manually created paths or not
> > > +removed submodules).
> >
> > I think "In this command, the restriction takes effect..." to the end
> > of the paragraph should be removed.  I don't want every subcommand's
> > behavior to be specified here; it'll grow unreadably long and be more
> > likely to eventually go stale.
>
> Yeah, I was also concerned about that. But wouldn't it be important to
> inform the users how the setting takes place in grep (specially with
> the corner cases)? And maybe others, in the future?
>
> What if we move the information that is only relevant to a single
> command into its own man page? I.e. git-grep.txt would have something
> like:

Moving it to grep's manpage seems ideal to me.  grep's behavior should
be defined in grep's manual.

> sparse.restrictCmds::
> See complete definition in linkgit:git-config[1]. In grep, the
> restriction takes effect in three cases: with --cached; when a
> commit-ish is given; when searching a working tree where some paths
> excluded by the sparsity patterns are present (e.g. manually created
> paths or not removed submodules).

That looks more than a little confusing.  Could this definition be
something more like "See base definition in linkgit:git-config[1].
grep honors sparse.restrictCmds by limiting searches to the sparsity
paths in three cases: when searching the working tree, when searching
the index with --cached, or when searching a specified commit"

> The only problem then is that the information would be a little
> scattered... But I think it shouldn't be a big deal, as a person
> interested in knowing how foo behaves with sparse.restrictCmds would
> only need to look into foo's man page, anyway.
>
> > > diff --git a/git.c b/git.c
> > > index a2d337eed7..6db1382ae4 100644
> > > --- a/git.c
> > > +++ b/git.c
> > > @@ -319,6 +324,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
> > >                 (*argv)++;
> > >                 (*argc)--;
> > >         }
> > > +
> > >         return (*argv) - orig_argv;
> > >  }
> > >
> >
> > Why the stray whitespace change?
>
> Oops, that shouldn't be there. Thanks!
>
> >
> > > diff --git a/sparse-checkout.c b/sparse-checkout.c
> > > new file mode 100644
> > > index 0000000000..9a9e50fd29
> > > --- /dev/null
> > > +++ b/sparse-checkout.c
> > > @@ -0,0 +1,16 @@
> > > +#include "cache.h"
> > > +#include "config.h"
> > > +#include "sparse-checkout.h"
> > > +
> > > +int restrict_to_sparse_paths(struct repository *repo)
> > > +{
> > > +       int ret;
> > > +
> > > +       if (opt_restrict_to_sparse_paths >= 0)
> > > +               return opt_restrict_to_sparse_paths;
> > > +
> > > +       if (repo_config_get_bool(repo, "sparse.restrictcmds", &ret))
> > > +               ret = 1;
> > > +
> > > +       return ret;
> > > +}
> >
> > Do we want to considering renaming this file to sparse.c, since it's
> > for sparse grep and sparse diff and etc., not just for the checkout
> > piece?  It would also go along well with our toplevel related config
> > being in the "sparse" namespace.
>
> Makes sense. But since Stolee is already working on
> "sparse-checkout.c" [1], if we use "sparse.c" in this series we will
> end up with two extra files. And as "sparse.c" is quite small, I think
> we could unify into the "sparse-checkout.c".
>
> [1]: https://lore.kernel.org/git/0181a134bfb6986dc0e54ae624c478446a1324a9.1588857462.git.gitgitgadget@gmail.com/

Or we could just suggest he use sparse.c too.  :-)

Stolee?


> > > diff --git a/t/t7817-grep-sparse-checkout.sh b/t/t7817-grep-sparse-checkout.sh
> > > index ce080cf572..1aef084186 100755
> > > --- a/t/t7817-grep-sparse-checkout.sh
> > > +++ b/t/t7817-grep-sparse-checkout.sh
> >
> > All these testcases look great (modulo the small typo I pointed out
> > earlier); I kept thinking "but what about case <x>?" and then I kept
> > reading and saw you covered it.  You even added some I wasn't thinking
> > about and might have overlooked but seem important.
>
> Thanks :)

  reply	other threads:[~2020-06-03  2:40 UTC|newest]

Thread overview: 123+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-24  6:04 [RFC PATCH 0/3] grep: honor sparse checkout and add option to ignore it Matheus Tavares
2020-03-24  6:11 ` [RFC PATCH 1/3] doc: grep: unify info on configuration variables Matheus Tavares
2020-03-24  7:57   ` Elijah Newren
2020-03-24 21:26     ` Junio C Hamano
2020-03-24 23:38       ` Matheus Tavares
2020-03-24  6:12 ` [RFC PATCH 2/3] grep: honor sparse checkout patterns Matheus Tavares
2020-03-24  7:15   ` Elijah Newren
2020-03-24 15:12     ` Derrick Stolee
2020-03-24 16:16       ` Elijah Newren
2020-03-24 17:02         ` Derrick Stolee
2020-03-24 23:01       ` Matheus Tavares Bernardino
2020-03-24 22:55     ` Matheus Tavares Bernardino
2020-04-21  2:10       ` Matheus Tavares Bernardino
2020-04-21  3:08         ` Elijah Newren
2020-04-22 12:08           ` Derrick Stolee
2020-04-23  6:09           ` Matheus Tavares Bernardino
2020-03-24  6:13 ` [RFC PATCH 3/3] grep: add option to ignore sparsity patterns Matheus Tavares
2020-03-24  7:54   ` Elijah Newren
2020-03-24 18:30     ` Junio C Hamano
2020-03-24 19:07       ` Elijah Newren
2020-03-25 20:18         ` Junio C Hamano
2020-03-30  3:23       ` Matheus Tavares Bernardino
2020-03-31 19:12         ` Elijah Newren
2020-03-31 20:02           ` Derrick Stolee
2020-04-27 17:15             ` Matheus Tavares Bernardino
2020-04-29 16:46               ` Elijah Newren
2020-04-29 17:21             ` Elijah Newren
2020-03-25 23:15     ` Matheus Tavares Bernardino
2020-03-26  6:02       ` Elijah Newren
2020-03-27 15:51         ` Junio C Hamano
2020-03-27 19:01           ` Elijah Newren
2020-03-30  1:12         ` Matheus Tavares Bernardino
2020-03-31 16:48           ` Elijah Newren
2020-05-10  0:41 ` [RFC PATCH v2 0/4] grep: honor sparse checkout and add option to ignore it Matheus Tavares
2020-05-10  0:41   ` [RFC PATCH v2 1/4] doc: grep: unify info on configuration variables Matheus Tavares
2020-05-10  0:41   ` [RFC PATCH v2 2/4] config: load the correct config.worktree file Matheus Tavares
2020-05-11 19:10     ` Junio C Hamano
2020-05-12 22:55       ` Matheus Tavares Bernardino
2020-05-12 23:22         ` Junio C Hamano
2020-05-10  0:41   ` [RFC PATCH v2 3/4] grep: honor sparse checkout patterns Matheus Tavares
2020-05-11 19:35     ` Junio C Hamano
2020-05-13  0:05       ` Matheus Tavares Bernardino
2020-05-13  0:17         ` Junio C Hamano
2020-05-21  7:26           ` Elijah Newren
2020-05-21 17:35             ` Matheus Tavares Bernardino
2020-05-21 17:52               ` Elijah Newren
2020-05-22  5:49                 ` Matheus Tavares Bernardino
2020-05-22 14:26                   ` Elijah Newren
2020-05-22 15:36                     ` Elijah Newren
2020-05-22 20:54                       ` Matheus Tavares Bernardino
2020-05-22 21:06                         ` Elijah Newren
2020-06-10 11:40                     ` Derrick Stolee
2020-06-10 16:22                       ` Matheus Tavares Bernardino
2020-06-10 17:42                         ` Derrick Stolee
2020-06-10 18:14                           ` Matheus Tavares Bernardino
2020-06-10 20:12                         ` Elijah Newren
2020-06-10 19:58                       ` Elijah Newren
2020-05-21  7:36       ` Elijah Newren
2020-05-10  0:41   ` [RFC PATCH v2 4/4] config: add setting to ignore sparsity patterns in some cmds Matheus Tavares
2020-05-10  4:23     ` Matheus Tavares Bernardino
2020-05-21 17:18       ` Elijah Newren
2020-05-21  7:09     ` Elijah Newren
2020-05-28  1:12   ` [PATCH v3 0/5] grep: honor sparse checkout and add option to ignore it Matheus Tavares
2020-05-28  1:12     ` [PATCH v3 1/5] doc: grep: unify info on configuration variables Matheus Tavares
2020-05-28  1:13     ` [PATCH v3 2/5] t/helper/test-config: return exit codes consistently Matheus Tavares
2020-05-30 14:29       ` Elijah Newren
2020-06-01  4:36         ` Matheus Tavares Bernardino
2020-05-28  1:13     ` [PATCH v3 3/5] config: correctly read worktree configs in submodules Matheus Tavares
2020-05-30 14:49       ` Elijah Newren
2020-06-01  4:38         ` Matheus Tavares Bernardino
2020-05-28  1:13     ` [PATCH v3 4/5] grep: honor sparse checkout patterns Matheus Tavares
2020-05-30 15:48       ` Elijah Newren
2020-06-01  4:44         ` Matheus Tavares Bernardino
2020-06-03  2:38           ` Elijah Newren
2020-06-10 17:08             ` Matheus Tavares Bernardino
2020-05-28  1:13     ` [PATCH v3 5/5] config: add setting to ignore sparsity patterns in some cmds Matheus Tavares
2020-05-30 16:18       ` Elijah Newren
2020-06-01  4:45         ` Matheus Tavares Bernardino
2020-06-03  2:39           ` Elijah Newren [this message]
2020-06-10 21:15             ` Matheus Tavares Bernardino
2020-06-11  0:35               ` Elijah Newren
2020-06-12 15:44     ` [PATCH v4 0/6] grep: honor sparse checkout and add option to ignore it Matheus Tavares
2020-06-12 15:44       ` [PATCH v4 1/6] doc: grep: unify info on configuration variables Matheus Tavares
2020-06-12 15:45       ` [PATCH v4 2/6] t/helper/test-config: return exit codes consistently Matheus Tavares
2020-06-12 15:45       ` [PATCH v4 3/6] t/helper/test-config: facilitate addition of new cli options Matheus Tavares
2020-06-12 15:45       ` [PATCH v4 4/6] config: correctly read worktree configs in submodules Matheus Tavares
2020-06-16 19:13         ` Elijah Newren
2020-06-21 16:05           ` Matheus Tavares Bernardino
2020-09-01  2:41         ` Jonathan Nieder
2020-09-01 21:44           ` Matheus Tavares Bernardino
2020-06-12 15:45       ` [PATCH v4 5/6] grep: honor sparse checkout patterns Matheus Tavares
2020-06-12 15:45       ` [PATCH v4 6/6] config: add setting to ignore sparsity patterns in some cmds Matheus Tavares
2020-06-16 22:31       ` [PATCH v4 0/6] grep: honor sparse checkout and add option to ignore it Elijah Newren
2020-09-02  6:17       ` [PATCH v5 0/8] " Matheus Tavares
2020-09-02  6:17         ` [PATCH v5 1/8] doc: grep: unify info on configuration variables Matheus Tavares
2020-09-02  6:17         ` [PATCH v5 2/8] t1308-config-set: avoid false positives when using test-config Matheus Tavares
2020-09-02  6:57           ` Eric Sunshine
2020-09-02 16:16             ` Matheus Tavares Bernardino
2020-09-02 16:38               ` Eric Sunshine
2020-09-02  6:17         ` [PATCH v5 3/8] t/helper/test-config: be consistent with exit codes Matheus Tavares
2020-09-02  6:17         ` [PATCH v5 4/8] t/helper/test-config: check argc before accessing argv Matheus Tavares
2020-09-02  7:18           ` Eric Sunshine
2020-09-02  6:17         ` [PATCH v5 5/8] t/helper/test-config: unify exit labels Matheus Tavares
2020-09-02  7:30           ` Eric Sunshine
2020-09-02  6:17         ` [PATCH v5 6/8] config: correctly read worktree configs in submodules Matheus Tavares
2020-09-02 20:15           ` Jonathan Nieder
2020-09-09 13:04             ` Matheus Tavares Bernardino
2020-09-09 23:32               ` Jonathan Nieder
2020-09-02  6:17         ` [PATCH v5 7/8] grep: honor sparse checkout patterns Matheus Tavares
2020-09-02  6:17         ` [PATCH v5 8/8] config: add setting to ignore sparsity patterns in some cmds Matheus Tavares
2020-09-10 17:21         ` [PATCH v6 0/9] grep: honor sparse checkout and add option to ignore it Matheus Tavares
2020-09-10 17:21           ` [PATCH v6 1/9] doc: grep: unify info on configuration variables Matheus Tavares
2020-09-10 17:21           ` [PATCH v6 2/9] t1308-config-set: avoid false positives when using test-config Matheus Tavares
2020-09-10 17:21           ` [PATCH v6 3/9] t/helper/test-config: be consistent with exit codes Matheus Tavares
2020-09-10 17:21           ` [PATCH v6 4/9] t/helper/test-config: diagnose missing arguments Matheus Tavares
2020-09-10 17:21           ` [PATCH v6 5/9] t/helper/test-config: unify exit labels Matheus Tavares
2020-09-10 17:21           ` [PATCH v6 6/9] config: make do_git_config_sequence receive a 'struct repository' Matheus Tavares
2020-09-10 17:21           ` [PATCH v6 7/9] config: correctly read worktree configs in submodules Matheus Tavares
2020-09-10 17:21           ` [PATCH v6 8/9] grep: honor sparse checkout patterns Matheus Tavares
2020-09-10 17:21           ` [PATCH v6 9/9] config: add setting to ignore sparsity patterns in some cmds Matheus Tavares
2021-02-09 21:33           ` [PATCH v7] grep: honor sparse-checkout on working tree searches Matheus Tavares
2021-02-09 23:23             ` Junio C Hamano
2021-02-10  6:12               ` Elijah Newren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CABPp-BH0d5WKz7bXLkFQGDBKXbd0tccS_+1_iNc1Wqr93h+KaQ@mail.gmail.com \
    --to=newren@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jonathantanmy@google.com \
    --cc=matheus.bernardino@usp.br \
    --cc=stolee@gmail.com \
    --subject='Re: [PATCH v3 5/5] config: add setting to ignore sparsity patterns in some cmds' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).