Re: [PATCH 3/6] odb: add `source` field to struct object_info_source

[Junio C Hamano <gitster@pobox.com>]

Justin Tobler <jltobler@gmail.com> writes:

>> @@ -1424,6 +1424,10 @@ int packed_object_info_with_index_pos(struct odb_source_packed *source UNUSED,
>>  	oi->whence = OI_PACKED;
>>  
>>  	if (oi->sourcep) {
>> +		if (!source)
>> +			BUG("cannot request source without an owning source");
>> +		oi->sourcep->source = &source->base;
>
> And here it is set for the packed backend. Looks good.
>
> Naive question: I understand that some `packed_info_object()` callers
> may not have the `struct odb_source` on hand, but when the `struct
> packed_git` is intially setup, is it not always known the ODB source it
> comes from? It makes me wonder if the ODB source should also be recorded
> when `struct packed_git` is initialized.

As with your reaction to [PATCH 1/6], I do share this puzzlement: if
the source can almost always be NULL, what is it good for and isn't
it something that can be computed from the available information?

Perhaps it is the naming?

I am confused what the above quoted code actually is doing ("if you
have a source, then grab its base and set it to .source member of
the struct the out parameter points at", makes it sound like the out
parameter sourcep should be pointing at a structure with .base
member, not .source member, or perhaps the caller should be passing
&oi->sourcep->source as *base to be assigned to, or something).