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

On Wed, Jan 7, 2026 at 11:05 AM Patrick Steinhardt [off-list ref] wrote:

On Tue, Dec 23, 2025 at 12:11:13PM +0100, Christian Couder wrote:

quoted

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"?

No, they wouldn't be honored. 'auto' means that the client fully
accepts the filters advertised by the server. Maybe we could add a new
mode for using the locally configured filter by default and only using
the advertised filter if there is no locally configured filter for the
remote, but we can do that later.

quoted

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`.

Right, anyway I have removed that `list_objects_filter_resolve_auto()`
function altogether by removing the patch that introduced it in v2.

Thanks!