Re: [PATCH v2 12/19] streaming: rely on object sources to create object stream

[Patrick Steinhardt <ps@pks.im>]

On Fri, Nov 21, 2025 at 11:32:38AM -0800, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> > @@ -463,30 +461,15 @@ static int istream_source(struct odb_read_stream **out,
> >  			  struct repository *r,
> >  			  const struct object_id *oid)
> >  {
> > -	unsigned long size;
> > -	int status;
> > -	struct object_info oi = OBJECT_INFO_INIT;
> > -
> > -	oi.sizep = &size;
> > -	status = odb_read_object_info_extended(r->objects, oid, &oi, 0);
> > -	if (status < 0)
> > -		return status;
> > +	struct odb_source *source;
> >  
> > -	switch (oi.whence) {
> > -	case OI_LOOSE:
> > -		if (open_istream_loose(out, r, oid) < 0)
> > -			break;
> > -		return 0;
> > -	case OI_PACKED:
> > -		if (oi.u.packed.is_delta ||
> > -		    repo_settings_get_big_file_threshold(the_repository) >= size ||
> > -		    open_istream_pack_non_delta(out, r, oid, oi.u.packed.pack,
> > -						oi.u.packed.offset) < 0)
> > -			break;
> > +	if (!open_istream_pack_non_delta(out, r->objects, oid))
> >  		return 0;
> > -	default:
> > -		break;
> > -	}
> > +
> > +	odb_prepare_alternates(r->objects);
> > +	for (source = r->objects->sources; source; source = source->next)
> > +		if (!open_istream_loose(out, source, oid))
> > +			return 0;
> 
> Hmph.
> 
> Earlier we let odb_read_object_info_extended() decide which one of
> the duplicated objects (e.g., perhaps a loose object is still there
> after packing), and then used the one it picked.  I think the
> odb_read_object_info_extended() encodes a particular order with with
> solid reasons like "do in-core cached one first", "favor objects in
> pack over loose ones".
> 
> Now we instead let the first one with the object in the linked list
> of sources, which may be different, unless the linked list is
> created with the same "why one source needs to be given precedence
> over the others" reasoning.
> 
> I do not know if/how it matters, this somewhat changes the
> semantics, no?

The semantics are slightly different now in case multiple sources have
the object, true. I don't really think that this matters though: the
stream doesn't even indicate to the caller which source the stream has
been opened from, and neither does it indicate whether the object was
loose or packed. So assuming that there is no hash collision the result
would be the same, as the object contents should be similar independent
of the source.

Will update the commit message and add an explanation.

Patrick