Merge branch 'jt/avoid-ls-refs'

Over some transports, fetching objects with an exact commit object
name can be done without first seeing the ref advertisements.  The
code has been optimized to exploit this.

* jt/avoid-ls-refs:
fetch: do not list refs if fetching only hashes
transport: list refs before fetch if necessary
transport: do not list refs if possible
transport: allow skipping of ref listing

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 6ce882ce824cd26273ffac73f96cf82b6bf591bb..8f7249f2b138279dabfe994248f58069e8d52a7b 100644 (file)
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1186,6 +1186,7 @@ static int do_fetch(struct transport *transport,
        int retcode = 0;
        const struct ref *remote_refs;
        struct argv_array ref_prefixes = ARGV_ARRAY_INIT;
+       int must_list_refs = 1;
 
        if (tags == TAGS_DEFAULT) {
                if (transport->remote->fetch_tags == 2)
@@ -1201,17 +1202,36 @@ static int do_fetch(struct transport *transport,
                        goto cleanup;
        }
 
-       if (rs->nr)
+       if (rs->nr) {
+               int i;
+
                refspec_ref_prefixes(rs, &ref_prefixes);
-       else if (transport->remote && transport->remote->fetch.nr)
+
+               /*
+                * We can avoid listing refs if all of them are exact
+                * OIDs
+                */
+               must_list_refs = 0;
+               for (i = 0; i < rs->nr; i++) {
+                       if (!rs->items[i].exact_sha1) {
+                               must_list_refs = 1;
+                               break;
+                       }
+               }
+       } else if (transport->remote && transport->remote->fetch.nr)
                refspec_ref_prefixes(&transport->remote->fetch, &ref_prefixes);
 
-       if (ref_prefixes.argc &&
-           (tags == TAGS_SET || (tags == TAGS_DEFAULT))) {
-               argv_array_push(&ref_prefixes, "refs/tags/");
+       if (tags == TAGS_SET || tags == TAGS_DEFAULT) {
+               must_list_refs = 1;
+               if (ref_prefixes.argc)
+                       argv_array_push(&ref_prefixes, "refs/tags/");
        }
 
-       remote_refs = transport_get_remote_refs(transport, &ref_prefixes);
+       if (must_list_refs)
+               remote_refs = transport_get_remote_refs(transport, &ref_prefixes);
+       else
+               remote_refs = NULL;
+
        argv_array_clear(&ref_prefixes);
 
        ref_map = get_ref_map(transport->remote, remote_refs, rs,

diff --git a/fetch-pack.c b/fetch-pack.c
index 49ab2666b9bdc889b6ddb17b43893b5c4fc6a308..bff05ee69e54c5beabdc6415aba1c4e3a0118244 100644 (file)
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1626,7 +1626,7 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
                parse_list_objects_filter(&args->filter_options, "blob:none");
        }
 
-       if (!ref) {
+       if (version != protocol_v2 && !ref) {
                packet_flush(fd[1]);
                die(_("no matching remote head"));
        }

diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
index 3dc8f8ecec2c03c63788bc5b0e9db88e55deb823..8630b0cc39045f913987463a36388bcd72d6e248 100755 (executable)
--- a/t/t5551-http-fetch-smart.sh
+++ b/t/t5551-http-fetch-smart.sh
@@ -381,6 +381,21 @@ test_expect_success 'using fetch command in remote-curl updates refs' '
        test_cmp expect actual
 '
 
+test_expect_success 'fetch by SHA-1 without tag following' '
+       SERVER="$HTTPD_DOCUMENT_ROOT_PATH/server" &&
+       rm -rf "$SERVER" client &&
+
+       git init "$SERVER" &&
+       test_commit -C "$SERVER" foo &&
+
+       git clone $HTTPD_URL/smart/server client &&
+
+       test_commit -C "$SERVER" bar &&
+       git -C "$SERVER" rev-parse bar >bar_hash &&
+       git -C client -c protocol.version=0 fetch \
+               --no-tags origin $(cat bar_hash)
+'
+
 test_expect_success 'GIT_REDACT_COOKIES redacts cookies' '
        rm -rf clone &&
        echo "Set-Cookie: Foo=1" >cookies &&

diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh
index 98fbf39da325ab574f9717a03c6a6588b15f0ffa..8360188c01037abe45ef701561fd19ddac6e7ecb 100755 (executable)
--- a/t/t5702-protocol-v2.sh
+++ b/t/t5702-protocol-v2.sh
@@ -79,6 +79,19 @@ test_expect_success 'fetch with git:// using protocol v2' '
        grep "fetch< version 2" log
 '
 
+test_expect_success 'fetch by hash without tag following with protocol v2 does not list refs' '
+       test_when_finished "rm -f log" &&
+
+       test_commit -C "$daemon_parent" two_a &&
+       git -C "$daemon_parent" rev-parse two_a >two_a_hash &&
+
+       GIT_TRACE_PACKET="$(pwd)/log" git -C daemon_child -c protocol.version=2 \
+               fetch --no-tags origin $(cat two_a_hash) &&
+
+       grep "fetch< version 2" log &&
+       ! grep "fetch> command=ls-refs" log
+'
+
 test_expect_success 'pull with git:// using protocol v2' '
        test_when_finished "rm -f log" &&
 
@@ -286,6 +299,10 @@ test_expect_success 'dynamically fetch missing object' '
        grep "version 2" trace
 '
 
+test_expect_success 'when dynamically fetching missing object, do not list refs' '
+       ! grep "git> command=ls-refs" trace
+'
+
 test_expect_success 'partial fetch' '
        rm -rf client "$(pwd)/trace" &&
        git init client &&

diff --git a/transport-helper.c b/transport-helper.c
index 143ca008c8f20384c8c4636e5372817946e66e14..7213fa0d320284e318c7895cc0915ef556ac4f6d 100644 (file)
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -1105,6 +1105,7 @@ static struct ref *get_refs_list(struct transport *transport, int for_push,
 }
 
 static struct transport_vtable vtable = {
+       0,
        set_helper_option,
        get_refs_list,
        fetch,

diff --git a/transport-internal.h b/transport-internal.h
index 1cde6258a73bcf8582b0746d1c44a23b30115dc9..004bee5e368f24963d925af997aec051189e1acf 100644 (file)
--- a/transport-internal.h
+++ b/transport-internal.h
@@ -6,6 +6,12 @@ struct transport;
 struct argv_array;
 
 struct transport_vtable {
+       /**
+        * This transport supports the fetch() function being called
+        * without get_refs_list() first being called.
+        */
+       unsigned fetch_without_list : 1;
+
        /**
         * Returns 0 if successful, positive if the option is not
         * recognized or is inapplicable, and negative if the option

diff --git a/transport.c b/transport.c
index dd7bd3436c5e5387097ff423adcf5db44bcbf43e..046b2f6d8c96cdb8e2c28556cb295282fc76f7b9 100644 (file)
--- a/transport.c
+++ b/transport.c
@@ -252,8 +252,18 @@ static int connect_setup(struct transport *transport, int for_push)
        return 0;
 }
 
-static struct ref *get_refs_via_connect(struct transport *transport, int for_push,
-                                       const struct argv_array *ref_prefixes)
+/*
+ * Obtains the protocol version from the transport and writes it to
+ * transport->data->version, first connecting if not already connected.
+ *
+ * If the protocol version is one that allows skipping the listing of remote
+ * refs, and must_list_refs is 0, the listing of remote refs is skipped and
+ * this function returns NULL. Otherwise, this function returns the list of
+ * remote refs.
+ */
+static struct ref *handshake(struct transport *transport, int for_push,
+                            const struct argv_array *ref_prefixes,
+                            int must_list_refs)
 {
        struct git_transport_data *data = transport->data;
        struct ref *refs = NULL;
@@ -268,8 +278,10 @@ static struct ref *get_refs_via_connect(struct transport *transport, int for_pus
        data->version = discover_version(&reader);
        switch (data->version) {
        case protocol_v2:
-               get_remote_refs(data->fd[1], &reader, &refs, for_push,
-                               ref_prefixes, transport->server_options);
+               if (must_list_refs)
+                       get_remote_refs(data->fd[1], &reader, &refs, for_push,
+                                       ref_prefixes,
+                                       transport->server_options);
                break;
        case protocol_v1:
        case protocol_v0:
@@ -283,9 +295,18 @@ static struct ref *get_refs_via_connect(struct transport *transport, int for_pus
        }
        data->got_remote_heads = 1;
 
+       if (reader.line_peeked)
+               BUG("buffer must be empty at the end of handshake()");
+
        return refs;
 }
 
+static struct ref *get_refs_via_connect(struct transport *transport, int for_push,
+                                       const struct argv_array *ref_prefixes)
+{
+       return handshake(transport, for_push, ref_prefixes, 1);
+}
+
 static int fetch_refs_via_pack(struct transport *transport,
                               int nr_heads, struct ref **to_fetch)
 {
@@ -320,8 +341,17 @@ static int fetch_refs_via_pack(struct transport *transport,
        args.server_options = transport->server_options;
        args.negotiation_tips = data->options.negotiation_tips;
 
-       if (!data->got_remote_heads)
-               refs_tmp = get_refs_via_connect(transport, 0, NULL);
+       if (!data->got_remote_heads) {
+               int i;
+               int must_list_refs = 0;
+               for (i = 0; i < nr_heads; i++) {
+                       if (!to_fetch[i]->exact_oid) {
+                               must_list_refs = 1;
+                               break;
+                       }
+               }
+               refs_tmp = handshake(transport, 0, NULL, must_list_refs);
+       }
 
        switch (data->version) {
        case protocol_v2:
@@ -703,6 +733,7 @@ static int disconnect_git(struct transport *transport)
 }
 
 static struct transport_vtable taken_over_vtable = {
+       1,
        NULL,
        get_refs_via_connect,
        fetch_refs_via_pack,
@@ -852,6 +883,7 @@ void transport_check_allowed(const char *type)
 }
 
 static struct transport_vtable bundle_vtable = {
+       0,
        NULL,
        get_refs_from_bundle,
        fetch_refs_from_bundle,
@@ -861,6 +893,7 @@ static struct transport_vtable bundle_vtable = {
 };
 
 static struct transport_vtable builtin_smart_vtable = {
+       1,
        NULL,
        get_refs_via_connect,
        fetch_refs_via_pack,
@@ -1227,6 +1260,15 @@ int transport_fetch_refs(struct transport *transport, struct ref *refs)
        struct ref **heads = NULL;
        struct ref *rm;
 
+       if (!transport->vtable->fetch_without_list)
+               /*
+                * Some transports (e.g. the built-in bundle transport and the
+                * transport helper interface) do not work when fetching is
+                * done immediately after transport creation. List the remote
+                * refs anyway (if not already listed) as a workaround.
+                */
+               transport_get_remote_refs(transport, NULL);
+
        for (rm = refs; rm; rm = rm->next) {
                nr_refs++;
                if (rm->peer_ref &&