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>

diff --git a/Documentation/technical/protocol-v2.txt b/Documentation/technical/protocol-v2.txt
index 09e4e0273fd515254b41cf2d36e3b6083de1d4ee..292060a9dc1bf7e8cced8d7edb7e99d0cb722cad 100644 (file)
--- a/Documentation/technical/protocol-v2.txt
+++ b/Documentation/technical/protocol-v2.txt
@@ -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
        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
 
 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

diff --git a/builtin/clone.c b/builtin/clone.c
index 15b142d64640e29c10e62d565ac21adbaaeebca4..8e05e8ad6c8d30cca723cb489a2e97b1ad75c3d9 100644 (file)
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1130,9 +1130,13 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
                                     option_upload_pack);
 
        if (filter_options.choice) {
                                     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,
                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");
                transport_set_option(transport, TRANS_OPT_FROM_PROMISOR, "1");

+               strbuf_release(&expanded_filter_spec);

        }
 
        if (transport->smart_options && !deepen && !filter_options.choice)
        }
 
        if (transport->smart_options && !deepen && !filter_options.choice)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index e0140327aab23654c69e7388c23a07b98b8ff913..8b8bb649214cbe5d555d8b00b791558fdf87da76 100644 (file)
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -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;
 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;
        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) {
        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,
                set_option(transport, TRANS_OPT_LIST_OBJECTS_FILTER,

-                          filter_options.filter_spec);
+                          expanded_filter_spec.buf);

                set_option(transport, TRANS_OPT_FROM_PROMISOR, "1");
                set_option(transport, TRANS_OPT_FROM_PROMISOR, "1");

+               strbuf_release(&expanded_filter_spec);

        }
        if (negotiation_tip.nr) {
                if (transport->smart_options)
        }
        if (negotiation_tip.nr) {
                if (transport->smart_options)

diff --git a/fetch-pack.c b/fetch-pack.c
index 9691046e6445837beefde3d8ae5e842063842575..485632fabee376003b631f63aece35f641b3c4e4 100644 (file)
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -329,9 +329,14 @@ static int find_common(struct fetch_negotiator *negotiator,
                        packet_buf_write(&req_buf, "deepen-not %s", s->string);
                }
        }
                        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",
                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;
 
        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) {
        /* 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"));
                print_verbose(args, _("Server supports filter"));

+               expand_list_objects_filter_spec(&args->filter_options,
+                                               &expanded_filter_spec);

                packet_buf_write(&req_buf, "filter %s",
                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");
        }
        } else if (args->filter_options.choice) {
                warning("filtering not recognized by server, ignoring");
        }

diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c
index 5285e7674dbbdc07d764f6b2b6dd5a54f061268b..9efb3e99023877c804cf0dcf6cf100d6c09c716b 100644 (file)
--- a/list-objects-filter-options.c
+++ b/list-objects-filter-options.c
@@ -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
  * 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,
  */
 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);
 }
 
        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)
 {
 void list_objects_filter_release(
        struct list_objects_filter_options *filter_options)
 {

diff --git a/list-objects-filter-options.h b/list-objects-filter-options.h
index 477cd970295ba29c1bb092d9a88192b9a40d97fe..e3adc78ebf74717d3b4e544c5bbb5215d9402a92 100644 (file)
--- a/list-objects-filter-options.h
+++ b/list-objects-filter-options.h
@@ -2,6 +2,7 @@
 #define LIST_OBJECTS_FILTER_OPTIONS_H
 
 #include "parse-options.h"
 #define LIST_OBJECTS_FILTER_OPTIONS_H
 
 #include "parse-options.h"

+#include "strbuf.h"

 
 /*
  * The list of defined filters for list-objects.
 
 /*
  * 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
        /*
         * '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;
 
         */
        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 }
 
          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);
 
 void list_objects_filter_release(
        struct list_objects_filter_options *filter_options);
 

diff --git a/t/t6112-rev-list-filters-objects.sh b/t/t6112-rev-list-filters-objects.sh
index eb9e4119e2d5b77415e77b14ff23abe659180313..9c114277196e5c231869dbfe51f88760c40ae0b8 100755 (executable)
--- a/t/t6112-rev-list-filters-objects.sh
+++ b/t/t6112-rev-list-filters-objects.sh
@@ -444,4 +444,21 @@ test_expect_success 'rev-list W/ missing=allow-any' '
        git -C r1 rev-list --quiet --missing=allow-any --objects HEAD
 '
 
        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
 test_done

diff --git a/transport-helper.c b/transport-helper.c
index bf225c698fac81a9a94eff6d3371988ac4ff0bac..01404bfac52d137b461a9f713472b3def1c2fd45 100644 (file)
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -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.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.");
 
        if (data->transport_options.negotiation_tips)
                warning("Ignoring --negotiation-tip because the protocol does not support it.");

diff --git a/upload-pack.c b/upload-pack.c
index 5e81f1ff24f141fc5357522cb0745c7ff0aeb189..1c6d73e5a2334635513f6b0a6c5c7a151f125f42 100644 (file)
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -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) {
        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;
                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",
                        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);

                }
        }
 
                }
        }