From: Justin Tobler <jltobler@gmail.com>
Date: Mon, 23 Jun 2025 19:59:17 GMT
Subject: Re: [PATCH v4 2/5] promisor-remote: allow a server to advertise more fields
Message-ID: <ojee3heqvvlk3v4ftuzaulm5vu7tvmc5idvqbda2ioiuurbmdw@sevh7jtsbbad>
In-Reply-To: <20250611134506.2975856-3-christian.couder@gmail.com>

On 25/06/11 03:45PM, Christian Couder wrote:
> 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>
> ---
[snip]
> +static char *fields_from_config(struct string_list *fields_list, const char *config_key)
> +{
> +	char *fields = NULL;
> +
> +	if (!git_config_get_string(config_key, &fields) && *fields) {
> +		string_list_split_in_place(fields_list, fields, ", ", -1);
> +		string_list_remove_empty_items(fields_list, 0);

Ok, in this version we now filter out empty entries from the
string_list. Previously if fields were specified with both a comma and
SP character (i.e. "partialCloneFilter, token"), an empty entry would be
parsed in the middle and lead to a warning message.

This change is good because it would be pretty natural for a user to
specify the config with both. It might be nice to leave a comment
explaining why we do this though as it may be confusing without context.

-Justin

> +		filter_string_list(fields_list, 0, is_valid_field, (void *)config_key);
> +	}
> +
> +	return fields;
> +}
> +