Thread (361 messages) 361 messages, 13 authors, 1d ago

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help