[PATCH v8 0/7] Make the "promisor-remote" capability support more fields
- From
Christian Couder <christian.couder@gmail.com>
- Date
- Sep 8, 2025, 05:30 UTC
- Message-ID
- <20250908053056.956907-1-christian.couder@gmail.com>
- In-Reply-To
- <20250731072401.3817074-1-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:nonethen "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