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

[Patrick Steinhardt <ps@pks.im>]

On Mon, Jun 29, 2026 at 12:01:47PM -0500, Justin Tobler wrote:
> On 26/06/24 02:19PM, Patrick Steinhardt wrote:
> > 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...

> > 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