Show 84 quoted lines
> El lun, 22 jun 2026 a las 22:43, Karthik Nayak
> (<karthik.188@gmail.com>) escribió:
>>
>> Pablo Sabater <pabloosabaterr@gmail.com> writes:
>>
>> [snip]
>>
>> > diff --git a/connect.c b/connect.c
>> > index 1dced8e632..78c69d4485 100644
>> > --- a/connect.c
>> > +++ b/connect.c
>> > @@ -700,16 +700,16 @@ int server_supports(const char *feature)
>> > return !!server_feature_value(feature, NULL);
>> > }
>> >
>> > -void write_fetch_command_and_capabilities(struct strbuf *req_buf,
>> > - const struct string_list *server_options)
>> > +void write_command_and_capabilities(struct strbuf *req_buf, const char *command,
>> > + const struct string_list *server_options)
>> > {
>> > const char *hash_name;
>> > int advertise_sid;
>> >
>> > repo_config_get_bool(the_repository, "transfer.advertisesid", &advertise_sid);
>> >
>> > - ensure_server_supports_v2("fetch");
>> > - packet_buf_write(req_buf, "command=fetch");
>> > + ensure_server_supports_v2(command);
>> > + packet_buf_write(req_buf, "command=%s", command);
>> > if (server_supports_v2("agent"))
>> > packet_buf_write(req_buf, "agent=%s", git_user_agent_sanitized());
>> > if (advertise_sid && server_supports_v2("session-id"))
>> > @@ -727,7 +727,7 @@ void write_fetch_command_and_capabilities(struct strbuf *req_buf,
>> > die(_("mismatched algorithms: client %s; server %s"),
>> > the_hash_algo->name, hash_name);
>> > packet_buf_write(req_buf, "object-format=%s", the_hash_algo->name);
>> > - } else if (hash_algo_by_ptr(the_hash_algo) != GIT_HASH_SHA1_LEGACY) {
>> > + } else if (hash_algo_by_ptr(the_hash_algo) != GIT_HASH_SHA1) {
>> > die(_("the server does not support algorithm '%s'"),
>> > the_hash_algo->name);
>> > }
>>
>> Why did we make this change? If the server doesn't support v2, then the
>> object format should be `GIT_HASH_SHA1_LEGACY`. While the value of it is
>> indeed `GIT_HASH_SHA1`, it indicates a scenario where there was no
>> option to select object hash, which is the scenario here.
>>
>> If there is a reason to make such a change, perhaps we should highlight
>> this in the commit message.
>
> Hi!
> There should be no diff related to that line, In some point between
> Eric's last version (v11) and mine's firs (v12) the original code
> changed. On the diff from v11 [1] the object format is the same, i
> didn't notice this change and it's wrong, I'll fix it for v14, Thanks!
>
>>
>> > diff --git a/connect.h b/connect.h
>> > index c4f6ea4b0a..8f4c523892 100644
>> > --- a/connect.h
>> > +++ b/connect.h
>> > @@ -34,8 +34,12 @@ void check_stateless_delimiter(int stateless_rpc,
>> > struct packet_reader *reader,
>> > const char *error);
>> >
>> > +/*
>> > + * Writes a command along with the requested server capabilities/features into a
>> > + * request buffer.
>> > + */
>> > struct string_list;
>>
>> The comment should be above the function and not the forward
>> declaration.
>
> True, I'll fix it for v14.
>
>>
>> While we're here, why not `#include "string-list.h"` and remove the
>> forward declaration, is there a circular dependency?
>
> I believe this was right because from what I know forward declarations
> are prefered in headers when in this case, the struct is only used as
> a pointer. Investigating, this came from a review from patrick [2].
>Karthik Nayak <karthik.188@gmail.com>