fetch-pack: wire up and enable auto filter logic

Previous commits have set up an infrastructure for `--filter=auto` to
automatically prepare a partial clone filter based on what the server
advertised and the client accepted.

Using that infrastructure, let's now enable the `--filter=auto` option
in `git clone` and `git fetch` by setting `allow_auto_filter` to 1.

Note that these small changes mean that when `git clone --filter=auto`
or `git fetch --filter=auto` are used, "auto" is automatically saved
as the partial clone filter for the server on the client. Therefore
subsequent calls to `git fetch` on the client will automatically use
this "auto" mode even without `--filter=auto`.

Let's also set `allow_auto_filter` to 1 in `transport.c`, as the
transport layer must be able to accept the "auto" filter spec even if
the invoking command hasn't fully parsed it yet.

When an "auto" filter is requested, let's have the "fetch-pack.c" code
in `do_fetch_pack_v2()` compute a filter and send it to the server.

In `do_fetch_pack_v2()` the logic also needs to check for the
"promisor-remote" capability and call `promisor_remote_reply()` to
parse advertised remotes and populate the list of those accepted (and
their filters).

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/fetch-options.adoc b/Documentation/fetch-options.adoc
index 70a981833146669f7a248d7b83cb1ab9373a8369..f7432d4b2975faadd5b8493e35e275391cd7e7bb 100644 (file)
--- a/Documentation/fetch-options.adoc
+++ b/Documentation/fetch-options.adoc
@@ -92,11 +92,20 @@ precedence over the `fetch.output` config option.
        Use the partial clone feature and request that the server sends
        a subset of reachable objects according to a given object filter.
        When using `--filter`, the supplied _<filter-spec>_ is used for
-       the partial fetch. For example, `--filter=blob:none` will filter
-       out all blobs (file contents) until needed by Git. Also,
-       `--filter=blob:limit=<size>` will filter out all blobs of size
-       at least _<size>_. For more details on filter specifications, see
-       the `--filter` option in linkgit:git-rev-list[1].
+       the partial fetch.
++
+If `--filter=auto` is used, the filter specification is determined
+automatically by combining the filter specifications advertised by
+the server for the promisor remotes that the client accepts (see
+linkgit:gitprotocol-v2[5] and the `promisor.acceptFromServer`
+configuration option in linkgit:git-config[1]).
++
+For details on all other available filter specifications, see the
+`--filter=<filter-spec>` option in linkgit:git-rev-list[1].
++
+For example, `--filter=blob:none` will filter out all blobs (file
+contents) until needed by Git. Also, `--filter=blob:limit=<size>` will
+filter out all blobs of size at least _<size>_.
 
 ifndef::git-pull[]
 `--write-fetch-head`::

diff --git a/Documentation/git-clone.adoc b/Documentation/git-clone.adoc
index 57cdfb7620571c7f2fef035ee2bf819e79b4321d..0db2d1e5f0465b42539328fbc2ac0ba401ce146e 100644 (file)
--- a/Documentation/git-clone.adoc
+++ b/Documentation/git-clone.adoc
@@ -187,11 +187,26 @@ objects from the source repository into a pack in the cloned repository.
        Use the partial clone feature and request that the server sends
        a subset of reachable objects according to a given object filter.
        When using `--filter`, the supplied _<filter-spec>_ is used for
-       the partial clone filter. For example, `--filter=blob:none` will
-       filter out all blobs (file contents) until needed by Git. Also,
-       `--filter=blob:limit=<size>` will filter out all blobs of size
-       at least _<size>_. For more details on filter specifications, see
-       the `--filter` option in linkgit:git-rev-list[1].
+       the partial clone filter.
++
+If `--filter=auto` is used the filter specification is determined
+automatically through the 'promisor-remote' protocol (see
+linkgit:gitprotocol-v2[5]) by combining the filter specifications
+advertised by the server for the promisor remotes that the client
+accepts (see the `promisor.acceptFromServer` configuration option in
+linkgit:git-config[1]). This allows the server to suggest the optimal
+filter for the available promisor remotes.
++
+As with other filter specifications, the "auto" value is persisted in
+the configuration. This ensures that future fetches will continue to
+adapt to the server's current recommendation.
++
+For details on all other available filter specifications, see the
+`--filter=<filter-spec>` option in linkgit:git-rev-list[1].
++
+For example, `--filter=blob:none` will filter out all blobs (file
+contents) until needed by Git. Also, `--filter=blob:limit=<size>` will
+filter out all blobs of size at least _<size>_.
 
 `--also-filter-submodules`::
        Also apply the partial clone filter to any submodules in the repository.

