Skip to content

Commit

Permalink
Merge branch 'ms/update-index-racy'
Browse files Browse the repository at this point in the history
"git update-index --refresh" has been taught to deal better with
racy timestamps (just like "git status" already does).

* ms/update-index-racy:
  update-index: refresh should rewrite index in case of racy timestamps
  t7508: add tests capturing racy timestamp handling
  t7508: fix bogus mtime verification
  test-lib: introduce API for verifying file mtime
  • Loading branch information
gitster committed Feb 5, 2022
2 parents 1b4d9b4 + 2ede073 commit ee52b35
Show file tree
Hide file tree
Showing 6 changed files with 135 additions and 6 deletions.
11 changes: 11 additions & 0 deletions builtin/update-index.c
Original file line number Diff line number Diff line change
Expand Up @@ -787,6 +787,17 @@ static int refresh(struct refresh_params *o, unsigned int flag)
setup_work_tree();
read_cache();
*o->has_errors |= refresh_cache(o->flags | flag);
if (has_racy_timestamp(&the_index)) {
/*
* Even if nothing else has changed, updating the file
* increases the chance that racy timestamps become
* non-racy, helping future run-time performance.
* We do that even in case of "errors" returned by
* refresh_cache() as these are no actual errors.
* cmd_status() does the same.
*/
active_cache_changed |= SOMETHING_CHANGED;
}
return 0;
}

Expand Down
1 change: 1 addition & 0 deletions cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -889,6 +889,7 @@ void *read_blob_data_from_index(struct index_state *, const char *, unsigned lon
#define CE_MATCH_IGNORE_FSMONITOR 0X20
int is_racy_timestamp(const struct index_state *istate,
const struct cache_entry *ce);
int has_racy_timestamp(struct index_state *istate);
int ie_match_stat(struct index_state *, const struct cache_entry *, struct stat *, unsigned int);
int ie_modified(struct index_state *, const struct cache_entry *, struct stat *, unsigned int);

Expand Down
2 changes: 1 addition & 1 deletion read-cache.c
Original file line number Diff line number Diff line change
Expand Up @@ -2775,7 +2775,7 @@ static int repo_verify_index(struct repository *repo)
return verify_index_from(repo->index, repo->index_file);
}

static int has_racy_timestamp(struct index_state *istate)
int has_racy_timestamp(struct index_state *istate)
{
int entries = istate->cache_nr;
int i;
Expand Down
64 changes: 64 additions & 0 deletions t/t2108-update-index-refresh-racy.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
#!/bin/sh

test_description='update-index refresh tests related to racy timestamps'

TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh

reset_files () {
echo content >file &&
echo content >other &&
test_set_magic_mtime file &&
test_set_magic_mtime other
}

update_assert_changed () {
test_set_magic_mtime .git/index &&
test_might_fail git update-index "$1" &&
! test_is_magic_mtime .git/index
}

test_expect_success 'setup' '
reset_files &&
# we are calling reset_files() a couple of times during tests;
# test-tool chmtime does not change the ctime; to not weaken
# or even break our tests, disable ctime-checks entirely
git config core.trustctime false &&
git add file other &&
git commit -m "initial import"
'

test_expect_success '--refresh has no racy timestamps to fix' '
reset_files &&
# set the index time far enough to the future;
# it must be at least 3 seconds for VFAT
test_set_magic_mtime .git/index +60 &&
git update-index --refresh &&
test_is_magic_mtime .git/index +60
'

test_expect_success '--refresh should fix racy timestamp' '
reset_files &&
update_assert_changed --refresh
'

test_expect_success '--really-refresh should fix racy timestamp' '
reset_files &&
update_assert_changed --really-refresh
'

test_expect_success '--refresh should fix racy timestamp if other file needs update' '
reset_files &&
echo content2 >other &&
test_set_magic_mtime other &&
update_assert_changed --refresh
'

test_expect_success '--refresh should fix racy timestamp if racy file needs update' '
reset_files &&
echo content2 >file &&
test_set_magic_mtime file &&
update_assert_changed --refresh
'

test_done
30 changes: 25 additions & 5 deletions t/t7508-status.sh
Original file line number Diff line number Diff line change
Expand Up @@ -1647,13 +1647,33 @@ test_expect_success '"Initial commit" should not be noted in commit template' '
'

test_expect_success '--no-optional-locks prevents index update' '
test-tool chmtime =1234567890 .git/index &&
test_set_magic_mtime .git/index &&
git --no-optional-locks status &&
test-tool chmtime --get .git/index >out &&
grep ^1234567890 out &&
test_is_magic_mtime .git/index &&
git status &&
test-tool chmtime --get .git/index >out &&
! grep ^1234567890 out
! test_is_magic_mtime .git/index
'

test_expect_success 'racy timestamps will be fixed for clean worktree' '
echo content >racy-dirty &&
echo content >racy-racy &&
git add racy* &&
git commit -m "racy test files" &&
# let status rewrite the index, if necessary; after that we expect
# no more index writes unless caused by racy timestamps; note that
# timestamps may already be racy now (depending on previous tests)
git status &&
test_set_magic_mtime .git/index &&
git status &&
! test_is_magic_mtime .git/index
'

test_expect_success 'racy timestamps will be fixed for dirty worktree' '
echo content2 >racy-dirty &&
git status &&
test_set_magic_mtime .git/index &&
git status &&
! test_is_magic_mtime .git/index
'

test_done
33 changes: 33 additions & 0 deletions t/test-lib-functions.sh
Original file line number Diff line number Diff line change
Expand Up @@ -1840,3 +1840,36 @@ test_region () {
test_readlink () {
perl -le 'print readlink($_) for @ARGV' "$@"
}

# Set mtime to a fixed "magic" timestamp in mid February 2009, before we
# run an operation that may or may not touch the file. If the file was
# touched, its timestamp will not accidentally have such an old timestamp,
# as long as your filesystem clock is reasonably correct. To verify the
# timestamp, follow up with test_is_magic_mtime.
#
# An optional increment to the magic timestamp may be specified as second
# argument.
test_set_magic_mtime () {
local inc=${2:-0} &&
local mtime=$((1234567890 + $inc)) &&
test-tool chmtime =$mtime "$1" &&
test_is_magic_mtime "$1" $inc
}

# Test whether the given file has the "magic" mtime set. This is meant to
# be used in combination with test_set_magic_mtime.
#
# An optional increment to the magic timestamp may be specified as second
# argument. Usually, this should be the same increment which was used for
# the associated test_set_magic_mtime.
test_is_magic_mtime () {
local inc=${2:-0} &&
local mtime=$((1234567890 + $inc)) &&
echo $mtime >.git/test-mtime-expect &&
test-tool chmtime --get "$1" >.git/test-mtime-actual &&
test_cmp .git/test-mtime-expect .git/test-mtime-actual
local ret=$?
rm -f .git/test-mtime-expect
rm -f .git/test-mtime-actual
return $ret
}

0 comments on commit ee52b35

Please sign in to comment.