[PATCH 0/9] Implement `promisor.storeFields` and `--filter=auto`

[PATCH 0/9] Implement `promisor.storeFields` and `--filter=auto`

[Christian Couder]

Introduction
============

A previous patch series added the possibility to pass additional
fields, a "partialCloneFilter" and a "token" for each advertised
promisor remote, from a server to a client through the
"promisor-remote" capability.

On the client side though, it has so far only been possible to use
this new information to compare it with local information and then
decide if the corresponding advertised promisor remote is accepted or
not.

For the "token" it would be useful if it could be stored on the
client. For example in a setup where the client uses specialized
remote helpers which need a token to access the promisor remotes
advertised by the server, storing the token would allow the token to
be used when the client directly accesses a promisor remote for
example to lazy fetch some blobs it now needs.

To enable such a workflow, where the server can rotate tokens and the
client can have updated tokens from the server by simply fetching from
it, the first part of this series introduces a new
"promisor.storeFields" configuration option on the client side,
similar to the "promisor.checkFields" configuration option. When field
names, "token" or "partialCloneFilter", are listed in this new
configuration option, then the values of these field names transmitted
by the server are stored in the local configuration on the client
side.

Note that for security reasons, the corresponding remote name and url
of the advertised promisor remotes must have already been configured
on the client side. No new remote name nor url are configured.

For the "partialCloneFilter" field, simply storing the value is not
enough to enable dynamic updates. Currently, when a user initiates a
partial clone with `--filter=<filter-spec>`, that specific
<filter-spec> is saved in the client's local configuration (e.g.,
remote.origin.partialCloneFilter). Subsequent fetches then reuse this
value, ignoring suggestions from the server.

To avoid breaking this mechanism and still be able to use the
<filter-spec> that the server suggests for the promisor remotes that
the client accepts, the second part of this series introduces a new
`--filter=auto` mode for `git clone` and `git fetch`.

When `--filter=auto` is used, then "auto" is still saved as the
<filter-spec> for the server locally on the client, and then when a
fetch-pack happens, instead of passing just "auto", the actual filter
requested by the client is computed by combining the <filter-spec>s
that the server suggested for the promisor remotes that the client
accepted. This uses the "combine" filter mechanism that already exists
in "list-objects-filter-options.{c,h}".

This way by just using `--filter=auto` when cloning, a client makes
sure it will use the <filter-spec>s suggested by the server for the
promisor remotes it accepts.

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.

Overview of the patches
=======================

Patches 1/9 and 2/9 are the first part of the series and implement the
new "promisor.storeFields" configuration option. Patch 1/9 is a small
preparatory refactoring.

Patches from 3/9 to 9/9 implement the `--filter=auto` option:

  - Patches 3/9 and 4/9 are cleanups of "builtin/clone.c" and
    "builtin/fetch.c" respectively that make the `filter_options`
    variable local to cmd_clone() or cmd_fetch().

  - Patch 5/9 is a doc update as `--filter=<filter-spec>` wasn't
    documented for `git fetch`.

  - Patches 6/9 and 7/9 improve "list-objects-filter-options.{c,h}" to
    support the new 'auto' mode.

  - Patch 8/9 improves "promisor-remote.{c,h}" to support the new
    'auto' mode.

  - Patch 9/9 make the new 'auto' mode actually work by wiring up
    everything together.

CI Report
=========

All the tests pass, see:

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

Christian Couder (9):
  promisor-remote: refactor initialising field lists
  promisor-remote: allow a client to store fields
  clone: make filter_options local to cmd_clone()
  fetch: make filter_options local to cmd_fetch()
  doc: fetch: document `--filter=<filter-spec>` option
  list-objects-filter-options: support 'auto' mode for --filter
  list-objects-filter-options: implement auto filter resolution
  promisor-remote: keep advertised filter in memory
  fetch-pack: wire up and enable auto filter logic

 Documentation/config/promisor.adoc           |  33 +++
 Documentation/fetch-options.adoc             |  19 ++
 Documentation/git-clone.adoc                 |  25 ++-
 Documentation/gitprotocol-v2.adoc            |  24 +-
 Makefile                                     |   1 +
 builtin/clone.c                              |  18 +-
 builtin/fetch.c                              |  50 +++--
 fetch-pack.c                                 |  20 ++
 list-objects-filter-options.c                |  71 +++++-
 list-objects-filter-options.h                |  25 +++
 list-objects-filter.c                        |   8 +
 promisor-remote.c                            | 222 +++++++++++++++++--
 promisor-remote.h                            |   6 +
 t/meson.build                                |   1 +
 t/t5710-promisor-remote-capability.sh        | 109 +++++++++
 t/unit-tests/u-list-objects-filter-options.c |  86 +++++++
 transport.c                                  |   1 +
 17 files changed, 663 insertions(+), 56 deletions(-)
 create mode 100644 t/unit-tests/u-list-objects-filter-options.c

-- 
2.52.0.319.gfcaffa7898

[PATCH 1/9] promisor-remote: refactor initialising field lists

[Christian Couder]

In "promisor-remote.c", the fields_sent() and fields_checked()
functions serve similar purposes and contain a small amount of
duplicated code.

As we are going to add a similar function in a following commit,
let's refactor this common code into a new initialize_fields_list()
function.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 promisor-remote.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/promisor-remote.c b/promisor-remote.c
index 77ebf537e2..5d8151cedb 100644
--- a/promisor-remote.c
+++ b/promisor-remote.c
@@ -375,18 +375,24 @@ static char *fields_from_config(struct string_list *fields_list, const char *con
 	return fields;
 }
 
+static struct string_list *initialize_fields_list(struct string_list *fields_list, int *initialized,
+						  const char *config_key)
+{
+	if (!*initialized) {
+		fields_list->cmp = strcasecmp;
+		fields_from_config(fields_list, config_key);
+		*initialized = 1;
+	}
+
+	return fields_list;
+}
+
 static struct string_list *fields_sent(void)
 {
 	static struct string_list fields_list = STRING_LIST_INIT_NODUP;
 	static int initialized;
 
-	if (!initialized) {
-		fields_list.cmp = strcasecmp;
-		fields_from_config(&fields_list, "promisor.sendFields");
-		initialized = 1;
-	}
-
-	return &fields_list;
+	return initialize_fields_list(&fields_list, &initialized, "promisor.sendFields");
 }
 
 static struct string_list *fields_checked(void)
@@ -394,13 +400,7 @@ static struct string_list *fields_checked(void)
 	static struct string_list fields_list = STRING_LIST_INIT_NODUP;
 	static int initialized;
 
