Re: [PATCH GSoC 1/5] protocol-caps: add type support to object-info

On Wed Jul 29, 2026 at 11:53 AM CEST, Chandra Pratap wrote:
> On Sat, 25 Jul 2026 at 17:25, Pablo Sabater <[email protected]> wrote:
>>
>> Teach the server-side object-info handler to accept type as a requested
>> field. When the client includes type in its object-info request, the
>> server returns the requested object type.
>>
>> While at it, fix requested_info->size bit field style.
>>
>> Mentored-by: Karthik Nayak <[email protected]>
>> Mentored-by: Chandra Pratap <[email protected]>
>> Signed-off-by: Pablo Sabater <[email protected]>
>> ---
>>  protocol-caps.c      | 21 ++++++++++++++++++---
>>  t/t5701-git-serve.sh | 27 +++++++++++++++++++++++++++
>>  2 files changed, 45 insertions(+), 3 deletions(-)
>>
>> diff --git a/protocol-caps.c b/protocol-caps.c
>> index 02261be14d..5531d388f0 100644
>> --- a/protocol-caps.c
>> +++ b/protocol-caps.c
>> @@ -11,7 +11,8 @@
>>  #include "strbuf.h"
>>
>>  struct requested_info {
>> -       unsigned size : 1;
>> +       unsigned size:1;
>> +       unsigned type:1;
>>  };
>>
>>  /*
>> @@ -73,15 +74,20 @@ static void send_info(struct repository *r, struct packet_writer *writer,
>>         if (info->size)
>>                 packet_writer_write(writer, "size");
>>
>> +       if (info->type)
>> +               packet_writer_write(writer, "type");
>> +
>>         for_each_string_list_item (item, oid_str_list) {
>>                 const char *oid_str = item->string;
>>                 struct object_id oid;
>>                 size_t object_size;
>> +               enum object_type object_type;
>>
>>                 if (get_oid_hex_algop(oid_str, &oid, r->hash_algo) < 0) {
>>                         packet_writer_error(
>>                                 writer,
>> -                               "object-info: protocol error, expected to get oid, not '%s'",
>> +                               "object-info: protocol error, expected to get "
>> +                               "oid, not '%s'",
>
> I assume this is a style change? The original line doesn't seem
> long enough to wrap though.
>
> Also, this would break the grep-ability of this error string.

Yes It is a style change and it's ~60columns long, I'll drop the change.
Turns out my nvim settings is showing the vertical guide at ~60 columns instead
of 80, but that's on me.

>
>>                                 oid_str);
>>                         continue;
>>                 }
>> @@ -93,7 +99,8 @@ static void send_info(struct repository *r, struct packet_writer *writer,
>>                  * If an object is not recognized by the server append SP to
>>                  * the response.
>>                  */
>> -               if (get_object_info(r->objects, &oid, &object_size) <= OBJ_NONE) {
>> +               object_type = get_object_info(r->objects, &oid, &object_size);
>> +               if (object_type <= OBJ_NONE) {
>>                         strbuf_addstr(&send_buffer, " ");
>>                         goto write;
>>                 }
>> @@ -103,6 +110,9 @@ static void send_info(struct repository *r, struct packet_writer *writer,
>>                                     (uintmax_t)object_size);
>>                 }
>>
>> +               if (info->type)
>> +                       strbuf_addf(&send_buffer, " %s", type_name(object_type));
>> +
>>  write:
>>                 packet_writer_write(writer, "%s", send_buffer.buf);
>>                 strbuf_reset(&send_buffer);
>> @@ -124,6 +134,11 @@ int cap_object_info(struct repository *r, struct packet_reader *request)
>>                         continue;
>>                 }
>>
>> +               if (!strcmp("type", request->line)) {
>> +                       info.type = 1;
>> +                       continue;
>> +               }
>> +
>>                 if (parse_oid(request->line, &oid_str_list))
>>                         continue;
>>
>> diff --git a/t/t5701-git-serve.sh b/t/t5701-git-serve.sh
>> index 9a575aa098..d7c93b5b55 100755
>> --- a/t/t5701-git-serve.sh
>> +++ b/t/t5701-git-serve.sh
>> @@ -366,6 +366,33 @@ test_expect_success 'basics of object-info' '
>>         test_cmp expect actual
>>  '
>>
>> +test_expect_success 'type' '
>> +       test_config transfer.advertiseObjectInfo true &&
>> +
>> +       test-tool pkt-line pack >in <<-EOF &&
>> +       command=object-info
>> +       object-format=$(test_oid algo)
>> +       0001
>> +       size
>> +       type
>> +       oid $(git rev-parse two:two.t)
>> +       oid $(git rev-parse two:two.t)
>> +       0000
>> +       EOF
>> +
>> +       cat >expect <<-EOF &&
>> +       size
>> +       type
>> +       $(git rev-parse two:two.t) $(wc -c <two.t | xargs) blob
>> +       $(git rev-parse two:two.t) $(wc -c <two.t | xargs) blob
>
> Can we not use the `test_file_size` tool to do this instead?
> That should also be much more portable.

Yes, I will use it, I didn't know about it.
A test on top of this one does the same pattern, I will fix it too.

Thanks for the feedback,
Pablo