From b562f44d6f901030ee6835b7d1218493112e6188 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Thu, 16 Oct 2025 14:33:37 +0200 Subject: [PATCH 1/7] promisor-remote: refactor initialising field lists 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 --- 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"); } /* -- GitLab From 99f1a211dab0645bcd288e4c9e54bab3e8f0eb73 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Wed, 26 Nov 2025 09:30:37 +0100 Subject: [PATCH 2/7] clone: make filter_options local to cmd_clone() The `struct list_objects_filter_options filter_options` variable used in "builtin/clone.c" to store the parsed filters specified by `--filter=` 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 --- builtin/clone.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/builtin/clone.c b/builtin/clone.c index b19b302b06..946935d392 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,7 +1627,9 @@ 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); string_list_clear(&option_not, 0); -- GitLab From e04c2be040d8406dce13fbcae0df19e3225400e1 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 21 Oct 2025 08:10:35 +0200 Subject: [PATCH 3/7] promisor-remote: allow a client to store fields 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 --- Documentation/config/promisor.adoc | 31 ++++++ promisor-remote.c | 148 +++++++++++++++++++++++++- t/t5710-promisor-remote-capability.sh | 56 ++++++++++ 3 files changed, 234 insertions(+), 1 deletion(-) diff --git a/Documentation/config/promisor.adoc b/Documentation/config/promisor.adoc index 93e5e0d9b5..a2bcc3c865 100644 --- a/Documentation/config/promisor.adoc +++ b/Documentation/config/promisor.adoc @@ -89,3 +89,34 @@ 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. ++ +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/promisor-remote.c b/promisor-remote.c index 5d8151cedb..f05a87f202 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.storeAndUseFields"); +} + /* * 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..6a8fc99f8f 100755 --- a/t/t5710-promisor-remote-capability.sh +++ b/t/t5710-promisor-remote-capability.sh @@ -360,6 +360,62 @@ test_expect_success "clone with promisor.checkFields" ' check_missing_objects server 1 "$oid" ' +test_expect_success "clone with promisor.storeAndUseFields=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.storeAndUseFields=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 filter from the server is used + # TODO + + # 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 && + + + exit 1 && + + + # 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 && -- GitLab From b08723205e075b0f9f92a504b8bd744a8e0d5845 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Mon, 24 Nov 2025 18:11:29 +0100 Subject: [PATCH 4/7] list-objects-filter-options: add 'auto' mode for --filter In a following commit, we are going to allow passing "auto" as a to the `--filter=` option, but only for some commands. Other commands that support the `--filter=` 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. 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. Signed-off-by: Christian Couder --- list-objects-filter-options.c | 34 +++++++++++++++++++++++++++++++--- list-objects-filter-options.h | 6 ++++++ 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c index 7420bf81fe..afb1409bb8 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); } 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. -- GitLab From 5b4ba1b257b4406bf4a27ed58299aab08f274c19 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Thu, 4 Dec 2025 04:21:50 +0100 Subject: [PATCH 5/7] list-objects-filter-options: implement auto filter resolution 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. Let's add a `list_objects_filter_resolve_auto()` function to take care of all the filter processing which includes especially: - calling the `promisor_remote_construct_filter()` function introduced by a previous commit, - parsing the filter spec returned by that function, and - handling potential parsing errors. Signed-off-by: Christian Couder --- list-objects-filter-options.c | 35 +++++++++++++++++++++++++++++++++++ list-objects-filter-options.h | 19 +++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c index afb1409bb8..3e41ab1587 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:+", where , , 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 */ -- GitLab From f01bae8019733eb6feba64ff1cf7d52497576e2c Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Wed, 26 Nov 2025 17:13:05 +0100 Subject: [PATCH 6/7] promisor-remote: keep advertised filter in memory 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. 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 --- promisor-remote.c | 41 +++++++++++++++++++++++++++++++++++++++++ promisor-remote.h | 6 ++++++ 2 files changed, 47 insertions(+) diff --git a/promisor-remote.c b/promisor-remote.c index f05a87f202..a40f8cd27a 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,18 @@ 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); } char *promisor_remote_reply(const char *info) @@ -935,3 +956,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 */ -- GitLab From 56b8a530e91c636d92275c71933bdc62d204d9a7 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Wed, 10 Dec 2025 09:10:25 +0100 Subject: [PATCH 7/7] fetch-pack: wire up and enable auto filter logic 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 then, when that option is set, let's have the "fetch-pack.c" code compute a filter and send it to the server. Signed-off-by: Christian Couder --- builtin/clone.c | 2 ++ fetch-pack.c | 14 ++++++++++++++ 2 files changed, 16 insertions(+) diff --git a/builtin/clone.c b/builtin/clone.c index 946935d392..a34d2031d6 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/fetch-pack.c b/fetch-pack.c index 40316c9a34..254a8a42d1 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; @@ -1662,6 +1663,19 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args, int i; struct strvec index_pack_args = STRVEC_INIT; + 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) fetch_negotiator_init_noop(negotiator); -- GitLab