[PATCH v8 0/7] Make the "promisor-remote" capability support more fields

[Christian Couder <christian.couder@gmail.com>]

The "promisor-remote" capability can only be used to pass the names
and URLs of the promisor remotes from the server to the client. After
that the client can use this information to decide if it accepts the
remotes or not.

It would be nice if the server could pass more fields about its
remotes and if the client could use that additional information to
decide about the remotes by comparing it with its local information
about the remotes.

This patch series implements this by adding the "promisor.sendFields"
on the server side and the "promisor.checkFields" on the client side.

For example, if "promisor.sendFields" is set to "partialCloneFilter",
and the server has the remote "foo" configured like this:

[remote "foo"]
        url = file:///tmp/foo.git
	partialCloneFilter = blob:none

then "name=foo,url=file:///tmp/foo.git,partialCloneFilter=blob:none"
will be sent by the server for this remote.

All the information passed through the "promisor-remote" capability is
still only used to decide if the remotes are accepted or not. The
client doesn't store it and doesn't use it for any other purpose.

Note that the filter mechanism already exists for a long time and this
series doesn't change how it works. For example, it has already been
possible for a long time to have different repos using the same
promisor remote with different filters. See the existing partial clone
documentation (like "Documentation/technical/partial-clone.adoc") for
more information on partial clone.

The fields that can be passed are limited to "partialCloneFilter" and
"token".

On the technical side, we get rid of 'struct strvec' and we use
'struct promisor_info' to store the data and 'struct string_list' to
store the 'struct promisor_info' instances instead. We also replace
all the calls to strbuf_split*() functions with calls to
string_list_split*() functions.

This work is part of the "LOP" effort documented in:

  Documentation/technical/large-object-promisors.adoc

See that doc for more information on the broader context.

Changes since v7
----------------

Thanks to Patrick, Junio, Karthik, Jean-Noël and Justin for their
comments on the previous versions.

