fetch: work around "transport-take-over" hack

A Git-aware "connect" transport allows the "transport_take_over" to
redirect generic transport requests like fetch(), push_refs() and
get_refs_list() to the native Git transport handling methods.  The
take-over process replaces transport->data with a fake data that
these method implementations understand.

While this hack works OK for a single request, it breaks when the
transport needs to make more than one requests.  transport->data
that used to hold necessary information for the specific helper to
work correctly is destroyed during the take-over process.

One codepath that this matters is "git fetch" in auto-follow mode;
when it does not get all the tags that ought to point at the history
it got (which can be determined by looking at the peeled tags in the
initial advertisement) from the primary transfer, it internally
makes a second request to complete the fetch.  Because "take-over"
hack has already destroyed the data necessary to talk to the
transport helper by the time this happens, the second request cannot
make a request to the helper to make another connection to fetch
these additional tags.

Mark such a transport as "cannot_reuse", and use a separate
transport to perform the backfill fetch in order to work around
this breakage.

Note that this problem does not manifest itself when running t5802,
because our upload-pack gives you all the necessary auto-followed
tags during the primary transfer.  You would need to step through
"git fetch" in a debugger, stop immediately after the primary
transfer finishes and writes these auto-followed tags, remove the
tag references and repack/prune the repository to convince the
"find-non-local-tags" procedure that the primary transfer failed to
give us all the necessary tags, and then let it continue, in order
to trigger the bug in the secondary transfer this patch fixes.

Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 0b21f071b37831b77c404549628ec118ec8ad45e..57ab7e4d6346ad6966e450341047cb949b74da30 100644 (file)
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -37,6 +37,7 @@ static const char *depth;
 static const char *upload_pack;
 static struct strbuf default_rla = STRBUF_INIT;
 static struct transport *gtransport;
 static const char *upload_pack;
 static struct strbuf default_rla = STRBUF_INIT;
 static struct transport *gtransport;

+static struct transport *gsecondary;

 static const char *submodule_prefix = "";
 static const char *recurse_submodules_default;
 
 static const char *submodule_prefix = "";
 static const char *recurse_submodules_default;
 


@@ -97,6 +98,8 @@ static void unlock_pack(void)
 {
        if (gtransport)
                transport_unlock_pack(gtransport);
 {
        if (gtransport)
                transport_unlock_pack(gtransport);

+       if (gsecondary)
+               transport_unlock_pack(gsecondary);

 }
 
 static void unlock_pack_on_signal(int signo)
 }
 
 static void unlock_pack_on_signal(int signo)


@@ -747,9 +750,19 @@ struct transport *prepare_transport(struct remote *remote)
 
 static void backfill_tags(struct transport *transport, struct ref *ref_map)
 {
 
 static void backfill_tags(struct transport *transport, struct ref *ref_map)
 {

+       if (transport->cannot_reuse) {
+               gsecondary = prepare_transport(transport->remote);
+               transport = gsecondary;
+       }
+

        transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL);
        transport_set_option(transport, TRANS_OPT_DEPTH, "0");
        fetch_refs(transport, ref_map);
        transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL);
        transport_set_option(transport, TRANS_OPT_DEPTH, "0");
        fetch_refs(transport, ref_map);

+
+       if (gsecondary) {
+               transport_disconnect(gsecondary);
+               gsecondary = NULL;
+       }

 }
 
 static int do_fetch(struct transport *transport,
 }
 
 static int do_fetch(struct transport *transport,

diff --git a/transport.c b/transport.c
index e15db9808c85a3918a8de3c409500aaa78d1328e..de255880a41248857990380d7622512fd18160b0 100644 (file)
--- a/transport.c
+++ b/transport.c
@@ -875,6 +875,8 @@ void transport_take_over(struct transport *transport,
        transport->push_refs = git_transport_push;
        transport->disconnect = disconnect_git;
        transport->smart_options = &(data->options);
        transport->push_refs = git_transport_push;
        transport->disconnect = disconnect_git;
        transport->smart_options = &(data->options);

+
+       transport->cannot_reuse = 1;

 }
 
 static int is_local(const char *url)
 }
 
 static int is_local(const char *url)

diff --git a/transport.h b/transport.h
index ea70ea7e4a1b61c3006c614712ddd1aa41205ac6..96e0ede89f6c98efe988425aecd2418538b202e5 100644 (file)
--- a/transport.h
+++ b/transport.h
@@ -27,6 +27,12 @@ struct transport {
         */
        unsigned got_remote_refs : 1;
 
         */
        unsigned got_remote_refs : 1;
 

+       /*
+        * Transports that call take-over destroys the data specific to
+        * the transport type while doing so, and cannot be reused.
+        */
+       unsigned cannot_reuse : 1;
+

        /**
         * Returns 0 if successful, positive if the option is not
         * recognized or is inapplicable, and negative if the option
        /**
         * Returns 0 if successful, positive if the option is not
         * recognized or is inapplicable, and negative if the option