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

From: Marc Strapetz <redacted>

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 <redacted>
---
    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

This patch makes update-index --refresh write the index if it contains racy
timestamps, as discussed at [1].

Changes since v1:

 * main commit message now uses 'git update-index' and the paragraph was
   dropped
 * t/t7508-status.sh: two tests added which capture status racy handling
 * builtin/update-index.c: comment improved
 * t/t2108-update-index-refresh-racy.sh: major overhaul
   * one test case added
   * mtime-manipulations simplified and aligned to t7508
   * code style fixes, as discussed

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

Marc Strapetz (2):
  t7508: add tests capturing racy timestamp handling
  update-index: refresh should rewrite index in case of racy timestamps

 builtin/update-index.c               | 11 +++++
 cache.h                              |  1 +
 read-cache.c                         |  2 +-
 t/t2108-update-index-refresh-racy.sh | 64 ++++++++++++++++++++++++++++
 t/t7508-status.sh                    | 28 ++++++++++++
 5 files changed, 105 insertions(+), 1 deletion(-)
 create mode 100755 t/t2108-update-index-refresh-racy.sh


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

Range-diff vs v1:

 -:  ----------- > 1:  7d58f806111 t7508: add tests capturing racy timestamp handling
 1:  8f9618a44c5 ! 2:  dfeabf6af15 update-index: refresh should rewrite index in case of racy timestamps
     @@ Metadata
       ## Commit message ##
          update-index: refresh should rewrite index in case of racy timestamps
      
     -    update-index --refresh and --really-refresh should force writing of the
     -    index file if racy timestamps have been encountered, as status already
     -    does [1].
     +    '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 update-index still does not guarantee that there will
     -    be no more racy timestamps afterwards (the same holds true for status):
     +    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 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 '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 update-index for timestamps which are set into the future
     -      will leave them racy
     +    - 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.
      
     -    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 [off-list ref]
     @@ builtin/update-index.c: static int refresh(struct refresh_params *o, unsigned in
       	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. */
     ++		/*
     ++		 * 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;
     @@ t/t2108-update-index-refresh-racy.sh (new)
      +
      +test_description='update-index refresh tests related to racy timestamps'
      +
     ++TEST_PASSES_SANITIZE_LEAK=true
      +. ./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 ]
     ++reset_files () {
     ++	echo content >file &&
     ++	echo content >other &&
     ++	test-tool chmtime =1234567890 file &&
     ++	test-tool chmtime =1234567890 other
      +}
      +
     -+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 ]
     ++update_assert_changed () {
     ++	test-tool chmtime =1234567890 .git/index &&
     ++	test_might_fail git update-index "$1" &&
     ++	test-tool chmtime --get .git/index >.git/out &&
     ++	! grep ^1234567890 .git/out
      +}
      +
      +test_expect_success 'setup' '
     -+	touch .git/fs-tstamp &&
     -+	test-tool chmtime -1 .git/fs-tstamp &&
     -+	echo content >file &&
     -+	reset_mtime file &&
     -+
     -+	git add file &&
     ++	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_mtime .git/index &&
     -+	test-tool chmtime +1 .git/index &&
     -+	update_assert_unchanged --refresh
     ++	reset_files &&
     ++	test-tool chmtime =1234567891 .git/index &&
     ++	git update-index --refresh &&
     ++	test-tool chmtime --get .git/index >.git/out &&
     ++	grep ^1234567891 .git/out
      +'
      +
      +test_expect_success '--refresh should fix racy timestamp' '
     -+	reset_mtime .git/index &&
     ++	reset_files &&
      +	update_assert_changed --refresh
      +'
      +
      +test_expect_success '--really-refresh should fix racy timestamp' '
     -+	reset_mtime .git/index &&
     ++	reset_files &&
      +	update_assert_changed --really-refresh
      +'
      +
     -+test_expect_success '--refresh should fix racy timestamp even if needs update' '
     ++test_expect_success '--refresh should fix racy timestamp if other file needs update' '
     ++	reset_files &&
     ++	echo content2 >other &&
     ++	test-tool chmtime =1234567890 other &&
     ++	update_assert_changed --refresh
     ++'
     ++
     ++test_expect_success '--refresh should fix racy timestamp if racy file needs update' '
     ++	reset_files &&
      +	echo content2 >file &&
     -+	reset_mtime file &&
     -+	reset_mtime .git/index &&
     ++	test-tool chmtime =1234567890 file &&
      +	update_assert_changed --refresh
      +'
      +

-- 
gitgitgadget

From: Marc Strapetz <redacted>

"git status" fixes racy timestamps regardless of the worktree being
dirty or not. The new test cases capture this behavior.

Signed-off-by: Marc Strapetz <redacted>
---
 t/t7508-status.sh | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index 05c6c02435d..652cbb5ed2e 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh

@@ -1656,4 +1656,32 @@ test_expect_success '--no-optional-locks prevents index update' '
 	! grep ^1234567890 out
 '
 
+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-tool chmtime =1234567890 .git/index &&
+	test-tool chmtime --get .git/index >out &&
+	grep ^1234567890 out &&
+	git status &&
+	test-tool chmtime --get .git/index >out &&
+	! grep ^1234567890 out
+'
+
+test_expect_success 'racy timestamps will be fixed for dirty worktree' '
+	echo content2 >racy-dirty &&
+	git status &&
+	test-tool chmtime =1234567890 .git/index &&
+	test-tool chmtime --get .git/index >out &&
+	grep ^1234567890 out &&
+	git status &&
+	test-tool chmtime --get .git/index >out &&
+	! grep ^1234567890 out
+'
+
 test_done

-- 
gitgitgadget

From: Marc Strapetz <redacted>

'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 <redacted>
---
 builtin/update-index.c               | 11 +++++
 cache.h                              |  1 +
 read-cache.c                         |  2 +-
 t/t2108-update-index-refresh-racy.sh | 64 ++++++++++++++++++++++++++++
 4 files changed, 77 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..7e0a0d9bf80 100644
--- 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 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..171c37ebec9
--- /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-tool chmtime =1234567890 file &&
+	test-tool chmtime =1234567890 other
+}
+
+update_assert_changed () {
+	test-tool chmtime =1234567890 .git/index &&
+	test_might_fail git update-index "$1" &&
+	test-tool chmtime --get .git/index >.git/out &&
+	! grep ^1234567890 .git/out
+}
+
+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 &&
+	test-tool chmtime =1234567891 .git/index &&
+	git update-index --refresh &&
+	test-tool chmtime --get .git/index >.git/out &&
+	grep ^1234567891 .git/out
+'
+
+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-tool chmtime =1234567890 other &&
+	update_assert_changed --refresh
+'
+
+test_expect_success '--refresh should fix racy timestamp if racy file needs update' '
+	reset_files &&
+	echo content2 >file &&
+	test-tool chmtime =1234567890 file &&
+	update_assert_changed --refresh
+'
+
+test_done

-- 
gitgitgadget

This patch makes update-index --refresh write the index if it contains racy
timestamps, as discussed at [1].

Changes since v2:

 * new patch: test-lib: introduce API for verifying file mtime
 * new patch: t7508: fix bogus mtime verification for test
   "--no-optional-locks prevents index update"
 * change new tests in t2108 and t7508 to use new test-lib mtime API
 * fix "--refresh has no racy timestamps to fix" to use +60s mtime to be
   save on VFAT

Changes since v1:

 * main commit message now uses 'git update-index' and the paragraph was
   dropped
 * t/t7508-status.sh: two tests added which capture status racy handling
 * builtin/update-index.c: comment improved
 * t/t2108-update-index-refresh-racy.sh: major overhaul
   * one test case added
   * mtime-manipulations simplified and aligned to t7508
   * code style fixes, as discussed

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

Marc Strapetz (4):
  test-lib: introduce API for verifying file mtime
  t7508: fix bogus mtime verification
  t7508: add tests capturing racy timestamp handling
  update-index: refresh should rewrite index in case of racy timestamps

 builtin/update-index.c               | 11 +++++
 cache.h                              |  1 +
 read-cache.c                         |  2 +-
 t/t2108-update-index-refresh-racy.sh | 64 ++++++++++++++++++++++++++++
 t/t7508-status.sh                    | 30 ++++++++++---
 t/test-lib-functions.sh              | 28 ++++++++++++
 6 files changed, 130 insertions(+), 6 deletions(-)
 create mode 100755 t/t2108-update-index-refresh-racy.sh


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

Range-diff vs v2:

 -:  ----------- > 1:  e6301e9d770 test-lib: introduce API for verifying file mtime
 -:  ----------- > 2:  d15a23cc804 t7508: fix bogus mtime verification
 1:  7d58f806111 ! 3:  3567ef91e7a t7508: add tests capturing racy timestamp handling
     @@ Commit message
      
       ## t/t7508-status.sh ##
      @@ t/t7508-status.sh: test_expect_success '--no-optional-locks prevents index update' '
     - 	! grep ^1234567890 out
     + 	! test_is_magic_mtime .git/index
       '
       
      +test_expect_success 'racy timestamps will be fixed for clean worktree' '
     @@ t/t7508-status.sh: test_expect_success '--no-optional-locks prevents index updat
      +	# no more index writes unless caused by racy timestamps; note that
      +	# timestamps may already be racy now (depending on previous tests)
      +	git status &&
     -+	test-tool chmtime =1234567890 .git/index &&
     -+	test-tool chmtime --get .git/index >out &&
     -+	grep ^1234567890 out &&
     ++	test_set_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 dirty worktree' '
      +	echo content2 >racy-dirty &&
      +	git status &&
     -+	test-tool chmtime =1234567890 .git/index &&
     -+	test-tool chmtime --get .git/index >out &&
     -+	grep ^1234567890 out &&
     ++	test_set_magic_mtime .git/index &&
      +	git status &&
     -+	test-tool chmtime --get .git/index >out &&
     -+	! grep ^1234567890 out
     ++	! test_is_magic_mtime .git/index
      +'
      +
       test_done
 2:  dfeabf6af15 ! 4:  4a6b18fb304 update-index: refresh should rewrite index in case of racy timestamps
     @@ t/t2108-update-index-refresh-racy.sh (new)
      +reset_files () {
      +	echo content >file &&
      +	echo content >other &&
     -+	test-tool chmtime =1234567890 file &&
     -+	test-tool chmtime =1234567890 other
     ++	test_set_magic_mtime file &&
     ++	test_set_magic_mtime other
      +}
      +
      +update_assert_changed () {
     -+	test-tool chmtime =1234567890 .git/index &&
     ++	test_set_magic_mtime .git/index &&
      +	test_might_fail git update-index "$1" &&
     -+	test-tool chmtime --get .git/index >.git/out &&
     -+	! grep ^1234567890 .git/out
     ++	! test_is_magic_mtime .git/index
      +}
      +
      +test_expect_success 'setup' '
     @@ t/t2108-update-index-refresh-racy.sh (new)
      +
      +test_expect_success '--refresh has no racy timestamps to fix' '
      +	reset_files &&
     -+	test-tool chmtime =1234567891 .git/index &&
     ++	# 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-tool chmtime --get .git/index >.git/out &&
     -+	grep ^1234567891 .git/out
     ++	test_is_magic_mtime .git/index +60
      +'
      +
      +test_expect_success '--refresh should fix racy timestamp' '
     @@ t/t2108-update-index-refresh-racy.sh (new)
      +test_expect_success '--refresh should fix racy timestamp if other file needs update' '
      +	reset_files &&
      +	echo content2 >other &&
     -+	test-tool chmtime =1234567890 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-tool chmtime =1234567890 file &&
     ++	test_set_magic_mtime file &&
      +	update_assert_changed --refresh
      +'
      +

-- 
gitgitgadget

From: Marc Strapetz <redacted>

Add functions `test_set_magic_mtime` and `test_is_magic_mtime` which can
be used to (re)set the mtime of a file to a predefined ("magic")
timestamp, then perform some operations and finally check for mtime
changes of the file.

The core implementation follows the suggestion from the
mailing list [1].

[1] https://lore.kernel.org/git/xmqqczl5hpaq.fsf@gitster.g/T/#u

Signed-off-by: Marc Strapetz <redacted>
---
 t/test-lib-functions.sh | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 389153e5916..01dd8c01f59 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh

@@ -1806,3 +1806,31 @@ test_region () {
 test_readlink () {
 	perl -le 'print readlink($_) for @ARGV' "$@"
 }
+
+# Set a fixed "magic" mtime to the given file,
+# with an optional increment specified as second argument.
+# Use in combination with test_is_magic_mtime.
+test_set_magic_mtime () {
+	# We are using 1234567890 because it's a common timestamp used in
+	# various tests. It represents date 2009-02-13 which should be safe
+	# to use as long as the filetime clock is reasonably accurate.
+	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,
+# with an optional increment specified as second argument.
+# Use in combination with 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 .git/test-mtime-expect
+	rm .git/test-mtime-actual
+	return $ret
+}

-- 
gitgitgadget

From: Marc Strapetz <redacted>

The current `grep`-approach in "--no-optional-locks prevents index
update" may fail e.g. for `out` file contents "1234567890999" [1].
Fix this by using test-lib's new mtime-verification API.

[1] https://lore.kernel.org/git/xmqqczl5hpaq.fsf@gitster.g/T/#u

Signed-off-by: Marc Strapetz <redacted>
---
 t/t7508-status.sh | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index 05c6c02435d..b9efd2613d0 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh

@@ -1647,13 +1647,11 @@ 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_done

-- 
gitgitgadget

From: Marc Strapetz <redacted>

"git status" fixes racy timestamps regardless of the worktree being
dirty or not. The new test cases capture this behavior.

Signed-off-by: Marc Strapetz <redacted>
---
 t/t7508-status.sh | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index b9efd2613d0..2b7ef6c41a4 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh

@@ -1654,4 +1654,26 @@ test_expect_success '--no-optional-locks prevents index update' '
 	! 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

-- 
gitgitgadget

From: Marc Strapetz <redacted>

'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 <redacted>
---
 builtin/update-index.c               | 11 +++++
 cache.h                              |  1 +
 read-cache.c                         |  2 +-
 t/t2108-update-index-refresh-racy.sh | 64 ++++++++++++++++++++++++++++
 4 files changed, 77 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..7e0a0d9bf80 100644
--- 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 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..bc5f2886faf
--- /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

-- 
gitgitgadget

This patch makes update-index --refresh write the index if it contains racy
timestamps, as discussed at [1].

Changes since v3:

 * test-lib: improve API for verifying file mtime
   * fix quoting around "$1"
   * use "rm -f" for cleanup of auxiliary files
   * improve API description comments
 * Note that gitgitgadget's "freebsd_12" check is failing since a couple of
   days (unrelated to this pull request); hence, this check hasn't been
   applied to this patch series

Changes since v2:

 * new patch: test-lib: introduce API for verifying file mtime
 * new patch: t7508: fix bogus mtime verification for test
   "--no-optional-locks prevents index update"
 * change new tests in t2108 and t7508 to use new test-lib mtime API
 * fix "--refresh has no racy timestamps to fix" to use +60s mtime to be
   save on VFAT

Changes since v1:

 * main commit message now uses 'git update-index' and the paragraph was
   dropped
 * t/t7508-status.sh: two tests added which capture status racy handling
 * builtin/update-index.c: comment improved
 * t/t2108-update-index-refresh-racy.sh: major overhaul
   * one test case added
   * mtime-manipulations simplified and aligned to t7508
   * code style fixes, as discussed

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

Marc Strapetz (4):
  test-lib: introduce API for verifying file mtime
  t7508: fix bogus mtime verification
  t7508: add tests capturing racy timestamp handling
  update-index: refresh should rewrite index in case of racy timestamps

 builtin/update-index.c               | 11 +++++
 cache.h                              |  1 +
 read-cache.c                         |  2 +-
 t/t2108-update-index-refresh-racy.sh | 64 ++++++++++++++++++++++++++++
 t/t7508-status.sh                    | 30 ++++++++++---
 t/test-lib-functions.sh              | 33 ++++++++++++++
 6 files changed, 135 insertions(+), 6 deletions(-)
 create mode 100755 t/t2108-update-index-refresh-racy.sh


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

Range-diff vs v3:

 1:  e6301e9d770 ! 1:  37c11bfafc4 test-lib: introduce API for verifying file mtime
     @@ t/test-lib-functions.sh: test_region () {
       	perl -le 'print readlink($_) for @ARGV' "$@"
       }
      +
     -+# Set a fixed "magic" mtime to the given file,
     -+# with an optional increment specified as second argument.
     -+# Use in combination with test_is_magic_mtime.
     ++# 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 () {
     -+	# We are using 1234567890 because it's a common timestamp used in
     -+	# various tests. It represents date 2009-02-13 which should be safe
     -+	# to use as long as the filetime clock is reasonably accurate.
      +	local inc=${2:-0} &&
      +	local mtime=$((1234567890 + $inc)) &&
     -+	test-tool chmtime =$mtime $1 &&
     -+	test_is_magic_mtime $1 $inc
     ++	test-tool chmtime =$mtime "$1" &&
     ++	test_is_magic_mtime "$1" $inc
      +}
      +
     -+# Test whether the given file has the "magic" mtime set,
     -+# with an optional increment specified as second argument.
     -+# Use in combination with test_set_magic_mtime.
     ++# 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-tool chmtime --get "$1" >.git/test-mtime-actual &&
      +	test_cmp .git/test-mtime-expect .git/test-mtime-actual
      +	local ret=$?
     -+	rm .git/test-mtime-expect
     -+	rm .git/test-mtime-actual
     ++	rm -f .git/test-mtime-expect
     ++	rm -f .git/test-mtime-actual
      +	return $ret
      +}
 2:  d15a23cc804 = 2:  c97a41af389 t7508: fix bogus mtime verification
 3:  3567ef91e7a = 3:  82d0b6ab8d2 t7508: add tests capturing racy timestamp handling
 4:  4a6b18fb304 = 4:  e31edb74e24 update-index: refresh should rewrite index in case of racy timestamps

-- 
gitgitgadget

From: Marc Strapetz <redacted>

Add functions `test_set_magic_mtime` and `test_is_magic_mtime` which can
be used to (re)set the mtime of a file to a predefined ("magic")
timestamp, then perform some operations and finally check for mtime
changes of the file.

The core implementation follows the suggestion from the
mailing list [1].

[1] https://lore.kernel.org/git/xmqqczl5hpaq.fsf@gitster.g/T/#u

Signed-off-by: Marc Strapetz <redacted>
---
 t/test-lib-functions.sh | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 389153e5916..c1afa0884bf 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh

@@ -1806,3 +1806,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
+}

-- 
gitgitgadget

From: Marc Strapetz <redacted>

The current `grep`-approach in "--no-optional-locks prevents index
update" may fail e.g. for `out` file contents "1234567890999" [1].
Fix this by using test-lib's new mtime-verification API.

[1] https://lore.kernel.org/git/xmqqczl5hpaq.fsf@gitster.g/T/#u

Signed-off-by: Marc Strapetz <redacted>
---
 t/t7508-status.sh | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index 05c6c02435d..b9efd2613d0 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh

@@ -1647,13 +1647,11 @@ 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_done

-- 
gitgitgadget

From: Marc Strapetz <redacted>

"git status" fixes racy timestamps regardless of the worktree being
dirty or not. The new test cases capture this behavior.

Signed-off-by: Marc Strapetz <redacted>
---
 t/t7508-status.sh | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index b9efd2613d0..2b7ef6c41a4 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh

@@ -1654,4 +1654,26 @@ test_expect_success '--no-optional-locks prevents index update' '
 	! 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

-- 
gitgitgadget

From: Marc Strapetz <redacted>

'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 <redacted>
---
 builtin/update-index.c               | 11 +++++
 cache.h                              |  1 +
 read-cache.c                         |  2 +-
 t/t2108-update-index-refresh-racy.sh | 64 ++++++++++++++++++++++++++++
 4 files changed, 77 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..7e0a0d9bf80 100644
--- 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 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..bc5f2886faf
--- /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

-- 
gitgitgadget