{"thread":{"id":"22669","subject":"git fetch origin hanging in 1.7.0","startedAt":"2010-02-16T00:08:25Z","lastAt":"2010-02-16T13:20:40Z","messageCount":7,"participants":["Kevin Menard","Jeff King","Tay Ray Chuan","Ilari Liusvaara"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"134681","messageId":"7e3605161002151608t44bd320cgcd589796a9ec902b@mail.gmail.com","threadId":"22669","inReplyTo":null,"subject":"git fetch origin hanging in 1.7.0","fromName":"Kevin Menard","fromEmail":"nirvdrum@gmail.com","sentAt":"2010-02-16T00:08:25Z","receivedAt":"2010-02-16T00:08:25Z","isPatch":false,"sender":{"key":"nirvdrum@gmail.com","avatar":"https://gravatar.com/avatar/96621fc3018e7b95ed0e4fc67d2898f3ab35c4805a69d48f023d80e2e6151f90?d=mp&s=160"},"body":"Hi,\n\nI've run into an issue that doing a \"git fetch origin\" is hanging for\nme in git 1.7.0. The setup here may be wrong, so I don't want to call\nit a bug. But basically I have an empty repo created on the server\nthat a client then clones. There's a job that fetches new updates to\nthe server periodically. If nothing has been pushed to the server\nyet, the client fetch hangs.\n\nThis worked fine in 1.6.6, so I'll just roll back my version for now.\nBut any help on how to do this different would be much appreciated.\n\n-- \nThanks,\nKevin\n"},{"id":"134703","messageId":"20100216063959.GC2169@coredump.intra.peff.net","threadId":"22669","inReplyTo":"7e3605161002151608t44bd320cgcd589796a9ec902b@mail.gmail.com","subject":"Re: git fetch origin hanging in 1.7.0","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-02-16T06:39:59Z","receivedAt":"2010-02-16T06:39:59Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 15, 2010 at 07:08:25PM -0500, Kevin Menard wrote:\n\n> I've run into an issue that doing a \"git fetch origin\" is hanging for\n> me in git 1.7.0. The setup here may be wrong, so I don't want to call\n> it a bug. But basically I have an empty repo created on the server\n> that a client then clones. There's a job that fetches new updates to\n> the server periodically. If nothing has been pushed to the server\n> yet, the client fetch hangs.\n> \n> This worked fine in 1.6.6, so I'll just roll back my version for now.\n> But any help on how to do this different would be much appreciated.\n\nNo, what you're doing should work OK (though it does actually produce\nugly \"the remote end hung up unexpectedly\" messages in v1.6.6).\n\nI can reproduce the bug with:\n\n $ mkdir foo && (cd foo && git init)\n $ git clone foo bar\n Initialized empty Git repository in /home/peff/bar/.git/\n warning: You appear to have cloned an empty repository.\n $ cd bar && git fetch\n\nwhich hangs on a pipe read(). It bisects to 61b075b (Support taking over\ntransports, 2009-12-09) from Ilari (cc'd). It looks like we get the\nempty ref list once in get_remote_heads, and then try to get it again\nand hang. Maybe we need this?\n\ndiff --git a/transport.c b/transport.c\nindex ad25b98..e6f9464 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -1010,7 +1010,8 @@ int transport_push(struct transport *transport,\n \n const struct ref *transport_get_remote_refs(struct transport *transport)\n {\n-\tif (!transport->remote_refs)\n+\tstruct git_transport_data *data = transport->data;\n+\tif (!data->got_remote_heads)\n \t\ttransport->remote_refs = transport->get_refs_list(transport, 0);\n \n \treturn transport->remote_refs;\n\nThat fixes the problem for me, but I am totally clueless about this\ncode. Do all transports have a git_transport_data (if so, then why is it\na void pointer?).\n\n-Peff\n"},{"id":"134709","messageId":"20100216151821.994ace31.rctay89@gmail.com","threadId":"22669","inReplyTo":"20100216063959.GC2169@coredump.intra.peff.net","subject":"Re: git fetch origin hanging in 1.7.0","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-02-16T07:18:21Z","receivedAt":"2010-02-16T07:18:21Z","isPatch":false,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Tue, 16 Feb 2010 01:39:59 -0500\nJeff King wrote:\n> I can reproduce the bug with:\n> \n> $ mkdir foo && (cd foo && git init)\n> $ git clone foo bar\n> Initialized empty Git repository in /home/peff/bar/.git/\n> warning: You appear to have cloned an empty repository.\n> $ cd bar && git fetch\n> \n> which hangs on a pipe read(). It bisects to 61b075b (Support taking over\n> transports, 2009-12-09) from Ilari (cc'd). It looks like we get the\n> empty ref list once in get_remote_heads, and then try to get it again\n> and hang.\n\nI agree with the diagnosis. With gdb, I find that program execution\nhangs on the packet_read_line() call in connect.c::get_remote_heads().\n\n> diff --git a/transport.c b/transport.c\n> index ad25b98..e6f9464 100644\n> --- a/transport.c\n> +++ b/transport.c\n> @@ -1010,7 +1010,8 @@ int transport_push(struct transport *transport,\n> \n> const struct ref *transport_get_remote_refs(struct transport *transport)\n> {\n> -\tif (!transport->remote_refs)\n> +\tstruct git_transport_data *data = transport->data;\n> +\tif (!data->got_remote_heads)\n> \t\ttransport->remote_refs = transport->get_refs_list(transport, 0);\n> \n> \treturn transport->remote_refs;\n\nNAK. This will only work if the given transport is git://. (My take\nbelow.)\n\n> That fixes the problem for me, but I am totally clueless about this\n> code. Do all transports have a git_transport_data (if so, then why is it\n> a void pointer?).\n\nIt is void so that it can be any struct - for example,\ngit_transport_data, bundle_transport_data. That way, transport->data\ncan point to any transport-specific data, while keeping the compiler\nsatisfied at compile-time.\n\n-- >8 --\nSubject: [PATCH] transport: add got_remote_refs flag\n\ntranport.c::transport_get_remote_refs() used to check\ntransport->remote_refs to determine whether transport->get_refs_list()\nshould be invoked.\n\nHowever, transport->remote_refs could evaluate to false (eg. if it is\nNULL), causingo transport->get_refs_list() to be invoked unnecessarily.\n\nIntroduce a flag, transport->got_remote_refs, and make\ntranport.c::transport_get_remote_refs() check this flag rather than\nevaluating transport->remote_refs.\n\nSigned-off-by: Tay Ray Chuan \n---\n transport.c | 5 ++++-\n transport.h | 6 ++++++\n 2 files changed, 10 insertions(+), 1 deletions(-)\n\ndiff --git a/transport.c b/transport.c\nindex 3846aac..08e4fa0 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -918,6 +918,7 @@ struct transport *transport_get(struct remote *remote, const char *url)\n \tif (!remote)\n \t\tdie(\"No remote provided to transport_get()\");\n \n+\tret->got_remote_refs = 0;\n \tret->remote = remote;\n \thelper = remote->foreign_vcs;\n \n@@ -1079,8 +1080,10 @@ int transport_push(struct transport *transport,\n \n const struct ref *transport_get_remote_refs(struct transport *transport)\n {\n-\tif (!transport->remote_refs)\n+\tif (!transport->got_remote_refs) {\n \t\ttransport->remote_refs = transport->get_refs_list(transport, 0);\n+\t\ttransport->got_remote_refs = 1;\n+\t}\n \n \treturn transport->remote_refs;\n }\ndiff --git a/transport.h b/transport.h\nindex 7cea5cc..1cca13b 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -20,6 +20,12 @@ struct transport {\n \tconst struct ref *remote_refs;\n \n \t/**\n+\t * Indicates whether the remote_refs member has been set. Set by\n+\t * transport.c::transport_get_remote_refs().\n+\t */\n+\tunsigned got_remote_refs : 1;\n+\n+\t/**\n \t * Returns 0 if successful, positive if the option is not\n \t * recognized or is inapplicable, and negative if the option\n \t * is applicable but the value is invalid.\n-- \n1.7.0.1.gcd472.dirty\n\n-- \nCheers,\nRay Chuan\n"},{"id":"134711","messageId":"20100216072456.GG2169@coredump.intra.peff.net","threadId":"22669","inReplyTo":"20100216151821.994ace31.rctay89@gmail.com","subject":"Re: git fetch origin hanging in 1.7.0","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-02-16T07:24:56Z","receivedAt":"2010-02-16T07:24:56Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 16, 2010 at 03:18:21PM +0800, Tay Ray Chuan wrote:\n\n> > That fixes the problem for me, but I am totally clueless about this\n> > code. Do all transports have a git_transport_data (if so, then why is it\n> > a void pointer?).\n> \n> It is void so that it can be any struct - for example,\n> git_transport_data, bundle_transport_data. That way, transport->data\n> can point to any transport-specific data, while keeping the compiler\n> satisfied at compile-time.\n\nOK, I figured it was something like that. In that case, your fix looks\nlike the right thing to do (and it fixes my test case).\n\nThanks.\n\n> -- >8 --\n> Subject: [PATCH] transport: add got_remote_refs flag\n\nAcked-by: Jeff King \n\n-Peff\n"},{"id":"134713","messageId":"be6fef0d1002152332k2983d750l7b76496118dc3952@mail.gmail.com","threadId":"22669","inReplyTo":"20100216072456.GG2169@coredump.intra.peff.net","subject":"Re: git fetch origin hanging in 1.7.0","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-02-16T07:32:43Z","receivedAt":"2010-02-16T07:32:43Z","isPatch":false,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Tue, Feb 16, 2010 at 3:24 PM, Jeff King wrote:\n> OK, I figured it was something like that. In that case, your fix looks\n> like the right thing to do (and it fixes my test case).\n>\n> Thanks.\n\nNo problem.\n\nKevin, I didn't add a Reported-by line, as I don't regard my patch as\nactually fixing the issue at hand (hanging read) - it just side-steps\nthe issue. Hope you don't mind this.\n\n-- \nCheers,\nRay Chuan\n"},{"id":"134730","messageId":"20100216122759.GA30898@Knoppix","threadId":"22669","inReplyTo":"20100216151821.994ace31.rctay89@gmail.com","subject":"Re: git fetch origin hanging in 1.7.0","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2010-02-16T12:27:59Z","receivedAt":"2010-02-16T12:27:59Z","isPatch":false,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"On Tue, Feb 16, 2010 at 03:18:21PM +0800, Tay Ray Chuan wrote:\n> NAK. This will only work if the given transport is git://. (My take\n> below.)\n> \n> \n> -- >8 --\n> Subject: [PATCH] transport: add got_remote_refs flag\n> \n> tranport.c::transport_get_remote_refs() used to check\n> transport->remote_refs to determine whether transport->get_refs_list()\n> should be invoked.\n> \n> However, transport->remote_refs could evaluate to false (eg. if it is\n> NULL), causingo transport->get_refs_list() to be invoked unnecessarily.\n> \n> Introduce a flag, transport->got_remote_refs, and make\n> tranport.c::transport_get_remote_refs() check this flag rather than\n> evaluating transport->remote_refs.\n\n\n\nSeems to work even in external transport case (not suprising, as at this\nlevel, there's no difference between git://, ssh://, file:// and external\ntransports).\n\n-Ilari\n"},{"id":"134732","messageId":"7e3605161002160520tf0509b1u700a8b9c7e39b3a@mail.gmail.com","threadId":"22669","inReplyTo":"be6fef0d1002152332k2983d750l7b76496118dc3952@mail.gmail.com","subject":"Re: git fetch origin hanging in 1.7.0","fromName":"Kevin Menard","fromEmail":"nirvdrum@gmail.com","sentAt":"2010-02-16T13:20:40Z","receivedAt":"2010-02-16T13:20:40Z","isPatch":false,"sender":{"key":"nirvdrum@gmail.com","avatar":"https://gravatar.com/avatar/96621fc3018e7b95ed0e4fc67d2898f3ab35c4805a69d48f023d80e2e6151f90?d=mp&s=160"},"body":"On Tue, Feb 16, 2010 at 2:32 AM, Tay Ray Chuan wrote:\n\n> Kevin, I didn't add a Reported-by line, as I don't regard my patch as\n> actually fixing the issue at hand (hanging read) - it just side-steps\n> the issue. Hope you don't mind this.\n\n\nNo worries. I'm just grateful someone more knowledgeable than me\ncould take a look, confirm it was an issue, and supply a patch.\nThanks to others that chimed in as well.\n\n-- \nKevin\n"}]}