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 b3bab9d2729
(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>

diff --git a/midx-write.c b/midx-write.c
index a25cab75abad11155d58b8178acd081e63fd4aac..9328f65a20187a9ac2fc50f65592a49d8f3eb7b2 100644 (file)
--- a/midx-write.c
+++ b/midx-write.c
@@ -1152,7 +1152,7 @@ static bool midx_needs_update(struct multi_pack_index *midx, struct write_midx_c
 
        /*
         * Ensure that we have a valid checksum before consulting the
-        * exisiting MIDX in order to determine if we can avoid an
+        * existing MIDX in order to determine if we can avoid an
         * update.
         *
         * 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
                        BUG("same pack added twice?");
        }
 
-       for (uint32_t i = 0; i < ctx->nr; i++) {
-               strbuf_reset(&buf);
-               strbuf_addstr(&buf, midx->pack_names[i]);
-               strbuf_strip_suffix(&buf, ".idx");
+       for (struct multi_pack_index *m = midx; m; m = m->base_midx) {
+               for (uint32_t i = 0; i < m->num_packs; i++) {
+                       strbuf_reset(&buf);
+                       strbuf_addstr(&buf, m->pack_names[i]);
+                       strbuf_strip_suffix(&buf, ".idx");
 
-               if (!strset_contains(&packs, buf.buf))
-                       goto out;
-               strset_remove(&packs, buf.buf);
+                       if (!strset_contains(&packs, buf.buf))
+                               goto out;
+                       strset_remove(&packs, buf.buf);
+               }
        }
 
        needed = false;

diff --git a/t/t5334-incremental-multi-pack-index.sh b/t/t5334-incremental-multi-pack-index.sh
index 99c7d44d8e9d342c077fab13e5bee2f98a473190..c9f5b4e87aa035d2a568365fd14e59693a725ce3 100755 (executable)
--- a/t/t5334-incremental-multi-pack-index.sh
+++ b/t/t5334-incremental-multi-pack-index.sh
@@ -132,4 +132,20 @@ test_expect_success 'relink existing MIDX layer' '
 
 '
 
+test_expect_success 'non-incremental write with existing incremental chain' '
+       git init non-incremental-write-with-existing &&
+       test_when_finished "rm -fr non-incremental-write-with-existing" &&
+
+       (
+               cd non-incremental-write-with-existing &&
+
+               git config set maintenance.auto false &&
+
+               write_midx_layer &&
+               write_midx_layer &&
+
+               git multi-pack-index write
+       )
+'
+
 test_done