Re: [PATCH 2/3] midx: pass custom '--base' through incremental writes

On Thu, Aug 13, 2026 at 10:49:00AM +0200, Patrick Steinhardt wrote:
> > Thread the parsed base through `write_midx_file()`, and update the
> > repack caller to pass NULL for the new argument where no custom base
> > selection is needed.
> >
> > This exposes a pre-existing problem in incremental writes with custom
> > bases: the writer skips packs from the full existing MIDX chain, even
> > when the caller selected an older base or no base at all.
>
> So as the "normal" write path didn't honor this option at all, I assume
> this bug here then refers to "--stdin-packs" being broken?

Yeah, that's right.

> > @@ -128,19 +128,33 @@ test_expect_success 'write MIDX layer with --base=none and --no-write-chain-file
> >  		--no-write-chain-file --base=none)" &&
> >
> >  	test_cmp "$midx_chain.bak" "$midx_chain" &&
> > -	test_path_is_file "$midxdir/multi-pack-index-$layer.midx"
> > +	test_path_is_file "$midxdir/multi-pack-index-$layer.midx" &&
> > +
> > +	echo "$layer" >"$midx_chain" &&
> > +	test-tool read-midx --show-objects "$objdir" "$layer" >midx.objects &&
> > +	test_grep "^$(git rev-parse 2.2) " midx.objects &&
> > +	cp "$midx_chain.bak" "$midx_chain"
> >  '
>
> Would it make sense to also test for an object from the first MIDX layer
> to be included? Otherwise we don't really assert that all layers are
> included in the new MIDX.

I don't think that is necessary in this case, but let me know if I am
missing something below.

The new layer is written with '--bitmap', and '--base=none' means that
there is no base layer from which the bitmap can inherit objects. Since
1.2 is an ancestor of 2.2, writing a bitmap for the new layer already
requires that it contain 1.2 and the rest of its reachable history.
Otherwise bitmap generation would fail with the missing-closure error
before we reached the assertion.

Checking 2.2 confirms that an object from the old tip was pulled into
the new layer; the successful bitmap write already establishes that its
objects from the earlier layer were pulled in, too.

Thanks,
Taylor