Re: [PATCH 2/5] grep: skip files outside sparse checkout area

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 31, 2009, 16:42 UTC
Message-ID: <7vljm4st0w.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1248850154-5469-3-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> ---
>  Documentation/git-grep.txt |    4 +++-
>  builtin-grep.c             |    7 ++++++-
>  2 files changed, 9 insertions(+), 2 deletions(-)

I do not think you can call this "CE_VALID fixup". This is a possibly useful but nevertheless a new, different behaviour.

We probably want opt->ignoreassumeunchanged in struct grep_opt, and control it via grep.ignoreassumeunchanged configuration, that perhaps defaults to true (i.e. "ignore"), with a command line override.

Oh, and perhaps a shorter-and-sweeter name than "ignoreAssumeUnchanged".

Show 10 quoted lines

> diff --git a/builtin-grep.c b/builtin-grep.c
> index ad0e0a5..55078cb 100644
> --- a/builtin-grep.c
> +++ b/builtin-grep.c
> @@ -455,6 +455,8 @@ static int external_grep(struct grep_opt *opt, const char **paths, int cached)
>  			continue;
>  		if (!pathspec_matches(paths, ce->name, opt->max_depth))
>  			continue;
> +		if (ce->ce_flags & CE_VALID)
> +			continue;

The test probably should come before pathspec_matches().

Show 13 quoted lines

> @@ -522,8 +524,11 @@ static int grep_cache(struct grep_opt *opt, const char **paths, int cached,
>  				continue;
>  			hit |= grep_sha1(opt, ce->sha1, ce->name, 0);
>  		}
> -		else
> +		else {
> +			if (ce->ce_flags & CE_VALID)
> +				continue;
>  			hit |= grep_file(opt, ce->name);
> +		}
>  		if (ce_stage(ce)) {
>  			do {
>  				nr++;

I do not think the new code in this hunk will be reached as the other arm of the "else" greps in the index when CE_VALID is set.

Has this series been tested or self-reviewed yet?