Git Mailing List Archive on lore.kernel.org
 help / color / Atom feed
From: Elijah Newren <newren@gmail.com>
To: Matheus Tavares Bernardino <matheus.bernardino@usp.br>
Cc: "Derrick Stolee" <stolee@gmail.com>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Git Mailing List" <git@vger.kernel.org>,
	"Derrick Stolee" <dstolee@microsoft.com>,
	"Nguyễn Thái Ngọc" <pclouds@gmail.com>,
	"Jonathan Tan" <jonathantanmy@google.com>
Subject: Re: [RFC PATCH 3/3] grep: add option to ignore sparsity patterns
Date: Wed, 29 Apr 2020 09:46:46 -0700
Message-ID: <CABPp-BGytfCugK0S99nLPH4_VXmcYPHWdVyLO59BZc4__4CT9w@mail.gmail.com> (raw)
In-Reply-To: <CAHd-oW4NBEcx4ZHfhTZoWttROmEd9TXQC7kLtXgGATHQY9Gr9w@mail.gmail.com>

On Mon, Apr 27, 2020 at 10:15 AM Matheus Tavares Bernardino
<matheus.bernardino@usp.br> wrote:
>
> Hi, Stolee and Elijah
>
> I think I just finished addressing the comments on patch 2/3 [1]. And
> I'm now looking at the ones in 3/3 (this one). Below are some
> questions, just to make sure I'm going in the right direction with
> this one.
>
> On Tue, Mar 31, 2020 at 5:02 PM Derrick Stolee <stolee@gmail.com> wrote:
> >
> > On 3/31/2020 3:12 PM, Elijah Newren wrote:
> > >
> > > 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.
> ...
> > > """
> > >
> > > Several questions here, of course:
> > >
> > >   * do people like or hate the name?  indifferent?  have alternate ideas?
> >
> > It's probably time to create a 'sparse-checkout' config space. That
> > would allow
> >
> >         sparse-checkout.restrictGrep = true
> >
> > as an option. Or a more general
> >
> >         sparse-checkout.restrictCommands = true
> >
> > to make it clear that it affects multiple commands.
>
> If we are creating the new namespace, 'core.sparseCheckout' should
> also be renamed to something like 'sparse-checkout.enabled', right?
> And maybe we could use 'sparsecheckout.*', instead? That seems to be
> the convention for settings on hyphenated commands (as in sendemail.*,
> uploadpack.* and gitgui.*).

Or maybe just call the namespace 'sparse.*' if we're going that route?

> As for compatibility, when running `git sparse-checkout init`, if the
> config file already has the core.sparseCheckout setting, should we
> remove it? Or just add the new sparsecheckout.enabled config, which
> will always be read first?

We seem to have two competing issues:

  * If you remove the core.sparseCheckout setting in favor of
sparse.enabled, then people can't use the repo with an older version
of git.  (This may be acceptable, but we've generally been somewhat
careful with index extensions and such to avoid such a state, with
slow transitions with index and pack versions and such.)
  * If you leave the core.sparseCheckout setting around as well as
having sparse.enabled, then we have two different settings that we can
keep in sync with newer git but which older git will only update one
of.  What do we do if we detect they are out of sync?  Throw an error?
 Pretend that one overrules?  If the older one overrules, what do we
accomplish with the new name?  If the newer name overrules, doesn't
that also potentially break using an older git version?

I'm not sure what to do here.  Maybe people who have worked on index
version and pack version transitions have some good suggestions for
us?

> Also, should we emit a warning about the former being deprecated? The
> good thing about deprecation warnings, IMO, is that users will know
> the name change faster. But, at least for `git grep <tree>`, where we
> read  core.sparseCheckout and core.sparseCheckoutCone for each
> submodule and each tree, there would be too much pollution in the
> output...

We've already started to steer away from users setting these values
and just have them get set/updated/unset by sparse-checkout init and
sparse-checkout disable.  Since users won't be setting these directly,
I don't think deprecation warnings make sense.

> Finally, about restrictCommands, the idea is to have both
> sparsecheckout.restrictCommands and `git --restrict-to-sparse-paths`,
> right? For now, the option/setting would only affect grep, but support
> would be added gradually to other commands in the future. I noticed

There should be both a config option and a global command line flag,
yes.  We might need the flag to default to
not-restricting-to-sparse-paths for now because that's consistent with
the only thing the current implementation of these commands can do.
But I'm really worried that this will remain the default and we'll
force users in the future to jump through a bunch of hoops to do a
simple thing:

$ git clone --sparse-paths $WANTED_DIRECTORIES user@server.name:path/to/repo.git
$ cd repo
<Enjoy their small view of the repo without every command suddenly
requiring a network connection and downloading huge reams of data they
don't even care about.>

> git-read-tree already has a --no-sparse-checkout option. Should we
> remove this option in favor of the global
> --[no]-restrict-to-sparse-paths?

read-tree is plumbing; we can't break backward compatibility.  We'll
have to leave that option there and just document that the two options
do the same thing.

> Sorry for too many questions. I just wanted to make sure that I
> understood the plan before diving into the implementation, to avoid
> going in the wrong direction.

Nah, these are all good questions.  Sorry for the delay in getting back to you.

  reply index

Thread overview: 63+ 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 [this message]
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

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-BGytfCugK0S99nLPH4_VXmcYPHWdVyLO59BZc4__4CT9w@mail.gmail.com \
    --to=newren@gmail.com \
    --cc=dstolee@microsoft.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jonathantanmy@google.com \
    --cc=matheus.bernardino@usp.br \
    --cc=pclouds@gmail.com \
    --cc=stolee@gmail.com \
    /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

Git Mailing List Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/git/0 git/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 git git/ https://lore.kernel.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.git


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git