git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Marc Strapetz via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Marc Strapetz <marc.strapetz@syntevo.com>
Subject: [PATCH] update-index: refresh should rewrite index in case of racy timestamps
Date: Wed, 22 Dec 2021 13:56:30 +0000	[thread overview]
Message-ID: <pull.1105.git.1640181390841.gitgitgadget@gmail.com> (raw)

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

             reply	other threads:[~2021-12-22 13:56 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-22 13:56 Marc Strapetz via GitGitGadget [this message]
2021-12-22 23:52 ` [PATCH] update-index: refresh should rewrite index in case of racy timestamps Junio C Hamano
2021-12-23 18:24   ` Marc Strapetz
2022-01-05 13:15 ` [PATCH v2 0/2] " Marc Strapetz via GitGitGadget
2022-01-05 13:15   ` [PATCH v2 1/2] t7508: add tests capturing racy timestamp handling Marc Strapetz via GitGitGadget
2022-01-05 20:59     ` Junio C Hamano
2022-01-06 10:21       ` Marc Strapetz
2022-01-05 13:15   ` [PATCH v2 2/2] update-index: refresh should rewrite index in case of racy timestamps Marc Strapetz via GitGitGadget
2022-01-05 21:03     ` Junio C Hamano
2022-01-06 22:34   ` [PATCH v3 0/4] " Marc Strapetz via GitGitGadget
2022-01-06 22:34     ` [PATCH v3 1/4] test-lib: introduce API for verifying file mtime Marc Strapetz via GitGitGadget
2022-01-06 23:55       ` Junio C Hamano
2022-01-06 22:34     ` [PATCH v3 2/4] t7508: fix bogus mtime verification Marc Strapetz via GitGitGadget
2022-01-06 22:34     ` [PATCH v3 3/4] t7508: add tests capturing racy timestamp handling Marc Strapetz via GitGitGadget
2022-01-06 22:34     ` [PATCH v3 4/4] update-index: refresh should rewrite index in case of racy timestamps Marc Strapetz via GitGitGadget
2022-01-07 11:17     ` [PATCH v4 0/4] " Marc Strapetz via GitGitGadget
2022-01-07 11:17       ` [PATCH v4 1/4] test-lib: introduce API for verifying file mtime Marc Strapetz via GitGitGadget
2022-01-07 11:17       ` [PATCH v4 2/4] t7508: fix bogus mtime verification Marc Strapetz via GitGitGadget
2022-01-07 11:17       ` [PATCH v4 3/4] t7508: add tests capturing racy timestamp handling Marc Strapetz via GitGitGadget
2022-01-07 11:17       ` [PATCH v4 4/4] update-index: refresh should rewrite index in case of racy timestamps Marc Strapetz via GitGitGadget

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.1105.git.1640181390841.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=marc.strapetz@syntevo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).