diff --git a/Documentation/config/promisor.adoc b/Documentation/config/promisor.adoc index 93e5e0d9b55eb4bb14a66e2ac4f06106b4a023cf..a2bcc3c86508ec7c090f65fef0b371e5522b676a 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/builtin/clone.c b/builtin/clone.c index b19b302b06546710b7c8bc20b696a0944c9f1b78..a34d2031d675aae52517c1bfc9d99df5806f76c8 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; @@ -999,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); @@ -1625,7 +1629,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); diff --git a/fetch-pack.c b/fetch-pack.c index 40316c9a348f23dc1545930eb4ca7ad0a447affd..254a8a42d1c637f0143fcd50ae0b61e668a12220 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); diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c index 7420bf81fe0103196f46e5850077f239a46c8e8d..3e41ab158772cd013febe6e44a18ee6fea66bd8e 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; } @@ -208,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. @@ -263,6 +320,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 +337,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 7b2108b98662c9937eec96dbe4d8bef97e291158..832d615c171e44a7286ddd412b28bfffd2c66d59 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. @@ -18,6 +19,7 @@ enum list_objects_filter_choice { LOFC_SPARSE_OID, LOFC_OBJECT_TYPE, LOFC_COMBINE, + LOFC_AUTO, LOFC__COUNT /* must be last */ }; @@ -50,6 +52,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. @@ -162,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 */ diff --git a/promisor-remote.c b/promisor-remote.c index 77ebf537e2b3ee3eb9904d3b904badf9c9e64e0e..a40f8cd27af8b71d1354b5c0a4b79d44f66d72d9 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); } @@ -375,18 +376,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 +401,15 @@ 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 initialize_fields_list(&fields_list, &initialized, "promisor.checkFields"); +} - return &fields_list; +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"); } /* @@ -692,6 +701,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 +835,10 @@ 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; + 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)) @@ -736,14 +874,43 @@ 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); + /* 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); } 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); + + /* 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) @@ -789,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 263d331a551b6ca80daa559d43fc61f7b081967e..98a0f05e03ba6d0f82f2993207aedde1e9a2fb83 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 */ diff --git a/t/t5710-promisor-remote-capability.sh b/t/t5710-promisor-remote-capability.sh index 023735d6a84ea8158896dafc132c43af6f4c38eb..6a8fc99f8f6b08041cb0a5af6535a85e0bfaa997 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 &&