Subject: Re: [PATCH v4 2/4] test-dump-cache-tree: invalid trees are not errors

From: Junio C Hamano <[email-masked]>
Date: Mon, 07 Jul 2014 12:27:35 -0700

David Turner <[email-masked]> writes:

> Do not treat known-invalid trees as errors even when their count is
> incorrect.  Because git already knows that these trees are invalid,
> nothing depends on the count field.

s/count/subtree_nr/; they are different.

"nothing depends on" is not quite correct.  The field keeps track of
the number of subdirectories in the directory recorded in the
cache-tree, and it *must* be maintained correctly (we iterate over
the it->down[] array up to that number).

The number of subdirectories the directory actually has, which we
can discover by counting entries in the main part of the index, may
be different from subtree_nr, if we haven't run update_one() on it,
e.g. we may have added a path in a new subdirectory, at which time
we would have invalidated the directory and its it->down[] array does
not know the new subdirectory.

While reading 3/4, I wondered if it makes sense to show the (N
subtrees) for invalidated node, but as a debugging measure it helped
me often while developping the framework, so we may not want to drop
the subtree_nr from the output for invalidated entries.

The change itself looks good.

Thanks.