From: Matheus Tavares Bernardino <matheus.bernardino@usp.br>
To: Elijah Newren <newren@gmail.com>
Cc: "Git Mailing List" <git@vger.kernel.org>,
"Derrick Stolee" <dstolee@microsoft.com>,
"Nguyễn Thái Ngọc" <pclouds@gmail.com>,
"Junio C Hamano" <gitster@pobox.com>
Subject: Re: [RFC PATCH 3/3] grep: add option to ignore sparsity patterns
Date: Wed, 25 Mar 2020 20:15:42 -0300 [thread overview]
Message-ID: <CAHd-oW6ANf_iNmYkgXt4jo1seB-AEx1TSH384v+oh1bicG8RDA@mail.gmail.com> (raw)
In-Reply-To: <CABPp-BEbNCYk0pCuEDQ_ViB2=varJPBsVODxNvJs0EVRyBqjBg@mail.gmail.com>
On Tue, Mar 24, 2020 at 4:55 AM Elijah Newren <newren@gmail.com> wrote:
>
> On Mon, Mar 23, 2020 at 11:13 PM Matheus Tavares
> <matheus.bernardino@usp.br> wrote:
>
> > Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
> > ---
> >
> > Note: I still have to make --ignore-sparsity be able to work together
> > with --untracked. Unfortunatelly, this won't be as simple because the
> > codeflow taken by --untracked goes to grep_directory() which just
> > iterates the working tree, without looking the index entries. So I will
> > have to either: make --untracked use grep_cache(), and grep the
> > untracked files later; or try matching the working tree paths against
> > the sparsity patterns, without looking for the skip_worktree bit in
> > the index (as I mentioned in the previous patch's comments). Any
> > preferences regarding these two approaches? (or other suggestions?)
>
> Hmm. So, 'tracked' in git is the idea that we are keeping information
> about specific files. 'sparse-checkout' is the idea that we have a
> subset of those that we can work with without materializing all the
> other tracked files; it's clearly a subset of the realm of 'tracked'.
> 'untracked' is about getting everything outside the set of 'tracked'
> files, which to me means it is clearly outside the set of sparsity
> paths too (and thus you could take --untracked as implying
> --ignore-sparsity, though whether you do might not matter in practice
> because of the items I'll discuss next). Of course, I am also
> assuming `--untracked` is incompatible with --cached or specifying
> revisions or trees (based on it's definiton of "In addition to
> searching in the tracked files in the *working tree*, search also in
> untracked files." -- emphasis added.)
Hm, I see the point now, but I'm still a little confused: The "in the
working tree" section of the definition would exclude non checked out
files, right? However, git-grep's description says "Look for specified
patterns in the tracked files *in the work tree*", and it still
searches non checked out files (loading them from the cache, even when
--cache is not given). I know that's exactly what we are trying to
change with this patchset, but we will still give the
--ignore-sparsity option to allow the old behavior when needed (unless
we prohibit using --ignore-sparsity without --cached or $REV). I guess
my doubt is whether the problem is in the implementation of the
working tree grep, which considers non checked out files, or in the
docs, which say "tracked files *in the work tree*".
I tend to go with the latter, since using `git grep --ignore-sparsity`
in a sparse checked out working tree, to grep not present files as
well, kind of makes sense to me. And if the problem is indeed in the
docs, then I think we should also allow --ignore-sparsity when
grepping with --untracked, since it's an analogous case.
> If the incompatibility of
> --untracked and --cached/REVSIONS/TREES is not enforced, we may want
> to look into erroring out if they are given together. Once we do, we
> don't have to worry about grep_cache() at all in the case of
> --untracked and shouldn't. Files with the skip_worktree bit won't
> exist in the working directory, and thus won't be searched (this is
> what makes --untracked imply --ignore-sparsity not really matter).
>
> In short: With --untracked you are grepping ALL (non-ignored) files in
> the working directory -- either because they are both tracked and in
> the sparsity paths (anything tracked that isn't in the sparsity paths
> has the skip_worktree bit and thus isn't present), or because it is an
> untracked file. [And this may be what grep_directory() already does.]
>
> Does that make sense?
It does, and thanks for a very detailed explanation. But as I
mentioned before, I'm a little uncertain about --untracked implying
--ignore-spasity. The commit that added --untracked (0a93fb8) says:
"grep --untracked" would find the specified patterns from files in
untracked files in addition to its usual behaviour of finding them in
the tracked files
So, in my mind, it feels like --untracked wasn't meant to limit the
search to "all non-ignored files in the working directory", but to add
untracked files to the search (which could also contain tracked but
non checked out files). Wouldn't the "all non-ignored files in the
working directory" case be the use of --no-index?
> > diff --git a/builtin/grep.c b/builtin/grep.c
> > index 52ec72a036..17eae3edd6 100644
> > --- a/builtin/grep.c
> > +++ b/builtin/grep.c
...
> >
> > @@ -487,7 +492,7 @@ static int grep_cache(struct grep_opt *opt,
> > for (nr = 0; nr < repo->index->cache_nr; nr++) {
> > const struct cache_entry *ce = repo->index->cache[nr];
> >
> > - if (ce_skip_worktree(ce))
> > + if (!ignore_sparsity && ce_skip_worktree(ce))
>
> Oh boy on the double negatives...maybe we want to rename this flag somehow?
Yeah, I also thought about that, but couldn't come up with a better
name myself... My alternatives were all too verbose.
...
> I'm super excited to see work in this area. I hope I'm not
> discouraging you by attempting to provide what I think is the bigger
> picture I'd like us to work towards.
Not at all! :) Thanks a lot for the bigger picture and other
explanations. They help me understand the long-term goals and make
better decisions now.
next prev parent reply other threads:[~2020-03-25 23:16 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 [this message]
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
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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAHd-oW6ANf_iNmYkgXt4jo1seB-AEx1TSH384v+oh1bicG8RDA@mail.gmail.com \
--to=matheus.bernardino@usp.br \
--cc=dstolee@microsoft.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=newren@gmail.com \
--cc=pclouds@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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).