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

On Thu, Aug 13, 2026 at 03:30:53PM -0500, Taylor Blau wrote:
> On Thu, Aug 13, 2026 at 10:49:00AM +0200, Patrick Steinhardt wrote:
> > > @@ -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.

I think that's a bit roundabout, as it simply tells us that the bitmap
was generated correctly, but not that the MIDX contains the objects. It
of course should if the bitmap was generated properly, but I would have
preferred if we verified the property directly.

Anyway, this is not a huge concern, more of a nitpick. Thanks!

Patrick