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.
On Sun, Jun 7, 2015 at 1:20 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
>
>> When running the test suite with GIT_TEST_SPLIT_INDEX set, tests 17-18
>> in t7063 fail. Unset GIT_TEST_SPLIT_INDEX at the beginning of the test,
>> in order to fix it.
>
> That is not fixing but sweeping the problem under the rug, is it?
Indeed.
> Duy, untracked-cache is a fairly new toy and I wouldn't be surprised
> if it has un-thought-out interaction with split-index which is also
> a fairly new exotic toy. As both are from you, can you take a look
> at it?
Still a bit slow to address, but I marked Thomas' mail for
investigation when it came.
> We may want to make it easier to run tests with TEST-SPLIT-INDEX, if
> we envision that the feature will bring us sufficient benefit and we
> would eventually want to encourage its use to more people. As it
> stands now, only people who are curious enough opt into trying it
> out by exporting the environment, which would be done by a tiny
> minority of developers and users.
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.
We could make the test suite run twice, a normal run and a
test-split-index run. But I'm not sure if it really justifies
doubling test time. We will have to deal with this situation when/if
pack-v4 is merged because we would want to exercise both v3 and v4 as
much as possible.
(*) the last part should keep index read time small enough even if
the index is very large and avoid lstat() at refresh time with
the help from watchman. read-cache.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Thomas Gummerer
