split-index; stop abusing the `base_oid` to strip the "link" extension

When a split-index is in effect, the `$GIT_DIR/index` file needs to contain a "link" extension that contains all the information about the split-index, including the information about the shared index. However, in some cases Git needs to suppress writing that "link" extension (i.e. to fall back to writing a full index) even if the in-memory index structure _has_ a `split_index` configured. This is the case e.g. when "too many not shared" index entries exist. In such instances, the current code sets the `base_oid` field of said `split_index` structure to all-zero to indicate that `do_write_index()` should skip writing the "link" extension. This can lead to problems later on, when the in-memory index is still used to perform other operations and eventually wants to write a split-index, detects the presence of the `split_index` and reuses that, too (under the assumption that it has been initialized correctly and still has a non-null `base_oid`). Let's stop zeroing out the `base_oid` to indicate that the "link" extension should not be written. One might be tempted to simply call `discard_split_index()` instead, under the assumption that Git decided to write a non-split index and therefore the `split_index` structure might no longer be wanted. However, that is not possible because that would release index entries in `split_index->base` that are likely to still be in use. Therefore we cannot do that. The next best thing we _can_ do is to introduce a bit field to indicate specifically which index extensions (not) to write. So that's what we do here. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Johannes Schindelin committed Mar 26, 2023 at 22:45 UTC 3b7a4475b0912d6d78c5e2354599dbc8f1ade479
2 files changed +33 -18
read-cache.c
+32 -17
@@ -2868,6 +2868,16 @@ static int record_ieot(void)
2868 return !git_config_get_index_threads(&val) && val != 1;
2869 }
2870
2871 +enum write_extensions {
2872 + WRITE_NO_EXTENSION = 0,
2873 + WRITE_SPLIT_INDEX_EXTENSION = 1<<0,
2874 + WRITE_CACHE_TREE_EXTENSION = 1<<1,
2875 + WRITE_RESOLVE_UNDO_EXTENSION = 1<<2,
2876 + WRITE_UNTRACKED_CACHE_EXTENSION = 1<<3,
2877 + WRITE_FSMONITOR_EXTENSION = 1<<4,
2878 +};
2879 +#define WRITE_ALL_EXTENSIONS ((enum write_extensions)-1)
2880 +
2881 /*
2882 * On success, `tempfile` is closed. If it is the temporary file
2883 * of a `struct lock_file`, we will therefore effectively perform
@@ -2876,7 +2886,7 @@ static int record_ieot(void)
2886 * rely on it.
2887 */
2888 static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
2879 - int strip_extensions, unsigned flags)
2889 + enum write_extensions write_extensions, unsigned flags)
2890 {
2891 uint64_t start = getnanotime();
2892 struct hashfile *f;
@@ -3045,8 +3055,8 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
3055 return -1;
3056 }
3057
3048 - if (!strip_extensions && istate->split_index &&
3049 - !is_null_oid(&istate->split_index->base_oid)) {
3058 + if (write_extensions & WRITE_SPLIT_INDEX_EXTENSION &&
3059 + istate->split_index) {
3060 struct strbuf sb = STRBUF_INIT;
3061
3062 if (istate->sparse_index)
@@ -3060,7 +3070,8 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
3070 if (err)
3071 return -1;
3072 }
3063 - if (!strip_extensions && !drop_cache_tree && istate->cache_tree) {
3073 + if (write_extensions & WRITE_CACHE_TREE_EXTENSION &&
3074 + !drop_cache_tree && istate->cache_tree) {
3075 struct strbuf sb = STRBUF_INIT;
3076
3077 cache_tree_write(&sb, istate->cache_tree);
@@ -3070,7 +3081,8 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
3081 if (err)
3082 return -1;
3083 }
3073 - if (!strip_extensions && istate->resolve_undo) {
3084 + if (write_extensions & WRITE_RESOLVE_UNDO_EXTENSION &&
3085 + istate->resolve_undo) {
3086 struct strbuf sb = STRBUF_INIT;
3087
3088 resolve_undo_write(&sb, istate->resolve_undo);
@@ -3081,7 +3093,8 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
3093 if (err)
3094 return -1;
3095 }
3084 - if (!strip_extensions && istate->untracked) {
3096 + if (write_extensions & WRITE_UNTRACKED_CACHE_EXTENSION &&
3097 + istate->untracked) {
3098 struct strbuf sb = STRBUF_INIT;
3099
3100 write_untracked_extension(&sb, istate->untracked);
@@ -3092,7 +3105,8 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
3105 if (err)
3106 return -1;
3107 }
3095 - if (!strip_extensions && istate->fsmonitor_last_update) {
3108 + if (write_extensions & WRITE_FSMONITOR_EXTENSION &&
3109 + istate->fsmonitor_last_update) {
3110 struct strbuf sb = STRBUF_INIT;
3111
3112 write_fsmonitor_extension(&sb, istate);
@@ -3166,8 +3180,10 @@ static int commit_locked_index(struct lock_file *lk)
3180 return commit_lock_file(lk);
3181 }
3182
3169 -static int do_write_locked_index(struct index_state *istate, struct lock_file *lock,
3170 - unsigned flags)
3183 +static int do_write_locked_index(struct index_state *istate,
3184 + struct lock_file *lock,
3185 + unsigned flags,
3186 + enum write_extensions write_extensions)
3187 {
3188 int ret;
3189 int was_full = istate->sparse_index == INDEX_EXPANDED;
@@ -3185,7 +3201,7 @@ static int do_write_locked_index(struct index_state *istate, struct lock_file *l
3201 */
3202 trace2_region_enter_printf("index", "do_write_index", the_repository,
3203 "%s", get_lock_file_path(lock));
3188 - ret = do_write_index(istate, lock->tempfile, 0, flags);
3204 + ret = do_write_index(istate, lock->tempfile, write_extensions, flags);
3205 trace2_region_leave_printf("index", "do_write_index", the_repository,
3206 "%s", get_lock_file_path(lock));
3207
@@ -3214,7 +3230,7 @@ static int write_split_index(struct index_state *istate,
3230 {
3231 int ret;
3232 prepare_to_write_split_index(istate);
3217 - ret = do_write_locked_index(istate, lock, flags);
3233 + ret = do_write_locked_index(istate, lock, flags, WRITE_ALL_EXTENSIONS);
3234 finish_writing_split_index(istate);
3235 return ret;
3236 }
@@ -3289,7 +3305,7 @@ static int write_shared_index(struct index_state *istate,
3305
3306 trace2_region_enter_printf("index", "shared/do_write_index",
3307 the_repository, "%s", get_tempfile_path(*temp));
3292 - ret = do_write_index(si->base, *temp, 1, flags);
3308 + ret = do_write_index(si->base, *temp, WRITE_NO_EXTENSION, flags);
3309 trace2_region_leave_printf("index", "shared/do_write_index",
3310 the_repository, "%s", get_tempfile_path(*temp));
3311
@@ -3366,9 +3382,8 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock,
3382 if ((!si && !test_split_index_env) ||
3383 alternate_index_output ||
3384 (istate->cache_changed & ~EXTMASK)) {
3369 - if (si)
3370 - oidclr(&si->base_oid);
3371 - ret = do_write_locked_index(istate, lock, flags);
3385 + ret = do_write_locked_index(istate, lock, flags,
3386 + ~WRITE_SPLIT_INDEX_EXTENSION);
3387 goto out;
3388 }
3389
@@ -3394,8 +3409,8 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock,
3409 /* Same initial permissions as the main .git/index file */
3410 temp = mks_tempfile_sm(git_path("sharedindex_XXXXXX"), 0, 0666);
3411 if (!temp) {
3397 - oidclr(&si->base_oid);
3398 - ret = do_write_locked_index(istate, lock, flags);
3412 + ret = do_write_locked_index(istate, lock, flags,
3413 + ~WRITE_SPLIT_INDEX_EXTENSION);
3414 goto out;
3415 }
3416 ret = write_shared_index(istate, &temp, flags);
t/t7527-builtin-fsmonitor.sh
+1 -1
@@ -1003,7 +1003,7 @@ test_expect_success !UNICODE_COMPOSITION_SENSITIVE 'Unicode nfc/nfd' '
1003 egrep "^event: nfd/d_${utf8_nfc}/?$" ./unicode.trace
1004 '
1005
1006 -test_expect_failure 'split-index and FSMonitor work well together' '
1006 +test_expect_success 'split-index and FSMonitor work well together' '
1007 git init split-index &&
1008 test_when_finished "git -C \"$PWD/split-index\" \
1009 fsmonitor--daemon stop" &&