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

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 7, 2014, 19:27 UTC
Message-ID: <xmqqa98lc6l4.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1404619619-4774-2-git-send-email-dturner@twitter.com>

David Turner <dturner@twopensource.com> 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.