Re: [PATCH v12 13/13] fsmonitor: fix split-index bitmap bounds in tweak_fsmonitor()

[Paul Tarjan <paul@paultarjan.com>]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> So the actual bug fix would be to ensure that `write_shared_index()`
> produces a hash even if `index.skipHash=true`, because that hash is
> needed to identify the shared index.

You're right, and I verified this. I reverted my fsmonitor.c patch
from seen, applied the fix below in write_shared_index(), and ran
t9210 with GIT_TEST_SPLIT_INDEX=yes on Fedora:

  Without either fix:     not ok 12, not ok 13 (BUG assertion)
  With only skipHash fix: passed all 22 test(s)

The fsmonitor.c patch is not needed. I'll drop it from the next
version of the series.

Happy to submit this as a separate patch or include it in my
series if that's helpful. Or if someone is already working on the
skipHash + split-index interaction, I'll stay out of the way.

--- >8 ---

diff --git a/read-cache.c b/read-cache.c
--- a/read-cache.c
+++ b/read-cache.c
@@ write_shared_index
 	move_cache_to_base_index(istate);
 	convert_to_sparse(istate, 0);

-	trace2_region_enter_printf("index", "shared/do_write_index",
-				   the_repository, "%s", get_tempfile_path(*temp));
-	ret = do_write_index(si->base, *temp, WRITE_NO_EXTENSION, flags);
-	trace2_region_leave_printf("index", "shared/do_write_index",
-				   the_repository, "%s", get_tempfile_path(*temp));
+	/*
+	 * The shared index is identified by the hash of its contents
+	 * (sharedindex.<oid>).  If index.skipHash is set, do_write_index()
+	 * would produce an all-zero hash and the shared index would not
+	 * be found on re-read (is_null_oid() check in read_index_from()).
+	 * Temporarily force hashing for the shared index write.
+	 */
+	{
+		struct repository *r = the_repository;
+		int save_skip_hash;
+
+		prepare_repo_settings(r);
+		save_skip_hash = r->settings.index_skip_hash;
+		r->settings.index_skip_hash = 0;
+
+		trace2_region_enter_printf("index", "shared/do_write_index",
+					   the_repository, "%s", get_tempfile_path(*temp));
+		ret = do_write_index(si->base, *temp, WRITE_NO_EXTENSION, flags);
+		trace2_region_leave_printf("index", "shared/do_write_index",
+					   the_repository, "%s", get_tempfile_path(*temp));
+
+		r->settings.index_skip_hash = save_skip_hash;
+	}

 	if (was_full)
 		ensure_full_index(istate);

Paul