Re: [PATCH 5/5] odb/source: drop `local` flag

[Patrick Steinhardt <ps@pks.im>]

On Tue, Oct 13, 2026 at 05:44:11PM -0500, Justin Tobler wrote:
> On 26/10/13 09:36AM, Patrick Steinhardt wrote:
> > The `local` flag indicates whether a given `struct odb_source` is a
> > local source or not, where "local" is defined as being the object
> > directory that is directly attached to the current repository. But now
> > that alternates are handled in the "files" backend, this property is not
> > something that even makes sense anymore to have on the generic source.
> > 
> > In fact, we already have a partial replacement for this field in `struct
> > odb_files_dir`. The only other site that requires it is the packed
> > object database source, because it propagates the bit into the packfiles
> > it is opening.
> > 
> > Drop the flag from `struct odb_source` and instead move it into the
> > packed source.
> > 
> > Note that we stop considering the `local` bit when iterating through all
> > loose objects with the `ODB_FOR_EACH_OBJECT_LOCAL_ONLY` flag. This is
> > fine though, as we already handle the flag in the "files" backend
> > itself, where we skip over any object directory that is not marked as
> > local.
> 
> I do wish we could also drop ODB_FOR_EACH_OBJECT_LOCAL_ONLY. This flag
> also feels rather leaky to me. I guess though we still need a way to
> ignore objects from certain sources when iterating over all sources such
> as the inmemory source.
> 
> Overall, I happy to see the `local` field dropped from the generic ODB
> source structure, but IMO is does make each `for_each_object` callback
> handling of the ODB_FOR_EACH_OBJECT_LOCAL_ONLY flag a bit ambiguous and
> awkward.
> 
> Naive question: Do all odb_for_each_object() call sites that use the
> ODB_FOR_EACH_OBJECT_LOCAL_ONLY flag have "files" backend specific
> reasons to do so?

It does make it awkward, I agree. There's only two callers that use the
flag right now:

  - git-pack-objects(1) with its "--local" flag. I consider this command
    to be too specific to the "files" backend anyway though, so arguably
    we could just let it handle this directly.

  - "reachable.c" via `add_unseen_recent_objects_to_traversal()`. This
    is invoke by git-prune(1) to mark reachable objects that are recent,
    and it's dubious that we only enumerate local objects here.

So we might be able to get rid of that flag altogether. But I'm inclined
to go one step at a time and leave this for a future patch series.

> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> [snip]
> > diff --git a/odb/source-inmemory.c b/odb/source-inmemory.c
> > index 868e723c81..75cf29fa9d 100644
> > --- a/odb/source-inmemory.c
> > +++ b/odb/source-inmemory.c
> > @@ -161,8 +161,12 @@ static int odb_source_inmemory_for_each_object(struct odb_source *source,
> >  	};
> >  	struct object_id null_oid = { 0 };
> >  
> > -	if ((opts->flags & ODB_FOR_EACH_OBJECT_PROMISOR_ONLY) ||
> > -	    (opts->flags & ODB_FOR_EACH_OBJECT_LOCAL_ONLY && !source->local))
> > +	/*
> > +	 * In-memory objects are neither promisor objects nor do they live in
> > +	 * the local object directory, so there is nothing to iterate over.
> > +	 */
> > +	if (opts->flags & (ODB_FOR_EACH_OBJECT_PROMISOR_ONLY |
> > +			   ODB_FOR_EACH_OBJECT_LOCAL_ONLY))
> 
> So for the inmemory source, we don't want to iterate over the objects at
> all if we are only interested in "local" objects.
> 
> Naive question: Are there any scenarios where we even would iterate over
> an inmemory source and only be interested in "local" objects?

Not that I'd know of.

Patrick