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 <emilyshaffer@google.com> Helped-by: Jeff Hostetler <git@jeffhostetler.com> Helped-by: Jeff King <peff@peff.net> 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
489fc9ee718b7c8594f17b55f090ac5292c655e1
7 files changed
+173
-10
Documentation/rev-list-options.txt
+16
@@ -738,6 +738,22 @@ explicitly-given commit or tree.
738
Note that the form '--filter=sparse:path=<path>' that wants to read
739
from an arbitrary path on the filesystem has been dropped for security
740
reasons.
741
++
742
+Multiple '--filter=' flags can be specified to combine filters. Only
743
+objects which are accepted by every filter are included.
744
++
745
+The form '--filter=combine:<filter1>+<filter2>+...<filterN>' can also be
746
+used to combined several filters, but this is harder than just repeating
747
+the '--filter' flag and is usually not necessary. Filters are joined by
748
+'{plus}' and individual filters are %-encoded (i.e. URL-encoded).
749
+Besides the '{plus}' and '%' characters, the following characters are
750
+reserved and also must be encoded: `~!@#$^&*()[]{}\;",<>?`+'`+
751
+as well as all characters with ASCII code <= `0x20`, which includes
752
+space and newline.
753
++
754
+Other arbitrary characters can also be encoded. For instance,
755
+'combine:tree:3+blob:none' and 'combine:tree%3A3+blob%3Anone' are
756
+equivalent.
757
758
--no-filter::
759
Turn off any previous `--filter=` argument.
list-objects-filter-options.c
+83
-5
@@ -6,6 +6,7 @@
6
#include "list-objects.h"
7
#include "list-objects-filter.h"
8
#include "list-objects-filter-options.h"
9
+#include "trace.h"
10
#include "url.h"
11
12
static int parse_combine_filter(
@@ -178,15 +179,92 @@ cleanup:
179
return result;
180
}
181
181
-int parse_list_objects_filter(struct list_objects_filter_options *filter_options,
182
- const char *arg)
182
+static int allow_unencoded(char ch)
183
+{
184
+ if (ch <= ' ' || ch == '%' || ch == '+')
185
+ return 0;
186
+ return !strchr(RESERVED_NON_WS, ch);
187
+}
188
+
189
+static void filter_spec_append_urlencode(
190
+ struct list_objects_filter_options *filter, const char *raw)
191
{
192
struct strbuf buf = STRBUF_INIT;
193
+ strbuf_addstr_urlencode(&buf, raw, allow_unencoded);
194
+ trace_printf("Add to combine filter-spec: %s\n", buf.buf);
195
+ string_list_append(&filter->filter_spec, strbuf_detach(&buf, NULL));
196
+}
197
+
198
+/*
199
+ * Changes filter_options into an equivalent LOFC_COMBINE filter options
200
+ * instance. Does not do anything if filter_options is already LOFC_COMBINE.
201
+ */
202
+static void transform_to_combine_type(
203
+ struct list_objects_filter_options *filter_options)
204
+{
205
+ assert(filter_options->choice);
206
+ if (filter_options->choice == LOFC_COMBINE)
207
+ return;
208
+ {
209
+ const int initial_sub_alloc = 2;
210
+ struct list_objects_filter_options *sub_array =
211
+ xcalloc(initial_sub_alloc, sizeof(*sub_array));
212
+ sub_array[0] = *filter_options;
213
+ memset(filter_options, 0, sizeof(*filter_options));
214
+ filter_options->sub = sub_array;
215
+ filter_options->sub_alloc = initial_sub_alloc;
216
+ }
217
+ filter_options->sub_nr = 1;
218
+ filter_options->choice = LOFC_COMBINE;
219
+ string_list_append(&filter_options->filter_spec, xstrdup("combine:"));
220
+ filter_spec_append_urlencode(
221
+ filter_options,
222
+ list_objects_filter_spec(&filter_options->sub[0]));
223
+ /*
224
+ * We don't need the filter_spec strings for subfilter specs, only the
225
+ * top level.
226
+ */
227
+ string_list_clear(&filter_options->sub[0].filter_spec, /*free_util=*/0);
228
+}
229
+
230
+void list_objects_filter_die_if_populated(
231
+ struct list_objects_filter_options *filter_options)
232
+{
233
if (filter_options->choice)
234
die(_("multiple filter-specs cannot be combined"));
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);
235
+}
236
+
237
+int parse_list_objects_filter(
238
+ struct list_objects_filter_options *filter_options,
239
+ const char *arg)
240
+{
241
+ struct strbuf errbuf = STRBUF_INIT;
242
+ int parse_error;
243
+
244
+ if (!filter_options->choice) {
245
+ string_list_append(&filter_options->filter_spec, xstrdup(arg));
246
+
247
+ parse_error = gently_parse_list_objects_filter(
248
+ filter_options, arg, &errbuf);
249
+ } else {
250
+ /*
251
+ * Make filter_options an LOFC_COMBINE spec so we can trivially
252
+ * add subspecs to it.
253
+ */
254
+ transform_to_combine_type(filter_options);
255
+
256
+ string_list_append(&filter_options->filter_spec, xstrdup("+"));
257
+ filter_spec_append_urlencode(filter_options, arg);
258
+ ALLOC_GROW(filter_options->sub, filter_options->sub_nr + 1,
259
+ filter_options->sub_alloc);
260
+ filter_options = &filter_options->sub[filter_options->sub_nr++];
261
+ memset(filter_options, 0, sizeof(*filter_options));
262
+
263
+ parse_error = gently_parse_list_objects_filter(
264
+ filter_options, arg, &errbuf);
265
+ }
266
+ if (parse_error)
267
+ die("%s", errbuf.buf);
268
return 0;
269
}
270
list-objects-filter-options.h
+11
@@ -63,6 +63,17 @@ struct list_objects_filter_options {
63
/* Normalized command line arguments */
64
#define CL_ARG__FILTER "filter"
65
66
+void list_objects_filter_die_if_populated(
67
+ struct list_objects_filter_options *filter_options);
68
+
69
+/*
70
+ * Parses the filter spec string given by arg and either (1) simply places the
71
+ * result in filter_options if it is not yet populated or (2) combines it with
72
+ * the filter already in filter_options if it is already populated. In the case
73
+ * of (2), the filter specs are combined as if specified with 'combine:'.
74
+ *
75
+ * Dies and prints a user-facing message if an error occurs.
76
+ */
77
int parse_list_objects_filter(
78
struct list_objects_filter_options *filter_options,
79
const char *arg);
t/t5616-partial-clone.sh
+19
@@ -208,6 +208,25 @@ test_expect_success 'use fsck before and after manually fetching a missing subtr
208
test_cmp unique_types.expected unique_types.observed
209
'
210
211
+test_expect_success 'implicitly construct combine: filter with repeated flags' '
212
+ GIT_TRACE=$(pwd)/trace git clone --bare \
213
+ --filter=blob:none --filter=tree:1 \
214
+ "file://$(pwd)/srv.bare" pc2 &&
215
+ grep "trace:.* git pack-objects .*--filter=combine:blob:none+tree:1" \
216
+ trace &&
217
+ git -C pc2 rev-list --objects --missing=allow-any HEAD >objects &&
218
+
219
+ # We should have gotten some root trees.
220
+ grep " $" objects &&
221
+ # Should not have gotten any non-root trees or blobs.
222
+ ! grep " ." objects &&
223
+
224
+ xargs -n 1 git -C pc2 cat-file -t <objects >types &&
225
+ sort -u types >unique_types.actual &&
226
+ test_write_lines commit tree >unique_types.expected &&
227
+ test_cmp unique_types.expected unique_types.actual
228
+'
229
+
230
test_expect_success 'partial clone fetches blobs pointed to by refs even if normally filtered out' '
231
rm -rf src dst &&
232
git init src &&
t/t6112-rev-list-filters-objects.sh
+41
-5
@@ -351,7 +351,16 @@ test_expect_success 'combine:... for a simple combination' '
351
expect_has HEAD dir1 &&
352
353
# There are also 2 commit objects
354
- test_line_count = 5 actual
354
+ test_line_count = 5 actual &&
355
+
356
+ cp actual expected &&
357
+
358
+ # Try again using repeated --filter - this is equivalent to a manual
359
+ # combine with "combine:...+..."
360
+ git -C r3 rev-list --objects --filter=combine:tree:2 \
361
+ --filter=blob:none HEAD >actual &&
362
+
363
+ test_cmp expected actual
364
'
365
366
test_expect_success 'combine:... with URL encoding' '
@@ -417,10 +426,12 @@ test_expect_success 'combine:... with edge-case hex digits: Ff Aa 0 9' '
426
test_line_count = 5 actual
427
'
428
420
-test_expect_success 'add a sparse pattern blob whose path has reserved chars' '
429
+test_expect_success 'add sparse pattern blobs whose paths have reserved chars' '
430
cp r3/pattern r3/pattern1+renamed% &&
422
- git -C r3 add pattern1+renamed% &&
423
- git -C r3 commit -m "add sparse pattern file with reserved chars"
431
+ cp r3/pattern "r3/p;at%ter+n" &&
432
+ cp r3/pattern r3/^~pattern &&
433
+ git -C r3 add pattern1+renamed% "p;at%ter+n" ^~pattern &&
434
+ git -C r3 commit -m "add sparse pattern files with reserved chars"
435
'
436
437
test_expect_success 'combine:... with more than two sub-filters' '
@@ -445,7 +456,32 @@ test_expect_success 'combine:... with more than two sub-filters' '
456
git -C r3 rev-list --objects \
457
--filter=combine:tree:3+blob:limit=40+sparse:oid=master:pattern1%2brenamed%25 \
458
HEAD >actual &&
448
- test_cmp expect actual
459
+ test_cmp expect actual &&
460
+
461
+ # Use the same composite filter again, but with a pattern file name that
462
+ # requires encoding multiple characters, and use implicit filter
463
+ # combining.
464
+ test_when_finished "rm -f trace1" &&
465
+ GIT_TRACE=$(pwd)/trace1 git -C r3 rev-list --objects \
466
+ --filter=tree:3 --filter=blob:limit=40 \
467
+ --filter=sparse:oid="master:p;at%ter+n" \
468
+ HEAD >actual &&
469
+
470
+ test_cmp expect actual &&
471
+ grep "Add to combine filter-spec: sparse:oid=master:p%3bat%25ter%2bn" \
472
+ trace1 &&
473
+
474
+ # Repeat the above test, but this time, the characters to encode are in
475
+ # the LHS of the combined filter.
476
+ test_when_finished "rm -f trace2" &&
477
+ GIT_TRACE=$(pwd)/trace2 git -C r3 rev-list --objects \
478
+ --filter=sparse:oid=master:^~pattern \
479
+ --filter=tree:3 --filter=blob:limit=40 \
480
+ HEAD >actual &&
481
+
482
+ test_cmp expect actual &&
483
+ grep "Add to combine filter-spec: sparse:oid=master:%5e%7epattern" \
484
+ trace2
485
'
486
487
# Test provisional omit collection logic with a repo that has objects appearing
transport.c
+1
@@ -224,6 +224,7 @@ static int set_git_option(struct git_transport_options *opts,
224
opts->no_dependents = !!value;
225
return 0;
226
} else if (!strcmp(name, TRANS_OPT_LIST_OBJECTS_FILTER)) {
227
+ list_objects_filter_die_if_populated(&opts->filter_options);
228
parse_list_objects_filter(&opts->filter_options, value);
229
return 0;
230
}
upload-pack.c
+2
@@ -883,6 +883,7 @@ static void receive_needs(struct packet_reader *reader, struct object_array *wan
883
if (skip_prefix(reader->line, "filter ", &arg)) {
884
if (!filter_capability_requested)
885
die("git upload-pack: filtering capability not negotiated");
886
+ list_objects_filter_die_if_populated(&filter_options);
887
parse_list_objects_filter(&filter_options, arg);
888
continue;
889
}
@@ -1304,6 +1305,7 @@ static void process_args(struct packet_reader *request,
1305
}
1306
1307
if (allow_filter && skip_prefix(arg, "filter ", &p)) {
1308
+ list_objects_filter_die_if_populated(&filter_options);
1309
parse_list_objects_filter(&filter_options, p);
1310
continue;
1311
}