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@xxxxxxxxx>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
---
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@xxxxxxxxx> wrote:
> Thomas Gummerer <t.gummerer@xxxxxxxxx> 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(-)
diff --git a/read-cache.c b/read-cache.c
index 723d48d..309ccc7 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -999,7 +999,8 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e
}
pos = -pos-1;
- untracked_cache_add_to_index(istate, ce->name);
+ if (!(option & ADD_CACHE_KEEP_CACHE_TREE))
+ untracked_cache_add_to_index(istate, ce->name);
/*
* Inserting a merged entry ("stage 0") into the index
--
2.3.0.rc1.137.g477eb31
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html