git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christian Couder <christian.couder@gmail.com>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
	Patrick Steinhardt <ps@pks.im>, Taylor Blau <me@ttaylorr.com>,
	Karthik Nayak <karthik.188@gmail.com>,
	Justin Tobler <jltobler@gmail.com>,
	Christian Couder <christian.couder@gmail.com>
Subject: [PATCH v5 0/5] Make the "promisor-remote" capability support more fields
Date: Wed, 25 Jun 2025 14:50:50 +0200	[thread overview]
Message-ID: <20250625125055.1375596-1-christian.couder@gmail.com> (raw)
In-Reply-To: <20250611134506.2975856-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: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 v4
----------------

Thanks to Patrick, Junio, Karthik and Justin for their comments on the
previous versions.

There are very few changes compared to v4 and they are quite small.

In patch 1/5, in the commit message:

  - a few sentences were added to explain why using 'struct strvec'
    for the new fields wouldn't scale well,

  - a typo "use use" was fixed.

In patch 1/5, in the code, a BUG() message was improved.

In patch 2/5, in the code, some code comments were added in
fields_from_config().

CI tests
--------

They all passed:

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

Range diff compared to v4
-------------------------

1:  8a4df71d2c ! 1:  3700939f67 promisor-remote: refactor to get rid of 'struct strvec'
    @@ Commit message
         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.
    +    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 use a 'struct string_list' to store
    -    the instances of 'struct promisor_info'. For each 'item' in the
    +    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.
    @@ promisor-remote.c: static int should_accept_remote(enum accept_promisor accept,
      
     -  if (!strcmp(urls->v[i], remote_url))
     +  if (!p->url)
    -+          BUG("bad config_info (invalid URL) for remote '%s'",
    ++          BUG("bad config_info (URL is NULL) for remote '%s'",
     +              remote_name);
     +
     +  if (!strcmp(p->url, remote_url))
2:  8f3111b4f2 ! 2:  f546756705 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);
     +          filter_string_list(fields_list, 0, is_valid_field, (void *)config_key);
     +  }
3:  c91a1ba04a = 3:  3ac73b14eb promisor-remote: refactor how we parse advertised fields
4:  ad68202057 = 4:  b75577eff2 promisor-remote: allow a client to check fields
5:  e8efe62b7f = 5:  149b275087 promisor-remote: use string constants for 'name' and 'url' too


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    |  62 ++++
 Documentation/gitprotocol-v2.adoc     |  59 +++-
 promisor-remote.c                     | 401 +++++++++++++++++++++-----
 t/t5710-promisor-remote-capability.sh |  65 +++++
 4 files changed, 499 insertions(+), 88 deletions(-)

-- 
2.50.0.2.g875523421d


  parent reply	other threads:[~2025-06-25 12:51 UTC|newest]