Here are the changes compared to v7:

  - Rebased on top of 6ad8021821 (The fifth batch, 2025-08-29) to
    avoid some conflicts with relatively new topics and to get
    functions that can simplify parts of the patches like
    string_list_split_in_place_f().

  - In all the patches, some calls to git_config_get_string*()
    functions have been replaced with calls to
    repo_config_get_string*() as git_config_get_string*() functions
    have been removed.

  - In patch 2/7 ("promisor-remote: allow a server to advertise more
    fields") a call to string_list_split_in_place() and a following
    call to string_list_remove_empty_items() have been replaced by a
    single call to string_list_split_in_place_f() which is passed both
    the STRING_LIST_SPLIT_TRIM and the STRING_LIST_SPLIT_NONEMPTY
    flags.

  - Patch 3/7 ("promisor-remote: use string constants for 'name' and
    'url' too") was moved earlier in the series. It used to be patch
    5/5 in v7. This is because it now introduces a new
    skip_field_name_prefix() helper function which should start to be
    used early in the series to avoid some churn.

  - In patch 4/7 ("promisor-remote: refactor how we parse advertised
    fields"), the parse_one_advertised_remote() function is improved
    in the following ways:

    - It is passed a `const char *` instead of a `struct strbuf *` as
      argument, because it doesn't really need a `struct strbuf *`.

    - It uses string_list_split() instead of
      string_list_split_in_place() to avoid munging the argument it is
      passed as this argument is used in a warning() in case we didn't
      find both a name and a url in the remote advertisement.

    - The parsing is simplified using the new skip_field_name_prefix()
      function.

  - Patch 5/7 is new. It replaces a call to strbuf_split_str() with a
    call to string_list_split() as we don't really needed the `struct
    strbuf` that strbuf_split_str() created. This is allowed by the
    previous patch which made parse_one_advertised_remote() accept a
    `const char *` instead of a `struct strbuf *`.

  - In patch 6/7 ("promisor-remote: allow a client to check fields")
    the new skip_field_name_prefix() function is used simplify parsing
    a filter and a token in remote advertisements.

  - Patch 7/7 ("promisor-remote: use string_list_split() in
    mark_remotes_as_accepted()") is also new. In the same way as patch
    5/7, a call to strbuf_split_str() is replaced with a call to
    string_list_split() as we don't need the `struct strbuf` that
    strbuf_split_str() created. This removes the last call to a
    strbuf_split*() function in "promisor-remote.c".

CI tests
--------

They have all passed, see:

https://github.com/chriscool/git/actions/runs/17408565173

Range diff compared to v7
-------------------------

1:  87a6ba5c48 ! 1:  c8716bf361 promisor-remote: refactor to get rid of 'struct strvec'
    @@ promisor-remote.c: static int allow_unsanitized(char ch)
     @@ promisor-remote.c: static void promisor_info_vecs(struct repository *repo,
      
      		/* Only add remotes with a non empty URL */
    - 		if (!git_config_get_string_tmp(url_key, &url) && *url) {
    + 		if (!repo_config_get_string_tmp(the_repository, url_key, &url) && *url) {
     -			strvec_push(names, r->name);
     -			strvec_push(urls, url);
     +			struct promisor_info *new_info = xcalloc(1, sizeof(*new_info));
    @@ promisor-remote.c: char *promisor_remote_info(struct repository *repo)
     +	struct string_list config_info = STRING_LIST_INIT_NODUP;
     +	struct string_list_item *item;
      
    - 	git_config_get_bool("promisor.advertise", &advertise_promisors);
    + 	repo_config_get_bool(the_repository, "promisor.advertise", &advertise_promisors);
      
      	if (!advertise_promisors)
      		return NULL;
    @@ promisor-remote.c: static void filter_promisor_remote(struct repository *repo,
     -	struct strvec urls = STRVEC_INIT;
     +	struct string_list config_info = STRING_LIST_INIT_NODUP;
      
    - 	if (!git_config_get_string_tmp("promisor.acceptfromserver", &accept_str)) {
    + 	if (!repo_config_get_string_tmp(the_repository, "promisor.acceptfromserver", &accept_str)) {
      		if (!*accept_str || !strcasecmp("None", accept_str))
     @@ promisor-remote.c: static void filter_promisor_remote(struct repository *repo,
      	if (accept == ACCEPT_NONE)
2:  c729c110d0 ! 2:  a07976245a promisor-remote: allow a server to advertise more fields
    @@ promisor-remote.c: static int allow_unsanitized(char ch)
     +{
     +	char *fields = NULL;
     +
    -+	if (!git_config_get_string(config_key, &fields) && *fields) {
    -+		/* Split on any comma or space character */
    -+		string_list_split_in_place(fields_list, fields, ", ", -1);
    -+		/*
    -+		 * Remove empty items that might result from trailing
    -+		 * commas, or from items being separated by both
    -+		 * commas and spaces.
    -+		 */
    -+		string_list_remove_empty_items(fields_list, 0);
    ++	if (!repo_config_get_string(the_repository, config_key, &fields) && *fields) {
    ++		string_list_split_in_place_f(fields_list, fields, ",", -1,
    ++					     STRING_LIST_SPLIT_TRIM |
    ++					     STRING_LIST_SPLIT_NONEMPTY);
     +		filter_string_list(fields_list, 0, is_valid_field, (void *)config_key);
     +	}
     +
    @@ promisor-remote.c: static void promisor_info_list_clear(struct string_list *list
     +	for_each_string_list_item(item, field_names) {
     +		char *key = xstrfmt("remote.%s.%s", p->name, item->string);
     +		const char *val;
    -+		if (!git_config_get_string_tmp(key, &val) && *val)
    ++		if (!repo_config_get_string_tmp(the_repository, key, &val) && *val)
     +			set_one_field(p, item->string, val);
     +		free(key);
     +	}
5:  d0f7fda912 ! 3:  e38f2711f5 promisor-remote: use string constants for 'name' and 'url' too
    @@ Commit message
         Let's do the same for "name" and "url" to avoid repeating them
         several times and for consistency with the other fields.
     
    +    For skipping "name=" or "url=" in advertisements, let's introduce
    +    a skip_field_name_prefix() helper function to keep parsing clean
    +    and easy to understand.
    +
         Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
     
      ## promisor-remote.c ##
    @@ promisor-remote.c: char *promisor_remote_info(struct repository *repo)
      		strbuf_addstr_urlencode(&sb, p->url, allow_unsanitized);
      
      		if (p->filter) {
    -@@ promisor-remote.c: static struct promisor_info *parse_one_advertised_remote(struct strbuf *remote_i
    - 		*p = '\0';
    - 		value = url_percent_decode(p + 1);
    +@@ promisor-remote.c: static int should_accept_remote(enum accept_promisor accept,
    + 	return 0;
    + }
    + 
    ++static int skip_field_name_prefix(const char *elem, const char *field_name, const char **value)
    ++{
    ++	const char *p;
    ++	if (!skip_prefix(elem, field_name, &p) || *p != '=')
    ++		return 0;
    ++	*value = p + 1;
    ++	return 1;
    ++}
    ++
    + static void filter_promisor_remote(struct repository *repo,
    + 				   struct strvec *accepted,
    + 				   const char *info)
    +@@ promisor-remote.c: static void filter_promisor_remote(struct repository *repo,
    + 
    + 		for (size_t j = 0; elems[j]; j++) {
    + 			strbuf_strip_suffix(elems[j], ",");
    +-			if (!skip_prefix(elems[j]->buf, "name=", &remote_name))
    +-				skip_prefix(elems[j]->buf, "url=", &remote_url);
    ++			if (!skip_field_name_prefix(elems[j]->buf, promisor_field_name, &remote_name))
    ++				skip_field_name_prefix(elems[j]->buf, promisor_field_url, &remote_url);
    + 		}
      
    --		if (!strcmp(elem, "name"))
    -+		if (!strcmp(elem, promisor_field_name))
    - 			info->name = value;
    --		else if (!strcmp(elem, "url"))
    -+		else if (!strcmp(elem, promisor_field_url))
    - 			info->url = value;
    - 		else if (!strcmp(elem, promisor_field_filter))
    - 			info->filter = value;
    + 		if (remote_name)
3:  9e0eccae21 ! 4:  4263918802 promisor-remote: refactor how we parse advertised fields
    @@ promisor-remote.c: enum accept_promisor {
      
      	if (accept == ACCEPT_ALL)
      		return 1;
    -@@ promisor-remote.c: static int should_accept_remote(enum accept_promisor accept,
    - 	return 0;
    +@@ promisor-remote.c: static int skip_field_name_prefix(const char *elem, const char *field_name, cons
    + 	return 1;
      }
      
    -+static struct promisor_info *parse_one_advertised_remote(struct strbuf *remote_info)
    ++static struct promisor_info *parse_one_advertised_remote(const char *remote_info)
     +{
     +	struct promisor_info *info = xcalloc(1, sizeof(*info));
    -+	struct string_list elem_list = STRING_LIST_INIT_NODUP;
    ++	struct string_list elem_list = STRING_LIST_INIT_DUP;
     +	struct string_list_item *item;
     +
    -+	string_list_split_in_place(&elem_list, remote_info->buf, ",", -1);
    ++	string_list_split(&elem_list, remote_info, ",", -1);
     +
     +	for_each_string_list_item(item, &elem_list) {
    -+		char *elem = item->string;
    -+		char *value;
    -+		char *p = strchr(elem, '=');
    ++		const char *elem = item->string;
    ++		const char *p = strchr(elem, '=');
     +
     +		if (!p) {
     +			warning(_("invalid element '%s' from remote info"), elem);
     +			continue;
     +		}
     +
    -+		*p = '\0';
    -+		value = url_percent_decode(p + 1);
    -+
    -+		if (!strcmp(elem, "name"))
    -+			info->name = value;
    -+		else if (!strcmp(elem, "url"))
    -+			info->url = value;
    -+		else
    -+			free(value);
    ++		if (skip_field_name_prefix(elem, promisor_field_name, &p))
    ++			info->name = url_percent_decode(p);
    ++		else if (skip_field_name_prefix(elem, promisor_field_url, &p))
    ++			info->url = url_percent_decode(p);
     +	}
     +
     +	string_list_clear(&elem_list, 0);
     +
     +	if (!info->name || !info->url) {
     +		warning(_("server advertised a promisor remote without a name or URL: %s"),
    -+			remote_info->buf);
    ++			remote_info);
     +		promisor_info_free(info);
     +		return NULL;
     +	}
    @@ promisor-remote.c: static void filter_promisor_remote(struct repository *repo,
      
     -		for (size_t j = 0; elems[j]; j++) {
     -			strbuf_strip_suffix(elems[j], ",");
    --			if (!skip_prefix(elems[j]->buf, "name=", &remote_name))
    --				skip_prefix(elems[j]->buf, "url=", &remote_url);
    +-			if (!skip_field_name_prefix(elems[j]->buf, promisor_field_name, &remote_name))
    +-				skip_field_name_prefix(elems[j]->buf, promisor_field_url, &remote_url);
     -		}
    -+		advertised = parse_one_advertised_remote(remotes[i]);
    ++		advertised = parse_one_advertised_remote(remotes[i]->buf);
      
     -		if (remote_name)
     -			decoded_name = url_percent_decode(remote_name);
-:  ---------- > 5:  c1f55b84f0 promisor-remote: use string_list_split() in filter_promisor_remote()
4:  b1a3384ddc ! 6:  0e37b7149f promisor-remote: allow a client to check fields
    @@ promisor-remote.c: static int should_accept_remote(enum accept_promisor accept,
      
      	warning(_("known remote named '%s' but with URL '%s' instead of '%s'"),
      		remote_name, p->url, remote_url);
    -@@ promisor-remote.c: static struct promisor_info *parse_one_advertised_remote(struct strbuf *remote_i
    - 			info->name = value;
    - 		else if (!strcmp(elem, "url"))
    - 			info->url = value;
    -+		else if (!strcmp(elem, promisor_field_filter))
    -+			info->filter = value;
    -+		else if (!strcmp(elem, promisor_field_token))
    -+			info->token = value;
    - 		else
    - 			free(value);
    +@@ promisor-remote.c: static struct promisor_info *parse_one_advertised_remote(const char *remote_info
    + 			info->name = url_percent_decode(p);
    + 		else if (skip_field_name_prefix(elem, promisor_field_url, &p))
    + 			info->url = url_percent_decode(p);
    ++		else if (skip_field_name_prefix(elem, promisor_field_filter, &p))
    ++			info->filter = url_percent_decode(p);
    ++		else if (skip_field_name_prefix(elem, promisor_field_token, &p))
    ++			info->token = url_percent_decode(p);
      	}
    + 
    + 	string_list_clear(&elem_list, 0);
     @@ promisor-remote.c: static void filter_promisor_remote(struct repository *repo,
      	if (accept == ACCEPT_NONE)
      		return;
    @@ promisor-remote.c: static void filter_promisor_remote(struct repository *repo,
     -
      	/* Parse remote info received */
      
    - 	remotes = strbuf_split_str(info, ';', 0);
    + 	string_list_split(&remote_info, info, ";", -1);
     @@ promisor-remote.c: static void filter_promisor_remote(struct repository *repo,
      		if (!advertised)
      			continue;
-:  ---------- > 7:  123d41a7fc promisor-remote: use string_list_split() in mark_remotes_as_accepted()

Christian Couder (7):
  promisor-remote: refactor to get rid of 'struct strvec'
  promisor-remote: allow a server to advertise more fields
  promisor-remote: use string constants for 'name' and 'url' too
  promisor-remote: refactor how we parse advertised fields
  promisor-remote: use string_list_split() in filter_promisor_remote()
  promisor-remote: allow a client to check fields
  promisor-remote: use string_list_split() in mark_remotes_as_accepted()

 Documentation/config/promisor.adoc    |  61 ++++
 Documentation/gitprotocol-v2.adoc     |  64 +++-
 promisor-remote.c                     | 424 ++++++++++++++++++++------
 t/t5710-promisor-remote-capability.sh |  65 ++++
 4 files changed, 511 insertions(+), 103 deletions(-)

-- 
2.51.0.168.gc8716bf361