Re: [PATCH 1/6] packfile: thread odb_source_packed through packed_object_info()

On Mon, Jun 29, 2026 at 12:01:47PM -0500, Justin Tobler wrote:

On 26/06/24 02:19PM, Patrick Steinhardt wrote:

quoted

Add an optional `struct odb_source_packed *source` parameter to
`packed_object_info()` and `packed_object_info_with_index_pos()`. This
parameter is unused at this point in time, but it will be used in a
follow-up commit so that we can record the source of a specific object.

Ok so `packed_object_info()` is responsible for populating `struct
object_info` from the provided packfile and object offset. By
additionally providing the object source, the ultimate goal is to store
the this information in `struct object_info` or some equivalent
structure.

At first, I wondered if it would make more sense for `struct packed_git`
to record the `struct odb_source_packed` it comes from, but maybe that
wouldn't be the best layer to handle this bookkeeping?

Yeah, I was thinking about that, too. But I feel like that would be a
layering violation: a packfile can in theory live standalone without a
source. So tracking that information as part of the packfile itself just
feels wrong to me.

We could in theory adapt all callers of `packed_object_info()` to track
the origin of the packfiles. I _think_ that should be feasible at almost
all sites. But I'm just not sure myself whether that really buys us much
in the first place, because...

quoted

Note that callers in "odb/source-packed.c" pass the already-available
source, but all other callers pass `NULL` instead. This is fine though,
as we only care about populating this info when called via the packed
store.

Hmmm, is this because knowing the ODB source the object comes from is
only useful for callers from in "odb/source-packed.c"? Maybe this will
become a bit more clear to me in subsequent patches.

... right now none none of the callers that call `packed_object_info()`
directly care about the source information at all. It's really only
callers of `odb_read_object_info()` that do.

So I understand that this feels a bit iffy. But arguably, the right way
to fix this is to stop using `packed_object_info()` altogether. It is an
internal implementation detail of the object source backend, and ideally
we shouldn't need to care about it.

I already have a patch series that fixes git-cat-file(1). The
commit-graph is a bigger building site, as I'm still not a 100% decided
on how to represent such auxiliary data structures with pluggable object
backends. And for the other commands I don't yet have a good answer.

Patrick