{"thread":{"id":"20798","subject":"clong an empty repo over ssh causes (harmless) fatal","startedAt":"2009-08-31T11:14:22Z","lastAt":"2009-09-04T02:30:13Z","messageCount":25,"participants":["Sitaram Chamarty","Matthieu Moy","Jeff King","Sverre Rabbelier","Björn Steinbrink","Daniel Barkalow","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"122180","messageId":"slrnh9nc4e.6bn.sitaramc@sitaramc.homelinux.net","threadId":"20798","inReplyTo":null,"subject":"clong an empty repo over ssh causes (harmless) fatal","fromName":"Sitaram Chamarty","fromEmail":"sitaramc@gmail.com","sentAt":"2009-08-31T11:14:22Z","receivedAt":"2009-08-31T11:14:22Z","isPatch":false,"sender":{"key":"sitaramc@gmail.com","avatar":"https://avatars.githubusercontent.com/u/43316?v=4"},"body":"Hello,\n\nCloning an empty repo seems to produce a spurious error.\nThe repo still seems fine though.\n\nAny thoughts?\n\n    $ git clone ssh://sitaram@localhost/home/sitaram/t/a b\n    Initialized empty Git repository in /home/sitaram/t/b/.git/\n    sitaram@localhost's password:\n    warning: You appear to have cloned an empty repository.\n    fatal: The remote end hung up unexpectedly\n\nThanks,\n\nSitaram\n"},{"id":"122182","messageId":"vpqskf8z0rj.fsf@bauges.imag.fr","threadId":"20798","inReplyTo":"slrnh9nc4e.6bn.sitaramc@sitaramc.homelinux.net","subject":"Re: clong an empty repo over ssh causes (harmless) fatal","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2009-08-31T11:22:08Z","receivedAt":"2009-08-31T11:22:08Z","isPatch":false,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Sitaram Chamarty <sitaramc@gmail.com> writes:\n\n> Hello,\n>\n> Cloning an empty repo seems to produce a spurious error.\n> The repo still seems fine though.\n\nCan't reproduce here:\n\n$ git clone ssh://.../tmp/empty cloned\nInitialized empty Git repository in /tmp/cloned/.git/\nwarning: You appear to have cloned an empty repository.\n$ git --version\ngit version 1.6.4.187.gd399.dirty\n\nMaybe you have an older version of Git?\n\n-- \nMatthieu\n"},{"id":"122188","messageId":"2e24e5b90908310730u53ee27d3nd2b66c1f58202e7@mail.gmail.com","threadId":"20798","inReplyTo":"vpqskf8z0rj.fsf@bauges.imag.fr","subject":"Re: clong an empty repo over ssh causes (harmless) fatal","fromName":"Sitaram Chamarty","fromEmail":"sitaramc@gmail.com","sentAt":"2009-08-31T14:30:41Z","receivedAt":"2009-08-31T14:30:41Z","isPatch":false,"sender":{"key":"sitaramc@gmail.com","avatar":"https://avatars.githubusercontent.com/u/43316?v=4"},"body":"[apologies if you get this twice]\n\nOn Mon, Aug 31, 2009 at 4:52 PM, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:\n>\n> Sitaram Chamarty <sitaramc@gmail.com> writes:\n>\n> > Hello,\n> >\n> > Cloning an empty repo seems to produce a spurious error.\n> > The repo still seems fine though.\n>\n> Can't reproduce here:\n>\n> $ git clone ssh://.../tmp/empty cloned\n> Initialized empty Git repository in /tmp/cloned/.git/\n> warning: You appear to have cloned an empty repository.\n> $ git --version\n> git version 1.6.4.187.gd399.dirty\n>\n> Maybe you have an older version of Git?\n\nHad 1.6.4, just tried with 1.6.4.2 -- the error is still there, exactly so.\n\nAnything I can do to provide more info?\n\nTo start with, this is Mandriva 2009.1, kernel 2.6.29, git built from\nscratch locally (not an RPM or something).  Should I try some other\nthings like strace?  It's too minor to bother, if that's all it is,\nassuming it's not a symptom of something larger :)\n\n\n>\n> --\n> Matthieu\n"},{"id":"122189","messageId":"vpq4oroqbvc.fsf@bauges.imag.fr","threadId":"20798","inReplyTo":"2e24e5b90908310730u53ee27d3nd2b66c1f58202e7@mail.gmail.com","subject":"Re: clong an empty repo over ssh causes (harmless) fatal","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2009-08-31T14:47:03Z","receivedAt":"2009-08-31T14:47:03Z","isPatch":false,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Sitaram Chamarty <sitaramc@gmail.com> writes:\n\n> To start with, this is Mandriva 2009.1, kernel 2.6.29, git built from\n> scratch locally (not an RPM or something).  Should I try some other\n> things like strace?\n\nSetting $GIT_TRACE to 2 can help, too. The problem is that the git\ninstance you want to debug is the remote one, I don't know if there's\na better way to do that than putting a wrapper script for\ngit-upload-pack ...\n\n-- \nMatthieu\n"},{"id":"122190","messageId":"20090831164146.GA23245@coredump.intra.peff.net","threadId":"20798","inReplyTo":"2e24e5b90908310730u53ee27d3nd2b66c1f58202e7@mail.gmail.com","subject":"Re: clong an empty repo over ssh causes (harmless) fatal","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-31T16:41:46Z","receivedAt":"2009-08-31T16:41:46Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 31, 2009 at 08:00:41PM +0530, Sitaram Chamarty wrote:\n\n> > Maybe you have an older version of Git?\n> \n> Had 1.6.4, just tried with 1.6.4.2 -- the error is still there, exactly so.\n> \n> Anything I can do to provide more info?\n\nIIRC, the message you are seeing comes when the _server_ is an older\nversion of git. It is harmless, though.\n\n-Peff\n"},{"id":"122192","messageId":"fabb9a1e0908311012q4cea2d51i2c2f0cbceac0cab@mail.gmail.com","threadId":"20798","inReplyTo":"20090831164146.GA23245@coredump.intra.peff.net","subject":"Re: clong an empty repo over ssh causes (harmless) fatal","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-08-31T17:12:44Z","receivedAt":"2009-08-31T17:12:44Z","isPatch":false,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Mon, Aug 31, 2009 at 18:41, Jeff King<peff@peff.net> wrote:\n> IIRC, the message you are seeing comes when the _server_ is an older\n> version of git. It is harmless, though.\n\nMhhhh, is it some weird interaction between 'empty repository' patch\nand old server versions, or did this happen too before my patch was\napplied?\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"122193","messageId":"vpqocpvevzx.fsf@bauges.imag.fr","threadId":"20798","inReplyTo":"20090831164146.GA23245@coredump.intra.peff.net","subject":"Re: clong an empty repo over ssh causes (harmless) fatal","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2009-08-31T17:25:22Z","receivedAt":"2009-08-31T17:25:22Z","isPatch":false,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Mon, Aug 31, 2009 at 08:00:41PM +0530, Sitaram Chamarty wrote:\n>\n>> > Maybe you have an older version of Git?\n>> \n>> Had 1.6.4, just tried with 1.6.4.2 -- the error is still there, exactly so.\n>> \n>> Anything I can do to provide more info?\n>\n> IIRC, the message you are seeing comes when the _server_ is an older\n> version of git. It is harmless, though.\n\nSince the client and server are the same machine:\n\n    $ git clone ssh://sitaram@localhost/home/sitaram/t/a b\n\nI'd bet Sitaram has two installations of git, and plain ssh to the\nmachine points to the old one (like a $PATH set in ~/.login and not\n~/.profile or something like that).\n\n-- \nMatthieu\n"},{"id":"122196","messageId":"20090831190827.GA4876@sigill.intra.peff.net","threadId":"20798","inReplyTo":"fabb9a1e0908311012q4cea2d51i2c2f0cbceac0cab@mail.gmail.com","subject":"Re: clong an empty repo over ssh causes (harmless) fatal","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-31T19:08:27Z","receivedAt":"2009-08-31T19:08:27Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 31, 2009 at 07:12:44PM +0200, Sverre Rabbelier wrote:\n\n> On Mon, Aug 31, 2009 at 18:41, Jeff King<peff@peff.net> wrote:\n> > IIRC, the message you are seeing comes when the _server_ is an older\n> > version of git. It is harmless, though.\n> \n> Mhhhh, is it some weird interaction between 'empty repository' patch\n> and old server versions, or did this happen too before my patch was\n> applied?\n\nI think the former. I thought it was discussed before, but the only\nreference I can find is this (see the end of the email):\n\n  http://article.gmane.org/gmane.comp.version-control.git/107626\n\nand I don't see any followup for that specific part of the mail.\n\n-Peff\n"},{"id":"122197","messageId":"fabb9a1e0908311209y4c83b60eg75ced6d3db615dff@mail.gmail.com","threadId":"20798","inReplyTo":"20090831190827.GA4876@sigill.intra.peff.net","subject":"Re: clong an empty repo over ssh causes (harmless) fatal","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-08-31T19:09:47Z","receivedAt":"2009-08-31T19:09:47Z","isPatch":false,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Mon, Aug 31, 2009 at 21:08, Jeff King<peff@peff.net> wrote:\n> I think the former. I thought it was discussed before, but the only\n> reference I can find is this (see the end of the email):\n>\n>  http://article.gmane.org/gmane.comp.version-control.git/107626\n\nAh, yeup, I see.\n\n> and I don't see any followup for that specific part of the mail.\n\nI don't remember any follow up to that either, shame on me :(.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"122198","messageId":"20090831191032.GB4876@sigill.intra.peff.net","threadId":"20798","inReplyTo":"vpqocpvevzx.fsf@bauges.imag.fr","subject":"Re: clong an empty repo over ssh causes (harmless) fatal","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-31T19:10:32Z","receivedAt":"2009-08-31T19:10:32Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 31, 2009 at 07:25:22PM +0200, Matthieu Moy wrote:\n\n> Since the client and server are the same machine:\n> \n>     $ git clone ssh://sitaram@localhost/home/sitaram/t/a b\n> \n> I'd bet Sitaram has two installations of git, and plain ssh to the\n> machine points to the old one (like a $PATH set in ~/.login and not\n> ~/.profile or something like that).\n\nOh, indeed. I didn't notice that his host was @localhost. :)\n\nBut yes, that would be my guess, as well. Trying \"ssh sitaram@localhost\ngit version\" would be a good clue.\n\n-Peff\n"},{"id":"122201","messageId":"20090831201911.GA24989@atjola.homenet","threadId":"20798","inReplyTo":"20090831191032.GB4876@sigill.intra.peff.net","subject":"Re: clong an empty repo over ssh causes (harmless) fatal","fromName":"Björn Steinbrink","fromEmail":"b.steinbrink@gmx.de","sentAt":"2009-08-31T20:19:11Z","receivedAt":"2009-08-31T20:19:11Z","isPatch":false,"sender":{"key":"b.steinbrink@gmx.de","avatar":"https://avatars.githubusercontent.com/u/230962?v=4"},"body":"On 2009.08.31 15:10:32 -0400, Jeff King wrote:\n> On Mon, Aug 31, 2009 at 07:25:22PM +0200, Matthieu Moy wrote:\n> \n> > Since the client and server are the same machine:\n> > \n> >     $ git clone ssh://sitaram@localhost/home/sitaram/t/a b\n> > \n> > I'd bet Sitaram has two installations of git, and plain ssh to the\n> > machine points to the old one (like a $PATH set in ~/.login and not\n> > ~/.profile or something like that).\n> \n> Oh, indeed. I didn't notice that his host was @localhost. :)\n> \n> But yes, that would be my guess, as well. Trying \"ssh sitaram@localhost\n> git version\" would be a good clue.\n\nI see the problem here, too.\n\ndoener@atjola:~ $ (mkdir a; cd a; git init)\nInitialized empty Git repository in /home/doener/a/.git/\n\ndoener@atjola:~ $ git clone localhost:a b\nInitialized empty Git repository in /home/doener/b/.git/\nwarning: You appear to have cloned an empty repository.\nfatal: The remote end hung up unexpectedly\n\ndoener@atjola:~ $ ssh localhost git --version\ngit version 1.6.4.2.236.gf324c\n\nBjörn\n"},{"id":"122207","messageId":"20090831224749.GA24190@sigill.intra.peff.net","threadId":"20798","inReplyTo":"20090831201911.GA24989@atjola.homenet","subject":"Re: clong an empty repo over ssh causes (harmless) fatal","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-31T22:47:49Z","receivedAt":"2009-08-31T22:47:49Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 31, 2009 at 10:19:11PM +0200, Björn Steinbrink wrote:\n\n> I see the problem here, too.\n> \n> doener@atjola:~ $ (mkdir a; cd a; git init)\n> Initialized empty Git repository in /home/doener/a/.git/\n> \n> doener@atjola:~ $ git clone localhost:a b\n> Initialized empty Git repository in /home/doener/b/.git/\n> warning: You appear to have cloned an empty repository.\n> fatal: The remote end hung up unexpectedly\n> \n> doener@atjola:~ $ ssh localhost git --version\n> git version 1.6.4.2.236.gf324c\n\nOK, it is definitely not about mixed versions, and it is definitely\nreproducible, even without ssh. The local clone optimization manages to\navoid it, but you can see it with:\n\n  git clone file://$PWD/a b\n\nIt also happens with git://, except that it is the _remote_ side\nproducing the message, so git-daemon gets \"the remote end hung up\nunexpectedly\" on its stderr channel.\n\nAFAICT, this problem goes back to v1.6.2, the first version which\nhandled empty clones. So I blame Sverre. ;)\n\n-Peff\n"},{"id":"122208","messageId":"fabb9a1e0908311550r1b549eb2k2df65c188a0ea6a0@mail.gmail.com","threadId":"20798","inReplyTo":"20090831224749.GA24190@sigill.intra.peff.net","subject":"Re: clong an empty repo over ssh causes (harmless) fatal","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-08-31T22:50:25Z","receivedAt":"2009-08-31T22:50:25Z","isPatch":false,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\n2009/9/1 Jeff King <peff@peff.net>:\n> AFAICT, this problem goes back to v1.6.2, the first version which\n> handled empty clones. So I blame Sverre. ;)\n\nEep :(. Any idea what is going on?\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"122216","messageId":"20090901010833.GA4033@sigill.intra.peff.net","threadId":"20798","inReplyTo":"fabb9a1e0908311550r1b549eb2k2df65c188a0ea6a0@mail.gmail.com","subject":"Re: clong an empty repo over ssh causes (harmless) fatal","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-09-01T01:08:33Z","receivedAt":"2009-09-01T01:08:33Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Sep 01, 2009 at 12:50:25AM +0200, Sverre Rabbelier wrote:\n\n> 2009/9/1 Jeff King <peff@peff.net>:\n> > AFAICT, this problem goes back to v1.6.2, the first version which\n> > handled empty clones. So I blame Sverre. ;)\n> \n> Eep :(. Any idea what is going on?\n\nYeah. We call upload-pack on the remote side, realize there are no refs,\nand then we just stop talking. Meanwhile upload-pack is waiting for a\npacket to say \"these are the refs that I want\". So the client really\nneeds to send an extra packet saying \"list of refs is finished\".\n\nThe patch below seems to work for me, but I'm a little concerned how it\nmight impact other transports. It actually calls the transport's\nfetch method when we have no refs that we want. So each transport must\nrecognize that we want zero refs and do the appropriate thing. In this\ncase, for the git protocol, we want to:\n\n  - do a packet_flush to signal \"no more refs\" to the remote side\n\n  - be aware that we might have zero refs and avoid establishing a new\n    connection in that case\n\nOther transports might need to be tweaked similarly, but I don't have\ntime to test at the moment.\n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex 0d2b4a8..f198c01 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -515,8 +515,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t     option_upload_pack);\n \n \t\trefs = transport_get_remote_refs(transport);\n-\t\tif(refs)\n-\t\t\ttransport_fetch_refs(transport, refs);\n+\t\ttransport_fetch_refs(transport, refs);\n \t}\n \n \tif (refs) {\ndiff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c\nindex 629735f..04a3776 100644\n--- a/builtin-fetch-pack.c\n+++ b/builtin-fetch-pack.c\n@@ -803,6 +803,8 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,\n \t\tnr_heads = remove_duplicates(nr_heads, heads);\n \tif (!ref) {\n \t\tpacket_flush(fd[1]);\n+\t\tif (!nr_heads)\n+\t\t\treturn NULL;\n \t\tdie(\"no matching remote head\");\n \t}\n \tref_cpy = do_fetch_pack(fd, ref, nr_heads, heads, pack_lockfile);\ndiff --git a/transport.c b/transport.c\nindex f2bd998..25e8946 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -512,6 +512,8 @@ static int fetch_refs_via_pack(struct transport *transport,\n \t\torigh[i] = heads[i] = xstrdup(to_fetch[i]->name);\n \n \tif (!data->conn) {\n+\t\tif (!nr_heads)\n+\t\t\treturn 0;\n \t\tconnect_setup(transport, 0, 0);\n \t\tget_remote_heads(data->fd[0], &refs_tmp, 0, NULL, 0, NULL);\n \t}\n"},{"id":"122280","messageId":"alpine.LNX.2.00.0909020024270.28290@iabervon.org","threadId":"20798","inReplyTo":"20090901010833.GA4033@sigill.intra.peff.net","subject":"Re: clong an empty repo over ssh causes (harmless) fatal","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-09-02T04:33:52Z","receivedAt":"2009-09-02T04:33:52Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Mon, 31 Aug 2009, Jeff King wrote:\n\n> On Tue, Sep 01, 2009 at 12:50:25AM +0200, Sverre Rabbelier wrote:\n> \n> > 2009/9/1 Jeff King <peff@peff.net>:\n> > > AFAICT, this problem goes back to v1.6.2, the first version which\n> > > handled empty clones. So I blame Sverre. ;)\n> > \n> > Eep :(. Any idea what is going on?\n> \n> Yeah. We call upload-pack on the remote side, realize there are no refs,\n> and then we just stop talking. Meanwhile upload-pack is waiting for a\n> packet to say \"these are the refs that I want\". So the client really\n> needs to send an extra packet saying \"list of refs is finished\".\n> \n> The patch below seems to work for me, but I'm a little concerned how it\n> might impact other transports.\n\nDoes putting a \"transport_disconnect(transport);\" after the \n\"transport_unlock_pack(transport);\" in builtin-clone.c also work for you? \nI think that's a cleaner solution, and should future-proof it in case we \nhave a future transport that both doesn't disconnect itself after a fetch \nand gives an error message if the connection is dropped suddenly.\n\nIt's kind of just an accident that the only transport that cares about \ndisconnect very much doesn't care if you've fetched after getting the \nrefs.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"122283","messageId":"20090902051615.GC12046@coredump.intra.peff.net","threadId":"20798","inReplyTo":"alpine.LNX.2.00.0909020024270.28290@iabervon.org","subject":"Re: clong an empty repo over ssh causes (harmless) fatal","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-09-02T05:16:15Z","receivedAt":"2009-09-02T05:16:15Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 02, 2009 at 12:33:52AM -0400, Daniel Barkalow wrote:\n\n> > The patch below seems to work for me, but I'm a little concerned how it\n> > might impact other transports.\n> \n> Does putting a \"transport_disconnect(transport);\" after the \n> \"transport_unlock_pack(transport);\" in builtin-clone.c also work for you? \n> I think that's a cleaner solution, and should future-proof it in case we \n> have a future transport that both doesn't disconnect itself after a fetch \n> and gives an error message if the connection is dropped suddenly.\n> \n> It's kind of just an accident that the only transport that cares about \n> disconnect very much doesn't care if you've fetched after getting the \n> refs.\n\nIt does work, and I think that is a much saner solution for the reasons\nyou mention. Thanks. Do you want to write it up and submit it, or should\nI?\n\n-Peff\n"},{"id":"122286","messageId":"2e24e5b90909012230s554b8776q515d21d88e6ed1d6@mail.gmail.com","threadId":"20798","inReplyTo":"20090831224749.GA24190@sigill.intra.peff.net","subject":"Re: clong an empty repo over ssh causes (harmless) fatal","fromName":"Sitaram Chamarty","fromEmail":"sitaramc@gmail.com","sentAt":"2009-09-02T05:30:54Z","receivedAt":"2009-09-02T05:30:54Z","isPatch":false,"sender":{"key":"sitaramc@gmail.com","avatar":"https://avatars.githubusercontent.com/u/43316?v=4"},"body":"2009/9/1 Jeff King <peff@peff.net>:\n\n> OK, it is definitely not about mixed versions, and it is definitely\n> reproducible, even without ssh. The local clone optimization manages to\n> avoid it, but you can see it with:\n>\n>  git clone file://$PWD/a b\n\nok I hadn't noticed that -- good spot!\n\nAnyway this whole thread went way over my head very quickly so just\nwanted to say I appreciate you guys looking at it and fixing it.\n\nDon't know if enough people say that :)\n\nSitaram\n"},{"id":"122289","messageId":"alpine.LNX.2.00.0909020159080.28290@iabervon.org","threadId":"20798","inReplyTo":"20090902051615.GC12046@coredump.intra.peff.net","subject":"Re: clong an empty repo over ssh causes (harmless) fatal","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-09-02T06:02:44Z","receivedAt":"2009-09-02T06:02:44Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Wed, 2 Sep 2009, Jeff King wrote:\n\n> On Wed, Sep 02, 2009 at 12:33:52AM -0400, Daniel Barkalow wrote:\n> \n> > > The patch below seems to work for me, but I'm a little concerned how it\n> > > might impact other transports.\n> > \n> > Does putting a \"transport_disconnect(transport);\" after the \n> > \"transport_unlock_pack(transport);\" in builtin-clone.c also work for you? \n> > I think that's a cleaner solution, and should future-proof it in case we \n> > have a future transport that both doesn't disconnect itself after a fetch \n> > and gives an error message if the connection is dropped suddenly.\n> > \n> > It's kind of just an accident that the only transport that cares about \n> > disconnect very much doesn't care if you've fetched after getting the \n> > refs.\n> \n> It does work, and I think that is a much saner solution for the reasons\n> you mention. Thanks. Do you want to write it up and submit it, or should\n> I?\n\nYou probably should; I'm not sure when I'd get to putting together a \npatch, and you did the hard part (figuring out what was going on) anyway.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"122291","messageId":"20090902063647.GA29559@coredump.intra.peff.net","threadId":"20798","inReplyTo":"alpine.LNX.2.00.0909020159080.28290@iabervon.org","subject":"[PATCH] clone: disconnect transport after fetching","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-09-02T06:36:47Z","receivedAt":"2009-09-02T06:36:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The current code just leaves the transport in whatever state\nit was in after performing the fetch.  For a non-empty clone\nover the git protocol, the transport code already\ndisconnects at the end of the fetch.\n\nBut for an empty clone, we leave the connection hanging, and\neventually close the socket when clone exits. This causes\nthe remote upload-pack to complain \"the remote end hung up\nunexpectedly\". While this message is harmless to the clone\nitself, it is unnecessarily scary for a user to see and may\npollute git-daemon logs.\n\nThis patch just explicitly calls disconnect after we are\ndone with the remote end, which sends a flush packet to\nupload-pack and cleanly disconnects, avoiding the error\nmessage.\n\nOther transports are unaffected or slightly improved:\n\n - for a non-empty repo over the git protocol, the second\n   disconnect is a no-op (since we are no longer connected)\n\n - for \"walker\" transports (like HTTP or FTP), we actually\n   free some used memory (which previously just sat until\n   the clone process exits)\n\n - for \"rsync\", disconnect is always a no-op anyway\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nThis was suggested by Daniel, so theoretically\n\n  Acked-by: Daniel Barkalow <barkalow@iabervon.org>\n\n:)\n\nAs you can see from the commit message, I did a little extra hunting to\nmake sure we are not going to impact any other code paths, and I am\npretty sure we are fine.\n\n builtin-clone.c |    4 +++-\n 1 files changed, 3 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex 991a7ae..0f231d8 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -580,8 +580,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\toption_no_checkout = 1;\n \t}\n \n-\tif (transport)\n+\tif (transport) {\n \t\ttransport_unlock_pack(transport);\n+\t\ttransport_disconnect(transport);\n+\t}\n \n \tif (!option_no_checkout) {\n \t\tstruct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));\n-- \n1.6.4.2.401.ga275f.dirty\n"},{"id":"122293","messageId":"fabb9a1e0909020009r3ee28b1fo3cba095abafec9d4@mail.gmail.com","threadId":"20798","inReplyTo":"20090902063647.GA29559@coredump.intra.peff.net","subject":"Re: [PATCH] clone: disconnect transport after fetching","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-09-02T07:09:19Z","receivedAt":"2009-09-02T07:09:19Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Wed, Sep 2, 2009 at 08:36, Jeff King<peff@peff.net> wrote:\n> As you can see from the commit message, I did a little extra hunting to\n> make sure we are not going to impact any other code paths, and I am\n> pretty sure we are fine.\n\nThank you for fixing my mistake :).\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"122294","messageId":"20090902072638.GC31528@coredump.intra.peff.net","threadId":"20798","inReplyTo":"fabb9a1e0909020009r3ee28b1fo3cba095abafec9d4@mail.gmail.com","subject":"Re: [PATCH] clone: disconnect transport after fetching","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-09-02T07:26:38Z","receivedAt":"2009-09-02T07:26:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 02, 2009 at 09:09:19AM +0200, Sverre Rabbelier wrote:\n\n> On Wed, Sep 2, 2009 at 08:36, Jeff King<peff@peff.net> wrote:\n> > As you can see from the commit message, I did a little extra hunting to\n> > make sure we are not going to impact any other code paths, and I am\n> > pretty sure we are fine.\n> \n> Thank you for fixing my mistake :).\n\nYou're welcome, though I am not sure it is your mistake. Arguably this\nis something we should have been doing all along. The point of\nabstracting transports was that we didn't need to know their details at\nthe outer layer, but in this case we were relying on the fact that no\ntransports (until empty-clone-over-git) needed an explicit\ntransport_disconnect to cleanly hang up on the other end.\n\nSo think of it as you exposing a long-standing bug. ;)\n\n-Peff\n"},{"id":"122295","messageId":"fabb9a1e0909020037m4ce6237ana6dec41088bba596@mail.gmail.com","threadId":"20798","inReplyTo":"20090902072638.GC31528@coredump.intra.peff.net","subject":"Re: [PATCH] clone: disconnect transport after fetching","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-09-02T07:37:40Z","receivedAt":"2009-09-02T07:37:40Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Wed, Sep 2, 2009 at 09:26, Jeff King<peff@peff.net> wrote:\n> So think of it as you exposing a long-standing bug. ;)\n\nAh, well in that case, you're all welcome :P.\n\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"122311","messageId":"alpine.LNX.2.00.0909021228570.28290@iabervon.org","threadId":"20798","inReplyTo":"20090902063647.GA29559@coredump.intra.peff.net","subject":"Re: [PATCH] clone: disconnect transport after fetching","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-09-02T16:38:35Z","receivedAt":"2009-09-02T16:38:35Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Wed, 2 Sep 2009, Jeff King wrote:\n\n> The current code just leaves the transport in whatever state\n> it was in after performing the fetch.  For a non-empty clone\n> over the git protocol, the transport code already\n> disconnects at the end of the fetch.\n> \n> But for an empty clone, we leave the connection hanging, and\n> eventually close the socket when clone exits. This causes\n> the remote upload-pack to complain \"the remote end hung up\n> unexpectedly\". While this message is harmless to the clone\n> itself, it is unnecessarily scary for a user to see and may\n> pollute git-daemon logs.\n> \n> This patch just explicitly calls disconnect after we are\n> done with the remote end, which sends a flush packet to\n> upload-pack and cleanly disconnects, avoiding the error\n> message.\n> \n> Other transports are unaffected or slightly improved:\n> \n>  - for a non-empty repo over the git protocol, the second\n>    disconnect is a no-op (since we are no longer connected)\n> \n>  - for \"walker\" transports (like HTTP or FTP), we actually\n>    free some used memory (which previously just sat until\n>    the clone process exits)\n> \n>  - for \"rsync\", disconnect is always a no-op anyway\n> \n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n> This was suggested by Daniel, so theoretically\n> \n>   Acked-by: Daniel Barkalow <barkalow@iabervon.org>\n> \n> :)\n\nThis is what I intended, so:\n\nAcked-by: Daniel Barkalow <barkalow@iabervon.org>\n\n> As you can see from the commit message, I did a little extra hunting to\n> make sure we are not going to impact any other code paths, and I am\n> pretty sure we are fine.\n\nAlso, builtin-fetch already does the explicit disconnect, and commonly \nexercises both the \"we want something\" and \"we don't want anything\" cases, \nso any problems would have to be surprisingly clone-specific.\n\n>  builtin-clone.c |    4 +++-\n>  1 files changed, 3 insertions(+), 1 deletions(-)\n> \n> diff --git a/builtin-clone.c b/builtin-clone.c\n> index 991a7ae..0f231d8 100644\n> --- a/builtin-clone.c\n> +++ b/builtin-clone.c\n> @@ -580,8 +580,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n>  \t\toption_no_checkout = 1;\n>  \t}\n>  \n> -\tif (transport)\n> +\tif (transport) {\n>  \t\ttransport_unlock_pack(transport);\n> +\t\ttransport_disconnect(transport);\n> +\t}\n>  \n>  \tif (!option_no_checkout) {\n>  \t\tstruct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));\n> -- \n> 1.6.4.2.401.ga275f.dirty\n> \n"},{"id":"122315","messageId":"7veiqp1b9v.fsf@alter.siamese.dyndns.org","threadId":"20798","inReplyTo":"alpine.LNX.2.00.0909021228570.28290@iabervon.org","subject":"Re: [PATCH] clone: disconnect transport after fetching","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-02T17:55:56Z","receivedAt":"2009-09-02T17:55:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow <barkalow@iabervon.org> writes:\n\n> On Wed, 2 Sep 2009, Jeff King wrote:\n> ...\n>> Signed-off-by: Jeff King <peff@peff.net>\n>> ---\n>> This was suggested by Daniel, so theoretically\n>> \n>>   Acked-by: Daniel Barkalow <barkalow@iabervon.org>\n>> \n>> :)\n>\n> This is what I intended, so:\n>\n> Acked-by: Daniel Barkalow <barkalow@iabervon.org>\n\nThanks, both.\n"},{"id":"122385","messageId":"20090904023013.GA14651@coredump.intra.peff.net","threadId":"20798","inReplyTo":"20090902063647.GA29559@coredump.intra.peff.net","subject":"Re: [PATCH] clone: disconnect transport after fetching","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-09-04T02:30:13Z","receivedAt":"2009-09-04T02:30:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 02, 2009 at 02:36:47AM -0400, Jeff King wrote:\n\n> This patch just explicitly calls disconnect after we are\n> done with the remote end, which sends a flush packet to\n> upload-pack and cleanly disconnects, avoiding the error\n> message.\n\nI see you applied this with some extra tests. I should have mentioned in\nthe original cover letter that I considered tests but intentionally did\nnot include them.\n\nThe problem is that clone forks upload-pack, and then hangs up on it by\nexiting, and then upload-pack spews the unwanted message. But control\nhas returned to the shell after clone exits, meaning that the message\nfrom upload-pack may or may not have gotten there by the time we grep\nstderr.\n\nSo I don't think your test will ever incorrectly show a failure, but I\nbelieve that it would pass randomly even without the related fix to the\ncode.\n\n-Peff\n"}]}