filter-options: expand scaled numbers
When communicating with a remote server or a subprocess, use expanded numbers rather than numbers with scaling suffix in the object filter spec (e.g. "limit:blob=1k" becomes "limit:blob=1024"). Update the protocol docs to note that clients should always perform this expansion, to allow for more compatibility between server implementations. Signed-off-by: Josh Steadmon <steadmon@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Josh Steadmon committed
Jan 7, 2019 at 16:17 UTC
87c2d9d3102e6c388f2d29f1bdb0b3a222d9a707
9 files changed
+94
-16
Documentation/technical/protocol-v2.txt
+7
-1
@@ -296,7 +296,13 @@ included in the client's request:
296
Request that various objects from the packfile be omitted
297
using one of several filtering techniques. These are intended
298
for use with partial clone and partial fetch operations. See
299
- `rev-list` for possible "filter-spec" values.
299
+ `rev-list` for possible "filter-spec" values. When communicating
300
+ with other processes, senders SHOULD translate scaled integers
301
+ (e.g. "1k") into a fully-expanded form (e.g. "1024") to aid
302
+ interoperability with older receivers that may not understand
303
+ newly-invented scaling suffixes. However, receivers SHOULD
304
+ accept the following suffixes: 'k', 'm', and 'g' for 1024,
305
+ 1048576, and 1073741824, respectively.
306
307
If the 'ref-in-want' feature is advertised, the following argument can
308
be included in the client's request as well as the potential addition of
builtin/clone.c
+5
-1
@@ -1130,9 +1130,13 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
1130
option_upload_pack);
1131
1132
if (filter_options.choice) {
1133
+ struct strbuf expanded_filter_spec = STRBUF_INIT;
1134
+ expand_list_objects_filter_spec(&filter_options,
1135
+ &expanded_filter_spec);
1136
transport_set_option(transport, TRANS_OPT_LIST_OBJECTS_FILTER,
1134
- filter_options.filter_spec);
1137
+ expanded_filter_spec.buf);
1138
transport_set_option(transport, TRANS_OPT_FROM_PROMISOR, "1");
1139
+ strbuf_release(&expanded_filter_spec);
1140
}
1141
1142
if (transport->smart_options && !deepen && !filter_options.choice)
builtin/fetch.c
+6
-1
@@ -1172,6 +1172,7 @@ static void add_negotiation_tips(struct git_transport_options *smart_options)
1172
static struct transport *prepare_transport(struct remote *remote, int deepen)
1173
{
1174
struct transport *transport;
1175
+
1176
transport = transport_get(remote, NULL);
1177
transport_set_verbosity(transport, verbosity, progress);
1178
transport->family = family;
@@ -1191,9 +1192,13 @@ static struct transport *prepare_transport(struct remote *remote, int deepen)
1192
if (update_shallow)
1193
set_option(transport, TRANS_OPT_UPDATE_SHALLOW, "yes");
1194
if (filter_options.choice) {
1195
+ struct strbuf expanded_filter_spec = STRBUF_INIT;
1196
+ expand_list_objects_filter_spec(&filter_options,
1197
+ &expanded_filter_spec);
1198
set_option(transport, TRANS_OPT_LIST_OBJECTS_FILTER,
1195
- filter_options.filter_spec);
1199
+ expanded_filter_spec.buf);
1200
set_option(transport, TRANS_OPT_FROM_PROMISOR, "1");
1201
+ strbuf_release(&expanded_filter_spec);
1202
}
1203
if (negotiation_tip.nr) {
1204
if (transport->smart_options)
fetch-pack.c
+12
-3
@@ -329,9 +329,14 @@ static int find_common(struct fetch_negotiator *negotiator,
329
packet_buf_write(&req_buf, "deepen-not %s", s->string);
330
}
331
}
332
- if (server_supports_filtering && args->filter_options.choice)
332
+ if (server_supports_filtering && args->filter_options.choice) {
333
+ struct strbuf expanded_filter_spec = STRBUF_INIT;
334
+ expand_list_objects_filter_spec(&args->filter_options,
335
+ &expanded_filter_spec);
336
packet_buf_write(&req_buf, "filter %s",
334
- args->filter_options.filter_spec);
337
+ expanded_filter_spec.buf);
338
+ strbuf_release(&expanded_filter_spec);
339
+ }
340
packet_buf_flush(&req_buf);
341
state_len = req_buf.len;
342
@@ -1155,9 +1160,13 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out,
1160
/* Add filter */
1161
if (server_supports_feature("fetch", "filter", 0) &&
1162
args->filter_options.choice) {
1163
+ struct strbuf expanded_filter_spec = STRBUF_INIT;
1164
print_verbose(args, _("Server supports filter"));
1165
+ expand_list_objects_filter_spec(&args->filter_options,
1166
+ &expanded_filter_spec);
1167
packet_buf_write(&req_buf, "filter %s",
1160
- args->filter_options.filter_spec);
1168
+ expanded_filter_spec.buf);
1169
+ strbuf_release(&expanded_filter_spec);
1170
} else if (args->filter_options.choice) {
1171
warning("filtering not recognized by server, ignoring");
1172
}
list-objects-filter-options.c
+18
-2
@@ -18,8 +18,9 @@
18
* See Documentation/rev-list-options.txt for allowed values for <arg>.
19
*
20
* Capture the given arg as the "filter_spec". This can be forwarded to
21
- * subordinate commands when necessary. We also "intern" the arg for
22
- * the convenience of the current command.
21
+ * subordinate commands when necessary (although it's better to pass it through
22
+ * expand_list_objects_filter_spec() first). We also "intern" the arg for the
23
+ * convenience of the current command.
24
*/
25
static int gently_parse_list_objects_filter(
26
struct list_objects_filter_options *filter_options,
@@ -111,6 +112,21 @@ int opt_parse_list_objects_filter(const struct option *opt,
112
return parse_list_objects_filter(filter_options, arg);
113
}
114
115
+void expand_list_objects_filter_spec(
116
+ const struct list_objects_filter_options *filter,
117
+ struct strbuf *expanded_spec)
118
+{
119
+ strbuf_init(expanded_spec, strlen(filter->filter_spec));
120
+ if (filter->choice == LOFC_BLOB_LIMIT)
121
+ strbuf_addf(expanded_spec, "blob:limit=%lu",
122
+ filter->blob_limit_value);
123
+ else if (filter->choice == LOFC_TREE_DEPTH)
124
+ strbuf_addf(expanded_spec, "tree:%lu",
125
+ filter->tree_exclude_depth);
126
+ else
127
+ strbuf_addstr(expanded_spec, filter->filter_spec);
128
+}
129
+
130
void list_objects_filter_release(
131
struct list_objects_filter_options *filter_options)
132
{
list-objects-filter-options.h
+15
-2
@@ -2,6 +2,7 @@
2
#define LIST_OBJECTS_FILTER_OPTIONS_H
3
4
#include "parse-options.h"
5
+#include "strbuf.h"
6
7
/*
8
* The list of defined filters for list-objects.
@@ -20,8 +21,9 @@ struct list_objects_filter_options {
21
/*
22
* 'filter_spec' is the raw argument value given on the command line
23
* or protocol request. (The part after the "--keyword=".) For
23
- * commands that launch filtering sub-processes, this value should be
24
- * passed to them as received by the current process.
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
*/
28
char *filter_spec;
29
@@ -62,6 +64,17 @@ int opt_parse_list_objects_filter(const struct option *opt,
64
N_("object filtering"), 0, \
65
opt_parse_list_objects_filter }
66
67
+/*
68
+ * Translates abbreviated numbers in the filter's filter_spec into their
69
+ * fully-expanded forms (e.g., "limit:blob=1k" becomes "limit:blob=1024").
70
+ *
71
+ * This form should be used instead of the raw filter_spec field when
72
+ * communicating with a remote process or subprocess.
73
+ */
74
+void expand_list_objects_filter_spec(
75
+ const struct list_objects_filter_options *filter,
76
+ struct strbuf *expanded_spec);
77
+
78
void list_objects_filter_release(
79
struct list_objects_filter_options *filter_options);
80
t/t6112-rev-list-filters-objects.sh
+17
@@ -444,4 +444,21 @@ test_expect_success 'rev-list W/ missing=allow-any' '
444
git -C r1 rev-list --quiet --missing=allow-any --objects HEAD
445
'
446
447
+# Test expansion of filter specs.
448
+
449
+test_expect_success 'expand blob limit in protocol' '
450
+ git -C r2 config --local uploadpack.allowfilter 1 &&
451
+ GIT_TRACE_PACKET="$(pwd)/trace" git -c protocol.version=2 clone \
452
+ --filter=blob:limit=1k "file://$(pwd)/r2" limit &&
453
+ ! grep "blob:limit=1k" trace &&
454
+ grep "blob:limit=1024" trace
455
+'
456
+
457
+test_expect_success 'expand tree depth limit in protocol' '
458
+ GIT_TRACE_PACKET="$(pwd)/tree_trace" git -c protocol.version=2 clone \
459
+ --filter=tree:0k "file://$(pwd)/r2" tree &&
460
+ ! grep "tree:0k" tree_trace &&
461
+ grep "tree:0" tree_trace
462
+'
463
+
464
test_done
transport-helper.c
+9
-4
@@ -679,10 +679,15 @@ static int fetch(struct transport *transport,
679
if (data->transport_options.update_shallow)
680
set_helper_option(transport, "update-shallow", "true");
681
682
- if (data->transport_options.filter_options.choice)
683
- set_helper_option(
684
- transport, "filter",
685
- data->transport_options.filter_options.filter_spec);
682
+ if (data->transport_options.filter_options.choice) {
683
+ struct strbuf expanded_filter_spec = STRBUF_INIT;
684
+ expand_list_objects_filter_spec(
685
+ &data->transport_options.filter_options,
686
+ &expanded_filter_spec);
687
+ set_helper_option(transport, "filter",
688
+ expanded_filter_spec.buf);
689
+ strbuf_release(&expanded_filter_spec);
690
+ }
691
692
if (data->transport_options.negotiation_tips)
693
warning("Ignoring --negotiation-tip because the protocol does not support it.");
upload-pack.c
+5
-2
@@ -140,14 +140,17 @@ static void create_pack_file(const struct object_array *have_obj,
140
if (use_include_tag)
141
argv_array_push(&pack_objects.args, "--include-tag");
142
if (filter_options.filter_spec) {
143
+ struct strbuf expanded_filter_spec = STRBUF_INIT;
144
+ expand_list_objects_filter_spec(&filter_options,
145
+ &expanded_filter_spec);
146
if (pack_objects.use_shell) {
147
struct strbuf buf = STRBUF_INIT;
145
- sq_quote_buf(&buf, filter_options.filter_spec);
148
+ sq_quote_buf(&buf, expanded_filter_spec.buf);
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",
150
- filter_options.filter_spec);
153
+ expanded_filter_spec.buf);
154
}
155
}
156