Re: git diff: support "-U" and "--unified" options properly

From: Linus Torvalds <torvalds@osdl.org>
Date: May 15, 2006, 03:49 UTC
Message-ID: <Pine.LNX.4.64.0605142044550.3866@g5.osdl.org>
In-Reply-To: <Pine.LNX.4.64.0605141745410.3866@g5.osdl.org>

On Sun, 14 May 2006, Linus Torvalds wrote:

Show 13 quoted lines

> 
> So I would actually assume that the solution is to simply make 
> grep_cache() have a simple
> 
> 	#ifdef __unix__
> 		if (!cached) {
> 			hit = external_grep(opt, paths, cached);
> 			if (hit >= 0)
> 				return hit;
> 		}
> 	#endif
> 
> at the top, so that we'd have the best of both worlds.

Ok. Here's a slightly tested version, my out-lined thing from the email was pretty close to working already.

It's not perfect, but it gets the "git grep some-random-string" down to the good old half-a-second range for the kernel.

It should convert more of the argument flags for "grep", that should be trivial to expand (I did a few just as an example). It should also bother to try to return the right "hit" value (which it doesn't, right now - the code is kind of there, but I didn't actually bother to do it _right_).

Also, right now it _just_ limits by number of arguments, but it should also strictly speaking limit by total argument size (ie add up the length of the filenames, and do the "exec_grep()" flush call if it's bigger than some random value like 32kB).

But I think that it's _conceptually_ doing all the right things, and it seems to work. So maybe somebody else can do some of the final polish.

		Linus

----

diff --git a/builtin-grep.c b/builtin-grep.c
index fead356..14471db 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -12,6 +12,7 @@ #include "tree-walk.h"
 #include "builtin.h"
 #include <regex.h>
 #include <fnmatch.h>
+#include <sys/wait.h>
 
 /*
  * git grep pathspecs are somewhat different from diff-tree pathspecs;
@@ -409,12 +410,90 @@ static int grep_file(struct grep_opt *op
 	return i;
 }
 
+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
+
+static int external_grep(struct grep_opt *opt, const char **paths, int cached)
+{
+	int i, nr, argc, hit;
+	const char *argv[MAXARGS+1];
+	struct grep_pat *p;
+
+	nr = 0;
+	argv[nr++] = "grep";
+	if (opt->word_regexp)
+		argv[nr++] = "-w";
+	if (opt->name_only)
+		argv[nr++] = "-l";
+	for (p = opt->pattern_list; p; p = p->next) {
+		argv[nr++] = "-e";
+		argv[nr++] = p->pattern;
+	}
+	argv[nr++] = "--";
+
+	hit = 0;
+	argc = nr;
+	for (i = 0; i < active_nr; i++) {
+		struct cache_entry *ce = active_cache[i];
+		if (ce_stage(ce) || !S_ISREG(ntohl(ce->ce_mode)))
+			continue;
+		if (!pathspec_matches(paths, ce->name))
+			continue;
+		argv[argc++] = ce->name;
+		if (argc < MAXARGS)
+			continue;
+		hit += exec_grep(argc, argv);
+		argc = nr;
+	}
+	if (argc > nr)
+		hit += exec_grep(argc, argv);
+	return 0;
+}
+
 static int grep_cache(struct grep_opt *opt, const char **paths, int cached)
 {
 	int hit = 0;
 	int nr;
 	read_cache();
 
+#ifdef __unix__
+	/*
+	 * 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) {
+		hit = external_grep(opt, paths, cached);
+		if (hit >= 0)
+			return hit;
+	}
+#endif
+
 	for (nr = 0; nr < active_nr; nr++) {
 		struct cache_entry *ce = active_cache[nr];
 		if (ce_stage(ce) || !S_ISREG(ntohl(ce->ce_mode)))