Re: [PATCH v3 04/14] object-file: introduce function to iterate through objects

From: Patrick Steinhardt <ps@pks.im>
Date: Jan 22, 2026, 06:52 UTC
Message-ID: <aXHJEBY1FnbGRtzK@pks.im>
In-Reply-To: <aXFsFAV/1J9DLQRY@nand.local>

On Wed, Jan 21, 2026 at 07:15:16PM -0500, Taylor Blau wrote:

Show 20 quoted lines

> On Wed, Jan 21, 2026 at 01:50:20PM +0100, Patrick Steinhardt wrote:
> > Introduce a new function `odb_source_loose_for_each_object()` to plug
> > this gap. This function doesn't take any data specific to loose objects,
> > but instead it accepts a `struct object_info` that will be populated the
> > exact same as if `odb_source_loose_read_object()` was called.
> 
> This may be a bit of a tangent, but I wonder if we are over-applying the
> function prefixing convention.
> 
> In general I am really happy with this convention, and it yields
> organized headers where functions are clearly grouped by what structure
> they operate on. But I have noticed a handful of times where we replaced
> a very concise function name with a longer prefixed version.
> 
> I think I don't have a clear sense of what the benefit of prefixing is
> in this particular instance. Supposing for a moment that we don't have
> an existing for_each_loose_object() function (which I think is the
> end-state of this series). What does the name
> "odb_source_loose_for_each_object()" convey that
> "for_each_loose_object()" does not?

As you say further down, it makes it easy to see that it's a function that belongs to `struct odb_source_loose`. It immediately gives the reader a sense what the main structure is it belongs to, thus gives scope and makes LSPs work better because of the common prefix.

Show 7 quoted lines

> I think if there were multiple ways to iterate over loose objects, it
> makes a lot of sense to prefix them such that they are grouped to avoid
> mixing interfaces or using one API when you meant to call another. But
> my understanding is that the intent here is to consolidate all of the
> different ways to iterate over objects which live in different
> odb_source implementations opaque to the caller. As a result, what other
> way exists to iterate over loose objects?

There will be more to come: iterating over objects with a prefix, for example. In general, this series is taking a layered approach:

  - `odb_for_each_object()` is the high-level function that users should
    use if possible. It is part of the ODB layer and abstracts away
    details about the ODB sources.

  - `odb_source_for_each_object()` will be introduced in the next patch
    series. It allows the user to take an ODB source and iterate over
    its contained objects, regardless of what the backend is.

  - `odb_source_loose_for_each_object()` is the low-level implementation
    for one specific backend. We also have equivalent functions for the
    other backends, like for example for packed objects.

The longer the function name, the more specific the logic becomes. Sure, eventually it becomes a mouthful, but ideally users wouldn't have to ever interact with the low-level details at all.

Show 11 quoted lines

> Another aspect of this is how approachable the function is to newcomers.
> On the one hand, I can see an argument that prefixing makes it clear
> which functions belong together, and so if a newcomer is familiar with
> the concept of ODB sources, then they should reasonably expect that a
> function to iterate over loose objects would begin with "odb_source_".
> 
> But on the other hand, while a newcomer may be familiar with the basics
> of Git's object model enough to understand the distinction between loose
> and packed objects, they may not be familiar with the concept of an ODB
> source. In that case, the prefix makes it somewhat more difficult to
> find the right function to use.

So I would claim that this is even intentional. If a reader is not aware what an ODB source is, then chances are high that using the function that iterates through one specific source is the wrong thing to do. They should rather use `odb_for_each_object()` in that case, which is the higher-level interface that doesn't require the reader to know about ODB sources in the first place.

I kind of see this as "guiding" the reader and giving them some hints what the preferred interface is.

Patrick