From: Karthik Nayak Date: Fri, 26 Jun 2026 16:57:22 GMT Subject: Re: [PATCH GSoC v14 06/13] fetch-pack: move function to connect.c Message-ID: In-Reply-To: <20260625-ps-eric-work-rebase-v14-6-09f7ffe21a53@gmail.com> Pablo Sabater writes: Nit: could we mention the function in the subject? > write_fetch_command_and_capabilities will be refactored in a > subsequent Super Nit: Some parts of your patches use backticks for quoting code or filenames and others skip the convention. It would be nice to be consistent. > commit where it will become a more general-purpose function, making it > more accessible to additional commands in the future. > > Move `write_fetch_command_and_capabilities()` to `connect.c`, where > there are similar purpose functions. > > Because string_list is only used as a pointer, use a forward > declaration [1]. > > [1]: https://lore.kernel.org/git/Z0RIqUAoEob8lGfM@pks.im/ > > Helped-by: Jonathan Tan > Helped-by: Christian Couder > Signed-off-by: Calvin Wan > Signed-off-by: Eric Ju > Signed-off-by: Pablo Sabater > --- > connect.c | 34 ++++++++++++++++++++++++++++++++++ > connect.h | 4 ++++ > fetch-pack.c | 34 ---------------------------------- > 3 files changed, 38 insertions(+), 34 deletions(-) > > diff --git a/connect.c b/connect.c > index 47e39d2a73..1dced8e632 100644 > --- a/connect.c > +++ b/connect.c > @@ -700,6 +700,40 @@ 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) > +{ > + 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"); > + if (server_supports_v2("agent")) > + packet_buf_write(req_buf, "agent=%s", git_user_agent_sanitized()); > + if (advertise_sid && server_supports_v2("session-id")) > + packet_buf_write(req_buf, "session-id=%s", trace2_session_id()); > + if (server_options && server_options->nr) { > + ensure_server_supports_v2("server-option"); > + for (size_t i = 0; i < server_options->nr; i++) > + packet_buf_write(req_buf, "server-option=%s", > + server_options->items[i].string); > + } > + > + if (server_feature_v2("object-format", &hash_name)) { > + const unsigned int hash_algo = hash_algo_by_name(hash_name); > + if (hash_algo_by_ptr(the_hash_algo) != hash_algo) > + 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) { > + die(_("the server does not support algorithm '%s'"), > + the_hash_algo->name); > + } > + packet_buf_delim(req_buf); > +} > + > static const char *url_scheme_name(enum url_scheme scheme) > { > switch (scheme) { > diff --git a/connect.h b/connect.h > index aa482a37fb..c4f6ea4b0a 100644 > --- a/connect.h > +++ b/connect.h > @@ -34,4 +34,8 @@ void check_stateless_delimiter(int stateless_rpc, > struct packet_reader *reader, > const char *error); > > +struct string_list; > +void write_fetch_command_and_capabilities(struct strbuf *req_buf, > + const struct string_list *server_options); > + > #endif > diff --git a/fetch-pack.c b/fetch-pack.c > index ad07603755..4a8a70b5f3 100644 > --- a/fetch-pack.c > +++ b/fetch-pack.c > @@ -1376,40 +1376,6 @@ static int add_haves(struct fetch_negotiator *negotiator, > return haves_added; > } > > -static void write_fetch_command_and_capabilities(struct strbuf *req_buf, > - 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"); > - if (server_supports_v2("agent")) > - packet_buf_write(req_buf, "agent=%s", git_user_agent_sanitized()); > - if (advertise_sid && server_supports_v2("session-id")) > - packet_buf_write(req_buf, "session-id=%s", trace2_session_id()); > - if (server_options && server_options->nr) { > - ensure_server_supports_v2("server-option"); > - for (size_t i = 0; i < server_options->nr; i++) > - packet_buf_write(req_buf, "server-option=%s", > - server_options->items[i].string); > - } > - > - if (server_feature_v2("object-format", &hash_name)) { > - const unsigned int hash_algo = hash_algo_by_name(hash_name); > - if (hash_algo_by_ptr(the_hash_algo) != hash_algo) > - 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) { > - die(_("the server does not support algorithm '%s'"), > - the_hash_algo->name); > - } > - packet_buf_delim(req_buf); > -} > - > static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out, > struct fetch_pack_args *args, > const struct ref *wants, struct oidset *common, > > -- > 2.54.0