Re: [PATCH] grep: do not do external grep on skip-worktree entries
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:47:57
Subsystem:
the rest · Maintainer:
Linus Torvalds
Possibly related (same subject, not in this thread)
- 2016-06-15 · Re: [PATCH] grep: do not do external grep on skip-worktree entries · Linus Torvalds <torvalds@linux-foundation.org>
- 2016-06-15 · Re: [PATCH] grep: do not do external grep on skip-worktree entries · Linus Torvalds <torvalds@linux-foundation.org>
- 2016-06-15 · Re: [PATCH] grep: do not do external grep on skip-worktree entries · Junio C Hamano <hidden>
- 2016-06-15 · Re: [PATCH] grep: do not do external grep on skip-worktree entries · Junio C Hamano <hidden>
- 2016-06-15 · Re: [PATCH] grep: do not do external grep on skip-worktree entries · Linus Torvalds <torvalds@linux-foundation.org>
Junio C Hamano [off-list ref] writes:
This looks a bit wrong for a couple of reasons: - external_grep() is designed to return negative without running external grep when it shouldn't be used (see the beginning of the function for how it refuses to run when opt->extended is set and other conditions). The new logic seems to belong there, i.e. "in addition to the existing case we decline, if ce_skip_worktree() entry exists in the cache, we decline";
IOW, something like this instead of your patch. You would want to tests to demonstrate the original breakage, perhaps? builtin-grep.c | 18 +++++++++++++++++- 1 files changed, 17 insertions(+), 1 deletions(-)
diff --git a/builtin-grep.c b/builtin-grep.c
index 813fe97..25ee75d 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c@@ -357,6 +357,21 @@ static void grep_add_color(struct strbuf *sb, const char *escape_seq) strbuf_setlen(sb, sb->len - 1); } +static int has_skip_worktree_entry(struct grep_opt *opt, const char **paths) +{ + int nr; + for (nr = 0; nr < active_nr; nr++) { + struct cache_entry *ce = active_cache[nr]; + if (!S_ISREG(ce->ce_mode)) + continue; + if (!pathspec_matches(paths, ce->name, opt->max_depth)) + continue; + if (ce_skip_worktree(ce)) + return 1; + } + return 0; +} + static int external_grep(struct grep_opt *opt, const char **paths, int cached) { int i, nr, argc, hit, len, status;
@@ -365,7 +380,8 @@ static int external_grep(struct grep_opt *opt, const char **paths, int cached) char *argptr = randarg; struct grep_pat *p; - if (opt->extended || (opt->relative && opt->prefix_length)) + if (opt->extended || (opt->relative && opt->prefix_length) + || has_skip_worktree_entry(opt, paths)) return -1; len = nr = 0; push_arg("grep");