-	if (!initialized) {
-		fields_list.cmp = strcasecmp;
-		fields_from_config(&fields_list, "promisor.checkFields");
-		initialized = 1;
-	}
-
-	return &fields_list;
+	return initialize_fields_list(&fields_list, &initialized, "promisor.checkFields");
 }
 
 /*
-- 
2.52.0.319.gfcaffa7898

[PATCH 2/9] promisor-remote: allow a client to store fields

[Christian Couder]

A previous commit allowed a server to pass additional fields through
the "promisor-remote" protocol capability after the "name" and "url"
fields, specifically the "partialCloneFilter" and "token" fields.

Another previous commit, c213820c51 (promisor-remote: allow a client
to check fields, 2025-09-08), has made it possible for a client to
decide if it accepts a promisor remote advertised by a server based
on these additional fields.

Often though, it would be interesting for the client to just store in
its configuration files these additional fields passed by the server,
so that it can use them when needed.

For example if a token is necessary to access a promisor remote, that
token could be updated frequently only on the server side and then
passed to all the clients through the "promisor-remote" capability,
avoiding the need to update it on all the clients manually.

Storing the token on the client side makes sure that the token is
available when the client needs to access the promisor remotes for a
lazy fetch.

In the same way, if it appears that it's better to use a different
filter to access a promisor remote, it could be helpful if the client
could automatically use it.

To allow this, let's introduce a new "promisor.storeFields"
configuration variable.

Like "promisor.checkFields" and "promisor.sendFields", it should
contain a comma or space separated list of field names. Only the
"partialCloneFilter" and "token" field names are supported for now.

When a server advertises a promisor remote, for example "foo", along
with for example "token=XXXXX" to a client, and on the client side
"promisor.storeFields" contains "token", then the client will store
XXXXX for the "remote.foo.token" variable in its configuration file
and reload its configuration so it can immediately use this new
configuration variable.

A message is emitted on stderr to warn users when the config is
changed.

Note that even if "promisor.acceptFromServer" is set to "all", a
promisor remote has to be already configured on the client side for
some of its config to be changed. In any case no new remote is
configured and no new URL is stored.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/config/promisor.adoc    |  33 ++++++
 Documentation/gitprotocol-v2.adoc     |  12 ++-
 promisor-remote.c                     | 148 +++++++++++++++++++++++++-
 t/t5710-promisor-remote-capability.sh |  49 +++++++++
 4 files changed, 236 insertions(+), 6 deletions(-)

diff --git a/Documentation/config/promisor.adoc b/Documentation/config/promisor.adoc
index 93e5e0d9b5..b0fa43b839 100644
--- a/Documentation/config/promisor.adoc
+++ b/Documentation/config/promisor.adoc
@@ -89,3 +89,36 @@ variable. The fields are checked only if the
 `promisor.acceptFromServer` config variable is not set to "None". If
 set to "None", this config variable has no effect. See
 linkgit:gitprotocol-v2[5].
+
+promisor.storeFields::
+	A comma or space separated list of additional remote related
+	field names. If a client accepts an advertised remote, the
+	client will store the values associated with these field names
+	taken from the remote advertisement into its configuration,
+	and then reload its remote configuration. Currently,
+	"partialCloneFilter" and "token" are the only supported field
+	names.
++
+For example if a server advertises "partialCloneFilter=blob:limit=20k"
+for remote "foo", and that remote is accepted, then "blob:limit=20k"
+will be stored for the "remote.foo.partialCloneFilter" configuration
+variable.
++
+If the new field value from an advertised remote is the same as the
+existing field value for that remote on the client side, then no
+change is made to the client configuration though.
++
+When a new value is stored, a message is printed to standard error to
+let users know about this.
++
+Note that for security reasons, if the remote is not already
+configured on the client side, nothing will be stored for that
+remote. In any case, no new remote will be created and no URL will be
+stored.
++
+Before storing a partial clone filter, it's parsed to check it's
+valid. If it's not, a warning is emitted and it's not stored.
++
+Before storing a token, a check is performed to ensure it contains no
+control character. If the check fails, a warning is emitted and it's
+not stored.
diff --git a/Documentation/gitprotocol-v2.adoc b/Documentation/gitprotocol-v2.adoc
index c7db103299..d93dd279ea 100644
--- a/Documentation/gitprotocol-v2.adoc
+++ b/Documentation/gitprotocol-v2.adoc
@@ -826,9 +826,10 @@ are case-sensitive and MUST be transmitted exactly as specified
 above. 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.
+The client can use information transmitted through these fields to
+decide if it accepts the advertised promisor remote. Also, the client
+can be configured to store the values of these fields (see
+"promisor.storeFields" in linkgit:git-config[1]).
 
 Field values MUST be urlencoded.
 
@@ -856,8 +857,9 @@ the server advertised, the client shouldn't advertise the
 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.
+can be used to control what it accepts, and the "promisor.storeFields"
+option, to control what it stores. See the documentation of these
+configuration options in linkgit:git-config[1] for more information.
 
 Note that in the future it would be nice if the "promisor-remote"
 protocol capability could be used by the server, when responding to
diff --git a/promisor-remote.c b/promisor-remote.c
index 5d8151cedb..8d6d2d7b76 100644
--- a/promisor-remote.c
+++ b/promisor-remote.c
@@ -403,6 +403,14 @@ static struct string_list *fields_checked(void)
 	return initialize_fields_list(&fields_list, &initialized, "promisor.checkFields");
 }
 
+static struct string_list *fields_stored(void)
+{
+	static struct string_list fields_list = STRING_LIST_INIT_NODUP;
+	static int initialized;
+
+	return initialize_fields_list(&fields_list, &initialized, "promisor.storeFields");
+}
+
 /*
  * Struct for promisor remotes involved in the "promisor-remote"
  * protocol capability.
@@ -692,6 +700,132 @@ static struct promisor_info *parse_one_advertised_remote(const char *remote_info
 	return info;
 }
 
+static bool store_one_field(struct repository *repo, const char *remote_name,
+			    const char *field_name, const char *field_key,
+			    const char *advertised, const char *current)
+{
+	if (advertised && (!current || strcmp(current, advertised))) {
+		char *key = xstrfmt("remote.%s.%s", remote_name, field_key);
+
+		fprintf(stderr, _("Storing new %s from server for remote '%s'.\n"
+				  "    '%s' -> '%s'\n"),
+			field_name, remote_name,
+			current ? current : "",
+			advertised);
+
+		repo_config_set_worktree_gently(repo, key, advertised);
+		free(key);
+
+		return true;
+	}
+
+	return false;
+}
+
+/* Check that a filter is valid by parsing it */
+static bool valid_filter(const char *filter, const char *remote_name)
+{
+	struct list_objects_filter_options filter_opts = LIST_OBJECTS_FILTER_INIT;
+	struct strbuf err = STRBUF_INIT;
+	int res = gently_parse_list_objects_filter(&filter_opts, filter, &err);
+
+	if (res)
+		warning(_("invalid filter '%s' for remote '%s' "
+			  "will not be stored: %s"),
+			filter, remote_name, err.buf);
+
+	list_objects_filter_release(&filter_opts);
+	strbuf_release(&err);
+
+	return !res;
+}
+
+/* Check that a token doesn't contain any control character */
+static bool valid_token(const char *token, const char *remote_name)
+{
+	const char *c = token;
+
+	for (; *c; c++)
+		if (iscntrl(*c)) {
+			warning(_("invalid token '%s' for remote '%s' "
+				  "will not be stored"),
+				token, remote_name);
+			return false;
+		}
+
+	return true;
+}
+
+struct store_info {
+	struct repository *repo;
+	struct string_list config_info;
+	bool store_filter;
+	bool store_token;
+};
+
+static struct store_info *new_store_info(struct repository *repo)
+{
+	struct string_list *fields_to_store = fields_stored();
+	struct store_info *s = xmalloc(sizeof(*s));
+
+	s->repo = repo;
+
+	string_list_init_nodup(&s->config_info);
+	promisor_config_info_list(repo, &s->config_info, fields_to_store);
+	string_list_sort(&s->config_info);
+
+	s->store_filter = !!string_list_lookup(fields_to_store, promisor_field_filter);
+	s->store_token = !!string_list_lookup(fields_to_store, promisor_field_token);
+
+	return s;
+}
+
+static void free_store_info(struct store_info *s)
+{
+	if (s) {
+		promisor_info_list_clear(&s->config_info);
+		free(s);
+	}
+}
+
+static bool promisor_store_advertised_fields(struct promisor_info *advertised,
+					     struct store_info *store_info)
+{
+	struct promisor_info *p;
+	struct string_list_item *item;
+	const char *remote_name = advertised->name;
+	bool reload_config = false;
+
+	if (!(store_info->store_filter || store_info->store_token))
+		return false;
+
+	/*
+	 * Get existing config info for the advertised promisor
+	 * remote. This ensures the remote is already configured on
+	 * the client side.
+	 */
+	item = string_list_lookup(&store_info->config_info, remote_name);
+
+	if (!item)
+		return false;
+
+	p = item->util;
+
+	if (store_info->store_filter && advertised->filter &&
+	    valid_filter(advertised->filter, remote_name))
+		reload_config |= store_one_field(store_info->repo, remote_name,
+						 "filter", promisor_field_filter,
+						 advertised->filter, p->filter);
+
+	if (store_info->store_token && advertised->token &&
+	    valid_token(advertised->token, remote_name))
+		reload_config |= store_one_field(store_info->repo, remote_name,
+						 "token", promisor_field_token,
+						 advertised->token, p->token);
+
+	return reload_config;
+}
+
 static void filter_promisor_remote(struct repository *repo,
 				   struct strvec *accepted,
 				   const char *info)
@@ -700,7 +834,9 @@ static void filter_promisor_remote(struct repository *repo,
 	enum accept_promisor accept = ACCEPT_NONE;
 	struct string_list config_info = STRING_LIST_INIT_NODUP;
 	struct string_list remote_info = STRING_LIST_INIT_DUP;
+	struct store_info *store_info = NULL;
 	struct string_list_item *item;
+	bool reload_config = false;
 
 	if (!repo_config_get_string_tmp(the_repository, "promisor.acceptfromserver", &accept_str)) {
 		if (!*accept_str || !strcasecmp("None", accept_str))
@@ -736,14 +872,24 @@ static void filter_promisor_remote(struct repository *repo,
 			string_list_sort(&config_info);
 		}
 
-		if (should_accept_remote(accept, advertised, &config_info))
+		if (should_accept_remote(accept, advertised, &config_info)) {
+			if (!store_info)
+				store_info = new_store_info(repo);
+			if (promisor_store_advertised_fields(advertised, store_info))
+				reload_config = true;
+
 			strvec_push(accepted, advertised->name);
+		}
 
 		promisor_info_free(advertised);
 	}
 
 	promisor_info_list_clear(&config_info);
 	string_list_clear(&remote_info, 0);
+	free_store_info(store_info);
+
+	if (reload_config)
+		repo_promisor_remote_reinit(repo);
 }
 
 char *promisor_remote_reply(const char *info)
diff --git a/t/t5710-promisor-remote-capability.sh b/t/t5710-promisor-remote-capability.sh
index 023735d6a8..a726af214a 100755
--- a/t/t5710-promisor-remote-capability.sh
+++ b/t/t5710-promisor-remote-capability.sh
@@ -360,6 +360,55 @@ test_expect_success "clone with promisor.checkFields" '
 	check_missing_objects server 1 "$oid"
 '
 
+test_expect_success "clone with promisor.storeFields=partialCloneFilter" '
+	git -C server config promisor.advertise true &&
+	test_when_finished "rm -rf client" &&
+
+	git -C server remote add otherLop "https://invalid.invalid"  &&
+	git -C server config remote.otherLop.token "fooBar" &&
+	git -C server config remote.otherLop.stuff "baz" &&
+	git -C server config remote.otherLop.partialCloneFilter "blob:limit=10k" &&
+	test_when_finished "git -C server remote remove otherLop" &&
+
+	git -C server config remote.lop.token "fooXXX" &&
+	git -C server config remote.lop.partialCloneFilter "blob:limit=8k" &&
+
+	test_config -C server promisor.sendFields "partialCloneFilter, token" &&
+	test_when_finished "rm trace" &&
+
+	# Clone from server to create a client
+	GIT_TRACE_PACKET="$(pwd)/trace" GIT_NO_LAZY_FETCH=0 git clone \
+		-c remote.lop.promisor=true \
+		-c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \
+		-c remote.lop.url="file://$(pwd)/lop" \
+		-c remote.lop.token="fooYYY" \
+		-c remote.lop.partialCloneFilter="blob:none" \
+		-c promisor.acceptfromserver=All \
+		-c promisor.storeFields=partialcloneFilter \
+		--no-local --filter="blob:limit=5k" server client 2>err &&
+
+	# Check that the filter from the server is stored
+	echo "blob:limit=8k" >expected &&
+	git -C client config remote.lop.partialCloneFilter >actual &&
+	test_cmp expected actual &&
+
+	# Check that user is notified when the filter is stored
+	test_grep "Storing new filter from server for remote '\''lop'\''" err &&
+	test_grep "'\''blob:none'\'' -> '\''blob:limit=8k'\''" err &&
+
+	# Check that the token from the server is NOT stored
+	echo "fooYYY" >expected &&
+	git -C client config remote.lop.token >actual &&
+	test_cmp expected actual &&
+	test_grep ! "Storing new token from server" err &&
+
+	# Check that the filter for an unknown remote is NOT stored
+	test_must_fail git -C client config remote.otherLop.partialCloneFilter >actual &&
+
+	# Check that the largest object is still missing on the server
+	check_missing_objects server 1 "$oid"
+'
+
 test_expect_success "clone with promisor.advertise set to 'true' but don't delete the client" '
 	git -C server config promisor.advertise true &&
 
-- 
2.52.0.319.gfcaffa7898

[PATCH 3/9] clone: make filter_options local to cmd_clone()

[Christian Couder]

The `struct list_objects_filter_options filter_options` variable used
in "builtin/clone.c" to store the parsed filters specified by
`--filter=<filterspec>` is currently a static variable global to the
file.

As we are going to use it more in a following commit, it could become
a bit less easy to understand how it's managed.

To avoid that, let's make it clear that it's owned by cmd_clone() by
moving its definition into that function and making it non-static.

The only additional change to make this work is to pass it as an
argument to checkout(). So it's a small quite cheap cleanup anyway.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/clone.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index b19b302b06..186e5498d4 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -77,7 +77,6 @@ static struct string_list option_required_reference = STRING_LIST_INIT_NODUP;
 static struct string_list option_optional_reference = STRING_LIST_INIT_NODUP;
 static int max_jobs = -1;
 static struct string_list option_recurse_submodules = STRING_LIST_INIT_NODUP;
-static struct list_objects_filter_options filter_options = LIST_OBJECTS_FILTER_INIT;
 static int config_filter_submodules = -1;    /* unspecified */
 static int option_remote_submodules;
 
@@ -634,7 +633,9 @@ static int git_sparse_checkout_init(const char *repo)
 	return result;
 }
 
