On Mon, May 26, 2025 at 09:23:59AM +0200, Patrick Steinhardt wrote:
> On Sun, May 25, 2025 at 02:41:57PM -0400, Taylor Blau wrote:
> > diff --git a/midx-write.c b/midx-write.c
> > index e4a3830d45..7802a4b694 100644
> > --- a/midx-write.c
> > +++ b/midx-write.c
> > @@ -943,6 +943,19 @@ static int fill_packs_from_midx(struct write_midx_context *ctx,
> > {
> > struct multi_pack_index *m;
> >
> > + if (preferred_pack_name) {
> > + /*
> > + * If a preferred pack is specified, need to have
> > + * packed_git's loaded to ensure the chosen preferred
> > + * pack has a non-zero object count.
> > + *
> > + * Trick ourselves into thinking that we're writing a
> > + * reverse index in this case in order to open up the
> > + * pack index file.
> > + */
> > + flags |= MIDX_WRITE_REV_INDEX;
> > + }
>
> This change feels a bit weird to me. Sure, it does allow us to pull out
> the loop in the subsequent patch. But honestly, that makes this
> workaround even weirder in that we now set unrelated flags in some
> function and expect a different function to only honor it in order to
> open the packfile.
>
> Shouldn't we instead have a separate flag for the new function that
> tells it whether or not it is supposed to prepare the pack?
Yeah, I like that much better, thanks!
Thanks,
Taylor
