From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 23 Jan 2026 09:43:00 GMT
Subject: Re: [PATCH v3 10/14] treewide: drop uses of `for_each_{loose,packed}_object()`
Message-ID: <aXNCpJ6q3HwcwhPG@pks.im>
In-Reply-To: <aXLEzAnNRTf5A6bt@nand.local>

On Thu, Jan 22, 2026 at 07:46:04PM -0500, Taylor Blau wrote:
> On Wed, Jan 21, 2026 at 01:50:26PM +0100, Patrick Steinhardt wrote:
> > diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> > index 6964a5a52c..7d16fbc1b8 100644
> > --- a/builtin/cat-file.c
> > +++ b/builtin/cat-file.c
> > @@ -846,8 +849,15 @@ static void batch_each_object(struct batch_options *opt,
> >  		.payload = _payload,
> >  	};
> >  	struct bitmap_index *bitmap = prepare_bitmap_git(the_repository);
> > +	struct odb_source *source;
> >
> > -	for_each_loose_object(the_repository->objects, batch_one_object_loose, &payload, 0);
> > +	odb_prepare_alternates(the_repository->objects);
> > +	for (source = the_repository->objects->sources; source; source = source->next) {
> > +		int ret = odb_source_loose_for_each_object(source, NULL, batch_one_object_oi,
> > +							   &payload, flags);
> > +		if (ret)
> > +			break;
> > +	}
> 
> OK, I'm guessing that this is one such case where we can't yet use
> odb_for_each_object() function directly because of the refactoring which
> you alluded to in the commit message. That seems reasonable, though I
> wonder if it's worth adding a /* TODO */ comment here to that effect.

Sure, I can add a comment.

> Just out of curiosity, what does that refactoring entail? I'm curious
> because I wonder whether the caller is just written in such a way that
> it makes it hard to immediately plug into the new API, or whether there
> are more fundamental issues at play that make the refactoring less than
> straightforward. If the latter, those could potentially help inform the
> direction here.
> 
> (To be clear, I figure that this is likely work that you have already
> done, I'm just curious to see if the details would yield any benefit to
> the immediate patch series under discussion.)

What this code here intends to do is to filter objects via an object
filter (e.g. "--filter=blobs:none"). The way I intend do introduce this
functinoality in a subsequent series is to introduce a `struct
odb_for_each_object_options` that contains optional parameters:

  - An object ID prefix that can be used to iterate over all objects
    that have a certain matching prefix. This will be used for example
    in "object-name.c".

  - An object filter that can be used to filter objects like we do here.

  - Potentially more things that I haven't discovered yet?

Once we have that, the filtering can then happen on the source level.
For the packfile store it would mean that we can try to filter via the
bitmap, if available, and that would allow us to move the logic that we
have here into the backend.

Patrick