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

[Patrick Steinhardt <ps@pks.im>]

On Wed, Oct 14, 2026 at 10:43:23AM -0500, Justin Tobler wrote:
> On 26/10/14 08:56AM, Patrick Steinhardt wrote:
> > 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.
> 
> That's fair. I don't think we need to get rid of this flag as part of
> this series, but it seem like it would be something nice to eventually
> remove.
> 
> Do we want to leave a TODO comment to mention this?

I think I'd rather keep it in the hive mind. It's a bit more intricate
to get rid of this, and TODO items often invit rather mindless
approaches to fixes.

Patrick