Re: [PATCH] midx-write: skip empty incremental layers

On Mon, Sep 07, 2026 at 06:46:40PM -0700, Pia Park wrote:

An incremental MIDX write can find new packs without finding any new
objects, either because the packs are empty or because their objects
are already indexed by an earlier layer.

The existing early exit checks the number of packs, so these cases

First, thanks for working on this :-).

Second, what you wrote makes sense. It may be worth saying "[...] checks
*only* the number of packs", or "[...] but not the number of objects".

publish a zero-object layer without a reverse index. A subsequent
incremental write with --bitmap fails when loading that reverse index.

Reproducible on master as of
b8242b093d9e941a34460d715e3ce616a34ac3fe (2026-09-07), using:

Nit: we typically would abbreviate this using the "reference" pretty
formatter, as in:

    b8242b093d (The 23rd batch, 2026-09-07)

, but I think that it would be more interesting to include the commit
that introduced this breakage, which I would guess (though haven't
bisected) that we've had this bug at least as long as we've been able to
write incremental MIDXs. Though see below for perhaps an earlier origin.

    git init --bare --object-format=sha1 empty.git &&
    (
        cd empty.git &&
        git config midx.version 2 &&
        git pack-objects objects/pack/pack </dev/null &&
        git multi-pack-index write --incremental --bitmap &&
        git multi-pack-index write --incremental --bitmap
    )

The first write succeeds but publishes the empty layer
3c8853aad425100c5ee2ee22209bb0bb3df9ca37. The second exits with status
255, reporting "could not load reverse index for MIDX".

Right. This patch message suggests (and I agree with) the fact that the
first layer wrote anything at all is a bug.

It only happened to work because the first invocation did not require
loading the empty reverse index, and so did not read the corruption that
it just wrote. The second invocation notices the bug because we eagerly
read reverse indexes for pack(s) in previous layer(s) when writing
reachability bitmaps.

That makes me wonder whether this bug is unique to incremental MIDXs at
all. I tried testing this out locally with:

    git.compile init --bare empty.git &&
    (
      cd empty.git &&

      git.compile pack-objects objects/pack/pack </dev/null &&
      git.compile multi-pack-index write
    )

, and it happily wrote a MIDX.

Return success through the existing cleanup path when
compute_sorted_entries() finds no entries for a non-compacting
incremental write. This prevents publishing an empty layer that causes
subsequent incremental writes with --bitmap to fail with exit status
255. Exit before acquiring a lock or creating a temporary MIDX file,
leaving the existing chain untouched.

So I wonder if we should apply the fix even earlier in
write_midx_internal(), perhaps like:

--- 8< ---

git rev-parse 2>/dev/null || cd ~/src/git; git: line 0: cd: /Users/ttaylorr/src/git: No such file or directory

diff --git a/midx-write.c b/midx-write.c
index 580724d21a..5b2aa9acc8 100644
--- a/midx-write.c
+++ b/midx-write.c

@@ -1617,9 +1617,8 @@ static int write_midx_internal(struct write_midx_opts *opts)
 	}

 	if (!ctx.entries_nr) {
-		if (opts->flags & MIDX_WRITE_BITMAP)
-			warning(_("refusing to write multi-pack .bitmap without any objects"));
-		opts->flags &= ~(MIDX_WRITE_REV_INDEX | MIDX_WRITE_BITMAP);
+		error(_("no objects to index."));
+		goto cleanup;
 	}

 	if (ctx.incremental) {
--- >8 ---

(as an aside, we can probably rework those error messages to be a bit
more descriptive, perhaps, "cannot create a multi-pack-index without any
packs". But that is besides the point of your patch.)

Hmm. So we will avoid writing an empty MIDX when we have no object
entries, but only when doing a non-compact, incremental write? I imagine
that we would want similar treatment for both incremental and
non-incremental MIDXs, regardless of whether we are compacting.

I suspect that these tests will change a bit, so I'll avoid reviewing
them too carefully for the time being. I am glad, however, that you are
testing cases besides explicitly empty packs, e.g., dropping objects
which are represented in earlier layers.

Thanks,
Taylor