Re: [PATCH GSoC v4 4/9] fetch-object-info: use dedicated struct for the results

On Tue Aug 4, 2026 at 10:58 PM CEST, Junio C Hamano wrote:

Pablo Sabater [off-list ref] writes:

quoted

 builtin/cat-file.c  | 59 +++++++++-----------------------------
 fetch-object-info.c | 81 ++++++++++++++++++++++++++---------------------------
 fetch-object-info.h | 27 +++++++++++++-----
 object-file.c       | 10 -------
 odb.h               |  3 --
 transport.c         |  3 +-
 transport.h         |  5 ++--
 7 files changed, 77 insertions(+), 111 deletions(-)

We lost ~30 lines mostly thanks to losing an intermediate structure
and need to flip pointer members in it to point at heap allocated
locations?

Yes, also contributes that the logic that filtered the allow-list is
dropped, and compared to the previous version, no type logic is in
this patch.

quoted

-struct protocol_placeholder_entry {
-	const char *option;
-	const char *atom;
-};
-
-static const struct protocol_placeholder_entry remote_atom_map[] = {
-	{"size", "objectsize"},
-	{"type", "objecttype"},
-	/*
-	 * Add new protocol options here. Even if the server doesn't support
-	 * them the allow_list will drop them if the server doesn't advertise
-	 * them.
-	 */
-};

Hmph, it would not make any difference in the final result, as we
are losing protocol_placeholder_entry and this list of atoms, but
I am somewhat confused.   Why we already had "type" here, even
though we are adding support for it much later in the series (and
unlike the previous iteration, this iteration did not do the
ask/want_type thing in the previous step)?

The list was designed so even if new placeholder were added, it would be
idemp. because nothing would happen until a server did advertise that
attribute; that's why there's no change in behavior.

It was there because it was interesting for the tests. A known
placeholder but unsupported in the server still outputs an empty string
(this would be the case of: new client vs old server).

This way we had tested:

  unknown + unsupported as %(deltabase) is.
  known + unsupported as %(objecttype) was. (that's why it's on the list)
  knwon + supported as %(objectsize) is.

Using object_info and the list made sense because object_info already had
a type field, so listing type cost nothing. Now that we have to dedicate
an array per attribute it would mean carrying a types array that nobody
fills, so I preferred to drop it here and implement type fully later.

Hope this makes sense,
Pablo