Skip to content

Commit d13ca9c

Browse files
ttaylorrvmg
authored andcommitted
midx-write: handle noop writes when converting incremental chains
When updating a MIDX, we optimize out writes that will result in an identical MIDX as the one we already have on disk. See b3bab9d (midx-write: extract function to test whether MIDX needs updating, 2025-12-10) for more details on exactly which writes are optimized out. If `midx_needs_update()` can't rule out any of the obvious cases (e.g., the checksum is invalid, we're requesting a different version, or performing compaction which always requires an update), then we compare the packs we're writing to the packs we already know about. If there are an equal number of packs being written as there are in any existing MIDX layer(s), then we compare the packs by their name. This comparison fails when we have an incremental MIDX chain with at least two layers, since we do not recursively peel through earlier layers, instead treating the `->pack_names` array of the tip MIDX layer as containing all `m->num_packs + m->num_packs_in_base` packs. Adjust this to instead look through the MIDX layers one by one when comparing pack names. While we're at it, fix a typo above in the same function. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent dbad4b1 commit d13ca9c

2 files changed

Lines changed: 26 additions & 8 deletions

File tree

midx-write.c

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1152,7 +1152,7 @@ static bool midx_needs_update(struct multi_pack_index *midx, struct write_midx_c
11521152

11531153
/*
11541154
* Ensure that we have a valid checksum before consulting the
1155-
* exisiting MIDX in order to determine if we can avoid an
1155+
* existing MIDX in order to determine if we can avoid an
11561156
* update.
11571157
*
11581158
* This is necessary because the given MIDX is loaded directly
@@ -1208,14 +1208,16 @@ static bool midx_needs_update(struct multi_pack_index *midx, struct write_midx_c
12081208
BUG("same pack added twice?");
12091209
}
12101210

1211-
for (uint32_t i = 0; i < ctx->nr; i++) {
1212-
strbuf_reset(&buf);
1213-
strbuf_addstr(&buf, midx->pack_names[i]);
1214-
strbuf_strip_suffix(&buf, ".idx");
1211+
for (struct multi_pack_index *m = midx; m; m = m->base_midx) {
1212+
for (uint32_t i = 0; i < m->num_packs; i++) {
1213+
strbuf_reset(&buf);
1214+
strbuf_addstr(&buf, m->pack_names[i]);
1215+
strbuf_strip_suffix(&buf, ".idx");
12151216

1216-
if (!strset_contains(&packs, buf.buf))
1217-
goto out;
1218-
strset_remove(&packs, buf.buf);
1217+
if (!strset_contains(&packs, buf.buf))
1218+
goto out;
1219+
strset_remove(&packs, buf.buf);
1220+
}
12191221
}
12201222

12211223
needed = false;

t/t5334-incremental-multi-pack-index.sh

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,4 +132,20 @@ test_expect_success 'relink existing MIDX layer' '
132132
133133
'
134134

135+
test_expect_success 'non-incremental write with existing incremental chain' '
136+
git init non-incremental-write-with-existing &&
137+
test_when_finished "rm -fr non-incremental-write-with-existing" &&
138+
139+
(
140+
cd non-incremental-write-with-existing &&
141+
142+
git config set maintenance.auto false &&
143+
144+
write_midx_layer &&
145+
write_midx_layer &&
146+
147+
git multi-pack-index write
148+
)
149+
'
150+
135151
test_done

0 commit comments

Comments
 (0)