Re: [PATCH 2/5] setup: detangle loading of loose object maps

On Tue, Jul 28, 2026 at 03:32:27PM -0500, Justin Tobler wrote:
> On 26/07/24 05:48AM, Patrick Steinhardt wrote:
> > When a repository is configured to use a compatibility hash function
> > then we load the loose object map when we initialize the repository.
> > This object map provides the mappings between the canonical object hash
> > and the compatibility object hash.
> > 
> > Loading the object map happens in `repo_set_compat_hash_algo()`, which
> > calls `repo_read_loose_object_map()` in case the compatibility object
> > hash is non-zero. This setup sequence has two major downsides:
> > 
> >   - We assume that the primary object database is the "files" object
> >     database so that we can extract its "loose" backend. This stops
> >     working with pluggable object databases.
> 
> So IIUC, does this mean that `repo_set_compat_hash_algo()` is directly
> reaching into the loose object source to load the compatibility object
> map? I suppose it should be the responsibility of the respective ODB
> backend to handle object compatibility.
> 
> >   - We require the object database to already have been initialized when
> >     configuring the object database. This means that we must intermix
> >     configuration of the repository and initialization of its
> >     sub-structures in a weird way.
> 
> If there any reason we need to eagerly load compatibility object
> mappings?

I'm not familiar enough with the compatibility mappings to really be
able to say. Naively I'd say "no", but I'm rather erring on the side of
caution and want to leave this as-is.

> > diff --git a/loose.c b/loose.c
> > index 9dad75373b..a3b2dcedc2 100644
> > --- a/loose.c
> > +++ b/loose.c
> > @@ -69,6 +69,9 @@ static int load_one_loose_object_map(struct odb_source_loose *loose)
> >  	FILE *fp;
> >  	int ret = -1;
> >  
> > +	if (!should_use_loose_object_map(repo))
> > +		return 0;
> 
> Previously the above condition has asserted in
> `repo_read_loose_object_map()` which calls `loose_object_map_load()` for
> each source. Do we expect each source to potentially answer differently
> though?

Not really, no. But as it's now the source that loads the object map it
has to verify for itself whether it should or should not load it.

Patrick