Expand abbreviated filters
cover Expand abbreviated filters
NOTE: this patch is based on top of md/list-objects-filter-by-depth
Currently, git clients pass filter specs verbatim over the network and to subprocesses. We support various numeric abbreviations for parameters on these limits (via git_parse_ulong()), but other implementations may not support the same abbreviations (or may support them differently; e.g., should "1k" == 1000 or 1024?). It would be better to only pass fully-expanded numbers in this case, and keep the expansion logic completely on the client side.
This patch updates the protocol-v2 doc to specify that clients MUST expand abbreviations in filter specifications before passing them to other processes. It adds a new function "expand_list_objects_filter_spec()" in list-objects-filter-options.c that implements the expansion logic, and updates users of the filter_spec field to instead expand the spec first.
Josh Steadmon (1):
filter-options: Expand abbreviated numbers
Documentation/technical/protocol-v2.txt | 5 ++++- builtin/clone.c | 6 +++++- builtin/fetch.c | 7 ++++++- fetch-pack.c | 15 ++++++++++++--- list-objects-filter-options.c | 20 ++++++++++++++++++-- list-objects-filter-options.h | 17 +++++++++++++++-- t/t6112-rev-list-filters-objects.sh | 17 +++++++++++++++++ transport-helper.c | 13 +++++++++---- upload-pack.c | 7 +++++-- 9 files changed, 91 insertions(+), 16 deletions(-)
Signature · 1 line
2.20.0.405.gbc1bbc6f85-goog
RFC PATCH 1/1 filter-options: Expand abbreviated numbers
When communicating with a remote server or a subprocess, use expanded numbers rather than abbreviated numbers 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.
--- Documentation/technical/protocol-v2.txt | 5 ++++- builtin/clone.c | 6 +++++- builtin/fetch.c | 7 ++++++- fetch-pack.c | 15 ++++++++++++--- list-objects-filter-options.c | 20 ++++++++++++++++++-- list-objects-filter-options.h | 17 +++++++++++++++-- t/t6112-rev-list-filters-objects.sh | 17 +++++++++++++++++ transport-helper.c | 13 +++++++++---- upload-pack.c | 7 +++++-- 9 files changed, 91 insertions(+), 16 deletions(-)
Documentation/technical/protocol-v2.txt +4 -1
@@ -296,7 +296,10 @@ included in the client's request: Request that various objects from the packfile be omitted using one of several filtering techniques. These are intended for use with partial clone and partial fetch operations. See - `rev-list` for possible "filter-spec" values. + `rev-list` for possible "filter-spec" values. Clients MUST + translate abbreviated numbers (e.g. "1k") into fully-expanded + numbers (e.g. "1024") on the client side, so that the server + does not need to implement unit parsing. If the 'ref-in-want' feature is advertised, the following argument can 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) option_upload_pack);if (filter_options.choice) { +struct strbuf expanded_filter_spec =STRBUF_INIT ; +expand_list_objects_filter_spec (&filter_options, + &expanded_filter_spec);transport_set_option (transport,TRANS_OPT_LIST_OBJECTS_FILTER , - filter_options.filter_spec); + expanded_filter_spec.buf);transport_set_option (transport,TRANS_OPT_FROM_PROMISOR ,"1" ); +strbuf_release (&expanded_filter_spec); }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)static struct transport *prepare_transport (struct remote *remote,int deepen) {struct transport *transport; + transport =transport_get (remote,NULL );transport_set_verbosity (transport, verbosity, progress); transport->family = family;
@@ -1191,9 +1192,13 @@ static struct transport *prepare_transport(struct remote *remote, int deepen)if (update_shallow)set_option (transport,TRANS_OPT_UPDATE_SHALLOW ,"yes" );if (filter_options.choice) { +struct strbuf expanded_filter_spec =STRBUF_INIT ; +expand_list_objects_filter_spec (&filter_options, + &expanded_filter_spec);set_option (transport,TRANS_OPT_LIST_OBJECTS_FILTER , - filter_options.filter_spec); + expanded_filter_spec.buf);set_option (transport,TRANS_OPT_FROM_PROMISOR ,"1" ); +strbuf_release (&expanded_filter_spec); }if (negotiation_tip.nr) {if (transport->smart_options)
fetch-pack.c +12 -3
@@ -329,9 +329,14 @@ static int find_common(struct fetch_negotiator *negotiator,packet_buf_write (&req_buf,"deepen-not %s" , s->string); } } -if (server_supports_filtering && args->filter_options.choice) +if (server_supports_filtering && args->filter_options.choice) { +struct strbuf expanded_filter_spec =STRBUF_INIT ; +expand_list_objects_filter_spec (&args->filter_options, + &expanded_filter_spec);packet_buf_write (&req_buf,"filter %s" , - args->filter_options.filter_spec); + expanded_filter_spec.buf); +strbuf_release (&expanded_filter_spec); + }packet_buf_flush (&req_buf); state_len = req_buf.len;
@@ -1155,9 +1160,13 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out,/* Add filter */ if (server_supports_feature ("fetch" ,"filter" ,0 ) && args->filter_options.choice) { +struct strbuf expanded_filter_spec =STRBUF_INIT ;print_verbose (args,_ ("Server supports filter" )); +expand_list_objects_filter_spec (&args->filter_options, + &expanded_filter_spec);packet_buf_write (&req_buf,"filter %s" , - args->filter_options.filter_spec); + expanded_filter_spec.buf); +strbuf_release (&expanded_filter_spec); }else if (args->filter_options.choice) {warning ("filtering not recognized by server, ignoring" ); }
list-objects-filter-options.c +18 -2
@@ -18,8 +18,9 @@* See Documentation/rev-list-options.txt for allowed values for <arg>. * * Capture the given arg as the "filter_spec". This can be forwarded to -* subordinate commands when necessary. We also "intern" the arg for -* the convenience of the current command. +* subordinate commands when necessary (although it's better to pass it through +* expand_list_objects_filter_spec() first). We also "intern" the arg for the +* convenience of the current command. */ static int gently_parse_list_objects_filter( struct list_objects_filter_options *filter_options,
@@ -111,6 +112,21 @@ 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) +{ +strbuf_init (expanded_spec,strlen (filter->filter_spec)); +if (filter->choice ==LOFC_BLOB_LIMIT ) +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); +} +void list_objects_filter_release (struct list_objects_filter_options *filter_options) {
list-objects-filter-options.h +15 -2
@@ -2,6 +2,7 @@#define LIST_OBJECTS_FILTER_OPTIONS_H #include "parse-options.h" +#include "strbuf.h" /* * The list of defined filters for list-objects.
@@ -20,8 +21,9 @@ struct list_objects_filter_options {/* * 'filter_spec' is the raw argument value given on the command line * or protocol request. (The part after the "--keyword=".) For -* commands that launch filtering sub-processes, this value should be -* passed to them as received by the current process. +* commands that launch filtering sub-processes, or for communication +* over the network, don't use this value; use the result of +* expand_list_objects_filter_spec() instead. */ char *filter_spec;
@@ -62,6 +64,17 @@ int opt_parse_list_objects_filter(const struct option *opt,N_ ("object filtering" ),0 , \ opt_parse_list_objects_filter } +/* +* Translates abbreviated numbers in the filter's filter_spec into their +* fully-expanded forms (e.g., "limit:blob=1k" becomes "limit:blob=1024"). +* +* This form should be used instead of the raw filter_spec field when +* communicating with a remote process or subprocess. +*/ +void expand_list_objects_filter_spec ( +const struct list_objects_filter_options *filter, +struct strbuf *expanded_spec); +void list_objects_filter_release (struct list_objects_filter_options *filter_options);
t/t6112-rev-list-filters-objects.sh +17 -0
@@ -437,4 +437,21 @@ test_expect_success 'rev-list W/ missing=allow-any' ' git -C r1 rev-list --quiet --missing=allow-any --objects HEAD ' +# Test expansion of filter specs. + +test_expect_success 'expand blob limit in protocol' ' + git -C r2 config --local uploadpack.allowfilter 1 && + GIT_TRACE_PACKET="$(pwd)/trace" git -c protocol.version=2 clone \ + --filter=blob:limit=1k "file://$(pwd)/r2" limit && + ! grep "blob:limit=1k" trace && + grep "blob:limit=1024" trace +' + +test_expect_success 'expand tree depth limit in protocol' ' + GIT_TRACE_PACKET="$(pwd)/tree_trace" git -c protocol.version=2 clone \ + --filter=tree:0k "file://$(pwd)/r2" tree && + ! grep "tree:0k" tree_trace && + grep "tree:0" tree_trace +' + test_done
transport-helper.c +9 -4
@@ -679,10 +679,15 @@ static int fetch(struct transport *transport,if (data->transport_options.update_shallow)set_helper_option (transport,"update-shallow" ,"true" ); -if (data->transport_options.filter_options.choice) -set_helper_option ( - transport,"filter" , - data->transport_options.filter_options.filter_spec); +if (data->transport_options.filter_options.choice) { +struct strbuf expanded_filter_spec =STRBUF_INIT ; +expand_list_objects_filter_spec ( + &data->transport_options.filter_options, + &expanded_filter_spec); +set_helper_option (transport,"filter" , + expanded_filter_spec.buf); +strbuf_release (&expanded_filter_spec); + }if (data->transport_options.negotiation_tips)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,if (use_include_tag)argv_array_push (&pack_objects.args,"--include-tag" );if (filter_options.filter_spec) { +struct strbuf expanded_filter_spec =STRBUF_INIT ; +expand_list_objects_filter_spec (&filter_options, + &expanded_filter_spec);if (pack_objects.use_shell) {struct strbuf buf =STRBUF_INIT ; -sq_quote_buf (&buf, filter_options.filter_spec); +sq_quote_buf (&buf, expanded_filter_spec.buf);argv_array_pushf (&pack_objects.args,"--filter=%s" , buf.buf);strbuf_release (&buf); }else {argv_array_pushf (&pack_objects.args,"--filter=%s" , - filter_options.filter_spec); + expanded_filter_spec.buf); } }
Signature · 1 line
2.20.0.405.gbc1bbc6f85-goog
Re: [RFC PATCH 1/1] filter-options: Expand abbreviated numbers
Quoted 26 lines · Josh Steadmon
When communicating with a remote server or a subprocess, use expanded numbers rather than abbreviated numbers 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 <[email protected]>--- Documentation/technical/protocol-v2.txt | 5 ++++- builtin/clone.c | 6 +++++- builtin/fetch.c | 7 ++++++- fetch-pack.c | 15 ++++++++++++--- list-objects-filter-options.c | 20 ++++++++++++++++++-- list-objects-filter-options.h | 17 +++++++++++++++-- t/t6112-rev-list-filters-objects.sh | 17 +++++++++++++++++ transport-helper.c | 13 +++++++++---- upload-pack.c | 7 +++++-- 9 files changed, 91 insertions(+), 16 deletions(-)Documentation/technical/protocol-v2.txt +0
-0
Documentation/technical/protocol-v2.txt included in the client's request: · line 302
Quoted 8 lines
Documentation/technical/protocol-v2.txt +4
-1@@ -296,7 +296,10 @@ included in the client's request: Request that various objects from the packfile be omitted using one of several filtering techniques. These are intended for use with partial clone and partial fetch operations. See - `rev-list` for possible "filter-spec" values. + `rev-list` for possible "filter-spec" values. Clients MUST + translate abbreviated numbers (e.g. "1k") into fully-expanded + numbers (e.g. "1024") on the client side, so that the server + does not need to implement unit parsing.
I suspect that it is too late now to retroactively say "MUST" here. The best we may be able to do is to say "The sender SHOULD send a plain integer without unit and the receiver SHOULD be prepared to scale an integer with unit".
Re: [RFC PATCH 1/1] filter-options: Expand abbreviated numbers
Quoted 33 lines · Junio C Hamano
Quoted 31 lines · Josh Steadmon
When communicating with a remote server or a subprocess, use expanded numbers rather than abbreviated numbers 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 <[email protected]>--- Documentation/technical/protocol-v2.txt | 5 ++++- builtin/clone.c | 6 +++++- builtin/fetch.c | 7 ++++++- fetch-pack.c | 15 ++++++++++++--- list-objects-filter-options.c | 20 ++++++++++++++++++-- list-objects-filter-options.h | 17 +++++++++++++++-- t/t6112-rev-list-filters-objects.sh | 17 +++++++++++++++++ transport-helper.c | 13 +++++++++---- upload-pack.c | 7 +++++-- 9 files changed, 91 insertions(+), 16 deletions(-)Documentation/technical/protocol-v2.txt +1
-1@@ -296,7 +296,10 @@ included in the client's request: Request that various objects from the packfile be omitted using one of several filtering techniques. These are intended for use with partial clone and partial fetch operations. See - `rev-list` for possible "filter-spec" values. + `rev-list` for possible "filter-spec" values. Clients MUST
Quoted 9 lines
Documentation/technical/protocol-v2.txt included in the client's request: · line 302
Quoted 3 lines
Documentation/technical/protocol-v2.txt +3
-0+ translate abbreviated numbers (e.g. "1k") into fully-expanded + numbers (e.g. "1024") on the client side, so that the server + does not need to implement unit parsing.I suspect that it is too late now to retroactively say "MUST" here. The best we may be able to do is to say "The sender SHOULD send a plain integer without unit and the receiver SHOULD be prepared to scale an integer with unit".
In that case, do you think we should also specify that units should be interpreted as powers-of-2 rather than powers-of-10?
Re: [RFC PATCH 1/1] filter-options: Expand abbreviated numbers
Quoted 12 lines · Josh Steadmon
Quoted 11 lines
Documentation/technical/protocol-v2.txt included in the client's request: · line 302
Quoted 5 lines
Documentation/technical/protocol-v2.txt +4
-1- `rev-list` for possible "filter-spec" values. + `rev-list` for possible "filter-spec" values. Clients MUST + translate abbreviated numbers (e.g. "1k") into fully-expanded + numbers (e.g. "1024") on the client side, so that the server + does not need to implement unit parsing.I suspect that it is too late now to retroactively say "MUST" here. The best we may be able to do is to say "The sender SHOULD send a plain integer without unit and the receiver SHOULD be prepared to scale an integer with unit".
Quoted 2 lines
In that case, do you think we should also specify that units should be interpreted as powers-of-2 rather than powers-of-10?
If we do not document the number scaling elsewhere, then we certainly should, but I somehow doubt it.
Documentation/git-config.txt does list these explicitly where it talks about --type=int, but these human-readable scaled integers are also used in command line arguments as well, so we may want to find a central place that is clear to the readers that the description applies to all of them and move the description there.
Re: [RFC PATCH 1/1] filter-options: Expand abbreviated numbers
Quoted 17 lines · Junio C Hamano
Quoted 12 lines · Josh Steadmon
Quoted 11 lines
Documentation/technical/protocol-v2.txt included in the client's request: · line 302
Quoted 5 lines
Documentation/technical/protocol-v2.txt +4
-1- `rev-list` for possible "filter-spec" values. + `rev-list` for possible "filter-spec" values. Clients MUST + translate abbreviated numbers (e.g. "1k") into fully-expanded + numbers (e.g. "1024") on the client side, so that the server + does not need to implement unit parsing.I suspect that it is too late now to retroactively say "MUST" here. The best we may be able to do is to say "The sender SHOULD send a plain integer without unit and the receiver SHOULD be prepared to scale an integer with unit".
Quoted 2 lines
In that case, do you think we should also specify that units should be interpreted as powers-of-2 rather than powers-of-10?
Quoted 8 lines
If we do not document the number scaling elsewhere, then we certainly should, but I somehow doubt it.
Documentation/git-config.txtdoes list these explicitly where it talks about --type=int, but these human-readable scaled integers are also used in command line arguments as well, so we may want to find a central place that is clear to the readers that the description applies to all of them and move the description there.
Another thing. We may probably end up adding more scaling factors, but going forward we would not want to require any and all Git reimplementations to adopt them in lock-step, so perhaps
rev-list for possible "filter-spec" values. Senders SHOULD translate a scaled integer (e.g. "1k") into a full expanded form (e.g. "1024") so that an older receiver that does not know newly invented scaling units can still interoperate with it, but receivers SHOULD accept the following scalilng units: 'k', 'm' and 'g' for 1024, 1048576 and 1073741824 respectively.
or something like that.
v2 cover Expand abbreviated filters
In reply to Josh SteadmonNOTE: this patch is based on top of md/list-objects-filter-by-depth
Currently, git clients pass filter specs verbatim over the network and to subprocesses. We support various scaling suffixes for parameters on these limits (via git_parse_ulong()), but other implementations may not support the same suffixes. It would be better to only pass fully- expanded numbers in this case, and keep the expansion logic completely on the client side.
This patch updates the protocol-v2 doc to specify that clients SHOULD expand abbreviations in filter specifications before passing them to other processes. It adds a new function "expand_list_objects_filter_spec()" in list-objects-filter-options.c that implements the expansion logic, and updates users of the filter_spec field to instead expand the spec first.
Changes since V1:
* Changed "MUST" to "SHOULD" in protocol-v2.txt
* Noted specific suffixes that receivers SHOULD accept
Josh Steadmon (1):
filter-options: Expand abbreviated numbers
Documentation/technical/protocol-v2.txt | 8 +++++++- builtin/clone.c | 6 +++++- builtin/fetch.c | 7 ++++++- fetch-pack.c | 15 ++++++++++++--- list-objects-filter-options.c | 20 ++++++++++++++++++-- list-objects-filter-options.h | 17 +++++++++++++++-- t/t6112-rev-list-filters-objects.sh | 17 +++++++++++++++++ transport-helper.c | 13 +++++++++---- upload-pack.c | 7 +++++-- 9 files changed, 94 insertions(+), 16 deletions(-)
Range-diff against v1:
1: d324e78369 ! 1: d35827de35 filter-options: Expand abbreviated numbers
@@ -20,10 +20,13 @@
using one of several filtering techniques. These are intended for use with partial clone and partial fetch operations. See
- rev-list for possible "filter-spec" values.
-+ rev-list for possible "filter-spec" values. Clients MUST
-+ translate abbreviated numbers (e.g. "1k") into fully-expanded
-+ numbers (e.g. "1024") on the client side, so that the server
-+ does not need to implement unit parsing.
++ rev-list for possible "filter-spec" values. When communicating
++ with other processes, senders SHOULD translate scaled integers
++ (e.g. "1k") into a fully-expanded form (e.g. "1024") to aid
++ interoperability with older receivers that may not understand
++ newly-invented scaling suffixes. However, receivers SHOULD
++ accept the following suffixes: 'k', 'm', and 'g' for 1024,
++ 1048576, and 1073741824, respectively.
If the 'ref-in-want' feature is advertised, the following argument can be included in the client's request as well as the potential addition of
Signature · 1 line
2.20.1.97.g81188d93c3-goog
PATCH v2 1/1 filter-options: Expand abbreviated numbers
When communicating with a remote server or a subprocess, use expanded numbers rather than abbreviated numbers 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.
--- Documentation/technical/protocol-v2.txt | 8 +++++++- builtin/clone.c | 6 +++++- builtin/fetch.c | 7 ++++++- fetch-pack.c | 15 ++++++++++++--- list-objects-filter-options.c | 20 ++++++++++++++++++-- list-objects-filter-options.h | 17 +++++++++++++++-- t/t6112-rev-list-filters-objects.sh | 17 +++++++++++++++++ transport-helper.c | 13 +++++++++---- upload-pack.c | 7 +++++-- 9 files changed, 94 insertions(+), 16 deletions(-)
Documentation/technical/protocol-v2.txt +7 -1
@@ -296,7 +296,13 @@ included in the client's request: Request that various objects from the packfile be omitted using one of several filtering techniques. These are intended for use with partial clone and partial fetch operations. See - `rev-list` for possible "filter-spec" values. + `rev-list` for possible "filter-spec" values. When communicating + with other processes, senders SHOULD translate scaled integers + (e.g. "1k") into a fully-expanded form (e.g. "1024") to aid + interoperability with older receivers that may not understand + newly-invented scaling suffixes. However, receivers SHOULD + accept the following suffixes: 'k', 'm', and 'g' for 1024, + 1048576, and 1073741824, respectively. If the 'ref-in-want' feature is advertised, the following argument can 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) option_upload_pack);if (filter_options.choice) { +struct strbuf expanded_filter_spec =STRBUF_INIT ; +expand_list_objects_filter_spec (&filter_options, + &expanded_filter_spec);transport_set_option (transport,TRANS_OPT_LIST_OBJECTS_FILTER , - filter_options.filter_spec); + expanded_filter_spec.buf);transport_set_option (transport,TRANS_OPT_FROM_PROMISOR ,"1" ); +strbuf_release (&expanded_filter_spec); }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)static struct transport *prepare_transport (struct remote *remote,int deepen) {struct transport *transport; + transport =transport_get (remote,NULL );transport_set_verbosity (transport, verbosity, progress); transport->family = family;
@@ -1191,9 +1192,13 @@ static struct transport *prepare_transport(struct remote *remote, int deepen)if (update_shallow)set_option (transport,TRANS_OPT_UPDATE_SHALLOW ,"yes" );if (filter_options.choice) { +struct strbuf expanded_filter_spec =STRBUF_INIT ; +expand_list_objects_filter_spec (&filter_options, + &expanded_filter_spec);set_option (transport,TRANS_OPT_LIST_OBJECTS_FILTER , - filter_options.filter_spec); + expanded_filter_spec.buf);set_option (transport,TRANS_OPT_FROM_PROMISOR ,"1" ); +strbuf_release (&expanded_filter_spec); }if (negotiation_tip.nr) {if (transport->smart_options)
fetch-pack.c +12 -3
@@ -329,9 +329,14 @@ static int find_common(struct fetch_negotiator *negotiator,packet_buf_write (&req_buf,"deepen-not %s" , s->string); } } -if (server_supports_filtering && args->filter_options.choice) +if (server_supports_filtering && args->filter_options.choice) { +struct strbuf expanded_filter_spec =STRBUF_INIT ; +expand_list_objects_filter_spec (&args->filter_options, + &expanded_filter_spec);packet_buf_write (&req_buf,"filter %s" , - args->filter_options.filter_spec); + expanded_filter_spec.buf); +strbuf_release (&expanded_filter_spec); + }packet_buf_flush (&req_buf); state_len = req_buf.len;
@@ -1155,9 +1160,13 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out,/* Add filter */ if (server_supports_feature ("fetch" ,"filter" ,0 ) && args->filter_options.choice) { +struct strbuf expanded_filter_spec =STRBUF_INIT ;print_verbose (args,_ ("Server supports filter" )); +expand_list_objects_filter_spec (&args->filter_options, + &expanded_filter_spec);packet_buf_write (&req_buf,"filter %s" , - args->filter_options.filter_spec); + expanded_filter_spec.buf); +strbuf_release (&expanded_filter_spec); }else if (args->filter_options.choice) {warning ("filtering not recognized by server, ignoring" ); }
list-objects-filter-options.c +18 -2
@@ -18,8 +18,9 @@* See Documentation/rev-list-options.txt for allowed values for <arg>. * * Capture the given arg as the "filter_spec". This can be forwarded to -* subordinate commands when necessary. We also "intern" the arg for -* the convenience of the current command. +* subordinate commands when necessary (although it's better to pass it through +* expand_list_objects_filter_spec() first). We also "intern" the arg for the +* convenience of the current command. */ static int gently_parse_list_objects_filter( struct list_objects_filter_options *filter_options,
@@ -111,6 +112,21 @@ 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) +{ +strbuf_init (expanded_spec,strlen (filter->filter_spec)); +if (filter->choice ==LOFC_BLOB_LIMIT ) +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); +} +void list_objects_filter_release (struct list_objects_filter_options *filter_options) {
list-objects-filter-options.h +15 -2
@@ -2,6 +2,7 @@#define LIST_OBJECTS_FILTER_OPTIONS_H #include "parse-options.h" +#include "strbuf.h" /* * The list of defined filters for list-objects.
@@ -20,8 +21,9 @@ struct list_objects_filter_options {/* * 'filter_spec' is the raw argument value given on the command line * or protocol request. (The part after the "--keyword=".) For -* commands that launch filtering sub-processes, this value should be -* passed to them as received by the current process. +* commands that launch filtering sub-processes, or for communication +* over the network, don't use this value; use the result of +* expand_list_objects_filter_spec() instead. */ char *filter_spec;
@@ -62,6 +64,17 @@ int opt_parse_list_objects_filter(const struct option *opt,N_ ("object filtering" ),0 , \ opt_parse_list_objects_filter } +/* +* Translates abbreviated numbers in the filter's filter_spec into their +* fully-expanded forms (e.g., "limit:blob=1k" becomes "limit:blob=1024"). +* +* This form should be used instead of the raw filter_spec field when +* communicating with a remote process or subprocess. +*/ +void expand_list_objects_filter_spec ( +const struct list_objects_filter_options *filter, +struct strbuf *expanded_spec); +void list_objects_filter_release (struct list_objects_filter_options *filter_options);
t/t6112-rev-list-filters-objects.sh +17 -0
@@ -437,4 +437,21 @@ test_expect_success 'rev-list W/ missing=allow-any' ' git -C r1 rev-list --quiet --missing=allow-any --objects HEAD ' +# Test expansion of filter specs. + +test_expect_success 'expand blob limit in protocol' ' + git -C r2 config --local uploadpack.allowfilter 1 && + GIT_TRACE_PACKET="$(pwd)/trace" git -c protocol.version=2 clone \ + --filter=blob:limit=1k "file://$(pwd)/r2" limit && + ! grep "blob:limit=1k" trace && + grep "blob:limit=1024" trace +' + +test_expect_success 'expand tree depth limit in protocol' ' + GIT_TRACE_PACKET="$(pwd)/tree_trace" git -c protocol.version=2 clone \ + --filter=tree:0k "file://$(pwd)/r2" tree && + ! grep "tree:0k" tree_trace && + grep "tree:0" tree_trace +' + test_done
transport-helper.c +9 -4
@@ -679,10 +679,15 @@ static int fetch(struct transport *transport,if (data->transport_options.update_shallow)set_helper_option (transport,"update-shallow" ,"true" ); -if (data->transport_options.filter_options.choice) -set_helper_option ( - transport,"filter" , - data->transport_options.filter_options.filter_spec); +if (data->transport_options.filter_options.choice) { +struct strbuf expanded_filter_spec =STRBUF_INIT ; +expand_list_objects_filter_spec ( + &data->transport_options.filter_options, + &expanded_filter_spec); +set_helper_option (transport,"filter" , + expanded_filter_spec.buf); +strbuf_release (&expanded_filter_spec); + }if (data->transport_options.negotiation_tips)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,if (use_include_tag)argv_array_push (&pack_objects.args,"--include-tag" );if (filter_options.filter_spec) { +struct strbuf expanded_filter_spec =STRBUF_INIT ; +expand_list_objects_filter_spec (&filter_options, + &expanded_filter_spec);if (pack_objects.use_shell) {struct strbuf buf =STRBUF_INIT ; -sq_quote_buf (&buf, filter_options.filter_spec); +sq_quote_buf (&buf, expanded_filter_spec.buf);argv_array_pushf (&pack_objects.args,"--filter=%s" , buf.buf);strbuf_release (&buf); }else {argv_array_pushf (&pack_objects.args,"--filter=%s" , - filter_options.filter_spec); + expanded_filter_spec.buf); } }
Signature · 1 line
2.20.1.97.g81188d93c3-goog
Re: [PATCH v2 1/1] filter-options: Expand abbreviated numbers
Quoted 14 lines · Josh Steadmon
list-objects-filter-options.c +5
-0@@ -111,6 +112,21 @@ 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) +{ +strbuf_init (expanded_spec,strlen (filter->filter_spec));
list-objects-filter-options.c int opt_parse_list_objects_filter(const struct option *opt, · line 129
Quoted 10 lines
list-objects-filter-options.c +10
-0+if (filter->choice ==LOFC_BLOB_LIMIT ) +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); +} +
All compilers error out with something like this:
list-objects-filter-options.c: In function
‘expand_list_objects_filter_spec’:
list-objects-filter-options.c:124:29: error: ‘LOFC_TREE_DEPTH’ undeclared (first use in this function); did you mean ‘LOFC_TREE_NONE’?
else if (filter->choice == LOFC_TREE_DEPTH)
^~~~~~~~~~~~~~~
LOFC_TREE_NONE
list-objects-filter-options.c:124:29: note: each undeclared identifier is reported only once for each function it appears in
list-objects-filter-options.c:126:14: error: ‘const struct list_objects_filter_options’ has no member named ‘tree_exclude_depth’
filter->tree_exclude_depth);
^~make: *** [list-objects-filter-options.o] Error 1
Re: [PATCH v2 1/1] filter-options: Expand abbreviated numbers
Quoted 31 lines · SZEDER Gábor
Quoted 14 lines · Josh Steadmon
list-objects-filter-options.c +5
-0@@ -111,6 +112,21 @@ 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) +{ +strbuf_init (expanded_spec,strlen (filter->filter_spec));list-objects-filter-options.c int opt_parse_list_objects_filter(const struct option *opt, · line 129
Quoted 10 lines
list-objects-filter-options.c +10
-0+if (filter->choice ==LOFC_BLOB_LIMIT ) +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); +} +All compilers error out with something like this:
list-objects-filter-options.c: In function
‘expand_list_objects_filter_spec’:
list-objects-filter-options.c:124:29: error: ‘LOFC_TREE_DEPTH’ undeclared (first use in this function); did you mean ‘LOFC_TREE_NONE’?
Quoted 9 lines
else if (filter->choice == LOFC_TREE_DEPTH) ^~~~~~~~~~~~~~~ LOFC_TREE_NONE list-objects-filter-options.c:124:29: note: each undeclared identifier is reported only once for each function it appears in list-objects-filter-options.c:126:14: error: ‘const struct list_objects_filter_options’ has no member named ‘tree_exclude_depth’ filter->tree_exclude_depth); ^~make: *** [list-objects-filter-options.o] Error 1
Hmm, looks like you may not have applied this on top of md/list-objects-filter-by-depth? However, the most recent version of that branch has its own compilation errors at the moment.
Re: [PATCH v2 1/1] filter-options: Expand abbreviated numbers
Quoted 42 lines · Josh Steadmon
Quoted 31 lines · SZEDER Gábor
Quoted 14 lines · Josh Steadmon
list-objects-filter-options.c +5
-0@@ -111,6 +112,21 @@ 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) +{ +strbuf_init (expanded_spec,strlen (filter->filter_spec));list-objects-filter-options.c int opt_parse_list_objects_filter(const struct option *opt, · line 129
Quoted 10 lines
list-objects-filter-options.c +10
-0+if (filter->choice ==LOFC_BLOB_LIMIT ) +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); +} +All compilers error out with something like this:
list-objects-filter-options.c: In function
‘expand_list_objects_filter_spec’:
list-objects-filter-options.c:124:29: error: ‘LOFC_TREE_DEPTH’ undeclared (first use in this function); did you mean ‘LOFC_TREE_NONE’?Quoted 9 lines
else if (filter->choice == LOFC_TREE_DEPTH) ^~~~~~~~~~~~~~~ LOFC_TREE_NONE list-objects-filter-options.c:124:29: note: each undeclared identifier is reported only once for each function it appears in list-objects-filter-options.c:126:14: error: ‘const struct list_objects_filter_options’ has no member named ‘tree_exclude_depth’ filter->tree_exclude_depth); ^~make: *** [list-objects-filter-options.o] Error 1
Quoted 3 lines
Hmm, looks like you may not have applied this on top of md/list-objects-filter-by-depth? However, the most recent version of that branch has its own compilation errors at the moment.
Ah, OK. I didn't actually apply this patch anywhere, I just tried to test this topic as it is in 'pu', where it branches off from current 'master'.