Re: git fetch origin hanging in 1.7.0
Hi,
On Tue, 16 Feb 2010 01:39:59 -0500 Jeff King <peff@peff.net> wrote:
Show 12 quoted lines
> I can reproduce the bug with:
>
> $ mkdir foo && (cd foo && git init)
> $ git clone foo bar
> Initialized empty Git repository in /home/peff/bar/.git/
> warning: You appear to have cloned an empty repository.
> $ cd bar && git fetch
>
> which hangs on a pipe read(). It bisects to 61b075b (Support taking over
> transports, 2009-12-09) from Ilari (cc'd). It looks like we get the
> empty ref list once in get_remote_heads, and then try to get it again
> and hang.
I agree with the diagnosis. With gdb, I find that program execution hangs on the packet_read_line() call in connect.c::get_remote_heads().
Show 14 quoted lines
> diff --git a/transport.c b/transport.c
> index ad25b98..e6f9464 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -1010,7 +1010,8 @@ int transport_push(struct transport *transport,
>
> const struct ref *transport_get_remote_refs(struct transport *transport)
> {
> - if (!transport->remote_refs)
> + struct git_transport_data *data = transport->data;
> + if (!data->got_remote_heads)
> transport->remote_refs = transport->get_refs_list(transport, 0);
>
> return transport->remote_refs;NAK. This will only work if the given transport is git://. (My take below.)
> That fixes the problem for me, but I am totally clueless about this
> code. Do all transports have a git_transport_data (if so, then why is it
> a void pointer?).
It is void so that it can be any struct - for example, git_transport_data, bundle_transport_data. That way, transport->data can point to any transport-specific data, while keeping the compiler satisfied at compile-time.
-- >8 --
Subject: [PATCH] transport: add got_remote_refs flag
tranport.c::transport_get_remote_refs() used to check transport->remote_refs to determine whether transport->get_refs_list() should be invoked.
However, transport->remote_refs could evaluate to false (eg. if it is NULL), causingo transport->get_refs_list() to be invoked unnecessarily.
Introduce a flag, transport->got_remote_refs, and make tranport.c::transport_get_remote_refs() check this flag rather than evaluating transport->remote_refs.
Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
transport.c | 5 ++++-
transport.h | 6 ++++++
2 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/transport.c b/transport.c
index 3846aac..08e4fa0 100644
--- a/transport.c
+++ b/transport.c
@@ -918,6 +918,7 @@ struct transport *transport_get(struct remote *remote, const char *url)
if (!remote)
die("No remote provided to transport_get()");
+ ret->got_remote_refs = 0;
ret->remote = remote;
helper = remote->foreign_vcs;
@@ -1079,8 +1080,10 @@ int transport_push(struct transport *transport,
const struct ref *transport_get_remote_refs(struct transport *transport)
{
- if (!transport->remote_refs)
+ if (!transport->got_remote_refs) {
transport->remote_refs = transport->get_refs_list(transport, 0);
+ transport->got_remote_refs = 1;
+ }
return transport->remote_refs;
}
diff --git a/transport.h b/transport.h
index 7cea5cc..1cca13b 100644
--- a/transport.h
+++ b/transport.h
@@ -20,6 +20,12 @@ struct transport {
const struct ref *remote_refs;
/**
+ * Indicates whether the remote_refs member has been set. Set by
+ * transport.c::transport_get_remote_refs().
+ */
+ unsigned got_remote_refs : 1;
+
+ /**
* Returns 0 if successful, positive if the option is not
* recognized or is inapplicable, and negative if the option
* is applicable but the value is invalid.
--
1.7.0.1.gcd472.dirty
--
Cheers,
Ray Chuan
Kevin Menard
