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>

diff --git a/builtin/update-index.c b/builtin/update-index.c
index 187203e8bb53cbeaf80d2f4b89c04134a2a5c95f..7e0a0d9bf80916ceb8509af5b914a5649676ef8a 100644 (file)
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -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;
 }
 

diff --git a/cache.h b/cache.h
index cfba463aa97cb40ffebb4742c087f24daf791460..dd1932e2d0ef586c08e402a85dd41bc9e30ce774 100644 (file)
--- a/cache.h
+++ b/cache.h
@@ -891,6 +891,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);
 

diff --git a/read-cache.c b/read-cache.c
index cbe73f14e5e7efc63b20e80a2702fb5c0dea9a5d..ed297635a33160c6d61d0483fccb060ff719f507 100644 (file)
--- a/read-cache.c
+++ b/read-cache.c
@@ -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;

diff --git a/t/t2108-update-index-refresh-racy.sh b/t/t2108-update-index-refresh-racy.sh
new file mode 100755 (executable)
index 0000000..bc5f288
--- /dev/null
+++ b/t/t2108-update-index-refresh-racy.sh
@@ -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