promisor-remote: refactor to get rid of 'struct strvec'

In a following commit, we will use the new 'promisor-remote' protocol capability introduced by d460267613 (Add 'promisor-remote' capability to protocol v2, 2025-02-18) to pass and process more information about promisor remotes than just their name and url. For that purpose, we will need to store information about other fields, especially information that might or might not be available for different promisor remotes. Unfortunately using 'struct strvec', as we currently do, to store information about the promisor remotes with one 'struct strvec' for each field like "name" or "url" does not scale easily in that case. We would need one 'struct strvec' for each new field, and then we would have to pass all these 'struct strvec' around. Let's refactor this and introduce a new 'struct promisor_info'. It will only store promisor remote information in its members. For now it has only a 'name' member for the promisor remote name and an 'url' member for its URL. We will use a 'struct string_list' to store the instances of 'struct promisor_info'. For each 'item' in the string_list, 'item->string' will point to the promisor remote name and 'item->util' will point to the corresponding 'struct promisor_info' instance. Explicit members are used within 'struct promisor_info' for type safety and clarity regarding the specific information being handled, rather than a generic key-value store. We want to specify and document each field and its content, so adding new members to the struct as more fields are supported is fine. Signed-off-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Christian Couder committed Sep 8, 2025 at 07:30 UTC 57af9cc2e628165bef849576e2d42d8b200717ee
1 file changed +66 -41
promisor-remote.c
+66 -41
@@ -314,9 +314,35 @@ static int allow_unsanitized(char ch)
314 return ch > 32 && ch < 127;
315 }
316
317 -static void promisor_info_vecs(struct repository *repo,
318 - struct strvec *names,
319 - struct strvec *urls)
317 +/*
318 + * Struct for promisor remotes involved in the "promisor-remote"
319 + * protocol capability.
320 + *
321 + * Except for "name", each <member> in this struct and its <value>
322 + * should correspond (either on the client side or on the server side)
323 + * to a "remote.<name>.<member>" config variable set to <value> where
324 + * "<name>" is a promisor remote name.
325 + */
326 +struct promisor_info {
327 + const char *name;
328 + const char *url;
329 +};
330 +
331 +static void promisor_info_list_clear(struct string_list *list)
332 +{
333 + for (size_t i = 0; i < list->nr; i++) {
334 + struct promisor_info *p = list->items[i].util;
335 + free((char *)p->name);
336 + free((char *)p->url);
337 + }
338 + string_list_clear(list, 1);
339 +}
340 +
341 +/*
342 + * Populate 'list' with promisor remote information from the config.
343 + * The 'util' pointer of each list item will hold a 'struct promisor_info'.
344 + */
345 +static void promisor_config_info_list(struct repository *repo, struct string_list *list)
346 {
347 struct promisor_remote *r;
348
@@ -328,8 +354,14 @@ static void promisor_info_vecs(struct repository *repo,
354
355 /* Only add remotes with a non empty URL */
356 if (!repo_config_get_string_tmp(the_repository, url_key, &url) && *url) {
331 - strvec_push(names, r->name);
332 - strvec_push(urls, url);
357 + struct promisor_info *new_info = xcalloc(1, sizeof(*new_info));
358 + struct string_list_item *item;
359 +
360 + new_info->name = xstrdup(r->name);
361 + new_info->url = xstrdup(url);
362 +
363 + item = string_list_append(list, new_info->name);
364 + item->util = new_info;
365 }
366
367 free(url_key);
@@ -340,47 +372,36 @@ char *promisor_remote_info(struct repository *repo)
372 {
373 struct strbuf sb = STRBUF_INIT;
374 int advertise_promisors = 0;
343 - struct strvec names = STRVEC_INIT;
344 - struct strvec urls = STRVEC_INIT;
375 + struct string_list config_info = STRING_LIST_INIT_NODUP;
376 + struct string_list_item *item;
377
378 repo_config_get_bool(the_repository, "promisor.advertise", &advertise_promisors);
379
380 if (!advertise_promisors)
381 return NULL;
382
351 - promisor_info_vecs(repo, &names, &urls);
383 + promisor_config_info_list(repo, &config_info);
384
353 - if (!names.nr)
385 + if (!config_info.nr)
386 return NULL;
387
356 - for (size_t i = 0; i < names.nr; i++) {
357 - if (i)
388 + for_each_string_list_item(item, &config_info) {
389 + struct promisor_info *p = item->util;
390 +
391 + if (item != config_info.items)
392 strbuf_addch(&sb, ';');
393 +
394 strbuf_addstr(&sb, "name=");
360 - strbuf_addstr_urlencode(&sb, names.v[i], allow_unsanitized);
395 + strbuf_addstr_urlencode(&sb, p->name, allow_unsanitized);
396 strbuf_addstr(&sb, ",url=");
362 - strbuf_addstr_urlencode(&sb, urls.v[i], allow_unsanitized);
397 + strbuf_addstr_urlencode(&sb, p->url, allow_unsanitized);
398 }
399
365 - strvec_clear(&names);
366 - strvec_clear(&urls);
400 + promisor_info_list_clear(&config_info);
401
402 return strbuf_detach(&sb, NULL);
403 }
404
371 -/*
372 - * Find first index of 'nicks' where there is 'nick'. 'nick' is
373 - * compared case sensitively to the strings in 'nicks'. If not found
374 - * 'nicks->nr' is returned.
375 - */
376 -static size_t remote_nick_find(struct strvec *nicks, const char *nick)
377 -{
378 - for (size_t i = 0; i < nicks->nr; i++)
379 - if (!strcmp(nicks->v[i], nick))
380 - return i;
381 - return nicks->nr;
382 -}
383 -
405 enum accept_promisor {
406 ACCEPT_NONE = 0,
407 ACCEPT_KNOWN_URL,
@@ -390,19 +411,23 @@ enum accept_promisor {
411
412 static int should_accept_remote(enum accept_promisor accept,
413 const char *remote_name, const char *remote_url,
393 - struct strvec *names, struct strvec *urls)
414 + struct string_list *config_info)
415 {
395 - size_t i;
416 + struct promisor_info *p;
417 + struct string_list_item *item;
418
419 if (accept == ACCEPT_ALL)
420 return 1;
421
400 - i = remote_nick_find(names, remote_name);
422 + /* Get config info for that promisor remote */
423 + item = string_list_lookup(config_info, remote_name);
424
402 - if (i >= names->nr)
425 + if (!item)
426 /* We don't know about that remote */
427 return 0;
428
429 + p = item->util;
430 +
431 if (accept == ACCEPT_KNOWN_NAME)
432 return 1;
433
@@ -414,11 +439,11 @@ static int should_accept_remote(enum accept_promisor accept,
439 return 0;
440 }
441
417 - if (!strcmp(urls->v[i], remote_url))
442 + if (!strcmp(p->url, remote_url))
443 return 1;
444
445 warning(_("known remote named '%s' but with URL '%s' instead of '%s'"),
421 - remote_name, urls->v[i], remote_url);
446 + remote_name, p->url, remote_url);
447
448 return 0;
449 }
@@ -430,8 +455,7 @@ static void filter_promisor_remote(struct repository *repo,
455 struct strbuf **remotes;
456 const char *accept_str;
457 enum accept_promisor accept = ACCEPT_NONE;
433 - struct strvec names = STRVEC_INIT;
434 - struct strvec urls = STRVEC_INIT;
458 + struct string_list config_info = STRING_LIST_INIT_NODUP;
459
460 if (!repo_config_get_string_tmp(the_repository, "promisor.acceptfromserver", &accept_str)) {
461 if (!*accept_str || !strcasecmp("None", accept_str))
@@ -450,8 +474,10 @@ static void filter_promisor_remote(struct repository *repo,
474 if (accept == ACCEPT_NONE)
475 return;
476
453 - if (accept != ACCEPT_ALL)
454 - promisor_info_vecs(repo, &names, &urls);
477 + if (accept != ACCEPT_ALL) {
478 + promisor_config_info_list(repo, &config_info);
479 + string_list_sort(&config_info);
480 + }
481
482 /* Parse remote info received */
483
@@ -482,7 +508,7 @@ static void filter_promisor_remote(struct repository *repo,
508 if (remote_url)
509 decoded_url = url_percent_decode(remote_url);
510
485 - if (decoded_name && should_accept_remote(accept, decoded_name, decoded_url, &names, &urls))
511 + if (decoded_name && should_accept_remote(accept, decoded_name, decoded_url, &config_info))
512 strvec_push(accepted, decoded_name);
513
514 strbuf_list_free(elems);
@@ -490,8 +516,7 @@ static void filter_promisor_remote(struct repository *repo,
516 free(decoded_url);
517 }
518
493 - strvec_clear(&names);
494 - strvec_clear(&urls);
519 + promisor_info_list_clear(&config_info);
520 strbuf_list_free(remotes);
521 }
522