Re: [PATCH GSoC 1/5] protocol-caps: add type support to object-info
- From
Pablo Sabater <pabloosabaterr@gmail.com>
- Date
- Jul 29, 2026, 11:18 UTC
- Message-ID
- <DKB0I229LCE8.CU4ME582YISN@gmail.com>
- In-Reply-To
- <CA+J6zkQFAqZvi-6UaQi6v_OBiT4ihZtCN45vyGCGTbo9TJLJbg@mail.gmail.com>
On Wed Jul 29, 2026 at 11:53 AM CEST, Chandra Pratap wrote:
Show 54 quoted lines
> On Sat, 25 Jul 2026 at 17:25, Pablo Sabater <pabloosabaterr@gmail.com> 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 <karthik.188@gmail.com>
>> Mentored-by: Chandra Pratap <chandrapratap3519@gmail.com>
>> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
>> ---
>> 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.
Show 66 quoted lines
>
>> 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