From: Patrick Steinhardt <ps@pks.im>
Date: Thu, 30 Oct 2025 08:58:39 GMT
Subject: Re: [PATCH 4/8] packfile: fix approximation of object counts
Message-ID: <aQMov9F59ZFXSqAG@pks.im>
In-Reply-To: <aQKZ7FW925zvscgh@nand.local>

On Wed, Oct 29, 2025 at 06:49:16PM -0400, Taylor Blau wrote:
> On Tue, Oct 28, 2025 at 12:08:34PM +0100, Patrick Steinhardt wrote:
> > diff --git a/packfile.c b/packfile.c
> > index 6aa2ca8ac9e..6722c3b2b88 100644
> > --- a/packfile.c
> > +++ b/packfile.c
[snip]
> > -		for (p = r->objects->packfiles->packs; p; p = p->next) {
> > -			if (open_pack_index(p))
> > +		repo_for_each_pack(r, p) {
> > +			if (open_pack_index(p) || p->multi_pack_index)
> 
> Do we care about opening the pack index if we already accounted for it
> via the MIDX path above? My guess is not, so I would probably suggest
> writing this conditional as:
> 
>     if (p->multi_pack_index || open_pack_index(p))
>         continue;
> 
> to avoid loading pack indexes unless we have to.

Makes sense indeed. We don't need it to have the MIDX prepared, so we
can avoid the function call if we don't have any.

Patrick