diff --git a/Documentation/gitprotocol-v2.adoc b/Documentation/gitprotocol-v2.adoc
index d93dd279ead7b285cd8bf1d1117c98d0b4426094..f985cb4c4749533f4653ae5c2eeec1551e9218bb 100644 (file)
--- a/Documentation/gitprotocol-v2.adoc
+++ b/Documentation/gitprotocol-v2.adoc
@@ -812,10 +812,15 @@ MUST appear first in each pr-fields, in that order.
 After these mandatory fields, the server MAY advertise the following
 optional fields in any order:
 
-`partialCloneFilter`:: The filter specification used by the remote.
+`partialCloneFilter`:: The filter specification for the remote. It
+corresponds to the "remote.<name>.partialCloneFilter" config setting.
 Clients can use this to determine if the remote's filtering strategy
-is compatible with their needs (e.g., checking if both use "blob:none").
-It corresponds to the "remote.<name>.partialCloneFilter" config setting.
+is compatible with their needs (e.g., checking if both use
+"blob:none"). Additionally they can use this through the
+`--filter=auto` option in linkgit:git-clone[1]. With that option, the
+filter specification of the clone will be automatically computed by
+combining the filter specifications of the promisor remotes the client
+accepts.
 
 `token`:: An authentication token that clients can use when
 connecting to the remote. It corresponds to the "remote.<name>.token"
@@ -828,8 +833,9 @@ future protocol extensions.
 
 The client can use information transmitted through these fields to
 decide if it accepts the advertised promisor remote. Also, the client
-can be configured to store the values of these fields (see
-"promisor.storeFields" in linkgit:git-config[1]).
+can be configured to store the values of these fields or use them
+to automatically configure the repository (see "promisor.storeFields"
+in linkgit:git-config[1] and `--filter=auto` in linkgit:git-clone[1]).
 
 Field values MUST be urlencoded.
 

diff --git a/builtin/clone.c b/builtin/clone.c
index 186e5498d4085be717874fbb3e7e295c91dc106f..41bbaea72a723ea48d3cf1ddf461d6b287f9f9fc 100644 (file)
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1001,6 +1001,8 @@ int cmd_clone(int argc,
                NULL
        };
 
+       filter_options.allow_auto_filter = 1;
+
        packet_trace_identity("clone");
 
        repo_config(the_repository, git_clone_config, NULL);

diff --git a/builtin/fetch.c b/builtin/fetch.c
index b9841734473964110b9a5f17f06614bbd48c18cb..ddc30a0d309097d2f71423df4c9b94d9f724532d 100644 (file)
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -2439,6 +2439,8 @@ int cmd_fetch(int argc,
                OPT_END()
        };
 
+       filter_options.allow_auto_filter = 1;
+
        packet_trace_identity("fetch");
 
        /* Record the command line for the reflog */

diff --git a/fetch-pack.c b/fetch-pack.c
index 40316c9a348f23dc1545930eb4ca7ad0a447affd..12ccea0dabe1802903314343c11eeb9d2e5a0161 100644 (file)
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -35,6 +35,7 @@
 #include "sigchain.h"
 #include "mergesort.h"
 #include "prio-queue.h"
+#include "promisor-remote.h"
 
 static int transfer_unpack_limit = -1;
 static int fetch_unpack_limit = -1;
@@ -1661,6 +1662,25 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
        struct string_list packfile_uris = STRING_LIST_INIT_DUP;
        int i;
        struct strvec index_pack_args = STRVEC_INIT;
