Re: [PATCH 1/2] odb/source: generalize `reprepare()` callback

[Patrick Steinhardt <ps@pks.im>]

On Fri, Jun 26, 2026 at 02:10:32PM +0200, Toon Claes wrote:
> > diff --git a/builtin/grep.c b/builtin/grep.c
> > index 8080d1bf5e..7361bf071e 100644
> > --- a/builtin/grep.c
> > +++ b/builtin/grep.c
> > @@ -1361,10 +1360,8 @@ int cmd_grep(int argc,
> >  			struct odb_source *source;
> >  
> >  			odb_prepare_alternates(the_repository->objects);
> > -			for (source = the_repository->objects->sources; source; source = source->next) {
> > -				struct odb_source_files *files = odb_source_files_downcast(source);
> 
> So you're downcasting inside the implementation by the backends itself.
> That makes sense, but would it be worth to say something about that in
> the commit message?

Hm. Would that provide much value? I'm probably quite a bit biased here,
but I think that it's implicit that the backends have to eventually cast
the generic structure to their own backend.

So I wouldn't really know how to clarify this. Did you have anything
specific in mind?

> > diff --git a/odb/source-packed.c b/odb/source-packed.c
> > index 42c28fba0e..fa5a072488 100644
> > --- a/odb/source-packed.c
> > +++ b/odb/source-packed.c
> > @@ -15,7 +15,7 @@ static int find_pack_entry(struct odb_source_packed *store,
> >  {
> >  	struct packfile_list_entry *l;
> >  
> > -	odb_source_packed_prepare(store);
> > +	odb_source_prepare(&store->base, 0);
> 
> Why are you not using ODB_PREPARE_FLUSH_CACHES here? It used to do
> before?

Because this was calling `odb_source_packed_prepare()` before, not
`odb_source_reprepare()`. So this was calling the non-flushing variant.

> >  	if (store->midx && fill_midx_entry(store->midx, oid, e))
> >  		return 1;
> >  
> > @@ -47,7 +47,7 @@ static int odb_source_packed_read_object_info(struct odb_source *source,
> >  	 * been added since the last time we have prepared the packfile store.
> >  	 */
> >  	if (flags & OBJECT_INFO_SECOND_READ)
> > -		odb_source_reprepare(source);
> > +		odb_source_prepare(source, ODB_PREPARE_FLUSH_CACHES);
> 
> I think the new code is correct, but why wasn't `packed` used here in
> the past? The old odb_source_reprepare() expected a downcasted, didn't
> it?

No, `odb_source_reprepare()` is the generic variant. The naming schema
is typically:

  - `odb_source_frobnicate()` for the generic variants, which receive a
    `struct odb_source` as input.

  - `odb_source_<type>_frobnitcate()` for their backend-specific
    implementations, which cast down the generic `struct odb_source` to
    their backend-specific struct.

Thanks!

Patrick