Subject: Re: [PATCH 8/9] promisor-remote: keep advertised filter in memory

From: Patrick Steinhardt <[email-masked]>
Date: Wed, 7 Jan 2026 11:05:41 +0100

On Tue, Dec 23, 2025 at 12:11:12PM +0100, Christian Couder wrote:
> diff --git a/promisor-remote.c b/promisor-remote.c
> index 8d6d2d7b76..d5f3223cd0 100644
> --- a/promisor-remote.c
> +++ b/promisor-remote.c
> @@ -837,6 +838,7 @@ static void filter_promisor_remote(struct repository *repo,
>  	struct store_info *store_info = NULL;
>  	struct string_list_item *item;
>  	bool reload_config = false;
> +	struct string_list captured_filters = STRING_LIST_INIT_DUP;
>  
>  	if (!repo_config_get_string_tmp(the_repository, "promisor.acceptfromserver", &accept_str)) {
>  		if (!*accept_str || !strcasecmp("None", accept_str))

Nit: I found the "captured" terminology to be somewhat confusing. Can we
maybe rename this to `advertised_filters` to clarify?

> @@ -890,6 +899,25 @@ static void filter_promisor_remote(struct repository *repo,
>  
>  	if (reload_config)
>  		repo_promisor_remote_reinit(repo);
> +
> +	/* Apply captured filters to the stable repo state */
> +	for_each_string_list_item(item, &captured_filters) {
> +		struct promisor_remote *r = repo_promisor_remote_find(repo, item->string);
> +		if (r) {
> +			free(r->advertised_filter);
> +			r->advertised_filter = item->util;
> +			item->util = NULL;
> +		}
> +	}
> +
> +	string_list_clear(&captured_filters, 1);

Ah, I was wondering about memory lifetime first because we ask
`string_list_clear()` to free the `->util` pointers. But above we set
that pointer to `NULL` in case we retain it.

> @@ -935,3 +963,23 @@ void mark_promisor_remotes_as_accepted(struct repository *r, const char *remotes
>  
>  	string_list_clear(&accepted_remotes, 0);
>  }
> +
> +char *promisor_remote_construct_filter(struct repository *repo)
> +{
> +	struct string_list advertised_filters = STRING_LIST_INIT_NODUP;
> +	struct promisor_remote *r;
> +	char *result;
> +
> +	promisor_remote_init(repo);
> +
> +	for (r = repo->promisor_remote_config->promisors; r; r = r->next) {
> +		if (r->accepted && r->advertised_filter)
> +			string_list_append(&advertised_filters, r->advertised_filter);

Would we ever accept a promisor remote that _doesn't_ have an advertised
filter? If not, should we maybe `BUG()` in case the advertised filter
has not been set?

Patrick