Re: [PATCH 3/3] connected: search promisor objects generically

[Patrick Steinhardt <ps@pks.im>]

On Mon, Jun 22, 2026 at 10:57:10AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
[snip]
> > -		 *
> > -		 * Before checking for promisor packs, be sure we have the
> > -		 * latest pack-files loaded into memory.
> >  		 */
> > -		odb_reprepare(the_repository->objects);
> 
> Hmph?

Oh, I think I completely misread this as `odb_alternates_prepare()`,
which is something you typically see before loops like this. By using a
helper like `odb_for_each_object_ext()` we of course wouldn't have to
call that function anymore.

But this here is of course different, as this call would also cause us
to reload packfiles and loose objects.

> >  		do {
> > -			struct packed_git *p;
> > -
> > -			repo_for_each_pack(the_repository, p) {
> > -				if (!p->pack_promisor)
> > -					continue;
> > -				if (find_pack_entry_one(oid, p))
> > -					goto promisor_pack_found;
> > +			opts.prefix = oid;
> > +
> > +			err = odb_for_each_object_ext(the_repository->objects,
> > +						      NULL, promised_object_cb,
> > +						      NULL, &opts);
> > +			if (err < 0)
> > +				break;
> > +			if (err > 0) {
> > +				err = 0;
> > +				continue;
> >  			}
> 
> So we used to manually iterate and stop when we have a matching pack
> entry, but now "stop when we find" is done by promisor_object_cb
> callback that returns 1.
> 
> What is the reason why we no longer odb_(re)prepare() upfront before
> going into the loop?  Would it make us miss a newly added promisor
> packs?  We will fall back to rev-list for correctness, so it may not
> matter, though.

So yes, this is a bug.

Thanks!

Patrick