-static int checkout(int submodule_progress, int filter_submodules,
+static int checkout(int submodule_progress,
+		    struct list_objects_filter_options *filter_options,
+		    int filter_submodules,
 		    enum ref_storage_format ref_storage_format)
 {
 	struct object_id oid;
@@ -723,9 +724,9 @@ static int checkout(int submodule_progress, int filter_submodules,
 			strvec_pushf(&cmd.args, "--ref-format=%s",
 				     ref_storage_format_to_name(ref_storage_format));
 
-		if (filter_submodules && filter_options.choice)
+		if (filter_submodules && filter_options->choice)
 			strvec_pushf(&cmd.args, "--filter=%s",
-				     expand_list_objects_filter_spec(&filter_options));
+				     expand_list_objects_filter_spec(filter_options));
 
 		if (option_single_branch >= 0)
 			strvec_push(&cmd.args, option_single_branch ?
@@ -903,6 +904,7 @@ int cmd_clone(int argc,
 	enum transport_family family = TRANSPORT_FAMILY_ALL;
 	struct string_list option_config = STRING_LIST_INIT_DUP;
 	int option_dissociate = 0;
+	struct list_objects_filter_options filter_options = LIST_OBJECTS_FILTER_INIT;
 	int option_filter_submodules = -1; /* unspecified */
 	struct string_list server_options = STRING_LIST_INIT_NODUP;
 	const char *bundle_uri = NULL;
@@ -1625,9 +1627,13 @@ int cmd_clone(int argc,
 		return 1;
 
 	junk_mode = JUNK_LEAVE_REPO;
-	err = checkout(submodule_progress, filter_submodules,
+	err = checkout(submodule_progress,
+		       &filter_options,
+		       filter_submodules,
 		       ref_storage_format);
 
+	list_objects_filter_release(&filter_options);
+
 	string_list_clear(&option_not, 0);
 	string_list_clear(&option_config, 0);
 	string_list_clear(&server_options, 0);
-- 
2.52.0.319.gfcaffa7898

[PATCH 4/9] fetch: make filter_options local to cmd_fetch()

[Christian Couder]

The `struct list_objects_filter_options filter_options` variable used
in "builtin/fetch.c" to store the parsed filters specified by
`--filter=<filterspec>` is currently a static variable global to the
file.

As we are going to use it more in a following commit, it could become a
bit less easy to understand how it's managed.

To avoid that, let's make it clear that it's owned by cmd_fetch() by
moving its definition into that function and making it non-static.

This requires passing a pointer to it through the prepare_transport(),
do_fetch(), backfill_tags(), fetch_one_setup_partial(), and fetch_one()
functions, but it's quite straightforward.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/fetch.c | 48 +++++++++++++++++++++++++++---------------------
 1 file changed, 27 insertions(+), 21 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 288d3772ea..b984173447 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -97,7 +97,6 @@ static struct strbuf default_rla = STRBUF_INIT;
 static struct transport *gtransport;
 static struct transport *gsecondary;
 static struct refspec refmap = REFSPEC_INIT_FETCH;
-static struct list_objects_filter_options filter_options = LIST_OBJECTS_FILTER_INIT;
 static struct string_list server_options = STRING_LIST_INIT_DUP;
 static struct string_list negotiation_tip = STRING_LIST_INIT_NODUP;
 
@@ -1449,7 +1448,8 @@ static void add_negotiation_tips(struct git_transport_options *smart_options)
 	smart_options->negotiation_tips = oids;
 }
 
-static struct transport *prepare_transport(struct remote *remote, int deepen)
+static struct transport *prepare_transport(struct remote *remote, int deepen,
+					   struct list_objects_filter_options *filter_options)
 {
 	struct transport *transport;
 
@@ -1473,9 +1473,9 @@ static struct transport *prepare_transport(struct remote *remote, int deepen)
 		set_option(transport, TRANS_OPT_UPDATE_SHALLOW, "yes");
 	if (refetch)
 		set_option(transport, TRANS_OPT_REFETCH, "yes");
-	if (filter_options.choice) {
+	if (filter_options->choice) {
 		const char *spec =
-			expand_list_objects_filter_spec(&filter_options);
+			expand_list_objects_filter_spec(filter_options);
 		set_option(transport, TRANS_OPT_LIST_OBJECTS_FILTER, spec);
 		set_option(transport, TRANS_OPT_FROM_PROMISOR, "1");
 	}
@@ -1493,7 +1493,8 @@ static int backfill_tags(struct display_state *display_state,
 			 struct ref_transaction *transaction,
 			 struct ref *ref_map,
 			 struct fetch_head *fetch_head,
-			 const struct fetch_config *config)
+			 const struct fetch_config *config,
+			 struct list_objects_filter_options *filter_options)
 {
 	int retcode, cannot_reuse;
 
@@ -1507,7 +1508,7 @@ static int backfill_tags(struct display_state *display_state,
 	cannot_reuse = transport->cannot_reuse ||
 		deepen_since || deepen_not.nr;
 	if (cannot_reuse) {
-		gsecondary = prepare_transport(transport->remote, 0);
+		gsecondary = prepare_transport(transport->remote, 0, filter_options);
 		transport = gsecondary;
 	}
 
@@ -1713,7 +1714,8 @@ static int commit_ref_transaction(struct ref_transaction **transaction,
 
 static int do_fetch(struct transport *transport,
 		    struct refspec *rs,
-		    const struct fetch_config *config)
+		    const struct fetch_config *config,
+		    struct list_objects_filter_options *filter_options)
 {
 	struct ref_transaction *transaction = NULL;
 	struct ref *ref_map = NULL;
@@ -1873,7 +1875,7 @@ static int do_fetch(struct transport *transport,
 			 * the transaction and don't commit anything.
 			 */
 			if (backfill_tags(&display_state, transport, transaction, tags_ref_map,
-					  &fetch_head, config))
+					  &fetch_head, config, filter_options))
 				retcode = 1;
 		}
 
@@ -2198,20 +2200,21 @@ static int fetch_multiple(struct string_list *list, int max_children,
  * Fetching from the promisor remote should use the given filter-spec
  * or inherit the default filter-spec from the config.
  */
-static inline void fetch_one_setup_partial(struct remote *remote)
+static inline void fetch_one_setup_partial(struct remote *remote,
+					   struct list_objects_filter_options *filter_options)
 {
 	/*
 	 * Explicit --no-filter argument overrides everything, regardless
 	 * of any prior partial clones and fetches.
 	 */
-	if (filter_options.no_filter)
+	if (filter_options->no_filter)
 		return;
 
 	/*
 	 * If no prior partial clone/fetch and the current fetch DID NOT
 	 * request a partial-fetch, do a normal fetch.
 	 */
-	if (!repo_has_promisor_remote(the_repository) && !filter_options.choice)
+	if (!repo_has_promisor_remote(the_repository) && !filter_options->choice)
 		return;
 
 	/*
@@ -2220,8 +2223,8 @@ static inline void fetch_one_setup_partial(struct remote *remote)
 	 * filter-spec as the default for subsequent fetches to this
 	 * remote if there is currently no default filter-spec.
 	 */
-	if (filter_options.choice) {
-		partial_clone_register(remote->name, &filter_options);
+	if (filter_options->choice) {
+		partial_clone_register(remote->name, filter_options);
 		return;
 	}
 
@@ -2230,14 +2233,15 @@ static inline void fetch_one_setup_partial(struct remote *remote)
 	 * explicitly given filter-spec or inherit the filter-spec from
 	 * the config.
 	 */
-	if (!filter_options.choice)
-		partial_clone_get_default_filter_spec(&filter_options, remote->name);
+	if (!filter_options->choice)
+		partial_clone_get_default_filter_spec(filter_options, remote->name);
 	return;
 }
 
 static int fetch_one(struct remote *remote, int argc, const char **argv,
 		     int prune_tags_ok, int use_stdin_refspecs,
-		     const struct fetch_config *config)
+		     const struct fetch_config *config,
+		     struct list_objects_filter_options *filter_options)
 {
 	struct refspec rs = REFSPEC_INIT_FETCH;
 	int i;
@@ -2249,7 +2253,7 @@ static int fetch_one(struct remote *remote, int argc, const char **argv,
 		die(_("no remote repository specified; please specify either a URL or a\n"
 		      "remote name from which new revisions should be fetched"));
 
-	gtransport = prepare_transport(remote, 1);
+	gtransport = prepare_transport(remote, 1, filter_options);
 
 	if (prune < 0) {
 		/* no command line request */
@@ -2304,7 +2308,7 @@ static int fetch_one(struct remote *remote, int argc, const char **argv,
 	sigchain_push_common(unlock_pack_on_signal);
 	atexit(unlock_pack_atexit);
 	sigchain_push(SIGPIPE, SIG_IGN);
-	exit_code = do_fetch(gtransport, &rs, config);
+	exit_code = do_fetch(gtransport, &rs, config, filter_options);
 	sigchain_pop(SIGPIPE);
 	refspec_clear(&rs);
 	transport_disconnect(gtransport);
@@ -2329,6 +2333,7 @@ int cmd_fetch(int argc,
 	const char *submodule_prefix = "";
 	const char *bundle_uri;
 	struct string_list list = STRING_LIST_INIT_DUP;
+	struct list_objects_filter_options filter_options = LIST_OBJECTS_FILTER_INIT;
 	struct remote *remote = NULL;
 	int all = -1, multiple = 0;
 	int result = 0;
@@ -2594,7 +2599,7 @@ int cmd_fetch(int argc,
 		trace2_region_enter("fetch", "negotiate-only", the_repository);
 		if (!remote)
 			die(_("must supply remote when using --negotiate-only"));
-		gtransport = prepare_transport(remote, 1);
+		gtransport = prepare_transport(remote, 1, &filter_options);
 		if (gtransport->smart_options) {
 			gtransport->smart_options->acked_commits = &acked_commits;
 		} else {
@@ -2616,12 +2621,12 @@ int cmd_fetch(int argc,
 	} else if (remote) {
 		if (filter_options.choice || repo_has_promisor_remote(the_repository)) {
 			trace2_region_enter("fetch", "setup-partial", the_repository);
-			fetch_one_setup_partial(remote);
+			fetch_one_setup_partial(remote, &filter_options);
 			trace2_region_leave("fetch", "setup-partial", the_repository);
 		}
 		trace2_region_enter("fetch", "fetch-one", the_repository);
 		result = fetch_one(remote, argc, argv, prune_tags_ok, stdin_refspecs,
-				   &config);
+				   &config, &filter_options);
 		trace2_region_leave("fetch", "fetch-one", the_repository);
 	} else {
 		int max_children = max_jobs;
@@ -2727,5 +2732,6 @@ int cmd_fetch(int argc,
 
  cleanup:
 	string_list_clear(&list, 0);
+	list_objects_filter_release(&filter_options);
 	return result;
 }
-- 
2.52.0.319.gfcaffa7898

[PATCH 5/9] doc: fetch: document `--filter=<filter-spec>` option

[Christian Couder]

The `--filter=<filter-spec>` option is documented in most commands that
support it except `git fetch`.

Let's fix that and document that option properly in the same way as it
is already documented for `git clone`.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/fetch-options.adoc | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/fetch-options.adoc b/Documentation/fetch-options.adoc
index fcba46ee9e..70a9818331 100644
--- a/Documentation/fetch-options.adoc
+++ b/Documentation/fetch-options.adoc
@@ -88,6 +88,16 @@ linkgit:git-config[1].
 This is incompatible with `--recurse-submodules=(yes|on-demand)` and takes
 precedence over the `fetch.output` config option.
 
+--filter=<filter-spec>::
+	Use the partial clone feature and request that the server sends
+	a subset of reachable objects according to a given object filter.
+	When using `--filter`, the supplied _<filter-spec>_ is used for
+	the partial fetch. For example, `--filter=blob:none` will filter
+	out all blobs (file contents) until needed by Git. Also,
+	`--filter=blob:limit=<size>` will filter out all blobs of size
+	at least _<size>_. For more details on filter specifications, see
+	the `--filter` option in linkgit:git-rev-list[1].
+
 ifndef::git-pull[]
 `--write-fetch-head`::
 `--no-write-fetch-head`::
-- 
2.52.0.319.gfcaffa7898

[PATCH 6/9] list-objects-filter-options: support 'auto' mode for --filter

[Christian Couder]

In a following commit, we are going to allow passing "auto" as a
<filterspec> to the `--filter=<filterspec>` option, but only for some
commands. Other commands that support the `--filter=<filterspec>`
option should still die() when 'auto' is passed.

Let's set up the "list-objects-filter-options.{c,h}" infrastructure to
support that:

- Add a new `unsigned int allow_auto_filter : 1;` flag to
  `struct list_objects_filter_options` which specifies if "auto" is
  accepted or not.
- Change gently_parse_list_objects_filter() to parse "auto" if it's
  accepted.
- Make sure we die() if "auto" is combined with another filter.
- Update list_objects_filter_release() to preserve the
  allow_auto_filter flag, as this function is often called (via
  opt_parse_list_objects_filter) to reset the struct before parsing a
  new value.

Let's also update `list-objects-filter.c` to recognize the new
`LOFC_AUTO` choice. Since "auto" must be resolved to a concrete filter
before filtering actually begins, initializing a filter with
`LOFC_AUTO` is invalid and will trigger a BUG().

Note that ideally combining "auto" with "auto" could be allowed, but in
practice, it's probably not worth the added code complexity. And if we
really want it, nothing prevents us to allow it in future work.

If we ever want to give a meaning to combining "auto" with a different
filter too, nothing prevents us to do that in future work either.

While at it, let's add a new "u-list-objects-filter-options.c" file for
`struct list_objects_filter_options` related unit tests. For now it
only tests gently_parse_list_objects_filter() though.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Makefile                                     |  1 +
 list-objects-filter-options.c                | 36 +++++++++++--
 list-objects-filter-options.h                |  6 +++
 list-objects-filter.c                        |  8 +++
 t/meson.build                                |  1 +
 t/unit-tests/u-list-objects-filter-options.c | 53 ++++++++++++++++++++
 6 files changed, 102 insertions(+), 3 deletions(-)
 create mode 100644 t/unit-tests/u-list-objects-filter-options.c

diff --git a/Makefile b/Makefile
index 89d8d73ec0..85b2ff09f4 100644
--- a/Makefile
+++ b/Makefile
@@ -1507,6 +1507,7 @@ CLAR_TEST_SUITES += u-dir
 CLAR_TEST_SUITES += u-example-decorate
 CLAR_TEST_SUITES += u-hash
 CLAR_TEST_SUITES += u-hashmap
+CLAR_TEST_SUITES += u-list-objects-filter-options
 CLAR_TEST_SUITES += u-mem-pool
 CLAR_TEST_SUITES += u-oid-array
 CLAR_TEST_SUITES += u-oidmap
diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c
index 7420bf81fe..f13ae5caeb 100644
--- a/list-objects-filter-options.c
+++ b/list-objects-filter-options.c
@@ -20,6 +20,8 @@ const char *list_object_filter_config_name(enum list_objects_filter_choice c)
 	case LOFC_DISABLED:
 		/* we have no name for "no filter at all" */
 		break;
+	case LOFC_AUTO:
+		return "auto";
 	case LOFC_BLOB_NONE:
 		return "blob:none";
 	case LOFC_BLOB_LIMIT:
@@ -52,7 +54,17 @@ int gently_parse_list_objects_filter(
 	if (filter_options->choice)
 		BUG("filter_options already populated");
 
-	if (!strcmp(arg, "blob:none")) {
+	if (!strcmp(arg, "auto")) {
+		if (!filter_options->allow_auto_filter) {
+			strbuf_addstr(
+				errbuf,
+				_("'auto' filter not supported by this command"));
+			return 1;
+		}
+		filter_options->choice = LOFC_AUTO;
+		return 0;
+
+	} else if (!strcmp(arg, "blob:none")) {
 		filter_options->choice = LOFC_BLOB_NONE;
 		return 0;
 
@@ -146,10 +158,20 @@ static int parse_combine_subfilter(
 
 	decoded = url_percent_decode(subspec->buf);
 
-	result = has_reserved_character(subspec, errbuf) ||
-		gently_parse_list_objects_filter(
+	result = has_reserved_character(subspec, errbuf);
+	if (result)
+		goto cleanup;
+
+	result = gently_parse_list_objects_filter(
 			&filter_options->sub[new_index], decoded, errbuf);
+	if (result)
+		goto cleanup;
+
+	result = (filter_options->sub[new_index].choice == LOFC_AUTO);
+	if (result)
+		strbuf_addstr(errbuf, _("an 'auto' filter cannot be combined"));
 
+cleanup:
 	free(decoded);
 	return result;
 }
@@ -263,6 +285,9 @@ void parse_list_objects_filter(
 	} else {
 		struct list_objects_filter_options *sub;
 
+		if (filter_options->choice == LOFC_AUTO)
+			die(_("an 'auto' filter is incompatible with any other filter"));
+
 		/*
 		 * Make filter_options an LOFC_COMBINE spec so we can trivially
 		 * add subspecs to it.
@@ -277,6 +302,9 @@ void parse_list_objects_filter(
 		if (gently_parse_list_objects_filter(sub, arg, &errbuf))
 			die("%s", errbuf.buf);
 
+		if (sub->choice == LOFC_AUTO)
+			die(_("an 'auto' filter is incompatible with any other filter"));
+
 		strbuf_addch(&filter_options->filter_spec, '+');
 		filter_spec_append_urlencode(filter_options, arg);
 	}
@@ -317,6 +345,7 @@ void list_objects_filter_release(
 	struct list_objects_filter_options *filter_options)
 {
 	size_t sub;
+	unsigned int allow_auto_filter = filter_options->allow_auto_filter;
 
 	if (!filter_options)
 		return;
@@ -326,6 +355,7 @@ void list_objects_filter_release(
 		list_objects_filter_release(&filter_options->sub[sub]);
 	free(filter_options->sub);
 	list_objects_filter_init(filter_options);
+	filter_options->allow_auto_filter = allow_auto_filter;
 }
 
 void partial_clone_register(
diff --git a/list-objects-filter-options.h b/list-objects-filter-options.h
index 7b2108b986..77d7bbc846 100644
--- a/list-objects-filter-options.h
+++ b/list-objects-filter-options.h
@@ -18,6 +18,7 @@ enum list_objects_filter_choice {
 	LOFC_SPARSE_OID,
 	LOFC_OBJECT_TYPE,
 	LOFC_COMBINE,
+	LOFC_AUTO,
 	LOFC__COUNT /* must be last */
 };
 
@@ -50,6 +51,11 @@ struct list_objects_filter_options {
 	 */
 	unsigned int no_filter : 1;
 
+	/*
+	 * Is LOFC_AUTO a valid option?
+	 */
+	unsigned int allow_auto_filter : 1;
+
 	/*
 	 * BEGIN choice-specific parsed values from within the filter-spec. Only
 	 * some values will be defined for any given choice.
diff --git a/list-objects-filter.c b/list-objects-filter.c
index acd65ebb73..78316e7f90 100644
--- a/list-objects-filter.c
+++ b/list-objects-filter.c
@@ -745,6 +745,13 @@ static void filter_combine__init(
 	filter->finalize_omits_fn = filter_combine__finalize_omits;
 }
 
+static void filter_auto__init(
+	struct list_objects_filter_options *filter_options UNUSED,
+	struct filter *filter UNUSED)
+{
+	BUG("LOFC_AUTO should have been resolved before initializing the filter");
+}
+
 typedef void (*filter_init_fn)(
 	struct list_objects_filter_options *filter_options,
 	struct filter *filter);
@@ -760,6 +767,7 @@ static filter_init_fn s_filters[] = {
 	filter_sparse_oid__init,
 	filter_object_type__init,
 	filter_combine__init,
+	filter_auto__init,
 };
 
 struct filter *list_objects_filter__init(
diff --git a/t/meson.build b/t/meson.build
index 459c52a489..0bd66cc6ce 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -4,6 +4,7 @@ clar_test_suites = [
   'unit-tests/u-example-decorate.c',
   'unit-tests/u-hash.c',
   'unit-tests/u-hashmap.c',
+  'unit-tests/u-list-objects-filter-options.c',
   'unit-tests/u-mem-pool.c',
   'unit-tests/u-oid-array.c',
   'unit-tests/u-oidmap.c',
diff --git a/t/unit-tests/u-list-objects-filter-options.c b/t/unit-tests/u-list-objects-filter-options.c
new file mode 100644
index 0000000000..f7d73701b5
--- /dev/null
+++ b/t/unit-tests/u-list-objects-filter-options.c
@@ -0,0 +1,53 @@
+#include "unit-test.h"
+#include "list-objects-filter-options.h"
+#include "strbuf.h"
+
+/* Helper to test gently_parse_list_objects_filter() */
+static void check_gentle_parse(const char *filter_spec,
+			       int expect_success,
+			       int allow_auto,
+			       enum list_objects_filter_choice expected_choice)
+{
+	struct list_objects_filter_options filter_options = LIST_OBJECTS_FILTER_INIT;
+	struct strbuf errbuf = STRBUF_INIT;
+	int ret;
+
+	filter_options.allow_auto_filter = allow_auto;
+
+	ret = gently_parse_list_objects_filter(&filter_options, filter_spec, &errbuf);
+
+	if (expect_success) {
+		cl_assert_equal_i(ret, 0);
+		cl_assert_equal_i(expected_choice, filter_options.choice);
+		cl_assert_equal_i(errbuf.len, 0);
+	} else {
+		cl_assert(ret != 0);
+		cl_assert(errbuf.len > 0);
+	}
+
+	strbuf_release(&errbuf);
+	list_objects_filter_release(&filter_options);
+}
+
+void test_list_objects_filter_options__regular_filters(void)
+{
+	check_gentle_parse("blob:none", 1, 0, LOFC_BLOB_NONE);
+	check_gentle_parse("blob:none", 1, 1, LOFC_BLOB_NONE);
+	check_gentle_parse("blob:limit=5k", 1, 0, LOFC_BLOB_LIMIT);
+	check_gentle_parse("blob:limit=5k", 1, 1, LOFC_BLOB_LIMIT);
+	check_gentle_parse("combine:blob:none+tree:0", 1, 0, LOFC_COMBINE);
+	check_gentle_parse("combine:blob:none+tree:0", 1, 1, LOFC_COMBINE);
+}
+
+void test_list_objects_filter_options__auto_allowed(void)
+{
+	check_gentle_parse("auto", 1, 1, LOFC_AUTO);
+	check_gentle_parse("auto", 0, 0, 0);
+}
+
+void test_list_objects_filter_options__combine_auto_fails(void)
+{
+	check_gentle_parse("combine:auto+blob:none", 0, 1, 0);
+	check_gentle_parse("combine:blob:none+auto", 0, 1, 0);
+	check_gentle_parse("combine:auto+auto", 0, 1, 0);
+}
-- 
2.52.0.319.gfcaffa7898

[PATCH 7/9] list-objects-filter-options: implement auto filter resolution

[Christian Couder]

In a following commit, we will need to aggregate filters from multiple
accepted promisor remotes into a single filter.

For that purpose, let's add a `list_objects_filter_combine()` helper
function that takes a list of filter specifications and combines them
into a single string. If multiple filters are provided, it constructs a
"combine:..." filter, ensuring that sub-filters are properly
URL-encoded using the existing `allow_unencoded` logic.

In a following commit, we will add a `--filter=auto` option that will
enable a client to use the filters suggested by the server for the
promisor remotes the client accepted.

To simplify the filter processing related to this new feature, let's
also add a small `list_objects_filter_resolve_auto()` function.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 list-objects-filter-options.c                | 35 ++++++++++++++++++++
 list-objects-filter-options.h                | 19 +++++++++++
 t/unit-tests/u-list-objects-filter-options.c | 33 ++++++++++++++++++
 3 files changed, 87 insertions(+)

diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c
index f13ae5caeb..4a9c1991c1 100644
--- a/list-objects-filter-options.c
+++ b/list-objects-filter-options.c
@@ -230,6 +230,41 @@ static void filter_spec_append_urlencode(
 		     filter->filter_spec.buf + orig_len);
 }
 
+char *list_objects_filter_combine(const struct string_list *specs)
+{
+	struct strbuf buf = STRBUF_INIT;
+
+	if (!specs->nr)
+		return NULL;
+
+	if (specs->nr == 1)
+		return xstrdup(specs->items[0].string);
+
+	strbuf_addstr(&buf, "combine:");
+
+	for (size_t i = 0; i < specs->nr; i++) {
+		const char *spec = specs->items[i].string;
+		if (i > 0)
+			strbuf_addch(&buf, '+');
+
+		strbuf_addstr_urlencode(&buf, spec, allow_unencoded);
+	}
+
+	return strbuf_detach(&buf, NULL);
+}
+
+void list_objects_filter_resolve_auto(struct list_objects_filter_options *filter_options,
+	char *new_filter, struct strbuf *errbuf)
+{
+	if (filter_options->choice != LOFC_AUTO)
+		return;
+
+	list_objects_filter_release(filter_options);
+
+	if (new_filter)
+		gently_parse_list_objects_filter(filter_options, new_filter, errbuf);
+}
+
 /*
  * Changes filter_options into an equivalent LOFC_COMBINE filter options
  * instance. Does not do anything if filter_options is already LOFC_COMBINE.
diff --git a/list-objects-filter-options.h b/list-objects-filter-options.h
index 77d7bbc846..832d615c17 100644
--- a/list-objects-filter-options.h
+++ b/list-objects-filter-options.h
@@ -6,6 +6,7 @@
 #include "strbuf.h"
 
 struct option;
+struct string_list;
 
 /*
  * The list of defined filters for list-objects.
@@ -168,4 +169,22 @@ void list_objects_filter_copy(
 	struct list_objects_filter_options *dest,
 	const struct list_objects_filter_options *src);
 
+/*
+ * Combine the filter specs in 'specs' into a combined filter string
+ * like "combine:<spec1>+<spec2>", where <spec1>, <spec2>, etc are
+ * properly urlencoded. If 'specs' contains no element, NULL is
+ * returned. If 'specs' contains a single element, a copy of that
+ * element is returned.
+ */
+char *list_objects_filter_combine(const struct string_list *specs);
+
+/*
+ * Check if 'filter_options' are an 'auto' filter, and if that's the
+ * case populate it with the filter specified by 'new_filter'.
+ */
+void list_objects_filter_resolve_auto(
+	struct list_objects_filter_options *filter_options,
+	char *new_filter,
+	struct strbuf *errbuf);
+
 #endif /* LIST_OBJECTS_FILTER_OPTIONS_H */
diff --git a/t/unit-tests/u-list-objects-filter-options.c b/t/unit-tests/u-list-objects-filter-options.c
index f7d73701b5..84a012af3c 100644
--- a/t/unit-tests/u-list-objects-filter-options.c
+++ b/t/unit-tests/u-list-objects-filter-options.c
@@ -1,6 +1,7 @@
 #include "unit-test.h"
 #include "list-objects-filter-options.h"
 #include "strbuf.h"
+#include "string-list.h"
 
 /* Helper to test gently_parse_list_objects_filter() */
 static void check_gentle_parse(const char *filter_spec,
@@ -51,3 +52,35 @@ void test_list_objects_filter_options__combine_auto_fails(void)
 	check_gentle_parse("combine:blob:none+auto", 0, 1, 0);
 	check_gentle_parse("combine:auto+auto", 0, 1, 0);
 }
+
+/* Helper to test list_objects_filter_combine() */
+static void check_combine(const char **specs, size_t nr, const char *expected)
+{
+	struct string_list spec_list = STRING_LIST_INIT_NODUP;
+	char *actual;
+
+	for (size_t i = 0; i < nr; i++)
+		string_list_append(&spec_list, specs[i]);
+
+	actual = list_objects_filter_combine(&spec_list);
+
+	cl_assert_equal_s(actual, expected);
+
+	free(actual);
+	string_list_clear(&spec_list, 0);
+}
+
+void test_list_objects_filter_options__combine_helper(void)
+{
+	const char *empty[] = { NULL };
+	const char *one[] = { "blob:none" };
+	const char *two[] = { "blob:none", "tree:0" };
+	const char *complex[] = { "blob:limit=1k", "object:type=tag" };
+	const char *needs_encoding[] = { "blob:none", "combine:tree:0+blob:limit=1k" };
+
+	check_combine(empty, 0, NULL);
+	check_combine(one, 1, "blob:none");
+	check_combine(two, 2, "combine:blob:none+tree:0");
+	check_combine(complex, 2, "combine:blob:limit=1k+object:type=tag");
+	check_combine(needs_encoding, 2, "combine:blob:none+combine:tree:0%2bblob:limit=1k");
+}
-- 
2.52.0.319.gfcaffa7898

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

[Christian Couder]

Currently, advertised filters are only kept in memory temporarily
during parsing, or persisted to disk if `promisor.storeFields`
contains 'partialCloneFilter'.

In a following commit though, we will add a `--filter=auto` option.
This option will enable the client to use the filters that the server
is suggesting for the promisor remotes the client accepts.

To use them even if `promisor.storeFields` is not configured, these
filters should be stored somewhere for the current session.

Let's add an `advertised_filter` field to `struct promisor_remote`
for that purpose.

To ensure that the filters are available in all cases,
filter_promisor_remote() captures them into a temporary list and
applies them to the `promisor_remote` structs after the potential
configuration reload.

Then the accepted remotes are marked as `accepted` in the repository
state. This ensures that subsequent calls to look up accepted remotes
(like in the filter construction below) actually find them.

In a following commit, we will add a `--filter=auto` option that will
enable a client to use the filters suggested by the server for the
promisor remotes the client accepted.

To enable the client to construct a filter spec based on these filters,
let's add a `promisor_remote_construct_filter(repo)` function.

This function:

- iterates over all accepted promisor remotes in the repository,
- collects the filters advertised for them (using `advertised_filter`
  which a previous commit added to `struct promisor_remote`), and
- generates a single filter spec for them (using the
  `list_objects_filter_combine()` function added by a previous commit).

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 promisor-remote.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++
 promisor-remote.h |  6 ++++++
 2 files changed, 54 insertions(+)

diff --git a/promisor-remote.c b/promisor-remote.c
index 8d6d2d7b76..d5f3223cd0 100644
--- a/promisor-remote.c
+++ b/promisor-remote.c
@@ -193,6 +193,7 @@ void promisor_remote_clear(struct promisor_remote_config *config)
 	while (config->promisors) {
 		struct promisor_remote *r = config->promisors;
 		free(r->partial_clone_filter);
+		free(r->advertised_filter);
 		config->promisors = config->promisors->next;
 		free(r);
 	}
@@ -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))
@@ -879,6 +881,13 @@ static void filter_promisor_remote(struct repository *repo,
 				reload_config = true;
 
 			strvec_push(accepted, advertised->name);
+
+			/* Capture advertised filters for accepted remotes */
+			if (advertised->filter) {
+				struct string_list_item *i;
+				i = string_list_append(&captured_filters, advertised->name);
+				i->util = xstrdup(advertised->filter);
+			}
 		}
 
 		promisor_info_free(advertised);
@@ -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);
+
+	/* Mark the remotes as accepted in the repository state */
+	for (size_t i = 0; i < accepted->nr; i++) {
+		struct promisor_remote *r = repo_promisor_remote_find(repo, accepted->v[i]);
+		if (r)
+			r->accepted = 1;
+	}
 }
 
 char *promisor_remote_reply(const char *info)
@@ -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);
+	}
+
+	result = list_objects_filter_combine(&advertised_filters);
+
+	string_list_clear(&advertised_filters, 0);
+
+	return result;
+}
diff --git a/promisor-remote.h b/promisor-remote.h
index 263d331a55..98a0f05e03 100644
--- a/promisor-remote.h
+++ b/promisor-remote.h
@@ -15,6 +15,7 @@ struct object_id;
 struct promisor_remote {
 	struct promisor_remote *next;
 	char *partial_clone_filter;
+	char *advertised_filter;
 	unsigned int accepted : 1;
 	const char name[FLEX_ARRAY];
 };
@@ -67,4 +68,9 @@ void mark_promisor_remotes_as_accepted(struct repository *repo, const char *remo
  */
 int repo_has_accepted_promisor_remote(struct repository *r);
 
+/*
+ * Use the filters from the accepted remotes to create a filter.
+ */
+char *promisor_remote_construct_filter(struct repository *repo);
+
 #endif /* PROMISOR_REMOTE_H */
-- 
2.52.0.319.gfcaffa7898

[PATCH 9/9] fetch-pack: wire up and enable auto filter logic

[Christian Couder]

Previous commits have set up an infrastructure for `--filter=auto` to
automatically prepare a partial clone filter based on what the server
advertised and the client accepted.

Using that infrastructure, let's now enable the `--filter=auto` option
in `git clone` and `git fetch` by setting `allow_auto_filter` to 1.

Note that these small changes mean that when `git clone --filter=auto`
or `git fetch --filter=auto` are used, "auto" is automatically saved
as the partial clone filter for the server on the client. Therefore
subsequent calls to `git fetch` on the client will automatically use
this "auto" mode even without `--filter=auto`.

Let's also set `allow_auto_filter` to 1 in `transport.c`, as the
transport layer must be able to accept the "auto" filter spec even if
the invoking command hasn't fully parsed it yet.

When an "auto" filter is requested, let's have the "fetch-pack.c" code
in `do_fetch_pack_v2()` compute a filter and send it to the server.

In `do_fetch_pack_v2()` the logic also needs to check for the
"promisor-remote" capability and call `promisor_remote_reply()` to
parse advertised remotes and populate the list of those accepted (and
their filters).

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/fetch-options.adoc      | 19 ++++++---
 Documentation/git-clone.adoc          | 25 ++++++++---
 Documentation/gitprotocol-v2.adoc     | 16 ++++---
 builtin/clone.c                       |  2 +
 builtin/fetch.c                       |  2 +
 fetch-pack.c                          | 20 +++++++++
 t/t5710-promisor-remote-capability.sh | 60 +++++++++++++++++++++++++++
 transport.c                           |  1 +
 8 files changed, 130 insertions(+), 15 deletions(-)

diff --git a/Documentation/fetch-options.adoc b/Documentation/fetch-options.adoc
index 70a9818331..f7432d4b29 100644
--- a/Documentation/fetch-options.adoc
+++ b/Documentation/fetch-options.adoc
@@ -92,11 +92,20 @@ precedence over the `fetch.output` config option.
 	Use the partial clone feature and request that the server sends
 	a subset of reachable objects according to a given object filter.
 	When using `--filter`, the supplied _<filter-spec>_ is used for
-	the partial fetch. For example, `--filter=blob:none` will filter
-	out all blobs (file contents) until needed by Git. Also,
-	`--filter=blob:limit=<size>` will filter out all blobs of size
-	at least _<size>_. For more details on filter specifications, see
-	the `--filter` option in linkgit:git-rev-list[1].
+	the partial fetch.
++
+If `--filter=auto` is used, the filter specification is determined
+automatically by combining the filter specifications advertised by
+the server for the promisor remotes that the client accepts (see
+linkgit:gitprotocol-v2[5] and the `promisor.acceptFromServer`
+configuration option in linkgit:git-config[1]).
++
+For details on all other available filter specifications, see the
+`--filter=<filter-spec>` option in linkgit:git-rev-list[1].
++
+For example, `--filter=blob:none` will filter out all blobs (file
+contents) until needed by Git. Also, `--filter=blob:limit=<size>` will
+filter out all blobs of size at least _<size>_.
 
 ifndef::git-pull[]
 `--write-fetch-head`::
diff --git a/Documentation/git-clone.adoc b/Documentation/git-clone.adoc
index 57cdfb7620..0db2d1e5f0 100644
--- a/Documentation/git-clone.adoc
+++ b/Documentation/git-clone.adoc
@@ -187,11 +187,26 @@ objects from the source repository into a pack in the cloned repository.
 	Use the partial clone feature and request that the server sends
 	a subset of reachable objects according to a given object filter.
 	When using `--filter`, the supplied _<filter-spec>_ is used for
-	the partial clone filter. For example, `--filter=blob:none` will
-	filter out all blobs (file contents) until needed by Git. Also,
-	`--filter=blob:limit=<size>` will filter out all blobs of size
-	at least _<size>_. For more details on filter specifications, see
-	the `--filter` option in linkgit:git-rev-list[1].
+	the partial clone filter.
++
+If `--filter=auto` is used the filter specification is determined
+automatically through the 'promisor-remote' protocol (see
+linkgit:gitprotocol-v2[5]) by combining the filter specifications
+advertised by the server for the promisor remotes that the client
+accepts (see the `promisor.acceptFromServer` configuration option in
+linkgit:git-config[1]). This allows the server to suggest the optimal
+filter for the available promisor remotes.
++
+As with other filter specifications, the "auto" value is persisted in
+the configuration. This ensures that future fetches will continue to
+adapt to the server's current recommendation.
++
+For details on all other available filter specifications, see the
+`--filter=<filter-spec>` option in linkgit:git-rev-list[1].
++
+For example, `--filter=blob:none` will filter out all blobs (file
+contents) until needed by Git. Also, `--filter=blob:limit=<size>` will
+filter out all blobs of size at least _<size>_.
 
 `--also-filter-submodules`::
 	Also apply the partial clone filter to any submodules in the repository.
diff --git a/Documentation/gitprotocol-v2.adoc b/Documentation/gitprotocol-v2.adoc
index d93dd279ea..f985cb4c47 100644
--- a/Documentation/gitprotocol-v2.adoc
+++ b/Documentation/gitprotocol-v2.adoc
@@ -812,10 +812,15 @@ MUST appear first in each pr-fields, in that order.
 After these mandatory fields, the server MAY advertise the following
 optional fields in any order:
 
-`partialCloneFilter`:: The filter specification used by the remote.
+`partialCloneFilter`:: The filter specification for the remote. It
+corresponds to the "remote.<name>.partialCloneFilter" config setting.
 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.
+is compatible with their needs (e.g., checking if both use
+"blob:none"). Additionally they can use this through the
+`--filter=auto` option in linkgit:git-clone[1]. With that option, the
+filter specification of the clone will be automatically computed by
+combining the filter specifications of the promisor remotes the client
+accepts.
 
 `token`:: An authentication token that clients can use when
 connecting to the remote. It corresponds to the "remote.<name>.token"
@@ -828,8 +833,9 @@ future protocol extensions.
 
 The client can use information transmitted through these fields to
 decide if it accepts the advertised promisor remote. Also, the client
-can be configured to store the values of these fields (see
-"promisor.storeFields" in linkgit:git-config[1]).
+can be configured to store the values of these fields or use them
+to automatically configure the repository (see "promisor.storeFields"
+in linkgit:git-config[1] and `--filter=auto` in linkgit:git-clone[1]).
 
 Field values MUST be urlencoded.
 
diff --git a/builtin/clone.c b/builtin/clone.c
index 186e5498d4..41bbaea72a 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1001,6 +1001,8 @@ int cmd_clone(int argc,
 		NULL
 	};
 
+	filter_options.allow_auto_filter = 1;
+
 	packet_trace_identity("clone");
 
 	repo_config(the_repository, git_clone_config, NULL);
diff --git a/builtin/fetch.c b/builtin/fetch.c
index b984173447..ddc30a0d30 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -2439,6 +2439,8 @@ int cmd_fetch(int argc,
 		OPT_END()
 	};
 
+	filter_options.allow_auto_filter = 1;
+
 	packet_trace_identity("fetch");
 
 	/* Record the command line for the reflog */
diff --git a/fetch-pack.c b/fetch-pack.c
index 40316c9a34..12ccea0dab 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -35,6 +35,7 @@
 #include "sigchain.h"
 #include "mergesort.h"
 #include "prio-queue.h"
+#include "promisor-remote.h"
 
 static int transfer_unpack_limit = -1;
 static int fetch_unpack_limit = -1;
@@ -1661,6 +1662,25 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
 	struct string_list packfile_uris = STRING_LIST_INIT_DUP;
 	int i;
 	struct strvec index_pack_args = STRVEC_INIT;
+	const char *promisor_remote_config;
+
+	if (server_feature_v2("promisor-remote", &promisor_remote_config)) {
+		char *remote_name = promisor_remote_reply(promisor_remote_config);
+		free(remote_name);
+	}
+
+	if (args->filter_options.choice == LOFC_AUTO) {
+		struct strbuf errbuf = STRBUF_INIT;
+		char *constructed_filter = promisor_remote_construct_filter(r);
+
+		list_objects_filter_resolve_auto(&args->filter_options,
+						 constructed_filter, &errbuf);
+		if (errbuf.len > 0)
+			die(_("couldn't resolve 'auto' filter: %s"), errbuf.buf);
+
+		free(constructed_filter);
+		strbuf_release(&errbuf);
+	}
 
 	negotiator = &negotiator_alloc;
 	if (args->refetch)
diff --git a/t/t5710-promisor-remote-capability.sh b/t/t5710-promisor-remote-capability.sh
index a726af214a..21543bce20 100755
--- a/t/t5710-promisor-remote-capability.sh
+++ b/t/t5710-promisor-remote-capability.sh
@@ -409,6 +409,66 @@ test_expect_success "clone with promisor.storeFields=partialCloneFilter" '
 	check_missing_objects server 1 "$oid"
 '
 
+test_expect_success "clone and fetch with --filter=auto" '
+	git -C server config promisor.advertise true &&
+	test_when_finished "rm -rf client trace" &&
+
+	git -C server config remote.lop.partialCloneFilter "blob:limit=9500" &&
+	test_config -C server promisor.sendFields "partialCloneFilter" &&
+
+	GIT_TRACE_PACKET="$(pwd)/trace" GIT_NO_LAZY_FETCH=0 git clone \
+		-c remote.lop.promisor=true \
+		-c remote.lop.url="file://$(pwd)/lop" \
+		-c promisor.acceptfromserver=All \
+		--no-local --filter=auto server client 2>err &&
+
+	test_grep "filter blob:limit=9500" trace &&
+	test_grep ! "filter auto" trace &&
+
+	# Verify "auto" is persisted in config
+	echo auto >expected &&
+	git -C client config remote.origin.partialCloneFilter >actual &&
+	test_cmp expected actual &&
+
+	# Check that the largest object is still missing on the server
+	check_missing_objects server 1 "$oid" &&
+
+	# Now change the filter on the server
+	git -C server config remote.lop.partialCloneFilter "blob:limit=5678" &&
+
+	# Get a new commit on the server to ensure "git fetch" actually runs fetch-pack
+	test_commit -C template new-commit &&
+	git -C template push --all "$(pwd)/server" &&
+
+	# Perform a fetch WITH --filter=auto
+	rm -rf trace &&
+	GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch --filter=auto &&
+
+	# Verify that the new filter was used
+	test_grep "filter blob:limit=5678" trace &&
+
+	# Check that the largest object is still missing on the server
+	check_missing_objects server 1 "$oid" &&
+
+	# Change the filter on the server again
+	git -C server config remote.lop.partialCloneFilter "blob:limit=5432" &&
+
+	# Get yet a new commit on the server to ensure fetch-pack runs
+	test_commit -C template yet-a-new-commit &&
+	git -C template push --all "$(pwd)/server" &&
+
+	# Perform a fetch WITHOUT --filter=auto
+	# Relies on "auto" being persisted in the client config
+	rm -rf trace &&
+	GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch &&
+
+	# Verify that the new filter was used
+	test_grep "filter blob:limit=5432" trace &&
+
+	# Check that the largest object is still missing on the server
+	check_missing_objects server 1 "$oid"
+'
+
 test_expect_success "clone with promisor.advertise set to 'true' but don't delete the client" '
 	git -C server config promisor.advertise true &&
 
diff --git a/transport.c b/transport.c
index c7f06a7382..cde8d83a57 100644
--- a/transport.c
+++ b/transport.c
@@ -1219,6 +1219,7 @@ struct transport *transport_get(struct remote *remote, const char *url)
 		 */
 		struct git_transport_data *data = xcalloc(1, sizeof(*data));
 		list_objects_filter_init(&data->options.filter_options);
+		data->options.filter_options.allow_auto_filter = 1;
 		ret->data = data;
 		ret->vtable = &builtin_smart_vtable;
 		ret->smart_options = &(data->options);
-- 
2.52.0.319.gfcaffa7898

Re: [PATCH 5/9] doc: fetch: document `--filter=<filter-spec>` option

[Jean-Noël AVILA]

On Tuesday, 23 December 2025 12:11:09 CET Christian Couder wrote:
> The `--filter=<filter-spec>` option is documented in most commands that
> support it except `git fetch`.
> 
> Let's fix that and document that option properly in the same way as it
> is already documented for `git clone`.
> 
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
>  Documentation/fetch-options.adoc | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/fetch-options.adoc b/Documentation/fetch-
options.adoc
> index fcba46ee9e..70a9818331 100644
> --- a/Documentation/fetch-options.adoc
> +++ b/Documentation/fetch-options.adoc
> @@ -88,6 +88,16 @@ linkgit:git-config[1].
>  This is incompatible with `--recurse-submodules=(yes|on-demand)` and takes
>  precedence over the `fetch.output` config option.
> 
> +--filter=<filter-spec>::

The option itself must also be back-ticked.

`--filter=<filter-spec>`::

> +	Use the partial clone feature and request that the server sends
> +	a subset of reachable objects according to a given object filter.
> +	When using `--filter`, the supplied _<filter-spec>_ is used for
> +	the partial fetch. For example, `--filter=blob:none` will filter


Isn't this second sentence redundant? What new information is brought?

> +	out all blobs (file contents) until needed by Git. Also,
> +	`--filter=blob:limit=<size>` will filter out all blobs of size
> +	at least _<size>_. For more details on filter specifications, see
> +	the `--filter` option in linkgit:git-rev-list[1].
> +
>  ifndef::git-pull[]
>  `--write-fetch-head`::
>  `--no-write-fetch-head`::

Thank you

Jean-Noël

Re: [PATCH 2/9] promisor-remote: allow a client to store fields

[Patrick Steinhardt]

On Tue, Dec 23, 2025 at 12:11:06PM +0100, Christian Couder wrote:
> A previous commit allowed a server to pass additional fields through
> the "promisor-remote" protocol capability after the "name" and "url"
> fields, specifically the "partialCloneFilter" and "token" fields.
> 
> Another previous commit, c213820c51 (promisor-remote: allow a client
> to check fields, 2025-09-08), has made it possible for a client to
> decide if it accepts a promisor remote advertised by a server based
> on these additional fields.
> 
> Often though, it would be interesting for the client to just store in
> its configuration files these additional fields passed by the server,
> so that it can use them when needed.
> 
> For example if a token is necessary to access a promisor remote, that
> token could be updated frequently only on the server side and then
> passed to all the clients through the "promisor-remote" capability,
> avoiding the need to update it on all the clients manually.
> 
> Storing the token on the client side makes sure that the token is
> available when the client needs to access the promisor remotes for a
> lazy fetch.

I guess another use case is that a client performs a fresh clone and
doesn't know anything about the remote's promisors yet, right? In that
case, the client may want to tell git-clone(1) to accept any of the
remote's advertised promisors, store it and then use that promisor's
filter to perform the actual clone.

> In the same way, if it appears that it's better to use a different
> filter to access a promisor remote, it could be helpful if the client
> could automatically use it.
> 
> To allow this, let's introduce a new "promisor.storeFields"
> configuration variable.
> 
> Like "promisor.checkFields" and "promisor.sendFields", it should
> contain a comma or space separated list of field names. Only the
> "partialCloneFilter" and "token" field names are supported for now.
> 
> When a server advertises a promisor remote, for example "foo", along
> with for example "token=XXXXX" to a client, and on the client side
> "promisor.storeFields" contains "token", then the client will store
> XXXXX for the "remote.foo.token" variable in its configuration file
> and reload its configuration so it can immediately use this new
> configuration variable.
> 
> A message is emitted on stderr to warn users when the config is
> changed.
> 
> Note that even if "promisor.acceptFromServer" is set to "all", a
> promisor remote has to be already configured on the client side for
> some of its config to be changed. In any case no new remote is
> configured and no new URL is stored.

Hm, okay, so that's not yet part of this series. I assume this is going
to be part of a subsequent patch series then?

> diff --git a/promisor-remote.c b/promisor-remote.c
> index 5d8151cedb..8d6d2d7b76 100644
> --- a/promisor-remote.c
> +++ b/promisor-remote.c
> @@ -403,6 +403,14 @@ static struct string_list *fields_checked(void)
>  	return initialize_fields_list(&fields_list, &initialized, "promisor.checkFields");
>  }
>  
> +static struct string_list *fields_stored(void)
> +{
> +	static struct string_list fields_list = STRING_LIST_INIT_NODUP;
> +	static int initialized;
> +
> +	return initialize_fields_list(&fields_list, &initialized, "promisor.storeFields");
> +}

I'm a bit worried about all the function-local state that we're
accumulating in those functions. Wouldn't it be preferable if we instead
had a `struct promisor_remote` that encapsulates the information?

> @@ -692,6 +700,132 @@ static struct promisor_info *parse_one_advertised_remote(const char *remote_info
>  	return info;
>  }
>  
> +static bool store_one_field(struct repository *repo, const char *remote_name,
> +			    const char *field_name, const char *field_key,
> +			    const char *advertised, const char *current)
> +{
> +	if (advertised && (!current || strcmp(current, advertised))) {
> +		char *key = xstrfmt("remote.%s.%s", remote_name, field_key);
> +
> +		fprintf(stderr, _("Storing new %s from server for remote '%s'.\n"
> +				  "    '%s' -> '%s'\n"),
> +			field_name, remote_name,
> +			current ? current : "",
> +			advertised);
> +
> +		repo_config_set_worktree_gently(repo, key, advertised);

Why do we store this information in the current per-worktree config? I'd
expect that this should be stored in the local config.

> +		free(key);
> +
> +		return true;
> +	}
> +
> +	return false;
> +}
> +
> +/* Check that a filter is valid by parsing it */
> +static bool valid_filter(const char *filter, const char *remote_name)
> +{
> +	struct list_objects_filter_options filter_opts = LIST_OBJECTS_FILTER_INIT;
> +	struct strbuf err = STRBUF_INIT;
> +	int res = gently_parse_list_objects_filter(&filter_opts, filter, &err);
> +
> +	if (res)
> +		warning(_("invalid filter '%s' for remote '%s' "
> +			  "will not be stored: %s"),
> +			filter, remote_name, err.buf);
> +
> +	list_objects_filter_release(&filter_opts);
> +	strbuf_release(&err);
> +
> +	return !res;
> +}
> +
> +/* Check that a token doesn't contain any control character */
> +static bool valid_token(const char *token, const char *remote_name)
> +{
> +	const char *c = token;
> +
> +	for (; *c; c++)
> +		if (iscntrl(*c)) {

Makes sense. I was also wondering about whether we want to check for
non-space whitespace characters, like newlines.

> +			warning(_("invalid token '%s' for remote '%s' "
> +				  "will not be stored"),
> +				token, remote_name);
> +			return false;
> +		}
> +
> +	return true;
> +}
> +
> +struct store_info {
> +	struct repository *repo;
> +	struct string_list config_info;
> +	bool store_filter;
> +	bool store_token;
> +};
> +
> +static struct store_info *new_store_info(struct repository *repo)

This should be called `store_info_new()` according to our coding
guidelines.

> +{
> +	struct string_list *fields_to_store = fields_stored();
> +	struct store_info *s = xmalloc(sizeof(*s));
> +
> +	s->repo = repo;
> +
> +	string_list_init_nodup(&s->config_info);
> +	promisor_config_info_list(repo, &s->config_info, fields_to_store);
> +	string_list_sort(&s->config_info);
> +
> +	s->store_filter = !!string_list_lookup(fields_to_store, promisor_field_filter);
> +	s->store_token = !!string_list_lookup(fields_to_store, promisor_field_token);
> +
> +	return s;
> +}
> +
> +static void free_store_info(struct store_info *s)

Likewise, this would be `store_info_free()`.

> diff --git a/t/t5710-promisor-remote-capability.sh b/t/t5710-promisor-remote-capability.sh
> index 023735d6a8..a726af214a 100755
> --- a/t/t5710-promisor-remote-capability.sh
> +++ b/t/t5710-promisor-remote-capability.sh
> @@ -360,6 +360,55 @@ test_expect_success "clone with promisor.checkFields" '
>  	check_missing_objects server 1 "$oid"
>  '
>  
> +test_expect_success "clone with promisor.storeFields=partialCloneFilter" '
> +	git -C server config promisor.advertise true &&
> +	test_when_finished "rm -rf client" &&
> +
> +	git -C server remote add otherLop "https://invalid.invalid"  &&
> +	git -C server config remote.otherLop.token "fooBar" &&
> +	git -C server config remote.otherLop.stuff "baz" &&
> +	git -C server config remote.otherLop.partialCloneFilter "blob:limit=10k" &&
> +	test_when_finished "git -C server remote remove otherLop" &&
> +
> +	git -C server config remote.lop.token "fooXXX" &&
> +	git -C server config remote.lop.partialCloneFilter "blob:limit=8k" &&
> +
> +	test_config -C server promisor.sendFields "partialCloneFilter, token" &&
> +	test_when_finished "rm trace" &&
> +
> +	# Clone from server to create a client
> +	GIT_TRACE_PACKET="$(pwd)/trace" GIT_NO_LAZY_FETCH=0 git clone \
> +		-c remote.lop.promisor=true \
> +		-c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \
> +		-c remote.lop.url="file://$(pwd)/lop" \
> +		-c remote.lop.token="fooYYY" \
> +		-c remote.lop.partialCloneFilter="blob:none" \
> +		-c promisor.acceptfromserver=All \
> +		-c promisor.storeFields=partialcloneFilter \
> +		--no-local --filter="blob:limit=5k" server client 2>err &&

Onet thing that's missing in these tests is to verify that a subsequent
git-fetch(1) updates the configuration.

Patrick

Re: [PATCH 4/9] fetch: make filter_options local to cmd_fetch()

[Patrick Steinhardt]

On Tue, Dec 23, 2025 at 12:11:08PM +0100, Christian Couder wrote:
> The `struct list_objects_filter_options filter_options` variable used
> in "builtin/fetch.c" to store the parsed filters specified by
> `--filter=<filterspec>` is currently a static variable global to the
> file.
> 
> As we are going to use it more in a following commit, it could become a
> bit less easy to understand how it's managed.
> 
> To avoid that, let's make it clear that it's owned by cmd_fetch() by
> moving its definition into that function and making it non-static.
> 
> This requires passing a pointer to it through the prepare_transport(),
> do_fetch(), backfill_tags(), fetch_one_setup_partial(), and fetch_one()
> functions, but it's quite straightforward.

Nice cleanups. I'm always happy to see less global state.

Patrick

Re: [PATCH 6/9] list-objects-filter-options: support 'auto' mode for --filter

[Patrick Steinhardt]

On Tue, Dec 23, 2025 at 12:11:10PM +0100, Christian Couder wrote:
> In a following commit, we are going to allow passing "auto" as a
> <filterspec> to the `--filter=<filterspec>` option, but only for some
> commands. Other commands that support the `--filter=<filterspec>`
> option should still die() when 'auto' is passed.

Okay. I assume the idea is that the user can eventually say `git clone
--filter=auto`, and Git would automatically pick the best filter
advertised by the remote. Sounds reasonable to me.

> Let's set up the "list-objects-filter-options.{c,h}" infrastructure to
> support that:
> 
> - Add a new `unsigned int allow_auto_filter : 1;` flag to
>   `struct list_objects_filter_options` which specifies if "auto" is
>   accepted or not.
> - Change gently_parse_list_objects_filter() to parse "auto" if it's
>   accepted.
> - Make sure we die() if "auto" is combined with another filter.
> - Update list_objects_filter_release() to preserve the
>   allow_auto_filter flag, as this function is often called (via
>   opt_parse_list_objects_filter) to reset the struct before parsing a
>   new value.
> 
> Let's also update `list-objects-filter.c` to recognize the new
> `LOFC_AUTO` choice. Since "auto" must be resolved to a concrete filter
> before filtering actually begins, initializing a filter with
> `LOFC_AUTO` is invalid and will trigger a BUG().
> 
> Note that ideally combining "auto" with "auto" could be allowed, but in
> practice, it's probably not worth the added code complexity. And if we
> really want it, nothing prevents us to allow it in future work.

I guess the question is what this would even mean, and I cannot think
of any benefit to allow `--filter=combine:auto+auto`. So agreed

> If we ever want to give a meaning to combining "auto" with a different
> filter too, nothing prevents us to do that in future work either.

So basically the case where the user knows that they definitely don't
want blobs, and in addition they want to pick the best filter advertised
by the server? Yeah, that sounds like it could eventually be a nice
addition.

> diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c
> index 7420bf81fe..f13ae5caeb 100644
> --- a/list-objects-filter-options.c
> +++ b/list-objects-filter-options.c
> @@ -52,7 +54,17 @@ int gently_parse_list_objects_filter(
>  	if (filter_options->choice)
>  		BUG("filter_options already populated");
>  
> -	if (!strcmp(arg, "blob:none")) {
> +	if (!strcmp(arg, "auto")) {
> +		if (!filter_options->allow_auto_filter) {
> +			strbuf_addstr(
> +				errbuf,
> +				_("'auto' filter not supported by this command"));

Tiny nit: the indentation looks a bit weird here.

> @@ -146,10 +158,20 @@ static int parse_combine_subfilter(
>  
>  	decoded = url_percent_decode(subspec->buf);
>  
> -	result = has_reserved_character(subspec, errbuf) ||
> -		gently_parse_list_objects_filter(
> +	result = has_reserved_character(subspec, errbuf);
> +	if (result)
> +		goto cleanup;
> +
> +	result = gently_parse_list_objects_filter(
>  			&filter_options->sub[new_index], decoded, errbuf);
> +	if (result)
> +		goto cleanup;
> +
> +	result = (filter_options->sub[new_index].choice == LOFC_AUTO);
> +	if (result)
> +		strbuf_addstr(errbuf, _("an 'auto' filter cannot be combined"));

Nit: let's maybe also add the `goto cleanup` here. I'm not a fan of
leaving it away for the final statement as it makes it easy to forget
backfilling it in case this function needs to be extended in the future.

> @@ -317,6 +345,7 @@ void list_objects_filter_release(
>  	struct list_objects_filter_options *filter_options)
>  {
>  	size_t sub;
> +	unsigned int allow_auto_filter = filter_options->allow_auto_filter;
>  
>  	if (!filter_options)
>  		return;
> @@ -326,6 +355,7 @@ void list_objects_filter_release(
>  		list_objects_filter_release(&filter_options->sub[sub]);
>  	free(filter_options->sub);
>  	list_objects_filter_init(filter_options);
> +	filter_options->allow_auto_filter = allow_auto_filter;
>  }

Why do we do this extra step to restore the `allow_auto_filter` option
here? Are there any callers that reuse the filter after it has been
released?

In any case, this function does have clearing semantics as it also knows
to re-init the filter options. So it's somewhat misnamed and really
should be called `list_objects_filter_clear()` according to our coding
guidelines. That's certainly outside the scope of this patch series
though.

Patrick

Re: [PATCH 7/9] list-objects-filter-options: implement auto filter resolution

[Patrick Steinhardt]

On Tue, Dec 23, 2025 at 12:11:11PM +0100, Christian Couder wrote:
> In a following commit, we will need to aggregate filters from multiple
> accepted promisor remotes into a single filter.

Ah, interesting. I was always operating under the assumption that when
the server advertises multiple promisors, the client will pick only one
of them. And that made me wonder how the client knows which one to pick
in the first place.

But of course it's possible to just pick _all_ of them by combining the
filter.

> diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c
> index f13ae5caeb..4a9c1991c1 100644
> --- a/list-objects-filter-options.c
> +++ b/list-objects-filter-options.c
> @@ -230,6 +230,41 @@ static void filter_spec_append_urlencode(
>  		     filter->filter_spec.buf + orig_len);
>  }
>  
> +char *list_objects_filter_combine(const struct string_list *specs)
> +{
> +	struct strbuf buf = STRBUF_INIT;
> +
> +	if (!specs->nr)
> +		return NULL;
> +
> +	if (specs->nr == 1)
> +		return xstrdup(specs->items[0].string);
> +
> +	strbuf_addstr(&buf, "combine:");
> +
> +	for (size_t i = 0; i < specs->nr; i++) {
> +		const char *spec = specs->items[i].string;
> +		if (i > 0)
> +			strbuf_addch(&buf, '+');
> +
> +		strbuf_addstr_urlencode(&buf, spec, allow_unencoded);

Shouldn't we use `filter_spec_append_urlencode()` to do this?

> +	}
> +
> +	return strbuf_detach(&buf, NULL);
> +}

I'm surprised we didn't have such a function yet.

> +void list_objects_filter_resolve_auto(struct list_objects_filter_options *filter_options,
> +	char *new_filter, struct strbuf *errbuf)
> +{
> +	if (filter_options->choice != LOFC_AUTO)
> +		return;

I wonder whether we should rather `BUG()` in case the filter is not an
"auto" filter. Otherwise it's easy to get the callsite wrong, as the
user may expect that the filter gets resolved tdo the new filter, but
it's actually not because the original filter wasn't an "auto" filter in
the first place.

> +	list_objects_filter_release(filter_options);
> +
> +	if (new_filter)
> +		gently_parse_list_objects_filter(filter_options, new_filter, errbuf);
> +}

So as menitoned in a preceding commit `list_objects_filter_release()`,
will retain the `allow_auto` option. But when resolving "auto" filters
I'd expect us to not accept "auto" in the resolved filter anymore.
Otherwise, if `new_filter` was "auto", we'd still end up with an auto
filter, wouldn't we? I'd rather expect us to abort in that case.

Patrick

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

[Patrick Steinhardt]

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

Re: [PATCH 9/9] fetch-pack: wire up and enable auto filter logic

[Patrick Steinhardt]

On Tue, Dec 23, 2025 at 12:11:13PM +0100, Christian Couder wrote:
> diff --git a/Documentation/fetch-options.adoc b/Documentation/fetch-options.adoc
> index 70a9818331..f7432d4b29 100644
> --- a/Documentation/fetch-options.adoc
> +++ b/Documentation/fetch-options.adoc
> @@ -92,11 +92,20 @@ precedence over the `fetch.output` config option.
>  	Use the partial clone feature and request that the server sends
>  	a subset of reachable objects according to a given object filter.
>  	When using `--filter`, the supplied _<filter-spec>_ is used for
> -	the partial fetch. For example, `--filter=blob:none` will filter
> -	out all blobs (file contents) until needed by Git. Also,
> -	`--filter=blob:limit=<size>` will filter out all blobs of size
> -	at least _<size>_. For more details on filter specifications, see
> -	the `--filter` option in linkgit:git-rev-list[1].
> +	the partial fetch.
> ++
> +If `--filter=auto` is used, the filter specification is determined
> +automatically by combining the filter specifications advertised by
> +the server for the promisor remotes that the client accepts (see
> +linkgit:gitprotocol-v2[5] and the `promisor.acceptFromServer`
> +configuration option in linkgit:git-config[1]).

Okay, so if "promisor.acceptFromServer" enables a subset of advertised
promisors we will automatically use their advertised filters. But what
about the case where we already have a set of local promisors with their
own filters, would those also honored by "--filter=auto"?

> diff --git a/builtin/clone.c b/builtin/clone.c
> index 186e5498d4..41bbaea72a 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -1001,6 +1001,8 @@ int cmd_clone(int argc,
>  		NULL
>  	};
>  
> +	filter_options.allow_auto_filter = 1;
> +
>  	packet_trace_identity("clone");
>  
>  	repo_config(the_repository, git_clone_config, NULL);
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index b984173447..ddc30a0d30 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -2439,6 +2439,8 @@ int cmd_fetch(int argc,
>  		OPT_END()
>  	};
>  
> +	filter_options.allow_auto_filter = 1;
> +
>  	packet_trace_identity("fetch");
>  
>  	/* Record the command line for the reflog */

Nice that both of these changes are so easy now.

> diff --git a/fetch-pack.c b/fetch-pack.c
> index 40316c9a34..12ccea0dab 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -1661,6 +1662,25 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
>  	struct string_list packfile_uris = STRING_LIST_INIT_DUP;
>  	int i;
>  	struct strvec index_pack_args = STRVEC_INIT;
> +	const char *promisor_remote_config;
> +
> +	if (server_feature_v2("promisor-remote", &promisor_remote_config)) {
> +		char *remote_name = promisor_remote_reply(promisor_remote_config);
> +		free(remote_name);
> +	}
> +
> +	if (args->filter_options.choice == LOFC_AUTO) {
> +		struct strbuf errbuf = STRBUF_INIT;
> +		char *constructed_filter = promisor_remote_construct_filter(r);
> +
> +		list_objects_filter_resolve_auto(&args->filter_options,
> +						 constructed_filter, &errbuf);
> +		if (errbuf.len > 0)
> +			die(_("couldn't resolve 'auto' filter: %s"), errbuf.buf);

Now that I see it being used I think that the calling convention of this
function is a bit weird. I would've expected the function to return an
error code that the caller can consult instead of having to check for
`errbuf.len`.

Patrick