On Wed, Jul 31, 2024 at 03:40:13PM +0200, Christian Couder wrote:
> diff --git a/Documentation/gitprotocol-v2.txt b/Documentation/gitprotocol-v2.txt
> index 414bc625d5..4d8d3839c4 100644
> --- a/Documentation/gitprotocol-v2.txt
> +++ b/Documentation/gitprotocol-v2.txt
> @@ -781,6 +781,43 @@ 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>
> +~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +The server may advertise some promisor remotes it is using, if it's OK
> +for the server that a client uses them too. In this case <pr-infos>
> +should be of the form:
> +
> + pr-infos = pr-info | pr-infos ";" pr-info
So <pr-infos> uses the ';' character to delimit multiple <pr-info>s,
which means that <pr-info> can't use ';' itself. You mention above that
<pr-info> is supposed to be generic so that we can add other fields to
it in the future. Do you imagine that any of those fields might want to
use the ';' in their values?
One that comes to mind is the shared token example you wrote about
above. It would be nice to not restrict what characters the token can
contain.
I wonder if it would instead be useful to have <pr-infos> first write
out how many <pr-info>s it contains, and then write out each <pr-info>
separated by a NUL byte, so that none of the files in the <pr-info>
itself are restricted in what characters they can use.
> +static void promisor_info_vecs(struct repository *repo,
> + struct strvec *names,
> + struct strvec *urls)
> +{
> + struct promisor_remote *r;
> +
> + promisor_remote_init(repo);
> +
> + for (r = repo->promisor_remote_config->promisors; r; r = r->next) {
> + char *url;
> + char *url_key = xstrfmt("remote.%s.url", r->name);
> +
> + strvec_push(names, r->name);
> + strvec_push(urls, git_config_get_string(url_key, &url) ? NULL : url);
Do you mean to push NULL onto urls here? It seems risky since you have
to check that each entry in the strvec is non-NULL before printing it
out (which you do below in promisor_remote_info()).
Or maybe you need to in order to advertise promisor remotes without
URLs? If so, I'm not sure what the benefit would be to the client if it
doesn't know where to go to retrieve any objects without having a URL.
Thanks,
Taylor
