summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJunio C Hamano <gitster@pobox.com>2013-09-09 14:50:37 -0700
committerJunio C Hamano <gitster@pobox.com>2013-09-09 14:50:37 -0700
commit20419de969b8ce0c49d133affdb1bb73eb62815e (patch)
tree8dd2f6f1b4adc1b2beee854ab1bce32a4f6dd48f
parent3b30ba55e4c14c6939a201e87d3542e157e2b318 (diff)
parent0f73f8bd7974fcf7f9e4608875323c96c6159829 (diff)
downloadgit-20419de969b8ce0c49d133affdb1bb73eb62815e.tar.gz
Merge branch 'jc/transport-do-not-use-connect-twice-in-fetch'
The auto-tag-following code in "git fetch" tries to reuse the same transport twice when the serving end does not cooperate and does not give tags that point to commits that are asked for as part of the primary transfer. Unfortunately, Git-aware transport helper interface is not designed to be used more than once, hence this does not work over smart-http transfer. * jc/transport-do-not-use-connect-twice-in-fetch: builtin/fetch.c: Fix a sparse warning fetch: work around "transport-take-over" hack fetch: refactor code that fetches leftover tags fetch: refactor code that prepares a transport fetch: rename file-scope global "transport" to "gtransport" t5802: add test for connect helper
-rw-r--r--builtin/fetch.c89
-rwxr-xr-xt/t5802-connect-helper.sh72
-rw-r--r--transport.c2
-rw-r--r--transport.h6
4 files changed, 136 insertions, 33 deletions
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 5936539552..9e654efa3b 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -40,7 +40,8 @@ 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 struct transport *gsecondary;
static const char *submodule_prefix = "";
static const char *recurse_submodules_default;
@@ -108,8 +109,10 @@ static struct option builtin_fetch_options[] = {
static void unlock_pack(void)
{
- if (transport)
- transport_unlock_pack(transport);
+ if (gtransport)
+ transport_unlock_pack(gtransport);
+ if (gsecondary)
+ transport_unlock_pack(gsecondary);
}
static void unlock_pack_on_signal(int signo)
@@ -733,6 +736,48 @@ 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);
+}
+
+static 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 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,
struct refspec *refs, int ref_count)
{
@@ -819,11 +864,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);
}
@@ -832,17 +874,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(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);
-}
-
static int get_one_remote_for_fetch(struct remote *remote, void *priv)
{
struct string_list *list = priv;
@@ -965,26 +996,18 @@ 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);
+ gtransport = prepare_transport(remote);
if (prune < 0) {
/* no command line request */
- if (0 <= transport->remote->prune)
- prune = transport->remote->prune;
+ if (0 <= gtransport->remote->prune)
+ prune = gtransport->remote->prune;
else if (0 <= fetch_prune_config)
prune = fetch_prune_config;
else
prune = PRUNE_BY_DEFAULT;
}
- transport_set_verbosity(transport, 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);
-
if (argc > 0) {
int j = 0;
refs = xcalloc(argc + 1, sizeof(const char *));
@@ -1010,10 +1033,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;
}
diff --git a/t/t5802-connect-helper.sh b/t/t5802-connect-helper.sh
new file mode 100755
index 0000000000..878faf2b63
--- /dev/null
+++ b/t/t5802-connect-helper.sh
@@ -0,0 +1,72 @@
+#!/bin/sh
+
+test_description='ext::cmd remote "connect" helper'
+. ./test-lib.sh
+
+test_expect_success setup '
+ test_tick &&
+ git commit --allow-empty -m initial &&
+ test_tick &&
+ git commit --allow-empty -m second &&
+ test_tick &&
+ git commit --allow-empty -m third &&
+ test_tick &&
+ git tag -a -m "tip three" three &&
+
+ test_tick &&
+ git commit --allow-empty -m fourth
+'
+
+test_expect_success clone '
+ cmd=$(echo "echo >&2 ext::sh invoked && %S .." | sed -e "s/ /% /g") &&
+ git clone "ext::sh -c %S% ." dst &&
+ git for-each-ref refs/heads/ refs/tags/ >expect &&
+ (
+ cd dst &&
+ git config remote.origin.url "ext::sh -c $cmd" &&
+ git for-each-ref refs/heads/ refs/tags/
+ ) >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'update following tag' '
+ test_tick &&
+ git commit --allow-empty -m fifth &&
+ test_tick &&
+ git tag -a -m "tip five" five &&
+ git for-each-ref refs/heads/ refs/tags/ >expect &&
+ (
+ cd dst &&
+ git pull &&
+ git for-each-ref refs/heads/ refs/tags/ >../actual
+ ) &&
+ test_cmp expect actual
+'
+
+test_expect_success 'update backfilled tag' '
+ test_tick &&
+ git commit --allow-empty -m sixth &&
+ test_tick &&
+ git tag -a -m "tip two" two three^1 &&
+ git for-each-ref refs/heads/ refs/tags/ >expect &&
+ (
+ cd dst &&
+ git pull &&
+ git for-each-ref refs/heads/ refs/tags/ >../actual
+ ) &&
+ test_cmp expect actual
+'
+
+test_expect_success 'update backfilled tag without primary transfer' '
+ test_tick &&
+ git tag -a -m "tip one " one two^1 &&
+ git for-each-ref refs/heads/ refs/tags/ >expect &&
+ (
+ cd dst &&
+ git pull &&
+ git for-each-ref refs/heads/ refs/tags/ >../actual
+ ) &&
+ test_cmp expect actual
+'
+
+test_done
diff --git a/transport.c b/transport.c
index b321d6a49d..7202b7777d 100644
--- a/transport.c
+++ b/transport.c
@@ -881,6 +881,8 @@ void transport_take_over(struct transport *transport,
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)
diff --git a/transport.h b/transport.h
index 10f7556001..8f96bed775 100644
--- a/transport.h
+++ b/transport.h
@@ -29,6 +29,12 @@ struct transport {
*/
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