[PATCH v7 0/5] 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.

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 v6
----------------

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

Here are the changes compared to v6:

- In patch 2/5 ("promisor-remote: allow a server to advertise more
  fields") in "Documentation/gitprotocol-v2.adoc" we now clarify that
  field names in the protocol are case-sensitive and MUST be
  transmitted exactly as specified.

- In patch 2/5 and 4/5 we don't initialize the `static int
  initialized` to `0`, we just let the BSS do it.

- In patch 3/5 ("promisor-remote: refactor how we parse advertised
  fields"), we now use string_list_split_in_place() instead of
  strbuf_split() and `struct string_list elem_list` instead of `struct
  strbuf **elems`. The useless call to strbuf_strip_suffix() has also
  been removed.

- In patch 4/5 ("promisor-remote: allow a client to check fields") in
  "Documentation/config/promisor.adoc" we removed "Field names are
  compared case-insensitively." as it's not true when field names are
  part of the protocol.

- Also in patch 4/5:

  - in all_fields_match((), we use `struct string_list *fields`
    instead of `struct string_list* fields` according to our style,
    and

  - in parse_one_advertised_remote() we use strcmp() instead of
    strcasecmp() as we decided that field names in the protocol are
    case-sensitive.

CI tests
--------

They have all passed, see:

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

Range diff compared to v6
-------------------------

1:  87a6ba5c48 = 1:  87a6ba5c48 promisor-remote: refactor to get rid of 'struct strvec'
2:  0543a42858 ! 2:  c729c110d0 promisor-remote: allow a server to advertise more fields
    @@ Documentation/gitprotocol-v2.adoc: retrieving the header from a bundle at the in
     +connecting to the remote. It corresponds to the "remote.<name>.token"
     +config setting.
     +
    -+No other fields are defined by the protocol at this time. Clients MUST
    -+ignore fields they don't recognize to allow for future protocol
    -+extensions.
    ++No other fields are defined by the protocol at this time. Field names
    ++are case-sensitive and MUST be transmitted exactly as specified
    ++above. Clients MUST ignore fields they don't recognize to allow for
    ++future protocol extensions.
     +
     +For now, the client can only use information transmitted through these
     +fields to decide if it accepts the advertised promisor remote. In the
    @@ promisor-remote.c: static int allow_unsanitized(char ch)
     +static struct string_list *fields_sent(void)
     +{
     +  static struct string_list fields_list = STRING_LIST_INIT_NODUP;
    -+  static int initialized = 0;
    ++  static int initialized;
     +
     +  if (!initialized) {
     +          fields_list.cmp = strcasecmp;
3:  d566719807 ! 3:  9e0eccae21 promisor-remote: refactor how we parse advertised fields
    @@ promisor-remote.c: static int should_accept_remote(enum accept_promisor accept,
     +static struct promisor_info *parse_one_advertised_remote(struct strbuf *remote_info)
     +{
     +  struct promisor_info *info = xcalloc(1, sizeof(*info));
    -+  struct strbuf **elems = strbuf_split(remote_info, ',');
    ++  struct string_list elem_list = STRING_LIST_INIT_NODUP;
    ++  struct string_list_item *item;
     +
    -+  for (size_t i = 0; elems[i]; i++) {
    -+          char *elem = elems[i]->buf;
    ++  string_list_split_in_place(&elem_list, remote_info->buf, ",", -1);
    ++
    ++  for_each_string_list_item(item, &elem_list) {
    ++          char *elem = item->string;
     +          char *value;
     +          char *p = strchr(elem, '=');
     +
    -+          strbuf_strip_suffix(elems[i], ",");
    -+
     +          if (!p) {
     +                  warning(_("invalid element '%s' from remote info"), elem);
     +                  continue;
    @@ promisor-remote.c: static int should_accept_remote(enum accept_promisor accept,
     +                  free(value);
     +  }
     +
    -+  strbuf_list_free(elems);
    ++  string_list_clear(&elem_list, 0);
     +
     +  if (!info->name || !info->url) {
     +          warning(_("server advertised a promisor remote without a name or URL: %s"),
4:  d2a13eabc0 ! 4:  b1a3384ddc promisor-remote: allow a client to check fields
    @@ Documentation/config/promisor.adoc: promisor.acceptFromServer::
     +be used to verify that authentication credentials match expected
     +values.
     ++
    -+Field names are compared case-insensitively. Field values are compared
    -+case-sensitively.
    ++Field values are compared case-sensitively.
     ++
     +The "name" and "url" fields are always checked according to the
     +`promisor.acceptFromServer` policy, independently of this setting.
    @@ promisor-remote.c: static struct string_list *fields_sent(void)
     +static struct string_list *fields_checked(void)
     +{
     +  static struct string_list fields_list = STRING_LIST_INIT_NODUP;
    -+  static int initialized = 0;
    ++  static int initialized;
     +
     +  if (!initialized) {
     +          fields_list.cmp = strcasecmp;
    @@ promisor-remote.c: enum accept_promisor {
     +                      struct string_list *config_info,
     +                      int in_list)
     +{
    -+  struct string_list* fields = fields_checked();
    ++  struct string_list *fields = fields_checked();
     +  struct string_list_item *item_checked;
     +
     +  for_each_string_list_item(item_checked, fields) {
    @@ promisor-remote.c: static struct promisor_info *parse_one_advertised_remote(stru
                        info->name = value;
                else if (!strcmp(elem, "url"))
                        info->url = value;
    -+          else if (!strcasecmp(elem, promisor_field_filter))
    ++          else if (!strcmp(elem, promisor_field_filter))
     +                  info->filter = value;
    -+          else if (!strcasecmp(elem, promisor_field_token))
    ++          else if (!strcmp(elem, promisor_field_token))
     +                  info->token = value;
                else
                        free(value);
5:  c7d7c83534 ! 5:  d0f7fda912 promisor-remote: use string constants for 'name' and 'url' too
    @@ promisor-remote.c: static struct promisor_info *parse_one_advertised_remote(stru
     -          else if (!strcmp(elem, "url"))
     +          else if (!strcmp(elem, promisor_field_url))
                        info->url = value;
    -           else if (!strcasecmp(elem, promisor_field_filter))
    +           else if (!strcmp(elem, promisor_field_filter))
                        info->filter = value;


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

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

-- 
2.50.1.323.g4e0625aa69.dirty