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

From: Marc Strapetz via GitGitGadget <gitgitgadget@gmail.com>
Date: Dec 22, 2021, 13:56 UTC
Message-ID: <pull.1105.git.1640181390841.gitgitgadget@gmail.com>

From: Marc Strapetz <marc.strapetz@syntevo.com>

update-index --refresh and --really-refresh should force writing of the index file if racy timestamps have been encountered, as status already does [1].

Note that calling update-index still does not guarantee that there will be no more racy timestamps afterwards (the same holds true for status):

- calling update-index 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 update-index 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.

Both --refresh and --really-refresh may in theory be used in combination with --unresolve and --again which may reset the "active_cache_changed" flag. There is no difference of whether we write the index due to racy timestamps or due to other reasons, like if --really-refresh has detected CE_ENTRY_CHANGED in refresh_cache(). Hence, we will set the "active_cache_changed" flag immediately after calling refresh_cache().

[1] https://lore.kernel.org/git/d3dd805c-7c1d-30a9-6574-a7bfcb7fc013@syntevo.com/

Signed-off-by: Marc Strapetz <marc.strapetz@syntevo.com>
---
    update-index: refresh should rewrite index in case of racy timestamps
    
    This patch makes update-index --refresh write the index if it contains
    racy timestamps, as discussed at [1].
    
    [1]
    https://lore.kernel.org/git/d3dd805c-7c1d-30a9-6574-a7bfcb7fc013@syntevo.com/

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1105%2Fmstrap%2Ffeature%2Fupdate-index-refresh-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1105/mstrap/feature/update-index-refresh-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1105

 builtin/update-index.c               |  6 +++
 cache.h                              |  1 +
 read-cache.c                         |  2 +-
 t/t2108-update-index-refresh-racy.sh | 58 ++++++++++++++++++++++++++++
 4 files changed, 66 insertions(+), 1 deletion(-)
 create mode 100755 t/t2108-update-index-refresh-racy.sh

diff --git a/builtin/update-index.c b/builtin/update-index.c
index 187203e8bb5..0a069281e23 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -787,6 +787,12 @@ 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)) {
+		/* For racy timestamps we should set active_cache_changed immediately:
+		 * other callbacks may follow for which some of them may reset
+		 * active_cache_changed. */
+		active_cache_changed |= SOMETHING_CHANGED;
+	}
 	return 0;
 }
 
diff --git a/cache.h b/cache.h
index cfba463aa97..dd1932e2d0e 100644
--- 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 cbe73f14e5e..ed297635a33 100644
--- 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
index 00000000000..ece1151847c
--- /dev/null
+++ b/t/t2108-update-index-refresh-racy.sh
@@ -0,0 +1,58 @@
+#!/bin/sh
+
+test_description='update-index refresh tests related to racy timestamps'
+
+. ./test-lib.sh
+
+reset_mtime() {
+	test-tool chmtime =$(test-tool chmtime --get .git/fs-tstamp) $1
+}
+
+update_assert_unchanged() {
+	local ts1=$(test-tool chmtime --get .git/index) &&
+	git update-index $1 &&
+	local ts2=$(test-tool chmtime --get .git/index) &&
+	[ $ts1 -eq $ts2 ]
+}
+
+update_assert_changed() {
+	local ts1=$(test-tool chmtime --get .git/index) &&
+	test_might_fail git update-index $1 &&
+	local ts2=$(test-tool chmtime --get .git/index) &&
+	[ $ts1 -ne $ts2 ]
+}
+
+test_expect_success 'setup' '
+	touch .git/fs-tstamp &&
+	test-tool chmtime -1 .git/fs-tstamp &&
+	echo content >file &&
+	reset_mtime file &&
+
+	git add file &&
+	git commit -m "initial import"
+'
+
+test_expect_success '--refresh has no racy timestamps to fix' '
+	reset_mtime .git/index &&
+	test-tool chmtime +1 .git/index &&
+	update_assert_unchanged --refresh
+'
+
+test_expect_success '--refresh should fix racy timestamp' '
+	reset_mtime .git/index &&
+	update_assert_changed --refresh
+'
+
+test_expect_success '--really-refresh should fix racy timestamp' '
+	reset_mtime .git/index &&
+	update_assert_changed --really-refresh
+'
+
+test_expect_success '--refresh should fix racy timestamp even if needs update' '
+	echo content2 >file &&
+	reset_mtime file &&
+	reset_mtime .git/index &&
+	update_assert_changed --refresh
+'
+
+test_done

base-commit: 597af311a2899bfd6640b9b107622c5795d5f998

-- 
gitgitgadget