Re: [PATCH 1/2] grep: accept relative paths outside current working directory

Clemens Buchacher [off-list ref] writes:

On Sat, Sep 05, 2009 at 12:58:50AM -0700, Junio C Hamano wrote:

quoted

quoted

quoted

If "git add -u ../.." (I mean "the grand parent directory", not "an
unnamed subdirectory") did not work 

In git.git:

$ cd t
$ git grep addremove -- ../
fatal: git grep: cannot generate relative filenames containing '..'

So here's a fix for that. And a configurable solution for add and grep's
scope as a follow-up. I did not look at any other commands yet.

Thanks.  This was oa breakage I pointed out in an earlier message in this
discussion, and it is worth fixing.

Your patch is queued in 'pu', but it seems to break the exit status in a
strange way with my limited test.

Here is a non-broken behaviour without the "look in uplevel":

: git.git/cb/maint-1.6.3-grep-relative-up; ./git grep adddelete .
: git.git/cb/maint-1.6.3-grep-relative-up; ./git grep adddelete .; echo $?
1
: git.git/cb/maint-1.6.3-grep-relative-up; ./git grep adddelete . >/dev/null; echo $?
1

Now we go down, and grep from an uplevel:

: git.git/cb/maint-1.6.3-grep-relative-up; cd t
: t/cb/maint-1.6.3-grep-relative-up; ../git grep adddelete ..
: t/cb/maint-1.6.3-grep-relative-up; ../git grep adddelete .. ; echo $?
1
: t/cb/maint-1.6.3-grep-relative-up; ../git grep adddelete .. >/dev/null; echo $?
0

The command should not give different exit status depending on the
destination of standard output stream.

On Sun, Sep 06, 2009 at 03:58:15PM -0700, Junio C Hamano wrote:

Your patch is queued in 'pu', but it seems to break the exit status in a
strange way with my limited test.

[...]

The command should not give different exit status depending on the
destination of standard output stream.

I could also reproduce this in master using

	:~/git/t$ git grep --no-color not-going-to-match .; echo $?

The problem is a small bug in grep_cache(), which is triggered here because
external_grep() cannot deal with relative paths (not sure why that's the
case though). Fix below.

Clemens
--o<--

If external_grep() is called and returns an error, grep_cache() mistakenly
reported a hit, even if there were none. The bug can be triggered by
calling "git grep --no-color" from a subdirectory.

Signed-off-by: Clemens Buchacher <redacted>
---
 builtin-grep.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin-grep.c b/builtin-grep.c
index ad0e0a5..b577738 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c

@@ -500,9 +500,9 @@ static int grep_cache(struct grep_opt *opt, const char **paths, int cached,
 	 * be a lot more optimized
 	 */
 	if (!cached && external_grep_allowed) {
-		hit = external_grep(opt, paths, cached);
-		if (hit >= 0)
-			return hit;
+		int ret = external_grep(opt, paths, cached);
+		if (ret >= 0)
+			return ret;
 	}
 #endif
 

-- 
1.6.4.2.266.gbaa17