From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
Miles Bader <miles@gnu.org>, Jeff King <peff@peff.net>,
Nguyen Thai Ngoc Duy <pclouds@gmail.com>
Subject: [PATCH 1/2] grep: rip out support for external grep
Date: Tue, 12 Jan 2010 22:48:10 -0800 [thread overview]
Message-ID: <7v4omqv6tx.fsf_-_@alter.siamese.dyndns.org> (raw)
In-Reply-To: <7v63774tfd.fsf@alter.siamese.dyndns.org> (Junio C. Hamano's message of "Tue\, 12 Jan 2010 00\:29\:58 -0800")
We still allow people to pass --[no-]ext-grep on the command line,
but the option is ignored.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Junio C Hamano <gitster@pobox.com> writes:
> Linus Torvalds <torvalds@linux-foundation.org> writes:
>
>> Ack. Works for me. And with that, I'd love for it to go in, and get rid of
>> the external grep.
> ...
> Before going forward, I found two small nits that should go to maint.
These nits out-of-way, we can now start doing this.
Makefile | 10 --
builtin-grep.c | 305 +------------------------------------------------------
t/t7002-grep.sh | 6 +-
3 files changed, 8 insertions(+), 313 deletions(-)
diff --git a/Makefile b/Makefile
index 4a1e5bc..a4b922e 100644
--- a/Makefile
+++ b/Makefile
@@ -185,10 +185,6 @@ all::
# is a simplified version of the merge sort used in glibc. This is
# recommended if Git triggers O(n^2) behavior in your platform's qsort().
#
-# Define NO_EXTERNAL_GREP if you don't want "git grep" to ever call
-# your external grep (e.g., if your system lacks grep, if its grep is
-# broken, or spawning external process is slower than built-in grep git has).
-#
# Define UNRELIABLE_FSTAT if your system's fstat does not return the same
# information on a not yet closed file that lstat would return for the same
# file after it was closed.
@@ -777,7 +773,6 @@ ifeq ($(uname_S),SunOS)
NO_MKDTEMP = YesPlease
NO_MKSTEMPS = YesPlease
NO_REGEX = YesPlease
- NO_EXTERNAL_GREP = YesPlease
THREADED_DELTA_SEARCH = YesPlease
ifeq ($(uname_R),5.7)
NEEDS_RESOLV = YesPlease
@@ -895,7 +890,6 @@ ifeq ($(uname_S),IRIX)
# NO_MMAP. If you suspect that your compiler is not affected by this
# issue, comment out the NO_MMAP statement.
NO_MMAP = YesPlease
- NO_EXTERNAL_GREP = UnfortunatelyYes
SNPRINTF_RETURNS_BOGUS = YesPlease
SHELL_PATH = /usr/gnu/bin/bash
NEEDS_LIBGEN = YesPlease
@@ -915,7 +909,6 @@ ifeq ($(uname_S),IRIX64)
# NO_MMAP. If you suspect that your compiler is not affected by this
# issue, comment out the NO_MMAP statement.
NO_MMAP = YesPlease
- NO_EXTERNAL_GREP = UnfortunatelyYes
SNPRINTF_RETURNS_BOGUS = YesPlease
SHELL_PATH=/usr/gnu/bin/bash
NEEDS_LIBGEN = YesPlease
@@ -1322,9 +1315,6 @@ endif
ifdef DIR_HAS_BSD_GROUP_SEMANTICS
COMPAT_CFLAGS += -DDIR_HAS_BSD_GROUP_SEMANTICS
endif
-ifdef NO_EXTERNAL_GREP
- BASIC_CFLAGS += -DNO_EXTERNAL_GREP
-endif
ifdef UNRELIABLE_FSTAT
BASIC_CFLAGS += -DUNRELIABLE_FSTAT
endif
diff --git a/builtin-grep.c b/builtin-grep.c
index a5b6719..4adb971 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -15,14 +15,6 @@
#include "grep.h"
#include "quote.h"
-#ifndef NO_EXTERNAL_GREP
-#ifdef __unix__
-#define NO_EXTERNAL_GREP 0
-#else
-#define NO_EXTERNAL_GREP 1
-#endif
-#endif
-
static char const * const grep_usage[] = {
"git grep [options] [-e] <pattern> [<rev>...] [[--] path...]",
NULL
@@ -215,292 +207,12 @@ static int grep_file(struct grep_opt *opt, const char *filename)
return i;
}
-#if !NO_EXTERNAL_GREP
-static int exec_grep(int argc, const char **argv)
-{
- pid_t pid;
- int status;
-
- argv[argc] = NULL;
- pid = fork();
- if (pid < 0)
- return pid;
- if (!pid) {
- execvp("grep", (char **) argv);
- exit(255);
- }
- while (waitpid(pid, &status, 0) < 0) {
- if (errno == EINTR)
- continue;
- return -1;
- }
- if (WIFEXITED(status)) {
- if (!WEXITSTATUS(status))
- return 1;
- return 0;
- }
- return -1;
-}
-
-#define MAXARGS 1000
-#define ARGBUF 4096
-#define push_arg(a) do { \
- if (nr < MAXARGS) argv[nr++] = (a); \
- else die("maximum number of args exceeded"); \
- } while (0)
-
-/*
- * If you send a singleton filename to grep, it does not give
- * the name of the file. GNU grep has "-H" but we would want
- * that behaviour in a portable way.
- *
- * So we keep two pathnames in argv buffer unsent to grep in
- * the main loop if we need to do more than one grep.
- */
-static int flush_grep(struct grep_opt *opt,
- int argc, int arg0, const char **argv, int *kept)
-{
- int status;
- int count = argc - arg0;
- const char *kept_0 = NULL;
-
- if (count <= 2) {
- /*
- * Because we keep at least 2 paths in the call from
- * the main loop (i.e. kept != NULL), and MAXARGS is
- * far greater than 2, this usually is a call to
- * conclude the grep. However, the user could attempt
- * to overflow the argv buffer by giving too many
- * options to leave very small number of real
- * arguments even for the call in the main loop.
- */
- if (kept)
- die("insanely many options to grep");
-
- /*
- * If we have two or more paths, we do not have to do
- * anything special, but we need to push /dev/null to
- * get "-H" behaviour of GNU grep portably but when we
- * are not doing "-l" nor "-L" nor "-c".
- */
- if (count == 1 &&
- !opt->name_only &&
- !opt->unmatch_name_only &&
- !opt->count) {
- argv[argc++] = "/dev/null";
- argv[argc] = NULL;
- }
- }
-
- else if (kept) {
- /*
- * Called because we found many paths and haven't finished
- * iterating over the cache yet. We keep two paths
- * for the concluding call. argv[argc-2] and argv[argc-1]
- * has the last two paths, so save the first one away,
- * replace it with NULL while sending the list to grep,
- * and recover them after we are done.
- */
- *kept = 2;
- kept_0 = argv[argc-2];
- argv[argc-2] = NULL;
- argc -= 2;
- }
-
- if (opt->pre_context || opt->post_context) {
- /*
- * grep handles hunk marks between files, but we need to
- * do that ourselves between multiple calls.
- */
- if (opt->show_hunk_mark)
- write_or_die(1, "--\n", 3);
- else
- opt->show_hunk_mark = 1;
- }
-
- status = exec_grep(argc, argv);
-
- if (kept_0) {
- /*
- * Then recover them. Now the last arg is beyond the
- * terminating NULL which is at argc, and the second
- * from the last is what we saved away in kept_0
- */
- argv[arg0++] = kept_0;
- argv[arg0] = argv[argc+1];
- }
- return status;
-}
-
-static void grep_add_color(struct strbuf *sb, const char *escape_seq)
-{
- size_t orig_len = sb->len;
-
- while (*escape_seq) {
- if (*escape_seq == 'm')
- strbuf_addch(sb, ';');
- else if (*escape_seq != '\033' && *escape_seq != '[')
- strbuf_addch(sb, *escape_seq);
- escape_seq++;
- }
- if (sb->len > orig_len && sb->buf[sb->len - 1] == ';')
- strbuf_setlen(sb, sb->len - 1);
-}
-
-static int external_grep(struct grep_opt *opt, const char **paths, int cached)
-{
- int i, nr, argc, hit, len, status;
- const char *argv[MAXARGS+1];
- char randarg[ARGBUF];
- char *argptr = randarg;
- struct grep_pat *p;
-
- if (opt->extended || (opt->relative && opt->prefix_length))
- return -1;
- len = nr = 0;
- push_arg("grep");
- if (opt->fixed)
- push_arg("-F");
- if (opt->linenum)
- push_arg("-n");
- if (!opt->pathname)
- push_arg("-h");
- if (opt->regflags & REG_EXTENDED)
- push_arg("-E");
- if (opt->ignore_case)
- push_arg("-i");
- if (opt->binary == GREP_BINARY_NOMATCH)
- push_arg("-I");
- if (opt->word_regexp)
- push_arg("-w");
- if (opt->name_only)
- push_arg("-l");
- if (opt->unmatch_name_only)
- push_arg("-L");
- if (opt->null_following_name)
- /* in GNU grep git's "-z" translates to "-Z" */
- push_arg("-Z");
- if (opt->count)
- push_arg("-c");
- if (opt->post_context || opt->pre_context) {
- if (opt->post_context != opt->pre_context) {
- if (opt->pre_context) {
- push_arg("-B");
- len += snprintf(argptr, sizeof(randarg)-len,
- "%u", opt->pre_context) + 1;
- if (sizeof(randarg) <= len)
- die("maximum length of args exceeded");
- push_arg(argptr);
- argptr += len;
- }
- if (opt->post_context) {
- push_arg("-A");
- len += snprintf(argptr, sizeof(randarg)-len,
- "%u", opt->post_context) + 1;
- if (sizeof(randarg) <= len)
- die("maximum length of args exceeded");
- push_arg(argptr);
- argptr += len;
- }
- }
- else {
- push_arg("-C");
- len += snprintf(argptr, sizeof(randarg)-len,
- "%u", opt->post_context) + 1;
- if (sizeof(randarg) <= len)
- die("maximum length of args exceeded");
- push_arg(argptr);
- argptr += len;
- }
- }
- for (p = opt->pattern_list; p; p = p->next) {
- push_arg("-e");
- push_arg(p->pattern);
- }
- if (opt->color) {
- struct strbuf sb = STRBUF_INIT;
-
- grep_add_color(&sb, opt->color_match);
- setenv("GREP_COLOR", sb.buf, 1);
-
- strbuf_reset(&sb);
- strbuf_addstr(&sb, "mt=");
- grep_add_color(&sb, opt->color_match);
- strbuf_addstr(&sb, ":sl=:cx=:fn=:ln=:bn=:se=");
- setenv("GREP_COLORS", sb.buf, 1);
-
- strbuf_release(&sb);
-
- if (opt->color_external && strlen(opt->color_external) > 0)
- push_arg(opt->color_external);
- } else {
- unsetenv("GREP_COLOR");
- unsetenv("GREP_COLORS");
- }
- unsetenv("GREP_OPTIONS");
-
- hit = 0;
- argc = nr;
- for (i = 0; i < active_nr; i++) {
- struct cache_entry *ce = active_cache[i];
- char *name;
- int kept;
- if (!S_ISREG(ce->ce_mode))
- continue;
- if (!pathspec_matches(paths, ce->name, opt->max_depth))
- continue;
- name = ce->name;
- if (name[0] == '-') {
- int len = ce_namelen(ce);
- name = xmalloc(len + 3);
- memcpy(name, "./", 2);
- memcpy(name + 2, ce->name, len + 1);
- }
- argv[argc++] = name;
- if (MAXARGS <= argc) {
- status = flush_grep(opt, argc, nr, argv, &kept);
- if (0 < status)
- hit = 1;
- argc = nr + kept;
- }
- if (ce_stage(ce)) {
- do {
- i++;
- } while (i < active_nr &&
- !strcmp(ce->name, active_cache[i]->name));
- i--; /* compensate for loop control */
- }
- }
- if (argc > nr) {
- status = flush_grep(opt, argc, nr, argv, NULL);
- if (0 < status)
- hit = 1;
- }
- return hit;
-}
-#endif
-
-static int grep_cache(struct grep_opt *opt, const char **paths, int cached,
- int external_grep_allowed)
+static int grep_cache(struct grep_opt *opt, const char **paths, int cached)
{
int hit = 0;
int nr;
read_cache();
-#if !NO_EXTERNAL_GREP
- /*
- * Use the external "grep" command for the case where
- * we grep through the checked-out files. It tends to
- * be a lot more optimized
- */
- if (!cached && external_grep_allowed) {
- hit = external_grep(opt, paths, cached);
- if (hit >= 0)
- return hit;
- hit = 0;
- }
-#endif
-
for (nr = 0; nr < active_nr; nr++) {
struct cache_entry *ce = active_cache[nr];
if (!S_ISREG(ce->ce_mode))
@@ -697,8 +409,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
{
int hit = 0;
int cached = 0;
- int external_grep_allowed = 1;
int seen_dashdash = 0;
+ int external_grep_allowed__ignored;
struct grep_opt opt;
struct object_array list = { 0, 0, NULL };
const char **paths = NULL;
@@ -780,13 +492,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
OPT_BOOLEAN(0, "all-match", &opt.all_match,
"show only matches from files that match all patterns"),
OPT_GROUP(""),
-#if NO_EXTERNAL_GREP
- OPT_BOOLEAN(0, "ext-grep", &external_grep_allowed,
- "allow calling of grep(1) (ignored by this build)"),
-#else
- OPT_BOOLEAN(0, "ext-grep", &external_grep_allowed,
- "allow calling of grep(1) (default)"),
-#endif
+ OPT_BOOLEAN(0, "ext-grep", &external_grep_allowed__ignored,
+ "allow calling of grep(1) (ignored by this build)"),
{ OPTION_CALLBACK, 0, "help-all", &options, NULL, "show usage",
PARSE_OPT_HIDDEN | PARSE_OPT_NOARG, help_callback },
OPT_END()
@@ -837,8 +544,6 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
argc--;
}
- if ((opt.color && !opt.color_external) || opt.funcname)
- external_grep_allowed = 0;
if (!opt.pattern_list)
die("no pattern given.");
if (!opt.fixed && opt.ignore_case)
@@ -884,7 +589,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
if (!list.nr) {
if (!cached)
setup_work_tree();
- return !grep_cache(&opt, paths, cached, external_grep_allowed);
+ return !grep_cache(&opt, paths, cached);
}
if (cached)
diff --git a/t/t7002-grep.sh b/t/t7002-grep.sh
index abd14bf..c369cdb 100755
--- a/t/t7002-grep.sh
+++ b/t/t7002-grep.sh
@@ -302,8 +302,8 @@ test_expect_success 'grep -C1, hunk mark between files' '
test_cmp expected actual
'
-test_expect_success 'grep -C1 --no-ext-grep, hunk mark between files' '
- git grep -C1 --no-ext-grep "^[yz]" >actual &&
+test_expect_success 'grep -C1 hunk mark between files' '
+ git grep -C1 "^[yz]" >actual &&
test_cmp expected actual
'
@@ -359,7 +359,7 @@ test_expect_success 'log grep (6)' '
test_expect_success 'grep with CE_VALID file' '
git update-index --assume-unchanged t/t &&
rm t/t &&
- test "$(git grep --no-ext-grep test)" = "t/t:test" &&
+ test "$(git grep test)" = "t/t:test" &&
git update-index --no-assume-unchanged t/t &&
git checkout t/t
'
--
1.6.6.292.ge84ea.dirty
next prev parent reply other threads:[~2010-01-13 6:48 UTC|newest]
Thread overview: 60+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-12-30 14:11 [PATCH] grep: do not do external grep on skip-worktree entries Nguyễn Thái Ngọc Duy
2009-12-31 7:01 ` Junio C Hamano
2009-12-31 7:09 ` Junio C Hamano
2010-01-02 11:50 ` Nguyen Thai Ngoc Duy
2010-01-02 18:44 ` Junio C Hamano
2010-01-02 19:15 ` Nguyen Thai Ngoc Duy
2010-01-02 19:45 ` Junio C Hamano
2010-01-03 2:35 ` Miles Bader
2010-01-03 2:47 ` Miles Bader
2010-01-03 3:08 ` Miles Bader
2010-01-03 19:32 ` Linus Torvalds
2010-01-03 20:49 ` Junio C Hamano
2010-01-04 5:31 ` Jeff King
2010-01-04 5:52 ` Junio C Hamano
2010-01-04 6:44 ` Jeff King
2010-01-04 7:08 ` Junio C Hamano
2010-01-04 7:14 ` Junio C Hamano
2010-01-04 7:29 ` Jeff King
2010-01-04 7:26 ` Jeff King
2010-01-04 8:09 ` Jeff King
2010-01-04 16:01 ` Linus Torvalds
2010-01-04 15:54 ` Linus Torvalds
2010-01-04 15:57 ` Miles Bader
2010-01-04 16:03 ` Linus Torvalds
2010-01-11 6:39 ` Junio C Hamano
2010-01-11 15:43 ` Linus Torvalds
2010-01-11 15:59 ` Linus Torvalds
2010-01-11 16:22 ` Junio C Hamano
2010-01-11 16:24 ` Junio C Hamano
2010-01-11 16:33 ` Linus Torvalds
2010-01-12 8:29 ` Junio C Hamano
2010-01-12 8:31 ` [PATCH] grep: lookahead optimization can be used with -L option Junio C Hamano
2010-01-12 8:32 ` [PATCH] grep: -L should show empty files Junio C Hamano
2010-01-12 21:27 ` Sverre Rabbelier
2010-01-13 6:56 ` Junio C Hamano
2010-01-13 16:04 ` Sverre Rabbelier
2010-01-13 19:48 ` Junio C Hamano
2010-01-13 6:48 ` Junio C Hamano [this message]
2010-01-13 8:29 ` [PATCH 1/2] grep: rip out support for external grep Jay Soffian
2010-01-13 8:59 ` Junio C Hamano
2010-01-13 15:20 ` Linus Torvalds
2010-01-13 6:51 ` [PATCH 2/2] grep: rip out pessimization to use fixmatch() Junio C Hamano
2010-01-12 16:21 ` [PATCH] grep: do not do external grep on skip-worktree entries Jeff King
2010-01-11 19:26 ` Fredrik Kuivinen
[not found] ` <4c8ef71001111119p253170f8q37bcd3708d894a62@mail.gmail.com>
2010-01-11 19:29 ` Linus Torvalds
2010-01-11 19:40 ` Fredrik Kuivinen
2010-01-11 20:07 ` Linus Torvalds
2010-01-11 21:07 ` Fredrik Kuivinen
2010-01-11 21:24 ` Linus Torvalds
2010-01-04 16:24 ` Linus Torvalds
2010-01-04 10:14 ` Nguyen Thai Ngoc Duy
2010-01-04 6:06 ` Mike Hommey
2010-01-04 7:04 ` Jeff King
2010-01-04 12:34 ` [PATCH 1/2] t7002: set test prerequisite "external-grep" if supported Nguyễn Thái Ngọc Duy
2010-01-07 2:37 ` Junio C Hamano
2010-01-07 4:29 ` Junio C Hamano
2010-01-07 13:27 ` Nguyen Thai Ngoc Duy
2010-01-07 14:04 ` Johannes Sixt
2010-01-07 14:26 ` Nguyen Thai Ngoc Duy
2010-01-04 12:34 ` [PATCH 2/2] t7002: add tests for skip-worktree fixes in commit a67e281 Nguyễn Thái Ngọc Duy
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=7v4omqv6tx.fsf_-_@alter.siamese.dyndns.org \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=miles@gnu.org \
--cc=pclouds@gmail.com \
--cc=peff@peff.net \
--cc=torvalds@linux-foundation.org \
/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.
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).