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:<depth> filter specs since there should be no server in existence that supports tree:# but not tree:#k etc. Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Matthew DeVore <matvore@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Matthew DeVore committed
Jun 27, 2019 at 15:54 UTC
cf9ceb5a12cad9c9153d227a0f497d1b522ce085
9 files changed
+78
-70
builtin/clone.c
+3
-5
@@ -1149,13 +1149,11 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
1149
transport->server_options = &server_options;
1150
1151
if (filter_options.choice) {
1152
- struct strbuf expanded_filter_spec = STRBUF_INIT;
1153
- expand_list_objects_filter_spec(&filter_options,
1154
- &expanded_filter_spec);
1152
+ const char *spec =
1153
+ expand_list_objects_filter_spec(&filter_options);
1154
transport_set_option(transport, TRANS_OPT_LIST_OBJECTS_FILTER,
1156
- expanded_filter_spec.buf);
1155
+ spec);
1156
transport_set_option(transport, TRANS_OPT_FROM_PROMISOR, "1");
1158
- strbuf_release(&expanded_filter_spec);
1157
}
1158
1159
if (transport->smart_options && !deepen && !filter_options.choice)
builtin/fetch.c
+3
-6
@@ -1188,13 +1188,10 @@ static struct transport *prepare_transport(struct remote *remote, int deepen)
1188
if (update_shallow)
1189
set_option(transport, TRANS_OPT_UPDATE_SHALLOW, "yes");
1190
if (filter_options.choice) {
1191
- struct strbuf expanded_filter_spec = STRBUF_INIT;
1192
- expand_list_objects_filter_spec(&filter_options,
1193
- &expanded_filter_spec);
1194
- set_option(transport, TRANS_OPT_LIST_OBJECTS_FILTER,
1195
- expanded_filter_spec.buf);
1191
+ const char *spec =
1192
+ expand_list_objects_filter_spec(&filter_options);
1193
+ set_option(transport, TRANS_OPT_LIST_OBJECTS_FILTER, spec);
1194
set_option(transport, TRANS_OPT_FROM_PROMISOR, "1");
1197
- strbuf_release(&expanded_filter_spec);
1195
}
1196
if (negotiation_tip.nr) {
1197
if (transport->smart_options)
builtin/rev-list.c
+4
-2
@@ -466,8 +466,10 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
466
die(_("object filtering requires --objects"));
467
if (filter_options.choice == LOFC_SPARSE_OID &&
468
!filter_options.sparse_oid_value)
469
- die(_("invalid sparse value '%s'"),
470
- filter_options.filter_spec);
469
+ die(
470
+ _("invalid sparse value '%s'"),
471
+ list_objects_filter_spec(
472
+ &filter_options));
473
continue;
474
}
475
if (!strcmp(arg, ("--no-" CL_ARG__FILTER))) {
fetch-pack.c
+7
-13
@@ -339,12 +339,9 @@ static int find_common(struct fetch_negotiator *negotiator,
339
}
340
}
341
if (server_supports_filtering && args->filter_options.choice) {
342
- struct strbuf expanded_filter_spec = STRBUF_INIT;
343
- expand_list_objects_filter_spec(&args->filter_options,
344
- &expanded_filter_spec);
345
- packet_buf_write(&req_buf, "filter %s",
346
- expanded_filter_spec.buf);
347
- strbuf_release(&expanded_filter_spec);
342
+ const char *spec =
343
+ expand_list_objects_filter_spec(&args->filter_options);
344
+ packet_buf_write(&req_buf, "filter %s", spec);
345
}
346
packet_buf_flush(&req_buf);
347
state_len = req_buf.len;
@@ -1099,7 +1096,7 @@ static int add_haves(struct fetch_negotiator *negotiator,
1096
}
1097
1098
static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out,
1102
- const struct fetch_pack_args *args,
1099
+ struct fetch_pack_args *args,
1100
const struct ref *wants, struct oidset *common,
1101
int *haves_to_send, int *in_vain,
1102
int sideband_all)
@@ -1140,13 +1137,10 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out,
1137
/* Add filter */
1138
if (server_supports_feature("fetch", "filter", 0) &&
1139
args->filter_options.choice) {
1143
- struct strbuf expanded_filter_spec = STRBUF_INIT;
1140
+ const char *spec =
1141
+ expand_list_objects_filter_spec(&args->filter_options);
1142
print_verbose(args, _("Server supports filter"));
1145
- expand_list_objects_filter_spec(&args->filter_options,
1146
- &expanded_filter_spec);
1147
- packet_buf_write(&req_buf, "filter %s",
1148
- expanded_filter_spec.buf);
1149
- strbuf_release(&expanded_filter_spec);
1143
+ packet_buf_write(&req_buf, "filter %s", spec);
1144
} else if (args->filter_options.choice) {
1145
warning("filtering not recognized by server, ignoring");
1146
}
list-objects-filter-options.c
+34
-16
@@ -184,7 +184,7 @@ int parse_list_objects_filter(struct list_objects_filter_options *filter_options
184
struct strbuf buf = STRBUF_INIT;
185
if (filter_options->choice)
186
die(_("multiple filter-specs cannot be combined"));
187
- filter_options->filter_spec = strdup(arg);
187
+ string_list_append(&filter_options->filter_spec, xstrdup(arg));
188
if (gently_parse_list_objects_filter(filter_options, arg, &buf))
189
die("%s", buf.buf);
190
return 0;
@@ -203,19 +203,36 @@ int opt_parse_list_objects_filter(const struct option *opt,
203
return parse_list_objects_filter(filter_options, arg);
204
}
205
206
-void expand_list_objects_filter_spec(
207
- const struct list_objects_filter_options *filter,
208
- struct strbuf *expanded_spec)
206
+const char *list_objects_filter_spec(struct list_objects_filter_options *filter)
207
{
210
- strbuf_init(expanded_spec, strlen(filter->filter_spec));
211
- if (filter->choice == LOFC_BLOB_LIMIT)
212
- strbuf_addf(expanded_spec, "blob:limit=%lu",
208
+ if (!filter->filter_spec.nr)
209
+ BUG("no filter_spec available for this filter");
210
+ if (filter->filter_spec.nr != 1) {
211
+ struct strbuf concatted = STRBUF_INIT;
212
+ strbuf_add_separated_string_list(
213
+ &concatted, "", &filter->filter_spec);
214
+ string_list_clear(&filter->filter_spec, /*free_util=*/0);
215
+ string_list_append(
216
+ &filter->filter_spec, strbuf_detach(&concatted, NULL));
217
+ }
218
+
219
+ return filter->filter_spec.items[0].string;
220
+}
221
+
222
+const char *expand_list_objects_filter_spec(
223
+ struct list_objects_filter_options *filter)
224
+{
225
+ if (filter->choice == LOFC_BLOB_LIMIT) {
226
+ struct strbuf expanded_spec = STRBUF_INIT;
227
+ strbuf_addf(&expanded_spec, "blob:limit=%lu",
228
filter->blob_limit_value);
214
- else if (filter->choice == LOFC_TREE_DEPTH)
215
- strbuf_addf(expanded_spec, "tree:%lu",
216
- filter->tree_exclude_depth);
217
- else
218
- strbuf_addstr(expanded_spec, filter->filter_spec);
229
+ string_list_clear(&filter->filter_spec, /*free_util=*/0);
230
+ string_list_append(
231
+ &filter->filter_spec,
232
+ strbuf_detach(&expanded_spec, NULL));
233
+ }
234
+
235
+ return list_objects_filter_spec(filter);
236
}
237
238
void list_objects_filter_release(
@@ -225,7 +242,7 @@ void list_objects_filter_release(
242
243
if (!filter_options)
244
return;
228
- free(filter_options->filter_spec);
245
+ string_list_clear(&filter_options->filter_spec, /*free_util=*/0);
246
free(filter_options->sparse_oid_value);
247
for (sub = 0; sub < filter_options->sub_nr; sub++)
248
list_objects_filter_release(&filter_options->sub[sub]);
@@ -235,7 +252,7 @@ void list_objects_filter_release(
252
253
void partial_clone_register(
254
const char *remote,
238
- const struct list_objects_filter_options *filter_options)
255
+ struct list_objects_filter_options *filter_options)
256
{
257
/*
258
* Record the name of the partial clone remote in the
@@ -258,7 +275,7 @@ void partial_clone_register(
275
* the default for subsequent fetches from this remote.
276
*/
277
core_partial_clone_filter_default =
261
- xstrdup(filter_options->filter_spec);
278
+ xstrdup(expand_list_objects_filter_spec(filter_options));
279
git_config_set("core.partialclonefilter",
280
core_partial_clone_filter_default);
281
}
@@ -274,7 +291,8 @@ void partial_clone_get_default_filter_spec(
291
if (!core_partial_clone_filter_default)
292
return;
293
277
- filter_options->filter_spec = strdup(core_partial_clone_filter_default);
294
+ string_list_append(&filter_options->filter_spec,
295
+ core_partial_clone_filter_default);
296
gently_parse_list_objects_filter(filter_options,
297
core_partial_clone_filter_default,
298
&errbuf);
list-objects-filter-options.h
+19
-8
@@ -2,7 +2,7 @@
2
#define LIST_OBJECTS_FILTER_OPTIONS_H
3
4
#include "parse-options.h"
5
-#include "strbuf.h"
5
+#include "string-list.h"
6
7
/*
8
* The list of defined filters for list-objects.
@@ -24,8 +24,10 @@ struct list_objects_filter_options {
24
* commands that launch filtering sub-processes, or for communication
25
* over the network, don't use this value; use the result of
26
* expand_list_objects_filter_spec() instead.
27
+ * To get the raw filter spec given by the user, use the result of
28
+ * list_objects_filter_spec().
29
*/
28
- char *filter_spec;
30
+ struct string_list filter_spec;
31
32
/*
33
* 'choice' is determined by parsing the filter-spec. This indicates
@@ -76,13 +78,22 @@ int opt_parse_list_objects_filter(const struct option *opt,
78
/*
79
* Translates abbreviated numbers in the filter's filter_spec into their
80
* fully-expanded forms (e.g., "limit:blob=1k" becomes "limit:blob=1024").
81
+ * Returns a string owned by the list_objects_filter_options object.
82
*
80
- * This form should be used instead of the raw filter_spec field when
81
- * communicating with a remote process or subprocess.
83
+ * This form should be used instead of the raw list_objects_filter_spec()
84
+ * value when communicating with a remote process or subprocess.
85
*/
83
-void expand_list_objects_filter_spec(
84
- const struct list_objects_filter_options *filter,
85
- struct strbuf *expanded_spec);
86
+const char *expand_list_objects_filter_spec(
87
+ struct list_objects_filter_options *filter);
88
+
89
+/*
90
+ * Returns the filter spec string more or less in the form as the user
91
+ * entered it. This form of the filter_spec can be used in user-facing
92
+ * messages. Returns a string owned by the list_objects_filter_options
93
+ * object.
94
+ */
95
+const char *list_objects_filter_spec(
96
+ struct list_objects_filter_options *filter);
97
98
void list_objects_filter_release(
99
struct list_objects_filter_options *filter_options);
@@ -96,7 +107,7 @@ static inline void list_objects_filter_set_no_filter(
107
108
void partial_clone_register(
109
const char *remote,
99
- const struct list_objects_filter_options *filter_options);
110
+ struct list_objects_filter_options *filter_options);
111
void partial_clone_get_default_filter_spec(
112
struct list_objects_filter_options *filter_options);
113
t/t6112-rev-list-filters-objects.sh
-7
@@ -590,11 +590,4 @@ test_expect_success 'expand blob limit in protocol' '
590
grep "blob:limit=1024" trace
591
'
592
593
-test_expect_success 'expand tree depth limit in protocol' '
594
- GIT_TRACE_PACKET="$(pwd)/tree_trace" git -c protocol.version=2 clone \
595
- --filter=tree:0k "file://$(pwd)/r2" tree &&
596
- ! grep "tree:0k" tree_trace &&
597
- grep "tree:0" tree_trace
598
-'
599
-
593
test_done
transport-helper.c
+3
-7
@@ -682,13 +682,9 @@ static int fetch(struct transport *transport,
682
set_helper_option(transport, "update-shallow", "true");
683
684
if (data->transport_options.filter_options.choice) {
685
- struct strbuf expanded_filter_spec = STRBUF_INIT;
686
- expand_list_objects_filter_spec(
687
- &data->transport_options.filter_options,
688
- &expanded_filter_spec);
689
- set_helper_option(transport, "filter",
690
- expanded_filter_spec.buf);
691
- strbuf_release(&expanded_filter_spec);
685
+ const char *spec = expand_list_objects_filter_spec(
686
+ &data->transport_options.filter_options);
687
+ set_helper_option(transport, "filter", spec);
688
}
689
690
if (data->transport_options.negotiation_tips)
upload-pack.c
+5
-6
@@ -140,18 +140,17 @@ static void create_pack_file(const struct object_array *have_obj,
140
argv_array_push(&pack_objects.args, "--delta-base-offset");
141
if (use_include_tag)
142
argv_array_push(&pack_objects.args, "--include-tag");
143
- if (filter_options.filter_spec) {
144
- struct strbuf expanded_filter_spec = STRBUF_INIT;
145
- expand_list_objects_filter_spec(&filter_options,
146
- &expanded_filter_spec);
143
+ if (filter_options.choice) {
144
+ const char *spec =
145
+ expand_list_objects_filter_spec(&filter_options);
146
if (pack_objects.use_shell) {
147
struct strbuf buf = STRBUF_INIT;
149
- sq_quote_buf(&buf, expanded_filter_spec.buf);
148
+ sq_quote_buf(&buf, spec);
149
argv_array_pushf(&pack_objects.args, "--filter=%s", buf.buf);
150
strbuf_release(&buf);
151
} else {
152
argv_array_pushf(&pack_objects.args, "--filter=%s",
154
- expanded_filter_spec.buf);
153
+ spec);
154
}
155
}
156