Re: [PATCH 3/3] diff-index: pass pathspec down to unpack-trees machinery

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jan 11, 2012, 06:31 UTC
Message-ID: <20120111063104.GA3153@burratino>
In-Reply-To: <1314653603-7533-4-git-send-email-gitster@pobox.com>

Hi,

Junio C Hamano wrote:

Show 5 quoted lines

> And finally, pass the pathspec down through unpack_trees() to traverse_trees()
> callchain.
>
> Before and after applying this series, looking for changes in the kernel
> repository with a fairly narrow pathspec gets a moderate speeds up.

Check this out:

	mkdir -p test-repo/subdir
	cd test-repo
	git init
	echo hi >subdir/hello.h
	git add subdir/hello.h
	git commit -m 'say hi'
	git diff-index --abbrev HEAD -- '*.h'

In git versions which include the patch described above, the unchanged subdir/hello.h shows up as a newly added file. Reverting that patch (v1.7.7.1~22^2, diff-index: pass pathspec down to unpack-trees machinery, 2011-08-29) makes "git diff HEAD" with wildcards work again.

It looks like the pruning of the preimage by pathspec is too aggressive and is omiting whole directories that do not match the pathspec without realizing that the path to a file contained within might match.

We could just add a test for this to the testsuite and do that revert, but I'd rather not yet, in case this is a symptom of some deeper unpack_trees() confusion.

Hints?

Thanks, Jonathan

Show 19 quoted lines

>  diff-lib.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/diff-lib.c b/diff-lib.c
> index f8454dd..ebe751e 100644
> --- a/diff-lib.c
> +++ b/diff-lib.c
> @@ -468,6 +468,7 @@ static int diff_cache(struct rev_info *revs,
>  	opts.unpack_data = revs;
>  	opts.src_index = &the_index;
>  	opts.dst_index = NULL;
> +	opts.pathspec = &revs->diffopt.pathspec;
>  
>  	init_tree_desc(&t, tree->buffer, tree->size);
>  	return unpack_trees(1, &t, &opts);
> -- 
> 1.7.7.rc0.70.g82660
>
>