From af23445925464b74b5f038271b6cfeaeb217481f Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 7 Aug 2013 15:38:45 -0700 Subject: fetch: rename file-scope global "transport" to "gtransport" Although many functions in this file take a "struct transport" as a parameter, "fetch_one()" assigns to the global singleton instance which is a file-scope static, in order to allow a parameterless signal handler unlock_pack() to access it. Rename the variable to gtransport to make sure these uses stand out. Signed-off-by: Junio C Hamano --- builtin/fetch.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) (limited to 'builtin/fetch.c') diff --git a/builtin/fetch.c b/builtin/fetch.c index d784b2e694..636b47ffb7 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -36,7 +36,7 @@ static int tags = TAGS_DEFAULT, unshallow; static const char *depth; static const char *upload_pack; static struct strbuf default_rla = STRBUF_INIT; -static struct transport *transport; +static struct transport *gtransport; static const char *submodule_prefix = ""; static const char *recurse_submodules_default; @@ -95,8 +95,8 @@ static struct option builtin_fetch_options[] = { static void unlock_pack(void) { - if (transport) - transport_unlock_pack(transport); + if (gtransport) + transport_unlock_pack(gtransport); } static void unlock_pack_on_signal(int signo) @@ -818,13 +818,13 @@ static int do_fetch(struct transport *transport, static void set_option(const char *name, const char *value) { - int r = transport_set_option(transport, name, value); + int r = transport_set_option(gtransport, name, value); if (r < 0) die(_("Option \"%s\" value \"%s\" is not valid for %s"), - name, value, transport->url); + name, value, gtransport->url); if (r > 0) warning(_("Option \"%s\" is ignored for %s\n"), - name, transport->url); + name, gtransport->url); } static int get_one_remote_for_fetch(struct remote *remote, void *priv) @@ -949,8 +949,8 @@ static int fetch_one(struct remote *remote, int argc, const char **argv) die(_("No remote repository specified. Please, specify either a URL or a\n" "remote name from which new revisions should be fetched.")); - transport = transport_get(remote, NULL); - transport_set_verbosity(transport, verbosity, progress); + gtransport = transport_get(remote, NULL); + transport_set_verbosity(gtransport, verbosity, progress); if (upload_pack) set_option(TRANS_OPT_UPLOADPACK, upload_pack); if (keep) @@ -983,10 +983,10 @@ static int fetch_one(struct remote *remote, int argc, const char **argv) sigchain_push_common(unlock_pack_on_signal); atexit(unlock_pack); refspec = parse_fetch_refspec(ref_nr, refs); - exit_code = do_fetch(transport, refspec, ref_nr); + exit_code = do_fetch(gtransport, refspec, ref_nr); free_refspec(ref_nr, refspec); - transport_disconnect(transport); - transport = NULL; + transport_disconnect(gtransport); + gtransport = NULL; return exit_code; } -- cgit v1.2.1 From db5723c6283d9a8dff3397c432af80cf5e2f7766 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 7 Aug 2013 14:43:20 -0700 Subject: fetch: refactor code that prepares a transport Make a helper function prepare_transport() that returns a transport to talk to a given remote. The set_option() helper that used to always affect the file-scope global "gtransport" now takes a transport as its parameter. Signed-off-by: Junio C Hamano --- builtin/fetch.c | 46 ++++++++++++++++++++++++++-------------------- 1 file changed, 26 insertions(+), 20 deletions(-) (limited to 'builtin/fetch.c') diff --git a/builtin/fetch.c b/builtin/fetch.c index 636b47ffb7..39a3fc8dea 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -720,6 +720,31 @@ static int truncate_fetch_head(void) return 0; } +static void set_option(struct transport *transport, const char *name, const char *value) +{ + int r = transport_set_option(transport, name, value); + if (r < 0) + die(_("Option \"%s\" value \"%s\" is not valid for %s"), + name, value, transport->url); + if (r > 0) + warning(_("Option \"%s\" is ignored for %s\n"), + name, transport->url); +} + +struct transport *prepare_transport(struct remote *remote) +{ + struct transport *transport; + transport = transport_get(remote, NULL); + transport_set_verbosity(transport, verbosity, progress); + if (upload_pack) + set_option(transport, TRANS_OPT_UPLOADPACK, upload_pack); + if (keep) + set_option(transport, TRANS_OPT_KEEP, "yes"); + if (depth) + set_option(transport, TRANS_OPT_DEPTH, depth); + return transport; +} + static int do_fetch(struct transport *transport, struct refspec *refs, int ref_count) { @@ -816,17 +841,6 @@ static int do_fetch(struct transport *transport, return retcode; } -static void set_option(const char *name, const char *value) -{ - int r = transport_set_option(gtransport, name, value); - if (r < 0) - die(_("Option \"%s\" value \"%s\" is not valid for %s"), - name, value, gtransport->url); - if (r > 0) - warning(_("Option \"%s\" is ignored for %s\n"), - name, gtransport->url); -} - static int get_one_remote_for_fetch(struct remote *remote, void *priv) { struct string_list *list = priv; @@ -949,15 +963,7 @@ static int fetch_one(struct remote *remote, int argc, const char **argv) die(_("No remote repository specified. Please, specify either a URL or a\n" "remote name from which new revisions should be fetched.")); - gtransport = transport_get(remote, NULL); - transport_set_verbosity(gtransport, verbosity, progress); - if (upload_pack) - set_option(TRANS_OPT_UPLOADPACK, upload_pack); - if (keep) - set_option(TRANS_OPT_KEEP, "yes"); - if (depth) - set_option(TRANS_OPT_DEPTH, depth); - + gtransport = prepare_transport(remote); if (argc > 0) { int j = 0; refs = xcalloc(argc + 1, sizeof(const char *)); -- cgit v1.2.1 From 069d50320257b76504921a7bc77696a6d858bfec Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 7 Aug 2013 15:14:45 -0700 Subject: fetch: refactor code that fetches leftover tags Usually the upload-pack process running on the other side will give us all the reachable tags we need during the primary object transfer in do_fetch(). If that does not happen (e.g. the other side may be running a third-party implementation of upload-pack), we will run another fetch to pick up leftover tags that we know point at the commits reachable from our updated tips. Separate out the code to run this second fetch into a helper function. Signed-off-by: Junio C Hamano --- builtin/fetch.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) (limited to 'builtin/fetch.c') diff --git a/builtin/fetch.c b/builtin/fetch.c index 39a3fc8dea..0b21f071b3 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -745,6 +745,13 @@ struct transport *prepare_transport(struct remote *remote) return transport; } +static void backfill_tags(struct transport *transport, struct ref *ref_map) +{ + transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL); + transport_set_option(transport, TRANS_OPT_DEPTH, "0"); + fetch_refs(transport, ref_map); +} + static int do_fetch(struct transport *transport, struct refspec *refs, int ref_count) { @@ -828,11 +835,8 @@ static int do_fetch(struct transport *transport, struct ref **tail = &ref_map; ref_map = NULL; find_non_local_tags(transport, &ref_map, &tail); - if (ref_map) { - transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL); - transport_set_option(transport, TRANS_OPT_DEPTH, "0"); - fetch_refs(transport, ref_map); - } + if (ref_map) + backfill_tags(transport, ref_map); free_refs(ref_map); } -- cgit v1.2.1 From b26ed4305f9e7043133e52990897afbbf1808d6d Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 7 Aug 2013 15:47:18 -0700 Subject: 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 --- builtin/fetch.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'builtin/fetch.c') diff --git a/builtin/fetch.c b/builtin/fetch.c index 0b21f071b3..57ab7e4d63 100644 --- 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 struct transport *gsecondary; 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 (gsecondary) + transport_unlock_pack(gsecondary); } 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) { + 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); + + if (gsecondary) { + transport_disconnect(gsecondary); + gsecondary = NULL; + } } static int do_fetch(struct transport *transport, -- cgit v1.2.1 From 0f73f8bd7974fcf7f9e4608875323c96c6159829 Mon Sep 17 00:00:00 2001 From: Ramsay Jones Date: Wed, 28 Aug 2013 19:56:17 +0100 Subject: builtin/fetch.c: Fix a sparse warning Sparse issues an "'prepare_transport' was not declared. Should it be static?" warning. In order to suppress the warning, since this symbol only requires file scope, we simply add the static modifier to it's declaration. Signed-off-by: Ramsay Jones Signed-off-by: Junio C Hamano --- builtin/fetch.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'builtin/fetch.c') diff --git a/builtin/fetch.c b/builtin/fetch.c index 57ab7e4d63..564705555b 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -734,7 +734,7 @@ static void set_option(struct transport *transport, const char *name, const char name, transport->url); } -struct transport *prepare_transport(struct remote *remote) +static struct transport *prepare_transport(struct remote *remote) { struct transport *transport; transport = transport_get(remote, NULL); -- cgit v1.2.1