mirror of
https://github.com/git/git.git
synced 2024-10-29 21:37:53 +01:00
5581a019ba
Ever since the split index feature was introduced [1], refreshing a
split index is prone to a variant of the classic racy git problem.
Consider the following sequence of commands updating the split index
when the shared index contains a racily clean cache entry, i.e. an
entry whose cached stat data matches with the corresponding file in
the worktree and the cached mtime matches that of the index:
echo "cached content" >file
git update-index --split-index --add file
echo "dirty worktree" >file # size stays the same!
# ... wait ...
git update-index --add other-file
Normally, when a non-split index is updated, then do_write_index()
(the function responsible for writing all kinds of indexes, "regular",
split, and shared) recognizes racily clean cache entries, and writes
them with smudged stat data, i.e. with file size set to 0. When
subsequent git commands read the index, they will notice that the
smudged stat data doesn't match with the file in the worktree, and
then go on to check the file's content and notice its dirtiness.
In the above example, however, in the second 'git update-index'
prepare_to_write_split_index() decides which cache entries stored only
in the shared index should be replaced in the new split index. Alas,
this function never looks out for racily clean cache entries, and
since the file's stat data in the worktree hasn't changed since the
shared index was written, it won't be replaced in the new split index.
Consequently, do_write_index() doesn't even get this racily clean
cache entry, and can't smudge its stat data. Subsequent git commands
will then see that the index has more recent mtime than the file and
that the (not smudged) cached stat data still matches with the file in
the worktree, and, ultimately, will erroneously consider the file
clean.
Modify prepare_to_write_split_index() to recognize racily clean cache
entries, and mark them to be added to the split index. Note that
there are two places where it should check raciness: first those cache
entries that are only stored in the shared index, and then those that
have been copied by unpack_trees() from the shared index while it
constructed a new index. This way do_write_index() will get these
racily clean cache entries as well, and will then write them with
smudged stat data to the new split index.
This change makes all tests in 't1701-racy-split-index.sh' pass, so
flip the two 'test_expect_failure' tests to success. Also add the '#'
(as in nr. of trial) to those tests' description that were omitted
when the tests expected failure.
Note that after this change if the index is split when it contains a
racily clean cache entry, then a smudged cache entry will be written
both to the new shared and to the new split indexes. This doesn't
affect regular git commands: as far as they are concerned this is just
an entry in the split index replacing an outdated entry in the shared
index. It did affect a few tests in 't1700-split-index.sh', though,
because they actually check which entries are stored in the split
index; a previous patch in this series has already made the necessary
adjustments in 't1700'. And racily clean cache entries and index
splitting are rare enough to not worry about the resulting duplicated
smudged cache entries, and the additional complexity required to
prevent them is not worth it.
Several tests failed occasionally when the test suite was run with
'GIT_TEST_SPLIT_INDEX=yes'. Here are those that I managed to trace
back to this racy split index problem, starting with those failing
more frequently, with a link to a failing Travis CI build job for
each. The highlighted line [2] shows when the racy file was written,
which is not always in the failing test but in a preceeding setup
test.
t3903-stash.sh:
https://travis-ci.org/git/git/jobs/385542084#L5858
t4024-diff-optimize-common.sh:
https://travis-ci.org/git/git/jobs/386531969#L3174
t4015-diff-whitespace.sh:
https://travis-ci.org/git/git/jobs/360797600#L8215
t2200-add-update.sh:
https://travis-ci.org/git/git/jobs/382543426#L3051
t0090-cache-tree.sh:
https://travis-ci.org/git/git/jobs/416583010#L3679
There might be others, e.g. perhaps 't1000-read-tree-m-3way.sh' and
others using 'lib-read-tree-m-3way.sh', but I couldn't confirm yet.
[1] In the branch leading to the merge commit v2.1.0-rc0~45 (Merge
branch 'nd/split-index', 2014-07-16).
[2] Note that those highlighted lines are in the 'after failure' fold,
and your browser might unhelpfully fold it up before you could
take a good look.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
214 lines
6.1 KiB
Bash
Executable file
214 lines
6.1 KiB
Bash
Executable file
#!/bin/sh
|
|
|
|
# This test can give false success if your machine is sufficiently
|
|
# slow or all trials happened to happen on second boundaries.
|
|
|
|
test_description='racy split index'
|
|
|
|
. ./test-lib.sh
|
|
|
|
test_expect_success 'setup' '
|
|
# Only split the index when the test explicitly says so.
|
|
sane_unset GIT_TEST_SPLIT_INDEX &&
|
|
git config splitIndex.maxPercentChange 100 &&
|
|
|
|
echo "cached content" >racy-file &&
|
|
git add racy-file &&
|
|
git commit -m initial &&
|
|
|
|
echo something >other-file &&
|
|
# No raciness with this file.
|
|
test-tool chmtime =-20 other-file &&
|
|
|
|
echo "+cached content" >expect
|
|
'
|
|
|
|
check_cached_diff () {
|
|
git diff-index --patch --cached $EMPTY_TREE racy-file >diff &&
|
|
tail -1 diff >actual &&
|
|
test_cmp expect actual
|
|
}
|
|
|
|
trials="0 1 2 3 4"
|
|
for trial in $trials
|
|
do
|
|
test_expect_success "split the index while adding a racily clean file #$trial" '
|
|
rm -f .git/index .git/sharedindex.* &&
|
|
|
|
# The next three commands must be run within the same
|
|
# second (so both writes to racy-file result in the same
|
|
# mtime) to create the interesting racy situation.
|
|
echo "cached content" >racy-file &&
|
|
|
|
# Update and split the index. The cache entry of
|
|
# racy-file will be stored only in the shared index.
|
|
git update-index --split-index --add racy-file &&
|
|
|
|
# File size must stay the same.
|
|
echo "dirty worktree" >racy-file &&
|
|
|
|
# Subsequent git commands should notice that racy-file
|
|
# and the split index have the same mtime, and check
|
|
# the content of the file to see if it is actually
|
|
# clean.
|
|
check_cached_diff
|
|
'
|
|
done
|
|
|
|
for trial in $trials
|
|
do
|
|
test_expect_success "add a racily clean file to an already split index #$trial" '
|
|
rm -f .git/index .git/sharedindex.* &&
|
|
|
|
git update-index --split-index &&
|
|
|
|
# The next three commands must be run within the same
|
|
# second.
|
|
echo "cached content" >racy-file &&
|
|
|
|
# Update the split index. The cache entry of racy-file
|
|
# will be stored only in the split index.
|
|
git update-index --add racy-file &&
|
|
|
|
# File size must stay the same.
|
|
echo "dirty worktree" >racy-file &&
|
|
|
|
# Subsequent git commands should notice that racy-file
|
|
# and the split index have the same mtime, and check
|
|
# the content of the file to see if it is actually
|
|
# clean.
|
|
check_cached_diff
|
|
'
|
|
done
|
|
|
|
for trial in $trials
|
|
do
|
|
test_expect_success "split the index when the index contains a racily clean cache entry #$trial" '
|
|
rm -f .git/index .git/sharedindex.* &&
|
|
|
|
# The next three commands must be run within the same
|
|
# second.
|
|
echo "cached content" >racy-file &&
|
|
|
|
git update-index --add racy-file &&
|
|
|
|
# File size must stay the same.
|
|
echo "dirty worktree" >racy-file &&
|
|
|
|
# Now wait a bit to ensure that the split index written
|
|
# below will get a more recent mtime than racy-file.
|
|
sleep 1 &&
|
|
|
|
# Update and split the index when the index contains
|
|
# the racily clean cache entry of racy-file.
|
|
# A corresponding replacement cache entry with smudged
|
|
# stat data should be added to the new split index.
|
|
git update-index --split-index --add other-file &&
|
|
|
|
# Subsequent git commands should notice the smudged
|
|
# stat data in the replacement cache entry and that it
|
|
# doesnt match with the file the worktree.
|
|
check_cached_diff
|
|
'
|
|
done
|
|
|
|
for trial in $trials
|
|
do
|
|
test_expect_success "update the split index when it contains a new racily clean cache entry #$trial" '
|
|
rm -f .git/index .git/sharedindex.* &&
|
|
|
|
git update-index --split-index &&
|
|
|
|
# The next three commands must be run within the same
|
|
# second.
|
|
echo "cached content" >racy-file &&
|
|
|
|
# Update the split index. The cache entry of racy-file
|
|
# will be stored only in the split index.
|
|
git update-index --add racy-file &&
|
|
|
|
# File size must stay the same.
|
|
echo "dirty worktree" >racy-file &&
|
|
|
|
# Now wait a bit to ensure that the split index written
|
|
# below will get a more recent mtime than racy-file.
|
|
sleep 1 &&
|
|
|
|
# Update the split index when the racily clean cache
|
|
# entry of racy-file is only stored in the split index.
|
|
# An updated cache entry with smudged stat data should
|
|
# be added to the new split index.
|
|
git update-index --add other-file &&
|
|
|
|
# Subsequent git commands should notice the smudged
|
|
# stat data.
|
|
check_cached_diff
|
|
'
|
|
done
|
|
|
|
for trial in $trials
|
|
do
|
|
test_expect_success "update the split index when a racily clean cache entry is stored only in the shared index #$trial" '
|
|
rm -f .git/index .git/sharedindex.* &&
|
|
|
|
# The next three commands must be run within the same
|
|
# second.
|
|
echo "cached content" >racy-file &&
|
|
|
|
# Update and split the index. The cache entry of
|
|
# racy-file will be stored only in the shared index.
|
|
git update-index --split-index --add racy-file &&
|
|
|
|
# File size must stay the same.
|
|
echo "dirty worktree" >racy-file &&
|
|
|
|
# Now wait a bit to ensure that the split index written
|
|
# below will get a more recent mtime than racy-file.
|
|
sleep 1 &&
|
|
|
|
# Update the split index when the racily clean cache
|
|
# entry of racy-file is only stored in the shared index.
|
|
# A corresponding replacement cache entry with smudged
|
|
# stat data should be added to the new split index.
|
|
git update-index --add other-file &&
|
|
|
|
# Subsequent git commands should notice the smudged
|
|
# stat data.
|
|
check_cached_diff
|
|
'
|
|
done
|
|
|
|
for trial in $trials
|
|
do
|
|
test_expect_success "update the split index after unpack trees() copied a racily clean cache entry from the shared index #$trial" '
|
|
rm -f .git/index .git/sharedindex.* &&
|
|
|
|
# The next three commands must be run within the same
|
|
# second.
|
|
echo "cached content" >racy-file &&
|
|
|
|
# Update and split the index. The cache entry of
|
|
# racy-file will be stored only in the shared index.
|
|
git update-index --split-index --add racy-file &&
|
|
|
|
# File size must stay the same.
|
|
echo "dirty worktree" >racy-file &&
|
|
|
|
# Now wait a bit to ensure that the split index written
|
|
# below will get a more recent mtime than racy-file.
|
|
sleep 1 &&
|
|
|
|
# Update the split index after unpack_trees() copied the
|
|
# racily clean cache entry of racy-file from the shared
|
|
# index. A corresponding replacement cache entry
|
|
# with smudged stat data should be added to the new
|
|
# split index.
|
|
git read-tree -m HEAD &&
|
|
|
|
# Subsequent git commands should notice the smudged
|
|
# stat data.
|
|
check_cached_diff
|
|
'
|
|
done
|
|
|
|
test_done
|