git grep -I bug

git grep -I bug

[Jeremy O'Brien]

[-- Attachment #1: Type: text/plain, Size: 291 bytes --]

I am running git version 1.6.1.2.309.g2ea3.

When I use

git grep -I "string_to_match"

to ignore binary files in my grep, binary files are returned anyway.

When I do a regular

grep "string_to_match" *

grep does not show the binary files. I believe this is a bug.

Thanks,
Jeremy O'Brien

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: git grep -I bug

[Junio C Hamano]

Jeremy O'Brien <obrien654j@gmail.com> writes:

> I am running git version 1.6.1.2.309.g2ea3.
>
> When I use
>
> git grep -I "string_to_match"
>
> to ignore binary files in my grep, binary files are returned anyway.

One sanity check.  What does 'git grep --cached -I "string_to_match"' do
in that case?

If it works as expected but without --cached it doesn't, then I think the
following patch will fix it.

-- >8 --
Subject: grep: pass -I (ignore binary) down to external grep

The external-grep codepath forgets to pass this option.  Fix it.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-grep.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git c/builtin-grep.c w/builtin-grep.c
index bebf15c..c799fdd 100644
--- c/builtin-grep.c
+++ w/builtin-grep.c
@@ -297,6 +297,8 @@ static int external_grep(struct grep_opt *opt, const char **paths, int cached)
 		push_arg("-l");
 	if (opt->unmatch_name_only)
 		push_arg("-L");
+	if (opt->binary == GREP_BINARY_NOMATCH)
+		push_arg("-I");
 	if (opt->null_following_name)
 		/* in GNU grep git's "-z" translates to "-Z" */
 		push_arg("-Z");

Re: git grep -I bug

[Jeremy O'Brien]

[-- Attachment #1: Type: text/plain, Size: 601 bytes --]

On Mon, Feb 02, 2009 at 09:54:31AM -0800, Junio C Hamano wrote:
> Jeremy O'Brien <obrien654j@gmail.com> writes:
> 
> > I am running git version 1.6.1.2.309.g2ea3.
> >
> > When I use
> >
> > git grep -I "string_to_match"
> >
> > to ignore binary files in my grep, binary files are returned anyway.
> 
> One sanity check.  What does 'git grep --cached -I "string_to_match"' do
> in that case?
> 

It works as expected. It is interesting that while my Linux install was
affected by this bug, my Mac OS X install did not seem to be affected by
it, while running the same version of git.

[-- Attachment #2: Type: application/pgp-signature, Size: 194 bytes --]

Re: git grep -I bug

[Junio C Hamano]

Jeremy O'Brien <obrien654j@gmail.com> writes:

> On Mon, Feb 02, 2009 at 09:54:31AM -0800, Junio C Hamano wrote:
>> Jeremy O'Brien <obrien654j@gmail.com> writes:
>> 
>> > I am running git version 1.6.1.2.309.g2ea3.
>> >
>> > When I use
>> >
>> > git grep -I "string_to_match"
>> >
>> > to ignore binary files in my grep, binary files are returned anyway.
>> 
>> One sanity check.  What does 'git grep --cached -I "string_to_match"' do
>> in that case?
>> 
>
> It works as expected. It is interesting that while my Linux install was
> affected by this bug, my Mac OS X install did not seem to be affected by
> it, while running the same version of git.

Perhaps your Mac OSX binary was built without external grep support.

Did the patch fix the issue, by the way?

Re: git grep -I bug

[Jeremy O'Brien]

On Mon, Feb 02, 2009 at 08:30:43PM -0800, Junio C Hamano wrote:
> Jeremy O'Brien <obrien654j@gmail.com> writes:
> 
> > On Mon, Feb 02, 2009 at 09:54:31AM -0800, Junio C Hamano wrote:
> >> Jeremy O'Brien <obrien654j@gmail.com> writes:
> >> 
> >> > I am running git version 1.6.1.2.309.g2ea3.
> >> >
> >> > When I use
> >> >
> >> > git grep -I "string_to_match"
> >> >
> >> > to ignore binary files in my grep, binary files are returned anyway.
> >> 
> >> One sanity check.  What does 'git grep --cached -I "string_to_match"' do
> >> in that case?
> >> 
> >
> > It works as expected. It is interesting that while my Linux install was
> > affected by this bug, my Mac OS X install did not seem to be affected by
> > it, while running the same version of git.
> 
> Perhaps your Mac OSX binary was built without external grep support.
> 
> Did the patch fix the issue, by the way?

I'm assuming that was the problem. And yes, the patch fixed it just
fine, thank you.