Thread overview: 107+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-14 16:03 [PATCH 0/4] Make the "promisor-remote" capability support extra fields Christian Couder
2025-04-14 16:03 ` [PATCH 1/4] config: move is_config_key_char() to "config.h" Christian Couder
2025-04-14 16:03 ` [PATCH 2/4] promisor-remote: refactor to get rid of 'struct strvec' Christian Couder
2025-04-22 10:13   ` Patrick Steinhardt
2025-04-29 15:12     ` Christian Couder
2025-04-14 16:03 ` [PATCH 3/4] promisor-remote: allow a server to advertise extra fields Christian Couder
2025-04-14 22:04   ` Junio C Hamano
2025-04-22 10:13     ` Patrick Steinhardt
2025-04-29 15:12       ` Christian Couder
2025-04-29 15:12     ` Christian Couder
2025-04-14 16:03 ` [PATCH 4/4] promisor-remote: allow a client to check " Christian Couder
2025-04-29 14:52 ` [PATCH v2 0/3] Make the "promisor-remote" capability support more fields Christian Couder
2025-04-29 14:52   ` [PATCH v2 1/3] promisor-remote: refactor to get rid of 'struct strvec' Christian Couder
2025-05-07  8:25     ` Patrick Steinhardt
2025-05-19 14:10       ` Christian Couder
2025-05-07 12:27     ` Karthik Nayak
2025-05-19 14:10       ` Christian Couder
2025-04-29 14:52   ` [PATCH v2 2/3] promisor-remote: allow a server to advertise more fields Christian Couder
2025-05-07  8:25     ` Patrick Steinhardt
2025-05-19 14:11       ` Christian Couder
2025-05-27  7:50         ` Patrick Steinhardt
2025-05-27 15:30           ` Junio C Hamano
2025-06-11 13:46           ` Christian Couder
2025-05-07 12:44     ` Karthik Nayak
2025-05-19 14:11       ` Christian Couder
2025-04-29 14:52   ` [PATCH v2 3/3] promisor-remote: allow a client to check fields Christian Couder
2025-05-07  8:25     ` Patrick Steinhardt
2025-05-19 14:11       ` Christian Couder
2025-05-02  9:34   ` [PATCH v2 0/3] Make the "promisor-remote" capability support more fields Christian Couder
2025-05-19 14:12   ` [PATCH v3 0/5] " Christian Couder
2025-05-19 14:12     ` [PATCH v3 1/5] promisor-remote: refactor to get rid of 'struct strvec' Christian Couder
2025-05-20  9:37       ` Karthik Nayak
2025-05-20 13:32         ` Christian Couder
2025-05-20 16:45           ` Junio C Hamano
2025-05-21  6:33             ` Christian Couder
2025-05-21 15:00               ` Junio C Hamano
2025-06-11 13:47                 ` Christian Couder
2025-05-19 14:12     ` [PATCH v3 2/5] promisor-remote: allow a server to advertise more fields Christian Couder
2025-05-21 20:31       ` Justin Tobler
2025-06-11 13:46         ` Christian Couder
2025-05-27  7:51       ` Patrick Steinhardt
2025-06-11 13:46         ` Christian Couder
2025-05-19 14:12     ` [PATCH v3 3/5] promisor-remote: refactor how we parse advertised fields Christian Couder
2025-05-19 14:12     ` [PATCH v3 4/5] promisor-remote: allow a client to check fields Christian Couder
2025-05-19 14:12     ` [PATCH v3 5/5] promisor-remote: use string constants for 'name' and 'url' too Christian Couder
2025-06-11 13:45     ` [PATCH v4 0/5] Make the "promisor-remote" capability support more fields Christian Couder
2025-06-11 13:45       ` [PATCH v4 1/5] promisor-remote: refactor to get rid of 'struct strvec' Christian Couder
2025-06-19 11:53         ` Karthik Nayak
2025-06-25 12:53           ` Christian Couder
2025-06-23 19:38         ` Justin Tobler
2025-06-25 12:52           ` Christian Couder
2025-06-11 13:45       ` [PATCH v4 2/5] promisor-remote: allow a server to advertise more fields Christian Couder
2025-06-19 12:15         ` Karthik Nayak
2025-06-25 12:51           ` Christian Couder
2025-06-23 19:59         ` Justin Tobler
2025-06-25 12:51           ` Christian Couder
2025-06-11 13:45       ` [PATCH v4 3/5] promisor-remote: refactor how we parse advertised fields Christian Couder
2025-06-11 13:45       ` [PATCH v4 4/5] promisor-remote: allow a client to check fields Christian Couder
2025-06-11 13:45       ` [PATCH v4 5/5] promisor-remote: use string constants for 'name' and 'url' too Christian Couder
2025-06-19 12:18       ` [PATCH v4 0/5] Make the "promisor-remote" capability support more fields Karthik Nayak
2025-06-25 12:50       ` Christian Couder [this message]
2025-06-25 12:50         ` [PATCH v5 1/5] promisor-remote: refactor to get rid of 'struct strvec' Christian Couder
2025-06-25 17:05           ` Junio C Hamano
2025-07-21 14:08             ` Christian Couder
2025-06-25 12:50         ` [PATCH v5 2/5] promisor-remote: allow a server to advertise more fields Christian Couder
2025-06-25 22:29           ` Junio C Hamano
2025-07-21 14:09             ` Christian Couder
2025-07-21 18:53               ` Junio C Hamano
2025-07-31  7:20                 ` Christian Couder
2025-06-27 18:47           ` Jean-Noël Avila
2025-07-21 14:09             ` Christian Couder
2025-06-25 12:50         ` [PATCH v5 3/5] promisor-remote: refactor how we parse advertised fields Christian Couder
2025-06-25 12:50         ` [PATCH v5 4/5] promisor-remote: allow a client to check fields Christian Couder
2025-06-25 12:50         ` [PATCH v5 5/5] promisor-remote: use string constants for 'name' and 'url' too Christian Couder
2025-07-07 22:35         ` [PATCH v5 0/5] Make the "promisor-remote" capability support more fields Junio C Hamano
2025-07-08  3:34           ` Christian Couder
2025-07-21 14:10         ` [PATCH v6 " Christian Couder
2025-07-21 14:10           ` [PATCH v6 1/5] promisor-remote: refactor to get rid of 'struct strvec' Christian Couder
2025-07-21 14:10           ` [PATCH v6 2/5] promisor-remote: allow a server to advertise more fields Christian Couder
2025-07-21 14:10           ` [PATCH v6 3/5] promisor-remote: refactor how we parse advertised fields Christian Couder
2025-07-21 20:39             ` Junio C Hamano
2025-07-31  7:22               ` Christian Couder
2025-07-21 14:10           ` [PATCH v6 4/5] promisor-remote: allow a client to check fields Christian Couder
2025-07-21 20:59             ` Junio C Hamano
2025-07-31  7:21               ` Christian Couder
2025-07-21 14:10           ` [PATCH v6 5/5] promisor-remote: use string constants for 'name' and 'url' too Christian Couder
2025-07-21 20:18             ` Junio C Hamano
2025-07-31  7:23           ` [PATCH v7 0/5] Make the "promisor-remote" capability support more fields Christian Couder
2025-07-31  7:23             ` [PATCH v7 1/5] promisor-remote: refactor to get rid of 'struct strvec' Christian Couder
2025-07-31  7:23             ` [PATCH v7 2/5] promisor-remote: allow a server to advertise more fields Christian Couder
2025-07-31  7:23             ` [PATCH v7 3/5] promisor-remote: refactor how we parse advertised fields Christian Couder
2025-07-31 16:03               ` Junio C Hamano
2025-09-08  5:31                 ` Christian Couder
2025-07-31  7:23             ` [PATCH v7 4/5] promisor-remote: allow a client to check fields Christian Couder
2025-07-31  7:23             ` [PATCH v7 5/5] promisor-remote: use string constants for 'name' and 'url' too Christian Couder
2025-07-31 15:48             ` [PATCH v7 0/5] Make the "promisor-remote" capability support more fields Junio C Hamano
2025-08-28 23:32             ` Junio C Hamano
2025-09-08  5:36               ` Christian Couder
2025-09-08  5:30             ` [PATCH v8 0/7] " Christian Couder
2025-09-08  5:30               ` [PATCH v8 1/7] promisor-remote: refactor to get rid of 'struct strvec' Christian Couder
2025-09-08  5:30               ` [PATCH v8 2/7] promisor-remote: allow a server to advertise more fields Christian Couder
2025-09-08  5:30               ` [PATCH v8 3/7] promisor-remote: use string constants for 'name' and 'url' too Christian Couder
2025-09-08  5:30               ` [PATCH v8 4/7] promisor-remote: refactor how we parse advertised fields Christian Couder
2025-09-08  5:30               ` [PATCH v8 5/7] promisor-remote: use string_list_split() in filter_promisor_remote() Christian Couder
2025-09-08  5:30               ` [PATCH v8 6/7] promisor-remote: allow a client to check fields Christian Couder
2025-09-08  5:30               ` [PATCH v8 7/7] promisor-remote: use string_list_split() in mark_remotes_as_accepted() Christian Couder
2025-09-08 17:34               ` [PATCH v8 0/7] Make the "promisor-remote" capability support more fields Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250625125055.1375596-1-christian.couder@gmail.com \
    --to=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jltobler@gmail.com \
    --cc=karthik.188@gmail.com \
    --cc=me@ttaylorr.com \
    --cc=ps@pks.im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).