From: "Jean-Noël Avila" <jn.avila@free•fr>
To: Christian Couder <christian.couder@gmail•com>, 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 <chriscool@tuxfamily•org>
Subject: Re: [PATCH v5 2/5] promisor-remote: allow a server to advertise more fields
Date: Fri, 27 Jun 2025 20:47:53 +0200 [thread overview]
Message-ID: <c49d73de-568a-4584-aa8f-9a9ffd68e4ce@free.fr> (raw)
In-Reply-To: <20250625125055.1375596-3-christian.couder@gmail.com>
Le 25/06/2025 à 14:50, Christian Couder a écrit :
> For now the "promisor-remote" protocol capability can only pass "name"
> and "url" information from a server to a client in the form
> "name=<remote_name>,url=<remote_url>".
>
> Let's make it possible to pass more information by introducing a new
> "promisor.sendFields" configuration variable. This variable should
> contain a comma or space separated list of field names that will be
> looked up in the configuration of the remote on the server to find the
> values that will be passed to the client.
>
> Only a set of predefined fields are allowed. The only fields in this
> set are "partialCloneFilter" and "token". The "partialCloneFilter"
> field specifies the filter definition used by the promisor remote,
> and the "token" field can provide an authentication credential for
> accessing it.
>
> For example, if "promisor.sendFields" is set to "partialCloneFilter",
> and the server has the "remote.<name>.partialCloneFilter" config
> variable set to a value for a remote, then that value will be passed
> in the form "partialCloneFilter=<value>" after the "name" and "url"
> fields.
>
> A following commit will allow the client to use the information to
> decide if it accepts the remote or not. For now the client doesn't do
> anything with the additional information it receives.
>
> Signed-off-by: Christian Couder <chriscool@tuxfamily•org>
> ---
> Documentation/config/promisor.adoc | 22 +++++
> Documentation/gitprotocol-v2.adoc | 59 +++++++++---
> promisor-remote.c | 134 ++++++++++++++++++++++++--
> t/t5710-promisor-remote-capability.sh | 31 ++++++
> 4 files changed, 221 insertions(+), 25 deletions(-)
>
> diff --git a/Documentation/config/promisor.adoc b/Documentation/config/promisor.adoc
> index 2638b01f83..beb8f65518 100644
> --- a/Documentation/config/promisor.adoc
> +++ b/Documentation/config/promisor.adoc
> @@ -9,6 +9,28 @@ promisor.advertise::
> "false", which means the "promisor-remote" capability is not
> advertised.
>
> +promisor.sendFields::
> + A comma or space separated list of additional remote related
> + field names. A server will send these field names and the
> + associated field values from its configuration when
> + advertising its promisor remotes using the "promisor-remote"
> + capability, see linkgit:gitprotocol-v2[5]. Currently, only the
> + "partialCloneFilter" and "token" field names are supported.
> ++
> +* "partialCloneFilter": contains the partial clone filter
> + used for the remote.
> ++
> +* "token": contains an authentication token for the remote.
> ++
This kind of text structure calls a description list instead and you can
already use backquotes:
`partialCloneFilter`:: contains the partial clone filter
> + used for the remote.
> ++
> +`token`:: contains an authentication token for the remote.
> +When a field name is part of this list and a corresponding
> +"remote.foo.<field name>" config variable is set on the server to a
Please no space in placeholders: <field-name>
> +non-empty value, then the field name and value will be sent when
> +advertising the promisor remote "foo".
> ++
> +This list has no effect unless the "promisor.advertise" config
> +variable is set to "true", and the "name" and "url" fields are always
> +advertised regardless of this setting.
> +
More generally, I am a bit annoyed by the usage of the "will" auxiliary
when not expressing a true future. For an international audience, this
can be misleading. The plain language[1] philosophy mandates to not use
auxiliaries other than where they are required (no convoluted sentences).
Would it make sense to start a style guide to help writing consistent
documentation that targets people whose first language is not English?
Being an non native speaker, I often find our docs too literate, with
lengthy sentences.
[1] https://en.wikipedia.org/wiki/Plain_language
> promisor.acceptFromServer::
> If set to "all", a client will accept all the promisor remotes
> a server might advertise using the "promisor-remote"
> diff --git a/Documentation/gitprotocol-v2.adoc b/Documentation/gitprotocol-v2.adoc
> index 9a57005d77..0583fafa09 100644
> --- a/Documentation/gitprotocol-v2.adoc
> +++ b/Documentation/gitprotocol-v2.adoc
> @@ -785,33 +785,59 @@ retrieving the header from a bundle at the indicated URI, and thus
> save themselves and the server(s) the request(s) needed to inspect the
> headers of that bundle or bundles.
>
> -promisor-remote=<pr-infos>
> +promisor-remote=<pr-info>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~
Be careful to adjust the length of the underline to the one of the title
>
> The server may advertise some promisor remotes it is using or knows
> about to a client which may want to use them as its promisor
remotes,> -instead of this repository. In this case <pr-infos> should be
of the
> +instead of this repository. In this case <pr-info> should be of the
> form:
>
> - pr-infos = pr-info | pr-infos ";" pr-info
> + pr-info = pr-fields | pr-info ";" pr-info
>
> - pr-info = "name=" pr-name | "name=" pr-name "," "url=" pr-url
> + pr-fields = field-name "=" field-value | pr-fields "," pr-fields
>
> -where `pr-name` is the urlencoded name of a promisor remote, and
> -`pr-url` the urlencoded URL of that promisor remote.
> +where all the `field-name` and `field-value` in a given `pr-fields`
> +are field names and values related to a single promisor remote.
>
> -In this case, if the client decides to use one or more promisor
> -remotes the server advertised, it can reply with
> -"promisor-remote=<pr-names>" where <pr-names> should be of the form:
> +The server MUST advertise at least the "name" and "url" field names
> +along with the associated field values, which are the name of a valid
> +remote and its URL, in each `pr-fields`. The "name" and "url" fields
> +MUST appear first in each pr-fields, in that order.
>
> - pr-names = pr-name | pr-names ";" pr-name
> +After these mandatory fields, the server MAY advertise the following
> +optional fields in any order:
> +
> +- "partialCloneFilter": The filter specification used by the remote.
> +Clients can use this to determine if the remote's filtering strategy
> +is compatible with their needs (e.g., checking if both use "blob:none").
> +It corresponds to the "remote.<name>.partialCloneFilter" config setting.
> +
> +- "token": An authentication token that clients can use when
> +connecting to the remote. It corresponds to the "remote.<name>.token"
> +config setting.
> +
This list can be turned into a description list.
> +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.
> +
> +For now, the client can only use information transmitted through these
> +fields to decide if it accepts the advertised promisor remote. In the
> +future that information might be used for other purposes though.
> +
> +Field values MUST be urlencoded.
> +
> +If the client decides to use one or more promisor remotes the server
> +advertised, it can reply with "promisor-remote=<pr-names>" where
> +<pr-names> should be of the form:
> +
> + pr-names = pr-name | pr-names ";" pr-names
Here the syntax used is not compatible with synopsis. Would it make
sense to uniformize it, or is BNF ok?
>
> where `pr-name` is the urlencoded name of a promisor remote the server
> advertised and the client accepts.
>
> -Note that, everywhere in this document, `pr-name` MUST be a valid
> -remote name, and the ';' and ',' characters MUST be encoded if they
> -appear in `pr-name` or `pr-url`.
> +Note that, everywhere in this document, the ';' and ',' characters
> +MUST be encoded if they appear in `pr-name` or `field-value`.
>
> If the server doesn't know any promisor remote that could be good for
> a client to use, or prefers a client not to use any promisor remote it
> @@ -822,9 +848,10 @@ In this case, or if the client doesn't want to use any promisor remote
> the server advertised, the client shouldn't advertise the
> "promisor-remote" capability at all in its reply.
>
> -The "promisor.advertise" and "promisor.acceptFromServer" configuration
> -options can be used on the server and client side to control what they
> -advertise or accept respectively. See the documentation of these
> +On the server side, the "promisor.advertise" and "promisor.sendFields"
> +configuration options can be used to control what it advertises. On
> +the client side, the "promisor.acceptFromServer" configuration option
> +can be used to control what it accepts. See the documentation of these
> configuration options for more information.
>
Thanks
Jean-Noël
next prev parent reply other threads:[~2025-06-27 18:53 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 ` [PATCH v5 " Christian Couder
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 [this message]
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=c49d73de-568a-4584-aa8f-9a9ffd68e4ce@free.fr \
--to=jn.avila@free$(echo .)fr \
--cc=chriscool@tuxfamily$(echo .)org \
--cc=christian.couder@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=jltobler@gmail$(echo .)com \
--cc=karthik.188@gmail$(echo .)com \
--cc=me@ttaylorr$(echo .)com \
--cc=ps@pks$(echo .)im \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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