Re: [PATCH] read-cache: fix untracked cache invalidation when split-index is used

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 8, 2015, 16:49 UTC
Message-ID: <xmqq7frecfi9.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1433673652-27720-1-git-send-email-pclouds@gmail.com>

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

Show 24 quoted lines

> Before this change, t7063.17 fails. The actual action though happens at
> t7063.16 where the entry "two" is added back to index after being
> removed in the .13. Here we expect a directory invalidate at .16 and
> none at .17 where untracked cache is refreshed. But things do not go as
> expected when GIT_TEST_SPLIT_INDEX is set.
>
> The different behavior that happens at .16 when split index is used: the
> entry "two", when deleted at .13, is simply marked "deleted". When .16
> executes, the entry resurfaces from the version in base index. This
> happens in merge_base_index() where add_index_entry() is called to add
> "two" back from the base index.
>
> This is where the bug comes from. The add_index_entry() is called with
> ADD_CACHE_KEEP_CACHE_TREE flag because this version of "two" is not new,
> it does not break either cache-tree or untracked cache. The code should
> check this flag and not invalidate untracked cache. This causes a second
> invalidation violates test expectation. The fix is obvious.
>
> Noticed-by: Thomas Gummerer <t.gummerer@gmail.com>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  PS. perhaps I should rename ADD_CACHE_KEEP_CACHE_TREE to
>  ADD_CACHE_KEEP_CACHE, or add a new flag .._KEEP_UNTRACKED_CACHE to
>  avoid confusion.

That may not be a bad idea, indeed.

>  Untracked cache, split index and the last part (*) all aim at a
>  smaller user audience with large work trees. They are not really what
>  a common user needs, but I hope they do have users.

I do hope that this can be made for everybody, though. Any project will get larger, not smaller, over time and these two (I am not sure what you refer to as 'last part', though) are meant to support projects with larger working trees better. After all, that is why I merged the untracked-cache series reasonably early to 'master' in this cycle to give us time to shake out little issues like this one. I think we killed two so far since it has been merged to 'master', so the plan is working rather nicely ;-).

Thanks.