From 842b00516aebee06fc99c51a663b6587f642d36d Mon Sep 17 00:00:00 2001 From: Matthew DeVore Date: Thu, 27 Jun 2019 15:54:07 -0700 Subject: list-objects-filter-options: always supply *errbuf Making errbuf an optional argument complicates error reporting. Fix this by making all callers supply an errbuf, even if they may ignore it. This will be important in follow-up patches where the filter-spec parsing has more pitfalls and possible errors. Signed-off-by: Matthew DeVore Signed-off-by: Junio C Hamano --- list-objects-filter-options.c | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) (limited to 'list-objects-filter-options.c') diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c index 1cb20c659c..7c3e397d29 100644 --- a/list-objects-filter-options.c +++ b/list-objects-filter-options.c @@ -30,11 +30,8 @@ static int gently_parse_list_objects_filter( const char *v0; if (filter_options->choice) { - if (errbuf) { - strbuf_addstr( - errbuf, - _("multiple filter-specs cannot be combined")); - } + strbuf_addstr( + errbuf, _("multiple filter-specs cannot be combined")); return 1; } @@ -52,11 +49,7 @@ static int gently_parse_list_objects_filter( } else if (skip_prefix(arg, "tree:", &v0)) { if (!git_parse_ulong(v0, &filter_options->tree_exclude_depth)) { - if (errbuf) { - strbuf_addstr( - errbuf, - _("expected 'tree:'")); - } + strbuf_addstr(errbuf, _("expected 'tree:'")); return 1; } filter_options->choice = LOFC_TREE_DEPTH; @@ -90,8 +83,7 @@ static int gently_parse_list_objects_filter( * add new filters */ - if (errbuf) - strbuf_addf(errbuf, _("invalid filter-spec '%s'"), arg); + strbuf_addf(errbuf, _("invalid filter-spec '%s'"), arg); memset(filter_options, 0, sizeof(*filter_options)); return 1; @@ -175,6 +167,8 @@ void partial_clone_register( void partial_clone_get_default_filter_spec( struct list_objects_filter_options *filter_options) { + struct strbuf errbuf = STRBUF_INIT; + /* * Parse default value, but silently ignore it if it is invalid. */ @@ -182,5 +176,6 @@ void partial_clone_get_default_filter_spec( return; gently_parse_list_objects_filter(filter_options, core_partial_clone_filter_default, - NULL); + &errbuf); + strbuf_release(&errbuf); } -- cgit v1.2.3 From e987df5fe62b8b29be4cdcdeb3704681ada2b29e Mon Sep 17 00:00:00 2001 From: Matthew DeVore Date: Thu, 27 Jun 2019 15:54:08 -0700 Subject: list-objects-filter: implement composite filters Allow combining filters such that only objects accepted by all filters are shown. The motivation for this is to allow getting directory listings without also fetching blobs. This can be done by combining blob:none with tree:. There are massive repositories that have larger-than-expected trees - even if you include only a single commit. A combined filter supports any number of subfilters, and is written in the following form: combine:++ Certain non-alphanumeric characters in each filter must be URL-encoded. For now, combined filters must be specified in this form. In a subsequent commit, rev-list will support multiple --filter arguments which will have the same effect as specifying one filter argument starting with "combine:". The documentation will be updated in that commit, as the URL-encoding scheme is in general not meant to be used directly by the user, and it is better to describe the URL-encoding feature in terms of the repeated flag. Helped-by: Emily Shaffer Helped-by: Jeff Hostetler Helped-by: Johannes Schindelin Helped-by: Jonathan Tan Helped-by: Junio C Hamano Signed-off-by: Matthew DeVore Signed-off-by: Junio C Hamano --- list-objects-filter-options.c | 106 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 104 insertions(+), 2 deletions(-) (limited to 'list-objects-filter-options.c') diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c index 7c3e397d29..75d0236ee2 100644 --- a/list-objects-filter-options.c +++ b/list-objects-filter-options.c @@ -6,6 +6,12 @@ #include "list-objects.h" #include "list-objects-filter.h" #include "list-objects-filter-options.h" +#include "url.h" + +static int parse_combine_filter( + struct list_objects_filter_options *filter_options, + const char *arg, + struct strbuf *errbuf); /* * Parse value of the argument to the "filter" keyword. @@ -35,8 +41,6 @@ static int gently_parse_list_objects_filter( return 1; } - filter_options->filter_spec = strdup(arg); - if (!strcmp(arg, "blob:none")) { filter_options->choice = LOFC_BLOB_NONE; return 0; @@ -77,6 +81,10 @@ static int gently_parse_list_objects_filter( _("sparse:path filters support has been dropped")); } return 1; + + } else if (skip_prefix(arg, "combine:", &v0)) { + return parse_combine_filter(filter_options, v0, errbuf); + } /* * Please update _git_fetch() in git-completion.bash when you @@ -89,10 +97,95 @@ static int gently_parse_list_objects_filter( return 1; } +static const char *RESERVED_NON_WS = "~`!@#$^&*()[]{}\\;'\",<>?"; + +static int has_reserved_character( + struct strbuf *sub_spec, struct strbuf *errbuf) +{ + const char *c = sub_spec->buf; + while (*c) { + if (*c <= ' ' || strchr(RESERVED_NON_WS, *c)) { + strbuf_addf( + errbuf, + _("must escape char in sub-filter-spec: '%c'"), + *c); + return 1; + } + c++; + } + + return 0; +} + +static int parse_combine_subfilter( + struct list_objects_filter_options *filter_options, + struct strbuf *subspec, + struct strbuf *errbuf) +{ + size_t new_index = filter_options->sub_nr++; + char *decoded; + int result; + + ALLOC_GROW(filter_options->sub, filter_options->sub_nr, + filter_options->sub_alloc); + memset(&filter_options->sub[new_index], 0, + sizeof(*filter_options->sub)); + + decoded = url_percent_decode(subspec->buf); + + result = has_reserved_character(subspec, errbuf) || + gently_parse_list_objects_filter( + &filter_options->sub[new_index], decoded, errbuf); + + free(decoded); + return result; +} + +static int parse_combine_filter( + struct list_objects_filter_options *filter_options, + const char *arg, + struct strbuf *errbuf) +{ + struct strbuf **subspecs = strbuf_split_str(arg, '+', 0); + size_t sub; + int result = 0; + + if (!subspecs[0]) { + strbuf_addstr(errbuf, _("expected something after combine:")); + result = 1; + goto cleanup; + } + + for (sub = 0; subspecs[sub] && !result; sub++) { + if (subspecs[sub + 1]) { + /* + * This is not the last subspec. Remove trailing "+" so + * we can parse it. + */ + size_t last = subspecs[sub]->len - 1; + assert(subspecs[sub]->buf[last] == '+'); + strbuf_remove(subspecs[sub], last, 1); + } + result = parse_combine_subfilter( + filter_options, subspecs[sub], errbuf); + } + + filter_options->choice = LOFC_COMBINE; + +cleanup: + strbuf_list_free(subspecs); + if (result) { + list_objects_filter_release(filter_options); + memset(filter_options, 0, sizeof(*filter_options)); + } + return result; +} + int parse_list_objects_filter(struct list_objects_filter_options *filter_options, const char *arg) { struct strbuf buf = STRBUF_INIT; + filter_options->filter_spec = strdup(arg); if (gently_parse_list_objects_filter(filter_options, arg, &buf)) die("%s", buf.buf); return 0; @@ -129,8 +222,15 @@ void expand_list_objects_filter_spec( void list_objects_filter_release( struct list_objects_filter_options *filter_options) { + size_t sub; + + if (!filter_options) + return; free(filter_options->filter_spec); free(filter_options->sparse_oid_value); + for (sub = 0; sub < filter_options->sub_nr; sub++) + list_objects_filter_release(&filter_options->sub[sub]); + free(filter_options->sub); memset(filter_options, 0, sizeof(*filter_options)); } @@ -174,6 +274,8 @@ void partial_clone_get_default_filter_spec( */ if (!core_partial_clone_filter_default) return; + + filter_options->filter_spec = strdup(core_partial_clone_filter_default); gently_parse_list_objects_filter(filter_options, core_partial_clone_filter_default, &errbuf); -- cgit v1.2.3 From f56f764279be2433ecf3cb3a484210bcaffea70c Mon Sep 17 00:00:00 2001 From: Matthew DeVore Date: Thu, 27 Jun 2019 15:54:09 -0700 Subject: list-objects-filter-options: move error check up Move the check that filter_options->choice is set to higher in the call stack. This can only be set when the gentle parse function is called from one of the two call sites. This is important because in an upcoming patch this may or may not be an error, and whether it is an error is only known to the parse_list_objects_filter function. Signed-off-by: Matthew DeVore Signed-off-by: Junio C Hamano --- list-objects-filter-options.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'list-objects-filter-options.c') diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c index 75d0236ee2..5fe2814841 100644 --- a/list-objects-filter-options.c +++ b/list-objects-filter-options.c @@ -35,11 +35,8 @@ static int gently_parse_list_objects_filter( { const char *v0; - if (filter_options->choice) { - strbuf_addstr( - errbuf, _("multiple filter-specs cannot be combined")); - return 1; - } + if (filter_options->choice) + BUG("filter_options already populated"); if (!strcmp(arg, "blob:none")) { filter_options->choice = LOFC_BLOB_NONE; @@ -185,6 +182,8 @@ int parse_list_objects_filter(struct list_objects_filter_options *filter_options const char *arg) { struct strbuf buf = STRBUF_INIT; + if (filter_options->choice) + die(_("multiple filter-specs cannot be combined")); filter_options->filter_spec = strdup(arg); if (gently_parse_list_objects_filter(filter_options, arg, &buf)) die("%s", buf.buf); -- cgit v1.2.3 From cf9ceb5a12cad9c9153d227a0f497d1b522ce085 Mon Sep 17 00:00:00 2001 From: Matthew DeVore Date: Thu, 27 Jun 2019 15:54:10 -0700 Subject: list-objects-filter-options: make filter_spec a string_list Make the filter_spec string a string_list rather than a raw C string. The list of strings must be concatted together to make a complete filter_spec. A future patch will use this capability to build "combine:" filter specs gradually. A strbuf would seem to be a more natural choice for this object, but it unfortunately requires initialization besides just zero'ing out the memory. This results in all container structs, and all containers of those structs, etc., to also require initialization. Initializing them all would be more cumbersome that simply using a string_list, which behaves properly when its contents are zero'd. For the purposes of code simplification, change behavior in how filter specs are conveyed over the protocol: do not normalize the tree: filter specs since there should be no server in existence that supports tree:# but not tree:#k etc. Helped-by: Junio C Hamano Signed-off-by: Matthew DeVore Signed-off-by: Junio C Hamano --- list-objects-filter-options.c | 50 +++++++++++++++++++++++++++++-------------- 1 file changed, 34 insertions(+), 16 deletions(-) (limited to 'list-objects-filter-options.c') diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c index 5fe2814841..01c0f13346 100644 --- a/list-objects-filter-options.c +++ b/list-objects-filter-options.c @@ -184,7 +184,7 @@ int parse_list_objects_filter(struct list_objects_filter_options *filter_options struct strbuf buf = STRBUF_INIT; if (filter_options->choice) die(_("multiple filter-specs cannot be combined")); - filter_options->filter_spec = strdup(arg); + string_list_append(&filter_options->filter_spec, xstrdup(arg)); if (gently_parse_list_objects_filter(filter_options, arg, &buf)) die("%s", buf.buf); return 0; @@ -203,19 +203,36 @@ int opt_parse_list_objects_filter(const struct option *opt, return parse_list_objects_filter(filter_options, arg); } -void expand_list_objects_filter_spec( - const struct list_objects_filter_options *filter, - struct strbuf *expanded_spec) +const char *list_objects_filter_spec(struct list_objects_filter_options *filter) { - strbuf_init(expanded_spec, strlen(filter->filter_spec)); - if (filter->choice == LOFC_BLOB_LIMIT) - strbuf_addf(expanded_spec, "blob:limit=%lu", + if (!filter->filter_spec.nr) + BUG("no filter_spec available for this filter"); + if (filter->filter_spec.nr != 1) { + struct strbuf concatted = STRBUF_INIT; + strbuf_add_separated_string_list( + &concatted, "", &filter->filter_spec); + string_list_clear(&filter->filter_spec, /*free_util=*/0); + string_list_append( + &filter->filter_spec, strbuf_detach(&concatted, NULL)); + } + + return filter->filter_spec.items[0].string; +} + +const char *expand_list_objects_filter_spec( + struct list_objects_filter_options *filter) +{ + if (filter->choice == LOFC_BLOB_LIMIT) { + struct strbuf expanded_spec = STRBUF_INIT; + strbuf_addf(&expanded_spec, "blob:limit=%lu", filter->blob_limit_value); - else if (filter->choice == LOFC_TREE_DEPTH) - strbuf_addf(expanded_spec, "tree:%lu", - filter->tree_exclude_depth); - else - strbuf_addstr(expanded_spec, filter->filter_spec); + string_list_clear(&filter->filter_spec, /*free_util=*/0); + string_list_append( + &filter->filter_spec, + strbuf_detach(&expanded_spec, NULL)); + } + + return list_objects_filter_spec(filter); } void list_objects_filter_release( @@ -225,7 +242,7 @@ void list_objects_filter_release( if (!filter_options) return; - free(filter_options->filter_spec); + string_list_clear(&filter_options->filter_spec, /*free_util=*/0); free(filter_options->sparse_oid_value); for (sub = 0; sub < filter_options->sub_nr; sub++) list_objects_filter_release(&filter_options->sub[sub]); @@ -235,7 +252,7 @@ void list_objects_filter_release( void partial_clone_register( const char *remote, - const struct list_objects_filter_options *filter_options) + struct list_objects_filter_options *filter_options) { /* * Record the name of the partial clone remote in the @@ -258,7 +275,7 @@ void partial_clone_register( * the default for subsequent fetches from this remote. */ core_partial_clone_filter_default = - xstrdup(filter_options->filter_spec); + xstrdup(expand_list_objects_filter_spec(filter_options)); git_config_set("core.partialclonefilter", core_partial_clone_filter_default); } @@ -274,7 +291,8 @@ void partial_clone_get_default_filter_spec( if (!core_partial_clone_filter_default) return; - filter_options->filter_spec = strdup(core_partial_clone_filter_default); + string_list_append(&filter_options->filter_spec, + core_partial_clone_filter_default); gently_parse_list_objects_filter(filter_options, core_partial_clone_filter_default, &errbuf); -- cgit v1.2.3 From 489fc9ee718b7c8594f17b55f090ac5292c655e1 Mon Sep 17 00:00:00 2001 From: Matthew DeVore Date: Thu, 27 Jun 2019 15:54:12 -0700 Subject: list-objects-filter-options: allow mult. --filter Allow combining of multiple filters by simply repeating the --filter flag. Before this patch, the user had to combine them in a single flag somewhat awkwardly (e.g. --filter=combine:FOO+BAR), including URL-encoding the individual filters. To make this work, in the --filter flag parsing callback, rather than error out when we detect that the filter_options struct is already populated, we modify it in-place to contain the added sub-filter. The existing sub-filter becomes the lhs of the combined filter, and the next sub-filter becomes the rhs. We also have to URL-encode the LHS and RHS sub-filters. We can simplify the operation if the LHS is already a combine: filter. In that case, we just append the URL-encoded RHS sub-filter to the LHS spec to get the new spec. Helped-by: Emily Shaffer Helped-by: Jeff Hostetler Helped-by: Jeff King Helped-by: Junio C Hamano Signed-off-by: Matthew DeVore Signed-off-by: Junio C Hamano --- list-objects-filter-options.c | 88 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 83 insertions(+), 5 deletions(-) (limited to 'list-objects-filter-options.c') diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c index 01c0f13346..2506dc8327 100644 --- a/list-objects-filter-options.c +++ b/list-objects-filter-options.c @@ -6,6 +6,7 @@ #include "list-objects.h" #include "list-objects-filter.h" #include "list-objects-filter-options.h" +#include "trace.h" #include "url.h" static int parse_combine_filter( @@ -178,15 +179,92 @@ cleanup: return result; } -int parse_list_objects_filter(struct list_objects_filter_options *filter_options, - const char *arg) +static int allow_unencoded(char ch) +{ + if (ch <= ' ' || ch == '%' || ch == '+') + return 0; + return !strchr(RESERVED_NON_WS, ch); +} + +static void filter_spec_append_urlencode( + struct list_objects_filter_options *filter, const char *raw) { struct strbuf buf = STRBUF_INIT; + strbuf_addstr_urlencode(&buf, raw, allow_unencoded); + trace_printf("Add to combine filter-spec: %s\n", buf.buf); + string_list_append(&filter->filter_spec, strbuf_detach(&buf, NULL)); +} + +/* + * Changes filter_options into an equivalent LOFC_COMBINE filter options + * instance. Does not do anything if filter_options is already LOFC_COMBINE. + */ +static void transform_to_combine_type( + struct list_objects_filter_options *filter_options) +{ + assert(filter_options->choice); + if (filter_options->choice == LOFC_COMBINE) + return; + { + const int initial_sub_alloc = 2; + struct list_objects_filter_options *sub_array = + xcalloc(initial_sub_alloc, sizeof(*sub_array)); + sub_array[0] = *filter_options; + memset(filter_options, 0, sizeof(*filter_options)); + filter_options->sub = sub_array; + filter_options->sub_alloc = initial_sub_alloc; + } + filter_options->sub_nr = 1; + filter_options->choice = LOFC_COMBINE; + string_list_append(&filter_options->filter_spec, xstrdup("combine:")); + filter_spec_append_urlencode( + filter_options, + list_objects_filter_spec(&filter_options->sub[0])); + /* + * We don't need the filter_spec strings for subfilter specs, only the + * top level. + */ + string_list_clear(&filter_options->sub[0].filter_spec, /*free_util=*/0); +} + +void list_objects_filter_die_if_populated( + struct list_objects_filter_options *filter_options) +{ if (filter_options->choice) die(_("multiple filter-specs cannot be combined")); - string_list_append(&filter_options->filter_spec, xstrdup(arg)); - if (gently_parse_list_objects_filter(filter_options, arg, &buf)) - die("%s", buf.buf); +} + +int parse_list_objects_filter( + struct list_objects_filter_options *filter_options, + const char *arg) +{ + struct strbuf errbuf = STRBUF_INIT; + int parse_error; + + if (!filter_options->choice) { + string_list_append(&filter_options->filter_spec, xstrdup(arg)); + + parse_error = gently_parse_list_objects_filter( + filter_options, arg, &errbuf); + } else { + /* + * Make filter_options an LOFC_COMBINE spec so we can trivially + * add subspecs to it. + */ + transform_to_combine_type(filter_options); + + string_list_append(&filter_options->filter_spec, xstrdup("+")); + filter_spec_append_urlencode(filter_options, arg); + ALLOC_GROW(filter_options->sub, filter_options->sub_nr + 1, + filter_options->sub_alloc); + filter_options = &filter_options->sub[filter_options->sub_nr++]; + memset(filter_options, 0, sizeof(*filter_options)); + + parse_error = gently_parse_list_objects_filter( + filter_options, arg, &errbuf); + } + if (parse_error) + die("%s", errbuf.buf); return 0; } -- cgit v1.2.3 From 5a133e8a7f7c28c57f7a0a85e57692b8b781d896 Mon Sep 17 00:00:00 2001 From: Matthew DeVore Date: Thu, 27 Jun 2019 15:54:13 -0700 Subject: list-objects-filter-options: clean up use of ALLOC_GROW Introduce a new macro ALLOC_GROW_BY which automatically zeros the added array elements and takes care of updating the nr value. Use the macro in code introduced earlier in this patchset. Signed-off-by: Matthew DeVore Signed-off-by: Junio C Hamano --- list-objects-filter-options.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) (limited to 'list-objects-filter-options.c') diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c index 2506dc8327..44bc1153d1 100644 --- a/list-objects-filter-options.c +++ b/list-objects-filter-options.c @@ -120,14 +120,12 @@ static int parse_combine_subfilter( struct strbuf *subspec, struct strbuf *errbuf) { - size_t new_index = filter_options->sub_nr++; + size_t new_index = filter_options->sub_nr; char *decoded; int result; - ALLOC_GROW(filter_options->sub, filter_options->sub_nr, - filter_options->sub_alloc); - memset(&filter_options->sub[new_index], 0, - sizeof(*filter_options->sub)); + ALLOC_GROW_BY(filter_options->sub, filter_options->sub_nr, 1, + filter_options->sub_alloc); decoded = url_percent_decode(subspec->buf); @@ -255,13 +253,12 @@ int parse_list_objects_filter( string_list_append(&filter_options->filter_spec, xstrdup("+")); filter_spec_append_urlencode(filter_options, arg); - ALLOC_GROW(filter_options->sub, filter_options->sub_nr + 1, - filter_options->sub_alloc); - filter_options = &filter_options->sub[filter_options->sub_nr++]; - memset(filter_options, 0, sizeof(*filter_options)); + ALLOC_GROW_BY(filter_options->sub, filter_options->sub_nr, 1, + filter_options->sub_alloc); parse_error = gently_parse_list_objects_filter( - filter_options, arg, &errbuf); + &filter_options->sub[filter_options->sub_nr - 1], arg, + &errbuf); } if (parse_error) die("%s", errbuf.buf); -- cgit v1.2.3 From 90d21f9ebf6906f0ebb4fb1b20ec9536072e2916 Mon Sep 17 00:00:00 2001 From: Matthew DeVore Date: Thu, 27 Jun 2019 15:54:14 -0700 Subject: list-objects-filter-options: make parser void This function always returns 0, so make it return void instead. Signed-off-by: Matthew DeVore Signed-off-by: Junio C Hamano --- list-objects-filter-options.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) (limited to 'list-objects-filter-options.c') diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c index 44bc1153d1..ba1425cb4a 100644 --- a/list-objects-filter-options.c +++ b/list-objects-filter-options.c @@ -232,7 +232,7 @@ void list_objects_filter_die_if_populated( die(_("multiple filter-specs cannot be combined")); } -int parse_list_objects_filter( +void parse_list_objects_filter( struct list_objects_filter_options *filter_options, const char *arg) { @@ -262,7 +262,6 @@ int parse_list_objects_filter( } if (parse_error) die("%s", errbuf.buf); - return 0; } int opt_parse_list_objects_filter(const struct option *opt, @@ -270,12 +269,11 @@ int opt_parse_list_objects_filter(const struct option *opt, { struct list_objects_filter_options *filter_options = opt->value; - if (unset || !arg) { + if (unset || !arg) list_objects_filter_set_no_filter(filter_options); - return 0; - } - - return parse_list_objects_filter(filter_options, arg); + else + parse_list_objects_filter(filter_options, arg); + return 0; } const char *list_objects_filter_spec(struct list_objects_filter_options *filter) -- cgit v1.2.3