+       const char *promisor_remote_config;
+
+       if (server_feature_v2("promisor-remote", &promisor_remote_config)) {
+               char *remote_name = promisor_remote_reply(promisor_remote_config);
+               free(remote_name);
+       }
+
+       if (args->filter_options.choice == LOFC_AUTO) {
+               struct strbuf errbuf = STRBUF_INIT;
+               char *constructed_filter = promisor_remote_construct_filter(r);
+
+               list_objects_filter_resolve_auto(&args->filter_options,
+                                                constructed_filter, &errbuf);
+               if (errbuf.len > 0)
+                       die(_("couldn't resolve 'auto' filter: %s"), errbuf.buf);
+
+               free(constructed_filter);
+               strbuf_release(&errbuf);
+       }
 
        negotiator = &negotiator_alloc;
        if (args->refetch)

diff --git a/t/t5710-promisor-remote-capability.sh b/t/t5710-promisor-remote-capability.sh
index a726af214a9df10cb0dac0b5a76012db496df490..21543bce20da07aff93c82cd0ff4ce903d6bb574 100755 (executable)
--- a/t/t5710-promisor-remote-capability.sh
+++ b/t/t5710-promisor-remote-capability.sh
@@ -409,6 +409,66 @@ test_expect_success "clone with promisor.storeFields=partialCloneFilter" '
        check_missing_objects server 1 "$oid"
 '
 
+test_expect_success "clone and fetch with --filter=auto" '
+       git -C server config promisor.advertise true &&
+       test_when_finished "rm -rf client trace" &&
+
+       git -C server config remote.lop.partialCloneFilter "blob:limit=9500" &&
+       test_config -C server promisor.sendFields "partialCloneFilter" &&
+
+       GIT_TRACE_PACKET="$(pwd)/trace" GIT_NO_LAZY_FETCH=0 git clone \
+               -c remote.lop.promisor=true \
+               -c remote.lop.url="file://$(pwd)/lop" \
+               -c promisor.acceptfromserver=All \
+               --no-local --filter=auto server client 2>err &&
+
+       test_grep "filter blob:limit=9500" trace &&
+       test_grep ! "filter auto" trace &&
+
+       # Verify "auto" is persisted in config
+       echo auto >expected &&
+       git -C client config remote.origin.partialCloneFilter >actual &&
+       test_cmp expected actual &&
+
+       # Check that the largest object is still missing on the server
+       check_missing_objects server 1 "$oid" &&
+
+       # Now change the filter on the server
+       git -C server config remote.lop.partialCloneFilter "blob:limit=5678" &&
+
+       # Get a new commit on the server to ensure "git fetch" actually runs fetch-pack
+       test_commit -C template new-commit &&
+       git -C template push --all "$(pwd)/server" &&
+
+       # Perform a fetch WITH --filter=auto
+       rm -rf trace &&
+       GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch --filter=auto &&
+
+       # Verify that the new filter was used
+       test_grep "filter blob:limit=5678" trace &&
+
+       # Check that the largest object is still missing on the server
+       check_missing_objects server 1 "$oid" &&
+
+       # Change the filter on the server again
+       git -C server config remote.lop.partialCloneFilter "blob:limit=5432" &&
+
+       # Get yet a new commit on the server to ensure fetch-pack runs
+       test_commit -C template yet-a-new-commit &&
+       git -C template push --all "$(pwd)/server" &&
+
+       # Perform a fetch WITHOUT --filter=auto
+       # Relies on "auto" being persisted in the client config
+       rm -rf trace &&
+       GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch &&
+
+       # Verify that the new filter was used
+       test_grep "filter blob:limit=5432" trace &&
+
+       # Check that the largest object is still missing on the server
+       check_missing_objects server 1 "$oid"
+'
+
 test_expect_success "clone with promisor.advertise set to 'true' but don't delete the client" '
        git -C server config promisor.advertise true &&
 

diff --git a/transport.c b/transport.c
index c7f06a7382e605a82316747836300446e5ce74c9..cde8d83a575a76c4ed553d11e15b7936b7bc49cb 100644 (file)
--- a/transport.c
+++ b/transport.c
@@ -1219,6 +1219,7 @@ struct transport *transport_get(struct remote *remote, const char *url)
                 */
                struct git_transport_data *data = xcalloc(1, sizeof(*data));
                list_objects_filter_init(&data->options.filter_options);
+               data->options.filter_options.allow_auto_filter = 1;
                ret->data = data;
                ret->vtable = &builtin_smart_vtable;
                ret->smart_options = &(data->options);