From: Paul Tarjan <paul@paultarjan.com>
Date: Sun, 05 Apr 2026 05:15:28 GMT
Subject: Re: [PATCH v12 13/13] fsmonitor: fix split-index bitmap bounds in tweak_fsmonitor()
Message-ID: <20260405051528.74435-1-github@paulisageek.com>
In-Reply-To: <b96ed977-525e-c3fc-a626-db1a4b3da376@gmx.de>

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