Re: [PATCH 2/3] cache-tree: detect mismatching number of index entries

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

> +	if (it->entry_count + pos > istate->cache_nr) {
> +		ret = error(_("corrupted cache-tree has entries not present in index"));
> +		goto out;
> +	}

Is it a safe assumption that the if() condition always indicates an
error?  When sparse-index is in effect, istate->cache_nr may be a
number that is smaller than the true number of paths in the index
(because all paths under a subdirectory we are not interested in are
folded into a single tree-ish entry), and I am not sure how it
should interact with it->entry_count (i.e. the number of paths under
the current directory we are looking at, which obviously cannot be a
sparsified entry) and pos (i.e. the index into active_cache[] that
represend the first path under the current directory)?

I guess as long as "it" is not folded, it does not matter how other
paths from different directories in active_cache[] are sparsified or
expanded, as long as "pos" keeps track of the current position
correctly.

> diff --git a/t/t4058-diff-duplicates.sh b/t/t4058-diff-duplicates.sh
> index 2501c89c1c9..3f602adb055 100755
> --- a/t/t4058-diff-duplicates.sh
> +++ b/t/t4058-diff-duplicates.sh
> @@ -132,15 +132,15 @@ test_expect_success 'create a few commits' '
>  	rm commit_id up final
>  '
>  
> -test_expect_failure 'git read-tree does not segfault' '
> -	test_when_finished rm .git/index.lock &&
> -	test_might_fail git read-tree --reset base
> +test_expect_success 'git read-tree does not segfault' '
> +	test_must_fail git read-tree --reset base 2>err &&
> +	test_grep "error: corrupted cache-tree has entries not present in index" err
>  '

Very good.  test_might_fail is a sign of trouble, and this gives us
a lot more predictability.