{"thread":{"id":"50071","subject":"[RFC PATCH 0/1] Expand abbreviated filters","startedAt":"2018-12-19T23:21:23Z","lastAt":"2019-01-09T19:20:08Z","messageCount":11,"participants":["Josh Steadmon","Junio C Hamano","SZEDER Gábor"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"365606","messageId":"cover.1545261186.git.steadmon@google.com","threadId":"50071","inReplyTo":null,"subject":"[RFC PATCH 0/1] Expand abbreviated filters","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2018-12-19T23:21:17Z","receivedAt":"2018-12-19T23:21:23Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"NOTE: this patch is based on top of md/list-objects-filter-by-depth\n\nCurrently, git clients pass filter specs verbatim over the network and\nto subprocesses. We support various numeric abbreviations for parameters\non these limits (via git_parse_ulong()), but other implementations may\nnot support the same abbreviations (or may support them differently;\ne.g., should \"1k\" == 1000 or 1024?). It would be better to only pass\nfully-expanded numbers in this case, and keep the expansion logic\ncompletely on the client side.\n\nThis patch updates the protocol-v2 doc to specify that clients MUST\nexpand abbreviations in filter specifications before passing them to\nother processes. It adds a new function\n\"expand_list_objects_filter_spec()\" in list-objects-filter-options.c\nthat implements the expansion logic, and updates users of the\nfilter_spec field to instead expand the spec first.\n\nJosh Steadmon (1):\n  filter-options: Expand abbreviated numbers\n\n Documentation/technical/protocol-v2.txt |  5 ++++-\n builtin/clone.c                         |  6 +++++-\n builtin/fetch.c                         |  7 ++++++-\n fetch-pack.c                            | 15 ++++++++++++---\n list-objects-filter-options.c           | 20 ++++++++++++++++++--\n list-objects-filter-options.h           | 17 +++++++++++++++--\n t/t6112-rev-list-filters-objects.sh     | 17 +++++++++++++++++\n transport-helper.c                      | 13 +++++++++----\n upload-pack.c                           |  7 +++++--\n 9 files changed, 91 insertions(+), 16 deletions(-)\n\n-- \n2.20.0.405.gbc1bbc6f85-goog\n\n"},{"id":"365607","messageId":"d324e7836928940a4df0b43da3ffeb8526feac61.1545261186.git.steadmon@google.com","threadId":"50071","inReplyTo":"cover.1545261186.git.steadmon@google.com","subject":"[RFC PATCH 1/1] filter-options: Expand abbreviated numbers","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2018-12-19T23:21:18Z","receivedAt":"2018-12-19T23:21:27Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"When communicating with a remote server or a subprocess, use expanded\nnumbers rather than abbreviated numbers in the object filter spec (e.g.\n\"limit:blob=1k\" becomes \"limit:blob=1024\").\n\nUpdate the protocol docs to note that clients should always perform this\nexpansion, to allow for more compatibility between server\nimplementations.\n\nSigned-off-by: Josh Steadmon <steadmon@google.com>\n---\n Documentation/technical/protocol-v2.txt |  5 ++++-\n builtin/clone.c                         |  6 +++++-\n builtin/fetch.c                         |  7 ++++++-\n fetch-pack.c                            | 15 ++++++++++++---\n list-objects-filter-options.c           | 20 ++++++++++++++++++--\n list-objects-filter-options.h           | 17 +++++++++++++++--\n t/t6112-rev-list-filters-objects.sh     | 17 +++++++++++++++++\n transport-helper.c                      | 13 +++++++++----\n upload-pack.c                           |  7 +++++--\n 9 files changed, 91 insertions(+), 16 deletions(-)\n\ndiff --git a/Documentation/technical/protocol-v2.txt b/Documentation/technical/protocol-v2.txt\nindex 09e4e0273f..d001372404 100644\n--- a/Documentation/technical/protocol-v2.txt\n+++ b/Documentation/technical/protocol-v2.txt\n@@ -296,7 +296,10 @@ included in the client's request:\n \tRequest that various objects from the packfile be omitted\n \tusing one of several filtering techniques. These are intended\n \tfor use with partial clone and partial fetch operations. See\n-\t`rev-list` for possible \"filter-spec\" values.\n+\t`rev-list` for possible \"filter-spec\" values. Clients MUST\n+\ttranslate abbreviated numbers (e.g. \"1k\") into fully-expanded\n+\tnumbers (e.g. \"1024\") on the client side, so that the server\n+\tdoes not need to implement unit parsing.\n \n If the 'ref-in-want' feature is advertised, the following argument can\n be included in the client's request as well as the potential addition of\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 15b142d646..8e05e8ad6c 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -1130,9 +1130,13 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\t\t\t     option_upload_pack);\n \n \tif (filter_options.choice) {\n+\t\tstruct strbuf expanded_filter_spec = STRBUF_INIT;\n+\t\texpand_list_objects_filter_spec(&filter_options,\n+\t\t\t\t\t\t&expanded_filter_spec);\n \t\ttransport_set_option(transport, TRANS_OPT_LIST_OBJECTS_FILTER,\n-\t\t\t\t     filter_options.filter_spec);\n+\t\t\t\t     expanded_filter_spec.buf);\n \t\ttransport_set_option(transport, TRANS_OPT_FROM_PROMISOR, \"1\");\n+\t\tstrbuf_release(&expanded_filter_spec);\n \t}\n \n \tif (transport->smart_options && !deepen && !filter_options.choice)\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex e0140327aa..8b8bb64921 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -1172,6 +1172,7 @@ static void add_negotiation_tips(struct git_transport_options *smart_options)\n static struct transport *prepare_transport(struct remote *remote, int deepen)\n {\n \tstruct transport *transport;\n+\n \ttransport = transport_get(remote, NULL);\n \ttransport_set_verbosity(transport, verbosity, progress);\n \ttransport->family = family;\n@@ -1191,9 +1192,13 @@ static struct transport *prepare_transport(struct remote *remote, int deepen)\n \tif (update_shallow)\n \t\tset_option(transport, TRANS_OPT_UPDATE_SHALLOW, \"yes\");\n \tif (filter_options.choice) {\n+\t\tstruct strbuf expanded_filter_spec = STRBUF_INIT;\n+\t\texpand_list_objects_filter_spec(&filter_options,\n+\t\t\t\t\t\t&expanded_filter_spec);\n \t\tset_option(transport, TRANS_OPT_LIST_OBJECTS_FILTER,\n-\t\t\t   filter_options.filter_spec);\n+\t\t\t   expanded_filter_spec.buf);\n \t\tset_option(transport, TRANS_OPT_FROM_PROMISOR, \"1\");\n+\t\tstrbuf_release(&expanded_filter_spec);\n \t}\n \tif (negotiation_tip.nr) {\n \t\tif (transport->smart_options)\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 9691046e64..485632fabe 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -329,9 +329,14 @@ static int find_common(struct fetch_negotiator *negotiator,\n \t\t\tpacket_buf_write(&req_buf, \"deepen-not %s\", s->string);\n \t\t}\n \t}\n-\tif (server_supports_filtering && args->filter_options.choice)\n+\tif (server_supports_filtering && args->filter_options.choice) {\n+\t\tstruct strbuf expanded_filter_spec = STRBUF_INIT;\n+\t\texpand_list_objects_filter_spec(&args->filter_options,\n+\t\t\t\t\t\t&expanded_filter_spec);\n \t\tpacket_buf_write(&req_buf, \"filter %s\",\n-\t\t\t\t args->filter_options.filter_spec);\n+\t\t\t\t expanded_filter_spec.buf);\n+\t\tstrbuf_release(&expanded_filter_spec);\n+\t}\n \tpacket_buf_flush(&req_buf);\n \tstate_len = req_buf.len;\n \n@@ -1155,9 +1160,13 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out,\n \t/* Add filter */\n \tif (server_supports_feature(\"fetch\", \"filter\", 0) &&\n \t    args->filter_options.choice) {\n+\t\tstruct strbuf expanded_filter_spec = STRBUF_INIT;\n \t\tprint_verbose(args, _(\"Server supports filter\"));\n+\t\texpand_list_objects_filter_spec(&args->filter_options,\n+\t\t\t\t\t\t&expanded_filter_spec);\n \t\tpacket_buf_write(&req_buf, \"filter %s\",\n-\t\t\t\t args->filter_options.filter_spec);\n+\t\t\t\t expanded_filter_spec.buf);\n+\t\tstrbuf_release(&expanded_filter_spec);\n \t} else if (args->filter_options.choice) {\n \t\twarning(\"filtering not recognized by server, ignoring\");\n \t}\ndiff --git a/list-objects-filter-options.c b/list-objects-filter-options.c\nindex 5285e7674d..9efb3e9902 100644\n--- a/list-objects-filter-options.c\n+++ b/list-objects-filter-options.c\n@@ -18,8 +18,9 @@\n  * See Documentation/rev-list-options.txt for allowed values for <arg>.\n  *\n  * Capture the given arg as the \"filter_spec\".  This can be forwarded to\n- * subordinate commands when necessary.  We also \"intern\" the arg for\n- * the convenience of the current command.\n+ * subordinate commands when necessary (although it's better to pass it through\n+ * expand_list_objects_filter_spec() first).  We also \"intern\" the arg for the\n+ * convenience of the current command.\n  */\n static int gently_parse_list_objects_filter(\n \tstruct list_objects_filter_options *filter_options,\n@@ -111,6 +112,21 @@ int opt_parse_list_objects_filter(const struct option *opt,\n \treturn parse_list_objects_filter(filter_options, arg);\n }\n \n+void expand_list_objects_filter_spec(\n+\tconst struct list_objects_filter_options *filter,\n+\tstruct strbuf *expanded_spec)\n+{\n+\tstrbuf_init(expanded_spec, strlen(filter->filter_spec));\n+\tif (filter->choice == LOFC_BLOB_LIMIT)\n+\t\tstrbuf_addf(expanded_spec, \"blob:limit=%lu\",\n+\t\t\t    filter->blob_limit_value);\n+\telse if (filter->choice == LOFC_TREE_DEPTH)\n+\t\tstrbuf_addf(expanded_spec, \"tree:%lu\",\n+\t\t\t    filter->tree_exclude_depth);\n+\telse\n+\t\tstrbuf_addstr(expanded_spec, filter->filter_spec);\n+}\n+\n void list_objects_filter_release(\n \tstruct list_objects_filter_options *filter_options)\n {\ndiff --git a/list-objects-filter-options.h b/list-objects-filter-options.h\nindex 477cd97029..e3adc78ebf 100644\n--- a/list-objects-filter-options.h\n+++ b/list-objects-filter-options.h\n@@ -2,6 +2,7 @@\n #define LIST_OBJECTS_FILTER_OPTIONS_H\n \n #include \"parse-options.h\"\n+#include \"strbuf.h\"\n \n /*\n  * The list of defined filters for list-objects.\n@@ -20,8 +21,9 @@ struct list_objects_filter_options {\n \t/*\n \t * 'filter_spec' is the raw argument value given on the command line\n \t * or protocol request.  (The part after the \"--keyword=\".)  For\n-\t * commands that launch filtering sub-processes, this value should be\n-\t * passed to them as received by the current process.\n+\t * commands that launch filtering sub-processes, or for communication\n+\t * over the network, don't use this value; use the result of\n+\t * expand_list_objects_filter_spec() instead.\n \t */\n \tchar *filter_spec;\n \n@@ -62,6 +64,17 @@ int opt_parse_list_objects_filter(const struct option *opt,\n \t  N_(\"object filtering\"), 0, \\\n \t  opt_parse_list_objects_filter }\n \n+/*\n+ * Translates abbreviated numbers in the filter's filter_spec into their\n+ * fully-expanded forms (e.g., \"limit:blob=1k\" becomes \"limit:blob=1024\").\n+ *\n+ * This form should be used instead of the raw filter_spec field when\n+ * communicating with a remote process or subprocess.\n+ */\n+void expand_list_objects_filter_spec(\n+\tconst struct list_objects_filter_options *filter,\n+\tstruct strbuf *expanded_spec);\n+\n void list_objects_filter_release(\n \tstruct list_objects_filter_options *filter_options);\n \ndiff --git a/t/t6112-rev-list-filters-objects.sh b/t/t6112-rev-list-filters-objects.sh\nindex 18b0b14d5a..f96f551fb5 100755\n--- a/t/t6112-rev-list-filters-objects.sh\n+++ b/t/t6112-rev-list-filters-objects.sh\n@@ -437,4 +437,21 @@ test_expect_success 'rev-list W/ missing=allow-any' '\n \tgit -C r1 rev-list --quiet --missing=allow-any --objects HEAD\n '\n \n+# Test expansion of filter specs.\n+\n+test_expect_success 'expand blob limit in protocol' '\n+\tgit -C r2 config --local uploadpack.allowfilter 1 &&\n+\tGIT_TRACE_PACKET=\"$(pwd)/trace\" git -c protocol.version=2 clone \\\n+\t\t--filter=blob:limit=1k \"file://$(pwd)/r2\" limit &&\n+\t! grep \"blob:limit=1k\" trace &&\n+\tgrep \"blob:limit=1024\" trace\n+'\n+\n+test_expect_success 'expand tree depth limit in protocol' '\n+\tGIT_TRACE_PACKET=\"$(pwd)/tree_trace\" git -c protocol.version=2 clone \\\n+\t\t--filter=tree:0k \"file://$(pwd)/r2\" tree &&\n+\t! grep \"tree:0k\" tree_trace &&\n+\tgrep \"tree:0\" tree_trace\n+'\n+\n test_done\ndiff --git a/transport-helper.c b/transport-helper.c\nindex bf225c698f..01404bfac5 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -679,10 +679,15 @@ static int fetch(struct transport *transport,\n \tif (data->transport_options.update_shallow)\n \t\tset_helper_option(transport, \"update-shallow\", \"true\");\n \n-\tif (data->transport_options.filter_options.choice)\n-\t\tset_helper_option(\n-\t\t\ttransport, \"filter\",\n-\t\t\tdata->transport_options.filter_options.filter_spec);\n+\tif (data->transport_options.filter_options.choice) {\n+\t\tstruct strbuf expanded_filter_spec = STRBUF_INIT;\n+\t\texpand_list_objects_filter_spec(\n+\t\t\t&data->transport_options.filter_options,\n+\t\t\t&expanded_filter_spec);\n+\t\tset_helper_option(transport, \"filter\",\n+\t\t\t\t  expanded_filter_spec.buf);\n+\t\tstrbuf_release(&expanded_filter_spec);\n+\t}\n \n \tif (data->transport_options.negotiation_tips)\n \t\twarning(\"Ignoring --negotiation-tip because the protocol does not support it.\");\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 5e81f1ff24..1c6d73e5a2 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -140,14 +140,17 @@ static void create_pack_file(const struct object_array *have_obj,\n \tif (use_include_tag)\n \t\targv_array_push(&pack_objects.args, \"--include-tag\");\n \tif (filter_options.filter_spec) {\n+\t\tstruct strbuf expanded_filter_spec = STRBUF_INIT;\n+\t\texpand_list_objects_filter_spec(&filter_options,\n+\t\t\t\t\t\t&expanded_filter_spec);\n \t\tif (pack_objects.use_shell) {\n \t\t\tstruct strbuf buf = STRBUF_INIT;\n-\t\t\tsq_quote_buf(&buf, filter_options.filter_spec);\n+\t\t\tsq_quote_buf(&buf, expanded_filter_spec.buf);\n \t\t\targv_array_pushf(&pack_objects.args, \"--filter=%s\", buf.buf);\n \t\t\tstrbuf_release(&buf);\n \t\t} else {\n \t\t\targv_array_pushf(&pack_objects.args, \"--filter=%s\",\n-\t\t\t\t\t filter_options.filter_spec);\n+\t\t\t\t\t expanded_filter_spec.buf);\n \t\t}\n \t}\n \n-- \n2.20.0.405.gbc1bbc6f85-goog\n\n"},{"id":"366058","messageId":"xmqqsgyaircj.fsf@gitster-ct.c.googlers.com","threadId":"50071","inReplyTo":"d324e7836928940a4df0b43da3ffeb8526feac61.1545261186.git.steadmon@google.com","subject":"Re: [RFC PATCH 1/1] filter-options: Expand abbreviated numbers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-01-02T23:15:24Z","receivedAt":"2019-01-02T23:15:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Josh Steadmon <steadmon@google.com> writes:\n\n> When communicating with a remote server or a subprocess, use expanded\n> numbers rather than abbreviated numbers in the object filter spec (e.g.\n> \"limit:blob=1k\" becomes \"limit:blob=1024\").\n>\n> Update the protocol docs to note that clients should always perform this\n> expansion, to allow for more compatibility between server\n> implementations.\n>\n> Signed-off-by: Josh Steadmon <steadmon@google.com>\n> ---\n>  Documentation/technical/protocol-v2.txt |  5 ++++-\n>  builtin/clone.c                         |  6 +++++-\n>  builtin/fetch.c                         |  7 ++++++-\n>  fetch-pack.c                            | 15 ++++++++++++---\n>  list-objects-filter-options.c           | 20 ++++++++++++++++++--\n>  list-objects-filter-options.h           | 17 +++++++++++++++--\n>  t/t6112-rev-list-filters-objects.sh     | 17 +++++++++++++++++\n>  transport-helper.c                      | 13 +++++++++----\n>  upload-pack.c                           |  7 +++++--\n>  9 files changed, 91 insertions(+), 16 deletions(-)\n>\n> diff --git a/Documentation/technical/protocol-v2.txt b/Documentation/technical/protocol-v2.txt\n> index 09e4e0273f..d001372404 100644\n> --- a/Documentation/technical/protocol-v2.txt\n> +++ b/Documentation/technical/protocol-v2.txt\n> @@ -296,7 +296,10 @@ included in the client's request:\n>  \tRequest that various objects from the packfile be omitted\n>  \tusing one of several filtering techniques. These are intended\n>  \tfor use with partial clone and partial fetch operations. See\n> -\t`rev-list` for possible \"filter-spec\" values.\n> +\t`rev-list` for possible \"filter-spec\" values. Clients MUST\n> +\ttranslate abbreviated numbers (e.g. \"1k\") into fully-expanded\n> +\tnumbers (e.g. \"1024\") on the client side, so that the server\n> +\tdoes not need to implement unit parsing.\n\nI suspect that it is too late now to retroactively say \"MUST\" here.\nThe best we may be able to do is to say \"The sender SHOULD send a\nplain integer without unit and the receiver SHOULD be prepared to\nscale an integer with unit\".\n\n"},{"id":"366290","messageId":"20190107212517.GA54613@google.com","threadId":"50071","inReplyTo":"xmqqsgyaircj.fsf@gitster-ct.c.googlers.com","subject":"Re: [RFC PATCH 1/1] filter-options: Expand abbreviated numbers","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2019-01-07T21:25:17Z","receivedAt":"2019-01-07T21:25:25Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"On 2019.01.02 15:15, Junio C Hamano wrote:\n> Josh Steadmon <steadmon@google.com> writes:\n> \n> > When communicating with a remote server or a subprocess, use expanded\n> > numbers rather than abbreviated numbers in the object filter spec (e.g.\n> > \"limit:blob=1k\" becomes \"limit:blob=1024\").\n> >\n> > Update the protocol docs to note that clients should always perform this\n> > expansion, to allow for more compatibility between server\n> > implementations.\n> >\n> > Signed-off-by: Josh Steadmon <steadmon@google.com>\n> > ---\n> >  Documentation/technical/protocol-v2.txt |  5 ++++-\n> >  builtin/clone.c                         |  6 +++++-\n> >  builtin/fetch.c                         |  7 ++++++-\n> >  fetch-pack.c                            | 15 ++++++++++++---\n> >  list-objects-filter-options.c           | 20 ++++++++++++++++++--\n> >  list-objects-filter-options.h           | 17 +++++++++++++++--\n> >  t/t6112-rev-list-filters-objects.sh     | 17 +++++++++++++++++\n> >  transport-helper.c                      | 13 +++++++++----\n> >  upload-pack.c                           |  7 +++++--\n> >  9 files changed, 91 insertions(+), 16 deletions(-)\n> >\n> > diff --git a/Documentation/technical/protocol-v2.txt b/Documentation/technical/protocol-v2.txt\n> > index 09e4e0273f..d001372404 100644\n> > --- a/Documentation/technical/protocol-v2.txt\n> > +++ b/Documentation/technical/protocol-v2.txt\n> > @@ -296,7 +296,10 @@ included in the client's request:\n> >  \tRequest that various objects from the packfile be omitted\n> >  \tusing one of several filtering techniques. These are intended\n> >  \tfor use with partial clone and partial fetch operations. See\n> > -\t`rev-list` for possible \"filter-spec\" values.\n> > +\t`rev-list` for possible \"filter-spec\" values. Clients MUST\n> > +\ttranslate abbreviated numbers (e.g. \"1k\") into fully-expanded\n> > +\tnumbers (e.g. \"1024\") on the client side, so that the server\n> > +\tdoes not need to implement unit parsing.\n> \n> I suspect that it is too late now to retroactively say \"MUST\" here.\n> The best we may be able to do is to say \"The sender SHOULD send a\n> plain integer without unit and the receiver SHOULD be prepared to\n> scale an integer with unit\".\n> \n\nIn that case, do you think we should also specify that units should be\ninterpreted as powers-of-2 rather than powers-of-10?\n"},{"id":"366293","messageId":"xmqqva306rsy.fsf@gitster-ct.c.googlers.com","threadId":"50071","inReplyTo":"20190107212517.GA54613@google.com","subject":"Re: [RFC PATCH 1/1] filter-options: Expand abbreviated numbers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-01-07T22:12:13Z","receivedAt":"2019-01-07T22:12:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Josh Steadmon <steadmon@google.com> writes:\n\n>> > -\t`rev-list` for possible \"filter-spec\" values.\n>> > +\t`rev-list` for possible \"filter-spec\" values. Clients MUST\n>> > +\ttranslate abbreviated numbers (e.g. \"1k\") into fully-expanded\n>> > +\tnumbers (e.g. \"1024\") on the client side, so that the server\n>> > +\tdoes not need to implement unit parsing.\n>> \n>> I suspect that it is too late now to retroactively say \"MUST\" here.\n>> The best we may be able to do is to say \"The sender SHOULD send a\n>> plain integer without unit and the receiver SHOULD be prepared to\n>> scale an integer with unit\".\n>> \n>\n> In that case, do you think we should also specify that units should be\n> interpreted as powers-of-2 rather than powers-of-10?\n\nIf we do not document the number scaling elsewhere, then we\ncertainly should, but I somehow doubt it.\n\nDocumentation/git-config.txt does list these explicitly where it\ntalks about --type=int, but these human-readable scaled integers are\nalso used in command line arguments as well, so we may want to find\na central place that is clear to the readers that the description\napplies to all of them and move the description there.\n\n"},{"id":"366294","messageId":"xmqqpnt86rii.fsf@gitster-ct.c.googlers.com","threadId":"50071","inReplyTo":"xmqqva306rsy.fsf@gitster-ct.c.googlers.com","subject":"Re: [RFC PATCH 1/1] filter-options: Expand abbreviated numbers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-01-07T22:18:29Z","receivedAt":"2019-01-07T22:18:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Josh Steadmon <steadmon@google.com> writes:\n>\n>>> > -\t`rev-list` for possible \"filter-spec\" values.\n>>> > +\t`rev-list` for possible \"filter-spec\" values. Clients MUST\n>>> > +\ttranslate abbreviated numbers (e.g. \"1k\") into fully-expanded\n>>> > +\tnumbers (e.g. \"1024\") on the client side, so that the server\n>>> > +\tdoes not need to implement unit parsing.\n>>> \n>>> I suspect that it is too late now to retroactively say \"MUST\" here.\n>>> The best we may be able to do is to say \"The sender SHOULD send a\n>>> plain integer without unit and the receiver SHOULD be prepared to\n>>> scale an integer with unit\".\n>>> \n>>\n>> In that case, do you think we should also specify that units should be\n>> interpreted as powers-of-2 rather than powers-of-10?\n>\n> If we do not document the number scaling elsewhere, then we\n> certainly should, but I somehow doubt it.\n>\n> Documentation/git-config.txt does list these explicitly where it\n> talks about --type=int, but these human-readable scaled integers are\n> also used in command line arguments as well, so we may want to find\n> a central place that is clear to the readers that the description\n> applies to all of them and move the description there.\n\nAnother thing.  We may probably end up adding more scaling factors,\nbut going forward we would not want to require any and all Git\nreimplementations to adopt them in lock-step, so perhaps\n\n    `rev-list` for possible \"filter-spec\" values.  Senders SHOULD\n    translate a scaled integer (e.g. \"1k\") into a full expanded form\n    (e.g. \"1024\") so that an older receiver that does not know newly\n    invented scaling units can still interoperate with it, but\n    receivers SHOULD accept the following scalilng units: 'k', 'm'\n    and 'g' for 1024, 1048576 and 1073741824 respectively.\n\nor something like that.\n"},{"id":"366304","messageId":"cover.1546906008.git.steadmon@google.com","threadId":"50071","inReplyTo":"cover.1545261186.git.steadmon@google.com","subject":"[PATCH v2 0/1] Expand abbreviated filters","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2019-01-08T00:17:08Z","receivedAt":"2019-01-08T00:17:14Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"NOTE: this patch is based on top of md/list-objects-filter-by-depth\n\nCurrently, git clients pass filter specs verbatim over the network and\nto subprocesses. We support various scaling suffixes for parameters on\nthese limits (via git_parse_ulong()), but other implementations may not\nsupport the same suffixes. It would be better to only pass fully-\nexpanded numbers in this case, and keep the expansion logic completely\non the client side.\n\nThis patch updates the protocol-v2 doc to specify that clients SHOULD\nexpand abbreviations in filter specifications before passing them to\nother processes. It adds a new function\n\"expand_list_objects_filter_spec()\" in list-objects-filter-options.c\nthat implements the expansion logic, and updates users of the\nfilter_spec field to instead expand the spec first.\n\nChanges since V1:\n  * Changed \"MUST\" to \"SHOULD\" in protocol-v2.txt\n  * Noted specific suffixes that receivers SHOULD accept\n\nJosh Steadmon (1):\n  filter-options: Expand abbreviated numbers\n\n Documentation/technical/protocol-v2.txt |  8 +++++++-\n builtin/clone.c                         |  6 +++++-\n builtin/fetch.c                         |  7 ++++++-\n fetch-pack.c                            | 15 ++++++++++++---\n list-objects-filter-options.c           | 20 ++++++++++++++++++--\n list-objects-filter-options.h           | 17 +++++++++++++++--\n t/t6112-rev-list-filters-objects.sh     | 17 +++++++++++++++++\n transport-helper.c                      | 13 +++++++++----\n upload-pack.c                           |  7 +++++--\n 9 files changed, 94 insertions(+), 16 deletions(-)\n\nRange-diff against v1:\n1:  d324e78369 ! 1:  d35827de35 filter-options: Expand abbreviated numbers\n    @@ -20,10 +20,13 @@\n      \tusing one of several filtering techniques. These are intended\n      \tfor use with partial clone and partial fetch operations. See\n     -\t`rev-list` for possible \"filter-spec\" values.\n    -+\t`rev-list` for possible \"filter-spec\" values. Clients MUST\n    -+\ttranslate abbreviated numbers (e.g. \"1k\") into fully-expanded\n    -+\tnumbers (e.g. \"1024\") on the client side, so that the server\n    -+\tdoes not need to implement unit parsing.\n    ++\t`rev-list` for possible \"filter-spec\" values. When communicating\n    ++\twith other processes, senders SHOULD translate scaled integers\n    ++\t(e.g. \"1k\") into a fully-expanded form (e.g. \"1024\") to aid\n    ++\tinteroperability with older receivers that may not understand\n    ++\tnewly-invented scaling suffixes. However, receivers SHOULD\n    ++\taccept the following suffixes: 'k', 'm', and 'g' for 1024,\n    ++\t1048576, and 1073741824, respectively.\n      \n      If the 'ref-in-want' feature is advertised, the following argument can\n      be included in the client's request as well as the potential addition of\n-- \n2.20.1.97.g81188d93c3-goog\n\n"},{"id":"366305","messageId":"d35827de35d2a158cd5325569eaaf355563bf028.1546906008.git.steadmon@google.com","threadId":"50071","inReplyTo":"cover.1546906008.git.steadmon@google.com","subject":"[PATCH v2 1/1] filter-options: Expand abbreviated numbers","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2019-01-08T00:17:09Z","receivedAt":"2019-01-08T00:17:18Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"When communicating with a remote server or a subprocess, use expanded\nnumbers rather than abbreviated numbers in the object filter spec (e.g.\n\"limit:blob=1k\" becomes \"limit:blob=1024\").\n\nUpdate the protocol docs to note that clients should always perform this\nexpansion, to allow for more compatibility between server\nimplementations.\n\nSigned-off-by: Josh Steadmon <steadmon@google.com>\n---\n Documentation/technical/protocol-v2.txt |  8 +++++++-\n builtin/clone.c                         |  6 +++++-\n builtin/fetch.c                         |  7 ++++++-\n fetch-pack.c                            | 15 ++++++++++++---\n list-objects-filter-options.c           | 20 ++++++++++++++++++--\n list-objects-filter-options.h           | 17 +++++++++++++++--\n t/t6112-rev-list-filters-objects.sh     | 17 +++++++++++++++++\n transport-helper.c                      | 13 +++++++++----\n upload-pack.c                           |  7 +++++--\n 9 files changed, 94 insertions(+), 16 deletions(-)\n\ndiff --git a/Documentation/technical/protocol-v2.txt b/Documentation/technical/protocol-v2.txt\nindex 09e4e0273f..292060a9dc 100644\n--- a/Documentation/technical/protocol-v2.txt\n+++ b/Documentation/technical/protocol-v2.txt\n@@ -296,7 +296,13 @@ included in the client's request:\n \tRequest that various objects from the packfile be omitted\n \tusing one of several filtering techniques. These are intended\n \tfor use with partial clone and partial fetch operations. See\n-\t`rev-list` for possible \"filter-spec\" values.\n+\t`rev-list` for possible \"filter-spec\" values. When communicating\n+\twith other processes, senders SHOULD translate scaled integers\n+\t(e.g. \"1k\") into a fully-expanded form (e.g. \"1024\") to aid\n+\tinteroperability with older receivers that may not understand\n+\tnewly-invented scaling suffixes. However, receivers SHOULD\n+\taccept the following suffixes: 'k', 'm', and 'g' for 1024,\n+\t1048576, and 1073741824, respectively.\n \n If the 'ref-in-want' feature is advertised, the following argument can\n be included in the client's request as well as the potential addition of\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 15b142d646..8e05e8ad6c 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -1130,9 +1130,13 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\t\t\t     option_upload_pack);\n \n \tif (filter_options.choice) {\n+\t\tstruct strbuf expanded_filter_spec = STRBUF_INIT;\n+\t\texpand_list_objects_filter_spec(&filter_options,\n+\t\t\t\t\t\t&expanded_filter_spec);\n \t\ttransport_set_option(transport, TRANS_OPT_LIST_OBJECTS_FILTER,\n-\t\t\t\t     filter_options.filter_spec);\n+\t\t\t\t     expanded_filter_spec.buf);\n \t\ttransport_set_option(transport, TRANS_OPT_FROM_PROMISOR, \"1\");\n+\t\tstrbuf_release(&expanded_filter_spec);\n \t}\n \n \tif (transport->smart_options && !deepen && !filter_options.choice)\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex e0140327aa..8b8bb64921 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -1172,6 +1172,7 @@ static void add_negotiation_tips(struct git_transport_options *smart_options)\n static struct transport *prepare_transport(struct remote *remote, int deepen)\n {\n \tstruct transport *transport;\n+\n \ttransport = transport_get(remote, NULL);\n \ttransport_set_verbosity(transport, verbosity, progress);\n \ttransport->family = family;\n@@ -1191,9 +1192,13 @@ static struct transport *prepare_transport(struct remote *remote, int deepen)\n \tif (update_shallow)\n \t\tset_option(transport, TRANS_OPT_UPDATE_SHALLOW, \"yes\");\n \tif (filter_options.choice) {\n+\t\tstruct strbuf expanded_filter_spec = STRBUF_INIT;\n+\t\texpand_list_objects_filter_spec(&filter_options,\n+\t\t\t\t\t\t&expanded_filter_spec);\n \t\tset_option(transport, TRANS_OPT_LIST_OBJECTS_FILTER,\n-\t\t\t   filter_options.filter_spec);\n+\t\t\t   expanded_filter_spec.buf);\n \t\tset_option(transport, TRANS_OPT_FROM_PROMISOR, \"1\");\n+\t\tstrbuf_release(&expanded_filter_spec);\n \t}\n \tif (negotiation_tip.nr) {\n \t\tif (transport->smart_options)\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 9691046e64..485632fabe 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -329,9 +329,14 @@ static int find_common(struct fetch_negotiator *negotiator,\n \t\t\tpacket_buf_write(&req_buf, \"deepen-not %s\", s->string);\n \t\t}\n \t}\n-\tif (server_supports_filtering && args->filter_options.choice)\n+\tif (server_supports_filtering && args->filter_options.choice) {\n+\t\tstruct strbuf expanded_filter_spec = STRBUF_INIT;\n+\t\texpand_list_objects_filter_spec(&args->filter_options,\n+\t\t\t\t\t\t&expanded_filter_spec);\n \t\tpacket_buf_write(&req_buf, \"filter %s\",\n-\t\t\t\t args->filter_options.filter_spec);\n+\t\t\t\t expanded_filter_spec.buf);\n+\t\tstrbuf_release(&expanded_filter_spec);\n+\t}\n \tpacket_buf_flush(&req_buf);\n \tstate_len = req_buf.len;\n \n@@ -1155,9 +1160,13 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out,\n \t/* Add filter */\n \tif (server_supports_feature(\"fetch\", \"filter\", 0) &&\n \t    args->filter_options.choice) {\n+\t\tstruct strbuf expanded_filter_spec = STRBUF_INIT;\n \t\tprint_verbose(args, _(\"Server supports filter\"));\n+\t\texpand_list_objects_filter_spec(&args->filter_options,\n+\t\t\t\t\t\t&expanded_filter_spec);\n \t\tpacket_buf_write(&req_buf, \"filter %s\",\n-\t\t\t\t args->filter_options.filter_spec);\n+\t\t\t\t expanded_filter_spec.buf);\n+\t\tstrbuf_release(&expanded_filter_spec);\n \t} else if (args->filter_options.choice) {\n \t\twarning(\"filtering not recognized by server, ignoring\");\n \t}\ndiff --git a/list-objects-filter-options.c b/list-objects-filter-options.c\nindex 5285e7674d..9efb3e9902 100644\n--- a/list-objects-filter-options.c\n+++ b/list-objects-filter-options.c\n@@ -18,8 +18,9 @@\n  * See Documentation/rev-list-options.txt for allowed values for <arg>.\n  *\n  * Capture the given arg as the \"filter_spec\".  This can be forwarded to\n- * subordinate commands when necessary.  We also \"intern\" the arg for\n- * the convenience of the current command.\n+ * subordinate commands when necessary (although it's better to pass it through\n+ * expand_list_objects_filter_spec() first).  We also \"intern\" the arg for the\n+ * convenience of the current command.\n  */\n static int gently_parse_list_objects_filter(\n \tstruct list_objects_filter_options *filter_options,\n@@ -111,6 +112,21 @@ int opt_parse_list_objects_filter(const struct option *opt,\n \treturn parse_list_objects_filter(filter_options, arg);\n }\n \n+void expand_list_objects_filter_spec(\n+\tconst struct list_objects_filter_options *filter,\n+\tstruct strbuf *expanded_spec)\n+{\n+\tstrbuf_init(expanded_spec, strlen(filter->filter_spec));\n+\tif (filter->choice == LOFC_BLOB_LIMIT)\n+\t\tstrbuf_addf(expanded_spec, \"blob:limit=%lu\",\n+\t\t\t    filter->blob_limit_value);\n+\telse if (filter->choice == LOFC_TREE_DEPTH)\n+\t\tstrbuf_addf(expanded_spec, \"tree:%lu\",\n+\t\t\t    filter->tree_exclude_depth);\n+\telse\n+\t\tstrbuf_addstr(expanded_spec, filter->filter_spec);\n+}\n+\n void list_objects_filter_release(\n \tstruct list_objects_filter_options *filter_options)\n {\ndiff --git a/list-objects-filter-options.h b/list-objects-filter-options.h\nindex 477cd97029..e3adc78ebf 100644\n--- a/list-objects-filter-options.h\n+++ b/list-objects-filter-options.h\n@@ -2,6 +2,7 @@\n #define LIST_OBJECTS_FILTER_OPTIONS_H\n \n #include \"parse-options.h\"\n+#include \"strbuf.h\"\n \n /*\n  * The list of defined filters for list-objects.\n@@ -20,8 +21,9 @@ struct list_objects_filter_options {\n \t/*\n \t * 'filter_spec' is the raw argument value given on the command line\n \t * or protocol request.  (The part after the \"--keyword=\".)  For\n-\t * commands that launch filtering sub-processes, this value should be\n-\t * passed to them as received by the current process.\n+\t * commands that launch filtering sub-processes, or for communication\n+\t * over the network, don't use this value; use the result of\n+\t * expand_list_objects_filter_spec() instead.\n \t */\n \tchar *filter_spec;\n \n@@ -62,6 +64,17 @@ int opt_parse_list_objects_filter(const struct option *opt,\n \t  N_(\"object filtering\"), 0, \\\n \t  opt_parse_list_objects_filter }\n \n+/*\n+ * Translates abbreviated numbers in the filter's filter_spec into their\n+ * fully-expanded forms (e.g., \"limit:blob=1k\" becomes \"limit:blob=1024\").\n+ *\n+ * This form should be used instead of the raw filter_spec field when\n+ * communicating with a remote process or subprocess.\n+ */\n+void expand_list_objects_filter_spec(\n+\tconst struct list_objects_filter_options *filter,\n+\tstruct strbuf *expanded_spec);\n+\n void list_objects_filter_release(\n \tstruct list_objects_filter_options *filter_options);\n \ndiff --git a/t/t6112-rev-list-filters-objects.sh b/t/t6112-rev-list-filters-objects.sh\nindex 18b0b14d5a..f96f551fb5 100755\n--- a/t/t6112-rev-list-filters-objects.sh\n+++ b/t/t6112-rev-list-filters-objects.sh\n@@ -437,4 +437,21 @@ test_expect_success 'rev-list W/ missing=allow-any' '\n \tgit -C r1 rev-list --quiet --missing=allow-any --objects HEAD\n '\n \n+# Test expansion of filter specs.\n+\n+test_expect_success 'expand blob limit in protocol' '\n+\tgit -C r2 config --local uploadpack.allowfilter 1 &&\n+\tGIT_TRACE_PACKET=\"$(pwd)/trace\" git -c protocol.version=2 clone \\\n+\t\t--filter=blob:limit=1k \"file://$(pwd)/r2\" limit &&\n+\t! grep \"blob:limit=1k\" trace &&\n+\tgrep \"blob:limit=1024\" trace\n+'\n+\n+test_expect_success 'expand tree depth limit in protocol' '\n+\tGIT_TRACE_PACKET=\"$(pwd)/tree_trace\" git -c protocol.version=2 clone \\\n+\t\t--filter=tree:0k \"file://$(pwd)/r2\" tree &&\n+\t! grep \"tree:0k\" tree_trace &&\n+\tgrep \"tree:0\" tree_trace\n+'\n+\n test_done\ndiff --git a/transport-helper.c b/transport-helper.c\nindex bf225c698f..01404bfac5 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -679,10 +679,15 @@ static int fetch(struct transport *transport,\n \tif (data->transport_options.update_shallow)\n \t\tset_helper_option(transport, \"update-shallow\", \"true\");\n \n-\tif (data->transport_options.filter_options.choice)\n-\t\tset_helper_option(\n-\t\t\ttransport, \"filter\",\n-\t\t\tdata->transport_options.filter_options.filter_spec);\n+\tif (data->transport_options.filter_options.choice) {\n+\t\tstruct strbuf expanded_filter_spec = STRBUF_INIT;\n+\t\texpand_list_objects_filter_spec(\n+\t\t\t&data->transport_options.filter_options,\n+\t\t\t&expanded_filter_spec);\n+\t\tset_helper_option(transport, \"filter\",\n+\t\t\t\t  expanded_filter_spec.buf);\n+\t\tstrbuf_release(&expanded_filter_spec);\n+\t}\n \n \tif (data->transport_options.negotiation_tips)\n \t\twarning(\"Ignoring --negotiation-tip because the protocol does not support it.\");\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 5e81f1ff24..1c6d73e5a2 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -140,14 +140,17 @@ static void create_pack_file(const struct object_array *have_obj,\n \tif (use_include_tag)\n \t\targv_array_push(&pack_objects.args, \"--include-tag\");\n \tif (filter_options.filter_spec) {\n+\t\tstruct strbuf expanded_filter_spec = STRBUF_INIT;\n+\t\texpand_list_objects_filter_spec(&filter_options,\n+\t\t\t\t\t\t&expanded_filter_spec);\n \t\tif (pack_objects.use_shell) {\n \t\t\tstruct strbuf buf = STRBUF_INIT;\n-\t\t\tsq_quote_buf(&buf, filter_options.filter_spec);\n+\t\t\tsq_quote_buf(&buf, expanded_filter_spec.buf);\n \t\t\targv_array_pushf(&pack_objects.args, \"--filter=%s\", buf.buf);\n \t\t\tstrbuf_release(&buf);\n \t\t} else {\n \t\t\targv_array_pushf(&pack_objects.args, \"--filter=%s\",\n-\t\t\t\t\t filter_options.filter_spec);\n+\t\t\t\t\t expanded_filter_spec.buf);\n \t\t}\n \t}\n \n-- \n2.20.1.97.g81188d93c3-goog\n\n"},{"id":"366410","messageId":"20190109122342.GF4673@szeder.dev","threadId":"50071","inReplyTo":"d35827de35d2a158cd5325569eaaf355563bf028.1546906008.git.steadmon@google.com","subject":"Re: [PATCH v2 1/1] filter-options: Expand abbreviated numbers","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-01-09T12:23:42Z","receivedAt":"2019-01-09T12:23:49Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Mon, Jan 07, 2019 at 04:17:09PM -0800, Josh Steadmon wrote:\n> diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c\n> index 5285e7674d..9efb3e9902 100644\n> --- a/list-objects-filter-options.c\n> +++ b/list-objects-filter-options.c\n\n> @@ -111,6 +112,21 @@ int opt_parse_list_objects_filter(const struct option *opt,\n>  \treturn parse_list_objects_filter(filter_options, arg);\n>  }\n>  \n> +void expand_list_objects_filter_spec(\n> +\tconst struct list_objects_filter_options *filter,\n> +\tstruct strbuf *expanded_spec)\n> +{\n> +\tstrbuf_init(expanded_spec, strlen(filter->filter_spec));\n> +\tif (filter->choice == LOFC_BLOB_LIMIT)\n> +\t\tstrbuf_addf(expanded_spec, \"blob:limit=%lu\",\n> +\t\t\t    filter->blob_limit_value);\n> +\telse if (filter->choice == LOFC_TREE_DEPTH)\n> +\t\tstrbuf_addf(expanded_spec, \"tree:%lu\",\n> +\t\t\t    filter->tree_exclude_depth);\n> +\telse\n> +\t\tstrbuf_addstr(expanded_spec, filter->filter_spec);\n> +}\n> +\n\nAll compilers error out with something like this:\n\n  list-objects-filter-options.c: In function\n  ‘expand_list_objects_filter_spec’:\n  list-objects-filter-options.c:124:29: error: ‘LOFC_TREE_DEPTH’ undeclared (first use in this function); did you mean ‘LOFC_TREE_NONE’?\n    else if (filter->choice == LOFC_TREE_DEPTH)\n                               ^~~~~~~~~~~~~~~\n                               LOFC_TREE_NONE\n  list-objects-filter-options.c:124:29: note: each undeclared identifier is reported only once for each function it appears in\n  list-objects-filter-options.c:126:14: error: ‘const struct list_objects_filter_options’ has no member named ‘tree_exclude_depth’\n          filter->tree_exclude_depth);\n                ^~\n  make: *** [list-objects-filter-options.o] Error 1\n\n"},{"id":"366438","messageId":"20190109185543.GD54613@google.com","threadId":"50071","inReplyTo":"20190109122342.GF4673@szeder.dev","subject":"Re: [PATCH v2 1/1] filter-options: Expand abbreviated numbers","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2019-01-09T18:55:43Z","receivedAt":"2019-01-09T18:55:51Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"On 2019.01.09 13:23, SZEDER Gábor wrote:\n> On Mon, Jan 07, 2019 at 04:17:09PM -0800, Josh Steadmon wrote:\n> > diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c\n> > index 5285e7674d..9efb3e9902 100644\n> > --- a/list-objects-filter-options.c\n> > +++ b/list-objects-filter-options.c\n> \n> > @@ -111,6 +112,21 @@ int opt_parse_list_objects_filter(const struct option *opt,\n> >  \treturn parse_list_objects_filter(filter_options, arg);\n> >  }\n> >  \n> > +void expand_list_objects_filter_spec(\n> > +\tconst struct list_objects_filter_options *filter,\n> > +\tstruct strbuf *expanded_spec)\n> > +{\n> > +\tstrbuf_init(expanded_spec, strlen(filter->filter_spec));\n> > +\tif (filter->choice == LOFC_BLOB_LIMIT)\n> > +\t\tstrbuf_addf(expanded_spec, \"blob:limit=%lu\",\n> > +\t\t\t    filter->blob_limit_value);\n> > +\telse if (filter->choice == LOFC_TREE_DEPTH)\n> > +\t\tstrbuf_addf(expanded_spec, \"tree:%lu\",\n> > +\t\t\t    filter->tree_exclude_depth);\n> > +\telse\n> > +\t\tstrbuf_addstr(expanded_spec, filter->filter_spec);\n> > +}\n> > +\n> \n> All compilers error out with something like this:\n> \n>   list-objects-filter-options.c: In function\n>   ‘expand_list_objects_filter_spec’:\n>   list-objects-filter-options.c:124:29: error: ‘LOFC_TREE_DEPTH’ undeclared (first use in this function); did you mean ‘LOFC_TREE_NONE’?\n>     else if (filter->choice == LOFC_TREE_DEPTH)\n>                                ^~~~~~~~~~~~~~~\n>                                LOFC_TREE_NONE\n>   list-objects-filter-options.c:124:29: note: each undeclared identifier is reported only once for each function it appears in\n>   list-objects-filter-options.c:126:14: error: ‘const struct list_objects_filter_options’ has no member named ‘tree_exclude_depth’\n>           filter->tree_exclude_depth);\n>                 ^~\n>   make: *** [list-objects-filter-options.o] Error 1\n> \n\nHmm, looks like you may not have applied this on top of\nmd/list-objects-filter-by-depth? However, the most recent version of\nthat branch has its own compilation errors at the moment.\n"},{"id":"366439","messageId":"20190109192001.GK4673@szeder.dev","threadId":"50071","inReplyTo":"20190109185543.GD54613@google.com","subject":"Re: [PATCH v2 1/1] filter-options: Expand abbreviated numbers","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-01-09T19:20:01Z","receivedAt":"2019-01-09T19:20:08Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Wed, Jan 09, 2019 at 10:55:43AM -0800, Josh Steadmon wrote:\n> On 2019.01.09 13:23, SZEDER Gábor wrote:\n> > On Mon, Jan 07, 2019 at 04:17:09PM -0800, Josh Steadmon wrote:\n> > > diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c\n> > > index 5285e7674d..9efb3e9902 100644\n> > > --- a/list-objects-filter-options.c\n> > > +++ b/list-objects-filter-options.c\n> > \n> > > @@ -111,6 +112,21 @@ int opt_parse_list_objects_filter(const struct option *opt,\n> > >  \treturn parse_list_objects_filter(filter_options, arg);\n> > >  }\n> > >  \n> > > +void expand_list_objects_filter_spec(\n> > > +\tconst struct list_objects_filter_options *filter,\n> > > +\tstruct strbuf *expanded_spec)\n> > > +{\n> > > +\tstrbuf_init(expanded_spec, strlen(filter->filter_spec));\n> > > +\tif (filter->choice == LOFC_BLOB_LIMIT)\n> > > +\t\tstrbuf_addf(expanded_spec, \"blob:limit=%lu\",\n> > > +\t\t\t    filter->blob_limit_value);\n> > > +\telse if (filter->choice == LOFC_TREE_DEPTH)\n> > > +\t\tstrbuf_addf(expanded_spec, \"tree:%lu\",\n> > > +\t\t\t    filter->tree_exclude_depth);\n> > > +\telse\n> > > +\t\tstrbuf_addstr(expanded_spec, filter->filter_spec);\n> > > +}\n> > > +\n> > \n> > All compilers error out with something like this:\n> > \n> >   list-objects-filter-options.c: In function\n> >   ‘expand_list_objects_filter_spec’:\n> >   list-objects-filter-options.c:124:29: error: ‘LOFC_TREE_DEPTH’ undeclared (first use in this function); did you mean ‘LOFC_TREE_NONE’?\n> >     else if (filter->choice == LOFC_TREE_DEPTH)\n> >                                ^~~~~~~~~~~~~~~\n> >                                LOFC_TREE_NONE\n> >   list-objects-filter-options.c:124:29: note: each undeclared identifier is reported only once for each function it appears in\n> >   list-objects-filter-options.c:126:14: error: ‘const struct list_objects_filter_options’ has no member named ‘tree_exclude_depth’\n> >           filter->tree_exclude_depth);\n> >                 ^~\n> >   make: *** [list-objects-filter-options.o] Error 1\n> > \n> \n> Hmm, looks like you may not have applied this on top of\n> md/list-objects-filter-by-depth? However, the most recent version of\n> that branch has its own compilation errors at the moment.\n\nAh, OK.  I didn't actually apply this patch anywhere, I just tried to\ntest this topic as it is in 'pu', where it branches off from current\n'master'.\n\n"}]}