Re: [PATCH] cache-tree: do not cache empty trees

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

> diff --git a/cache-tree.c b/cache-tree.c
> index f755590..03732ad 100644
> --- a/cache-tree.c
> +++ b/cache-tree.c
> @@ -621,9 +621,18 @@ static void prime_cache_tree_rec(struct cache_tree *it, struct tree *tree)
>  			struct tree *subtree = lookup_tree(entry.sha1);
>  			if (!subtree->object.parsed)
>  				parse_tree(subtree);
> +			if (!hashcmp(entry.sha1, (unsigned char *)EMPTY_TREE_SHA1_BIN)) {
> +				warning("empty tree detected! Will be removed in new commits");
> +				cnt = -1;
> +				break;
> +			}

You shouldn't need the cast (if you did, then hashcmp() macro should be
fixed so that you don't need to).

I don't think warning() is warranted for an operation you introduced to
keep the internal data structure consistent.

Should this comparison done after we parsed the subtree, or should we be
doing that before it?

If you are adding this new check to a point where we have already parsed
the subtree object, don't you have a better and cheaper way to detect if
the subtree is empty than the 20-byte comparision, namely, perhaps by
looking at subtree->size?