Re: [PATCH GSoC v19 05/13] fetch-pack: move write_fetch_command_and_capabilities() to connect.c
From: Pablo Sabater <hidden>
Date: 2026-07-18 19:00:40
On Sat Jul 18, 2026 at 8:34 PM CEST, SZEDER Gábor wrote:
On Fri, Jul 17, 2026 at 05:05:15PM +0200, Pablo Sabater wrote:quoted
In a subsequent commit write_fetch_command_and_capabilities() will be refactored to 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/ (local) Helped-by: Jonathan Tan [off-list ref] Helped-by: Christian Couder [off-list ref] Signed-off-by: Calvin Wan <redacted> Signed-off-by: Eric Ju <redacted> Signed-off-by: Pablo Sabater <redacted> --- 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..31e5ab958b 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 = 0; + + 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)) { + int hash_algo = hash_algo_by_name(hash_name); + if (hash_algo_by_ptr(the_hash_algo) != hash_algo)hash_algo_by_ptr() returns an unsigned integer, and comparing that to a signed integer results in: connect.c: In function ‘write_fetch_command_and_capabilities’: connect.c:726:53: error: comparison of integer expressions of different signedness: ‘uint32_t’ {aka ‘unsigned int’} and ‘int’ [-Werror=sign-compare] 726 | if (hash_algo_by_ptr(the_hash_algo) != hash_algo) | ^~ cc1: all warnings being treated as errors make: *** [Makefile:2924: connect.o] Error 1 This was not an issue while the function lived in "fetch-pack.c", because that file uses #define DISABLE_SIGN_COMPARE_WARNINGS, but "connect.c" doesn't. I think as a preparatory step the type of the 'hash_algo' variable should be changed to unsigned int while the function is still in "fetch-pack.c" to avoid this; you'll convert it in the next patch anyway.
Thanks for noticing it. Yes, then the prep patch needs to be done in fetch-pack.c before moving it to connect.c. I had it after because 2 hunks of that patch fixes calls for connect.c so this way there were all for connect.c instead of mixing. [snip] Thanks, Pablo