Re: [RFC PATCH 5/5] split-index: smudge and add racily clean cache entries to split index

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Sep 7, 2018, 03:49 UTC
Message-ID: <20180907034942.GA10370@localhost>
In-Reply-To: <87k1nyqzq2.fsf@evledraar.gmail.com>

On Thu, Sep 06, 2018 at 07:53:41PM +0200, Ævar Arnfjörð Bjarmason wrote:

Show 8 quoted lines

> I got 436 OK runs with that and 3 failures before I gave up and ctrl+c'd
> it. And the 3 failures were:
> 
>     t3903-stash.sh               (Wstat: 256 Tests: 90 Failed: 1)
>       Failed test:  55
>       Non-zero exit status: 1
> 
> So it's back to failing on the same test as before your patches.

Ah, what a pity :)

Luckily, the tests in t3903 are mostly self-contained, and copying test #55 into a dedicated test script still works. Then running it repeatedly is much faster than running the whole t3903, and failed in a reasonable amount of time. I could then eventually narrow it down to the diff below, which fails rather reliably, in fact I've yet to see it succeed. Unfortunately, 'git stash' is rather busy with index operations, and I couldn't figure out yet what exactly goes wrong or how to turn it into a proper test.

diff --git a/git-stash.sh b/git-stash.sh
index 94793c1a91..04fa2a4f43 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -328,6 +328,7 @@ push_stash () {
 			git diff-index -p --cached --binary HEAD -- "$@" |
 			git apply --index -R
 		else
+			GIT_TEST_SPLIT_INDEX=yes \
 			git reset --hard -q
 		fi
 
@@ -671,6 +672,7 @@ apply_to_branch () {
 	set -- --index "$@"
 	assert_stash_like "$@"
 
+	sleep 1
 	git checkout -b $branch $REV^ &&
 	apply_stash "$@" && {
 		test -z "$IS_STASH_REF" || drop_stash "$@"
diff --git a/read-cache.c b/read-cache.c
index 8f644f68b4..9f39f29221 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2761,9 +2761,8 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock,
 	}
 
 	if (git_env_bool("GIT_TEST_SPLIT_INDEX", 0)) {
-		int v = si->base_oid.hash[0];
-		if ((v & 15) < 6)
-			istate->cache_changed |= SPLIT_INDEX_ORDERED;
+		/* always split, to make it more deterministic */
+		istate->cache_changed |= SPLIT_INDEX_ORDERED;
 	}
 	if (too_many_not_shared_entries(istate))
 		istate->cache_changed |= SPLIT_INDEX_ORDERED;
diff --git a/t/t9999-stash-vs-split-index.sh b/t/t9999-stash-vs-split-index.sh
new file mode 100755
index 0000000000..bc2b7ccf6b
--- /dev/null
+++ b/t/t9999-stash-vs-split-index.sh
@@ -0,0 +1,20 @@
+test_description='stash vs. split index'
+
+. ./test-lib.sh
+
+test_expect_success 'stash vs. split index' '
+	sane_unset GIT_TEST_SPLIT_INDEX &&
+	git config splitIndex.maxPercentChange 100 &&
+
+	echo foo >file &&
+	GIT_TEST_SPLIT_INDEX=yes git add file &&
+	git commit -m initial &&
+	echo bar >file &&
+	git stash &&
+	echo baz >file &&
+	test_when_finished "git checkout master" &&
+	test_must_fail git stash branch new_branch stash@{0} &&
+	git rev-parse stash@{0} --
+'
+
+test_done

-- 
2.19.0.rc2.140.g09cf9e37c9