Re: [PATCH GSoC v2 4/6] fetch-object-info: parse type from server response

From: Jeff King <peff@peff.net>
Date: Aug 2, 2026, 16:38 UTC
Message-ID: <20260802163806.GA21296@coredump.intra.peff.net>
In-Reply-To: <xmqqcxw04hjm.fsf@gitster.g>

On Sun, Aug 02, 2026 at 09:24:13AM -0700, Junio C Hamano wrote:

Show 28 quoted lines

> "Pablo Sabater" <pabloosabaterr@gmail.com> writes:
> 
> > What I understood is that fetch_object_info shouldn't use object_info to
> > store the results, because it doesn't call read_object_info() like other
> > commands like 'info' do. Then, it should use its own data structure to
> > hold the results with flags like wants_size and wants_type. Something
> > like:
> >
> > 	struct object_info_results {
> > 		enum object_type *types;
> > 		size_t *sizes;
> > 		unsigned *unrecognized;
> > 		size_t nr;
> > 		unsigned wants_size:1;
> > 		unsigned wants_type:1;
> > 	};
> 
> I would have expected this to be an array of struct, i.e.
> 
> 	struct {
> 		struct oid *oid;
> 		enum object_type type;
> 		size_t size;
> 	} *result;
> 	size_t result_nr, result_alloc;
> 
> if you do not have the number of things you query upfront, or it may
> be an array of fixed size (i.e. no nr/alloc, just nr).

I think that could work, but two gotchas:

  - an array-of-struct allocates each item for every object. So if we
    are only asking about type, we have to allocate nr * size_t space to
    hold "size" fields nobody cares about.

    This is true of object_info, too, but there we don't care about
    memory cost because we're only using one at a time. Whereas here the
    intent is to hold many results at once.

  - you do need to signal somewhere whether "type" is valid (i.e.,
    whether the remote side supported it). You can put that flag into
    the result struct, but it is a little wasteful. It is really a
    property of the whole query, not of each individual object. So you'd
    have to carry extra flags around (one per type). Whereas NULL-ness
    of the array can signal that same information.

> If you'll be making the same query for many different objects, you
> know if you are asking for type for all of them or for none of them,
> so depending on how the caller uses it, you may not need the valid
> bit.  Or type==OBJ_NONE could signal "we have no info".

Yeah, we sometimes use OBJ_NONE or OBJ_BAD as a sentinel value for type. But if we're not asking for a type field at all, I think that gets awkward.

So for unknown objects, I think a separate bit is less awkward.

For signaling "the server refused to tell us this item" we could use sentinel types like OBJ_NONE. But I don't think that extends to other fields (e.g., there is no useful sentinel value for "size").

Show 17 quoted lines

> And you'd be using the second pattern I outlined, i.e.
> 
> 	for (size_t it = 0; it < result_nr; it++) {
>         	/*
> 		 * you may selectively populate the oi to signal
> 		 * you do not need some values, but you get the
>         	 * idea.
> 		 */
> 		struct object_info oi = {
> 			type_p = &result[it].type,
> 			size_p = &result[it].size,
> 			...
> 		};
> 		... ask about result[it].oid using &oi ...
> 	}
> 
> to populate the result[] array with values, I would imagine.

I think that is a perfectly reasonable direction for asking many responses from read_object_info(). But ultimately this is all getting shipped to the remote over the object-info protocol. So we never need an object_info at all, and even if we used one, we really would need N of them, because we're going to fill N requests at once (to reduce server round-trips).

-Peff