update-index: refresh should rewrite index in case of racy timestamps

'git update-index --refresh' and '--really-refresh' should force writing of the index file if racy timestamps have been encountered, as 'git status' already does [1]. Note that calling 'git update-index --refresh' still does not guarantee that there will be no more racy timestamps afterwards (the same holds true for 'git status'): - calling 'git update-index --refresh' immediately after touching and adding a file may still leave racy timestamps if all three operations occur within the racy-tolerance (usually 1 second unless USE_NSEC has been defined) - calling 'git update-index --refresh' for timestamps which are set into the future will leave them racy To guarantee that such racy timestamps will be resolved would require to wait until the system clock has passed beyond these timestamps and only then write the index file. Especially for future timestamps, this does not seem feasible because of possibly long delays/hangs. [1] https://lore.kernel.org/git/d3dd805c-7c1d-30a9-6574-a7bfcb7fc013@syntevo.com/ Signed-off-by: Marc Strapetz <marc.strapetz@syntevo.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Marc Strapetz committed Jan 7, 2022 at 11:17 UTC 2ede073fd2287eb91af26484c2e7dafc15ecb7b7
4 files changed +77 -1
builtin/update-index.c
+11
@@ -787,6 +787,17 @@ static int refresh(struct refresh_params *o, unsigned int flag)
787 setup_work_tree();
788 read_cache();
789 *o->has_errors |= refresh_cache(o->flags | flag);
790 + if (has_racy_timestamp(&the_index)) {
791 + /*
792 + * Even if nothing else has changed, updating the file
793 + * increases the chance that racy timestamps become
794 + * non-racy, helping future run-time performance.
795 + * We do that even in case of "errors" returned by
796 + * refresh_cache() as these are no actual errors.
797 + * cmd_status() does the same.
798 + */
799 + active_cache_changed |= SOMETHING_CHANGED;
800 + }
801 return 0;
802 }
803
cache.h
+1
@@ -891,6 +891,7 @@ void *read_blob_data_from_index(struct index_state *, const char *, unsigned lon
891 #define CE_MATCH_IGNORE_FSMONITOR 0X20
892 int is_racy_timestamp(const struct index_state *istate,
893 const struct cache_entry *ce);
894 +int has_racy_timestamp(struct index_state *istate);
895 int ie_match_stat(struct index_state *, const struct cache_entry *, struct stat *, unsigned int);
896 int ie_modified(struct index_state *, const struct cache_entry *, struct stat *, unsigned int);
897
read-cache.c
+1 -1
@@ -2775,7 +2775,7 @@ static int repo_verify_index(struct repository *repo)
2775 return verify_index_from(repo->index, repo->index_file);
2776 }
2777
2778 -static int has_racy_timestamp(struct index_state *istate)
2778 +int has_racy_timestamp(struct index_state *istate)
2779 {
2780 int entries = istate->cache_nr;
2781 int i;
t/t2108-update-index-refresh-racy.sh new
+64
@@ -0,0 +1,64 @@
1 +#!/bin/sh
2 +
3 +test_description='update-index refresh tests related to racy timestamps'
4 +
5 +TEST_PASSES_SANITIZE_LEAK=true
6 +. ./test-lib.sh
7 +
8 +reset_files () {
9 + echo content >file &&
10 + echo content >other &&
11 + test_set_magic_mtime file &&
12 + test_set_magic_mtime other
13 +}
14 +
15 +update_assert_changed () {
16 + test_set_magic_mtime .git/index &&
17 + test_might_fail git update-index "$1" &&
18 + ! test_is_magic_mtime .git/index
19 +}
20 +
21 +test_expect_success 'setup' '
22 + reset_files &&
23 + # we are calling reset_files() a couple of times during tests;
24 + # test-tool chmtime does not change the ctime; to not weaken
25 + # or even break our tests, disable ctime-checks entirely
26 + git config core.trustctime false &&
27 + git add file other &&
28 + git commit -m "initial import"
29 +'
30 +
31 +test_expect_success '--refresh has no racy timestamps to fix' '
32 + reset_files &&
33 + # set the index time far enough to the future;
34 + # it must be at least 3 seconds for VFAT
35 + test_set_magic_mtime .git/index +60 &&
36 + git update-index --refresh &&
37 + test_is_magic_mtime .git/index +60
38 +'
39 +
40 +test_expect_success '--refresh should fix racy timestamp' '
41 + reset_files &&
42 + update_assert_changed --refresh
43 +'
44 +
45 +test_expect_success '--really-refresh should fix racy timestamp' '
46 + reset_files &&
47 + update_assert_changed --really-refresh
48 +'
49 +
50 +test_expect_success '--refresh should fix racy timestamp if other file needs update' '
51 + reset_files &&
52 + echo content2 >other &&
53 + test_set_magic_mtime other &&
54 + update_assert_changed --refresh
55 +'
56 +
57 +test_expect_success '--refresh should fix racy timestamp if racy file needs update' '
58 + reset_files &&
59 + echo content2 >file &&
60 + test_set_magic_mtime file &&
61 + update_assert_changed --refresh
62 +'
63 +
64 +test_done