Re: [PATCH 1/5] Prevent diff machinery from examining worktree outside sparse checkout

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

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

> ---

Sign off?  Oh, this is an RFC... Ok...

Show 21 quoted lines

> diff --git a/diff-lib.c b/diff-lib.c
> index b7813af..4094f18 100644
> --- a/diff-lib.c
> +++ b/diff-lib.c
> @@ -337,6 +337,8 @@ static void do_oneway_diff(struct unpack_trees_options *o,
>  	struct rev_info *revs = o->unpack_data;
>  	int match_missing, cached;
>  
> +	/* if the entry is not checked out, don't examine work tree */
> +	cached = o->index_only || (idx && idx->ce_flags & CE_VALID);
>  	/*
>  	 * Backward compatibility wart - "diff-index -m" does
>  	 * not mean "do not ignore merges", but "match_missing".
> @@ -344,7 +346,6 @@ static void do_oneway_diff(struct unpack_trees_options *o,
>  	 * But with the revision flag parsing, that's found in
>  	 * "!revs->ignore_merges".
>  	 */
> -	cached = o->index_only;
>  	match_missing = !revs->ignore_merges;
>  
>  	if (cached && idx && ce_stage(idx)) {

The big comment is about match_missing and not about cached, so the code movement is good. When we pass NULL idx to this function, it means that the tree has it but the index doesn't, so it cannot possibly be marked as "exists in the index but is not checked out". So the hunk makes sense.

Show 15 quoted lines

> diff --git a/diff.c b/diff.c
> index 91d6ea2..de1bd87 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -1810,8 +1810,10 @@ static int reuse_worktree_file(const char *name, const unsigned char *sha1, int
>  
>  	/*
>  	 * If ce matches the file in the work tree, we can reuse it.
> +	 * For sparse checkout case, ce_uptodate() may be true although
> +	 * the file may or may not exist in the work tree.
>  	 */
> -	if (ce_uptodate(ce) ||
> +	if ((ce_uptodate(ce) && !(ce->ce_flags & CE_VALID)) ||
>  	    (!lstat(name, &st) && !ce_match_stat(ce, &st, 0)))
>  		return 1;

Why is this hunk necessary? A few lines above this we return 0 for any CE_VALID cache entry.