Re: [PATCH] cache-tree: invalidate i-t-a paths after writing trees

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 9, 2012, 11:57 UTC
Message-ID: <7vy5ibouo4.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1352459040-14452-1-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

Show 13 quoted lines

> diff --git a/cache-tree.c b/cache-tree.c
> index 28ed657..30a8018 100644
> --- a/cache-tree.c
> +++ b/cache-tree.c
> @@ -381,6 +381,9 @@ int cache_tree_update(struct cache_tree *it,
>  	i = update_one(it, cache, entries, "", 0, flags);
>  	if (i < 0)
>  		return i;
> +	for (i = 0; i < entries; i++)
> +		if (cache[i]->ce_flags & CE_INTENT_TO_ADD)
> +			cache_tree_invalidate_path(it, cache[i]->name);
>  	return 0;
>  }

I notice there is another special case for CE_REMOVE but there is nothing that adjusts the cache-tree for these entries in the current codebase.

I suspect the original code before we (perhaps incorrectly) updated the code not to error out upon I-T-A entries was fine only because we do not attempt to fully populate the cache-tree during a merge in the unpack-trees codepath, which will mark the index entries that are to be removed with CE_REMOVE in the resulting index.

The solution implemented with this patch will break if we start updating the cache tree after a successful merge in unpack-trees, I suspect.

An alternative might be to add a "phoney" bit next to "used" in the cache_tree structure, mark the cache tree as phoney when we skip an entry marked as CE_REMOVE or CE_ITA, and make the postprocessing loop this patch adds aware of that bit, instead of iterating over the index entries; instead, it would recurse the resulting cache tree and invalidate parts of the tree that have subtrees with the "phoney" bit set, or something.