{"thread":{"id":"17315","subject":"[PATCH] Allow cloning an empty repository","startedAt":"2009-01-23T00:07:32Z","lastAt":"2009-01-25T18:33:01Z","messageCount":23,"participants":["Sverre Rabbelier","Junio C Hamano","Johannes Schindelin","Miklos Vajna","Mike Hommey","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"101592","messageId":"1232669252-21881-1-git-send-email-srabbelier@gmail.com","threadId":"17315","inReplyTo":null,"subject":"[PATCH] Allow cloning an empty repository","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-01-23T00:07:32Z","receivedAt":"2009-01-23T00:07:32Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Cloning an empty repository manually (that is, doing 'git init' and\nthen doing all configuration by hand) can be a lot of work. Save the\nuser this work by allowing the cloning of empty repositories.\n\nSigned-off-by: Sverre Rabbelier \n---\n\nThanks to Dscho for giving some pointers on how to do this.\n\n builtin-clone.c | 17 +++++++++++++----\n t/t5701-clone-local.sh | 16 ++++++++++++++++\n 2 files changed, 29 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex f7e5a7b..d5bba0e 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -522,14 +522,23 @@ 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\ttransport_fetch_refs(transport, refs);\n+\t\tif(refs)\n+\t\t\ttransport_fetch_refs(transport, refs);\n \t}\n \n-\tclear_extra_refs();\n+\tif(refs) {\n+\t\tclear_extra_refs();\n \n-\tmapped_refs = write_remote_refs(refs, &refspec, reflog_msg.buf);\n+\t\tmapped_refs = write_remote_refs(refs, &refspec, reflog_msg.buf);\n \n-\thead_points_at = locate_head(refs, mapped_refs, &remote_head);\n+\t\thead_points_at = locate_head(refs, mapped_refs, &remote_head);\n+\t}\n+\telse {\n+\t\twarning(\"You appear to have cloned an empty repository.\");\n+\t\thead_points_at = NULL;\n+\t\tremote_head = NULL;\n+\t\toption_no_checkout = 1;\n+\t}\n \n \tif (head_points_at) {\n \t\t/* Local default branch link */\ndiff --git a/t/t5701-clone-local.sh b/t/t5701-clone-local.sh\nindex 8dfaaa4..fbd9bfa 100755\n--- a/t/t5701-clone-local.sh\n+++ b/t/t5701-clone-local.sh\n@@ -116,4 +116,20 @@ test_expect_success 'bundle clone with nonexistent HEAD' '\n \ttest ! -e .git/refs/heads/master\n '\n \n+test_expect_success 'clone empty repository' '\n+\tcd \"$D\" &&\n+\tmkdir empty &&\n+\t(cd empty && git init) &&\n+\tgit clone empty empty-clone &&\n+\ttest_tick &&\n+\t(cd empty-clone\n+\t echo \"content\" >> foo &&\n+\t git add foo &&\n+\t git commit -m \"Initial commit\" &&\n+\t git push origin master &&\n+\t expected=$(git rev-parse master) &&\n+\t actual=$(git --git-dir=../empty/.git rev-parse master) &&\n+\t test $actual = $expected)\n+'\n+\n test_done\n-- \n1.6.1.399.g0d272.dirty\n"},{"id":"101595","messageId":"7vwscm4xx0.fsf@gitster.siamese.dyndns.org","threadId":"17315","inReplyTo":"1232669252-21881-1-git-send-email-srabbelier@gmail.com","subject":"Re: [PATCH] Allow cloning an empty repository","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-23T01:24:27Z","receivedAt":"2009-01-23T01:24:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sverre Rabbelier writes:\n\n> diff --git a/builtin-clone.c b/builtin-clone.c\n> index f7e5a7b..d5bba0e 100644\n> --- a/builtin-clone.c\n> +++ b/builtin-clone.c\n> @@ -522,14 +522,23 @@ 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\ttransport_fetch_refs(transport, refs);\n> +\t\tif(refs)\n> +\t\t\ttransport_fetch_refs(transport, refs);\n\nThanks.\n\nI think the basic idea is Ok, but is it a reliable check at this point to\nsee if (refs == NULL) to tell if the target repository is an empty one?\n\nI am mostly worried about a failure case (connected but couldn't get the\nrefs, or perhaps connection failed to start). If you get a NULL in such a\ncase you may end up saying \"oh you cloned a void\" when you should say\n\"nah, such a remote repository does not exist\".\n\nIf transport_get_remote_refs() dies without returning NULL, that would be\nsufficient, but I didn't check.\n"},{"id":"101597","messageId":"bd6139dc0901221746h258f548etf857ab37399133da@mail.gmail.com","threadId":"17315","inReplyTo":"7vwscm4xx0.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Allow cloning an empty repository","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-01-23T01:46:42Z","receivedAt":"2009-01-23T01:46:42Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"On Fri, Jan 23, 2009 at 02:24, Junio C Hamano wrote:\n> I think the basic idea is Ok, but is it a reliable check at this point to\n> see if (refs == NULL) to tell if the target repository is an empty one?\n\nThis is the question I asked Dscho, and he said/guessed that it was.\n\n> I am mostly worried about a failure case (connected but couldn't get the\n> refs, or perhaps connection failed to start). If you get a NULL in such a\n> case you may end up saying \"oh you cloned a void\" when you should say\n> \"nah, such a remote repository does not exist\".\n\nYes, this was my concern as well.\n\n> If transport_get_remote_refs() dies without returning NULL, that would be\n> sufficient, but I didn't check.\n\nIt does, transport_get_remote_refs() calls fetch_pack, which dies when\n'refs' (the fourth argument) is NULL:\n\nstatic int fetch_refs_via_pack(...) {\n// setup code snipped\n\tif (!data->conn) {\n\t\tconnect_setup(transport);\n\t\tget_remote_heads(data->fd[0], &refs_tmp, 0, NULL, 0, NULL);\n\t}\n\n\trefs = fetch_pack(&args, data->fd, data->conn,\n\t\t\t refs_tmp ? refs_tmp : transport->remote_refs,\n\t\t\t dest, nr_heads, heads, &transport->pack_lockfile);\n\nSo unless get_remote_heads messes with it, it will remain NULL. Now I\nmust admit that I'm not familiar enough with get_remote_heads to know\nif this could cause a false positive, Dscho?\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"101599","messageId":"alpine.DEB.1.00.0901230333060.3586@pacific.mpi-cbg.de","threadId":"17315","inReplyTo":"bd6139dc0901221746h258f548etf857ab37399133da@mail.gmail.com","subject":"Re: [PATCH] Allow cloning an empty repository","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-23T02:42:24Z","receivedAt":"2009-01-23T02:42:24Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 23 Jan 2009, Sverre Rabbelier wrote:\n\n> On Fri, Jan 23, 2009 at 02:24, Junio C Hamano wrote:\n> > I think the basic idea is Ok, but is it a reliable check at this point to\n> > see if (refs == NULL) to tell if the target repository is an empty one?\n> \n> This is the question I asked Dscho, and he said/guessed that it was.\n> \n> > I am mostly worried about a failure case (connected but couldn't get \n> > the refs, or perhaps connection failed to start). If you get a NULL \n> > in such a case you may end up saying \"oh you cloned a void\" when you \n> > should say \"nah, such a remote repository does not exist\".\n> \n> Yes, this was my concern as well.\n\n>From what I can see in get_remote_heads(), the native protocols would \ndie(), as would rsync().\n\nHTTP transport, however, would not die() on connection errors, from my \ncursory look.\n\nThat might be skewed, though, as I am on top of Mike's patches (in the \nhopefully not so futile hope that Mike -- after letting me wait for over \none year -- finishes his work.\n\nHTTP being 2nd class citizen anyway (and we can always fix it after Mike's \ncleanups), I'd say this patch is ready to roll.\n\nCiao,\nDscho\n"},{"id":"101600","messageId":"bd6139dc0901221847u6b8aeadcl580a091751de3d26@mail.gmail.com","threadId":"17315","inReplyTo":"alpine.DEB.1.00.0901230333060.3586@pacific.mpi-cbg.de","subject":"Re: [PATCH] Allow cloning an empty repository","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-01-23T02:47:42Z","receivedAt":"2009-01-23T02:47:42Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"On Fri, Jan 23, 2009 at 03:42, Johannes Schindelin\n wrote:\n> HTTP transport, however, would not die() on connection errors, from my\n> cursory look.\n\nHeh, this is why I asked you, my cursory look told me \"no clue\" instead.\n\n> HTTP being 2nd class citizen anyway (and we can always fix it after Mike's\n> cleanups), I'd say this patch is ready to roll.\n\nAside from that, am I correct in asserting that in the \"worst case\"\nscenario the repo is cloned instead of erroring out, and a simple \"git\nfetch\" would fix the issue?\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"101603","messageId":"alpine.DEB.1.00.0901230419080.3586@pacific.mpi-cbg.de","threadId":"17315","inReplyTo":"bd6139dc0901221847u6b8aeadcl580a091751de3d26@mail.gmail.com","subject":"Re: [PATCH] Allow cloning an empty repository","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-23T03:20:34Z","receivedAt":"2009-01-23T03:20:34Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 23 Jan 2009, Sverre Rabbelier wrote:\n\n> On Fri, Jan 23, 2009 at 03:42, Johannes Schindelin\n> wrote:\n> \n> > HTTP being 2nd class citizen anyway (and we can always fix it after \n> > Mike's cleanups), I'd say this patch is ready to roll.\n> \n> Aside from that, am I correct in asserting that in the \"worst case\" \n> scenario the repo is cloned instead of erroring out, and a simple \"git \n> fetch\" would fix the issue?\n\nProbably. Note, however, that scripts might rely on a fail if there were \nproblems.\n\nBut then, scripts have no business cloning repositories (fetching, yes. \nBut cloning?)\n\nCiao,\nDscho\n"},{"id":"101663","messageId":"20090123165549.GI21473@genesis.frugalware.org","threadId":"17315","inReplyTo":"alpine.DEB.1.00.0901230419080.3586@pacific.mpi-cbg.de","subject":"Re: [PATCH] Allow cloning an empty repository","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2009-01-23T16:55:49Z","receivedAt":"2009-01-23T16:55:49Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Fri, Jan 23, 2009 at 04:20:34AM +0100, Johannes Schindelin wrote:\n> But then, scripts have no business cloning repositories (fetching, yes. \n> But cloning?)\n\nI think portals like repo.or.cz may do it.\n\nIsn't setting errno (or similar variable) in the HTTP code an option?\nThen we could see why the transport failed and make a difference between\n\"network error\" and \"no refs found\".\n"},{"id":"101685","messageId":"alpine.DEB.1.00.0901232100020.21467@intel-tinevez-2-302","threadId":"17315","inReplyTo":"20090123165549.GI21473@genesis.frugalware.org","subject":"Re: [PATCH] Allow cloning an empty repository","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-23T20:05:57Z","receivedAt":"2009-01-23T20:05:57Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 23 Jan 2009, Miklos Vajna wrote:\n\n> On Fri, Jan 23, 2009 at 04:20:34AM +0100, Johannes Schindelin \n> wrote:\n>\n> > But then, scripts have no business cloning repositories (fetching, \n> > yes. But cloning?)\n> \n> I think portals like repo.or.cz may do it.\n\n\"git grep git.clone\" in repo.git suggests that only clonecheck.sh uses it, \nwhich is not called anywhere.\n\nBut more generally, the same should apply to such scripts as to human \nusers: HTTP _is_ 2nd class, and the fetch _could_ succeed later.\n\n> Isn't setting errno (or similar variable) in the HTTP code an option? \n> Then we could see why the transport failed and make a difference between \n> \"network error\" and \"no refs found\".\n\nI'd rather see the HTTP code path taken by transport.c _fail_ (i.e. die()) \nwhen there is some sort of connection error.\n\nCiao,\nDscho\n"},{"id":"101697","messageId":"20090123215559.GA5561@glandium.org","threadId":"17315","inReplyTo":"alpine.DEB.1.00.0901230333060.3586@pacific.mpi-cbg.de","subject":"Re: [PATCH] Allow cloning an empty repository","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2009-01-23T21:55:59Z","receivedAt":"2009-01-23T21:55:59Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Fri, Jan 23, 2009 at 03:42:24AM +0100, Johannes Schindelin wrote:\n> Hi,\n> \n> On Fri, 23 Jan 2009, Sverre Rabbelier wrote:\n> \n> > On Fri, Jan 23, 2009 at 02:24, Junio C Hamano wrote:\n> > > I think the basic idea is Ok, but is it a reliable check at this point to\n> > > see if (refs == NULL) to tell if the target repository is an empty one?\n> > \n> > This is the question I asked Dscho, and he said/guessed that it was.\n> > \n> > > I am mostly worried about a failure case (connected but couldn't get \n> > > the refs, or perhaps connection failed to start). If you get a NULL \n> > > in such a case you may end up saying \"oh you cloned a void\" when you \n> > > should say \"nah, such a remote repository does not exist\".\n> > \n> > Yes, this was my concern as well.\n> \n> From what I can see in get_remote_heads(), the native protocols would \n> die(), as would rsync().\n> \n> HTTP transport, however, would not die() on connection errors, from my \n> cursory look.\n> \n> That might be skewed, though, as I am on top of Mike's patches (in the \n> hopefully not so futile hope that Mike -- after letting me wait for over \n> one year -- finishes his work.\n\nAs I said when posting my patch batch, I don't have much time nor\nmotivation to work on this series. But let's make a deal: if someone\nwrites a good enough http test suite, I'll polish the http code.\n\nMike\n"},{"id":"101700","messageId":"20090123223205.GA11491@coredump.intra.peff.net","threadId":"17315","inReplyTo":"alpine.DEB.1.00.0901230419080.3586@pacific.mpi-cbg.de","subject":"Re: [PATCH] Allow cloning an empty repository","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-01-23T22:32:05Z","receivedAt":"2009-01-23T22:32:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 23, 2009 at 04:20:34AM +0100, Johannes Schindelin wrote:\n\n> Probably. Note, however, that scripts might rely on a fail if there were \n> problems.\n> \n> But then, scripts have no business cloning repositories (fetching, yes. \n> But cloning?)\n\nReally? I have scripts that call clone (usually followed by building the\nresult). Are you proposing that all scripts should \"git init && git\nremote add && git fetch\"?\n\nSo I am strongly in favor of telling the difference between failure and\nemptiness.\n\n-Peff\n"},{"id":"101701","messageId":"bd6139dc0901231434n6d6af5a6ma8bb12a634d5b3ba@mail.gmail.com","threadId":"17315","inReplyTo":"20090123223205.GA11491@coredump.intra.peff.net","subject":"Re: [PATCH] Allow cloning an empty repository","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-01-23T22:34:04Z","receivedAt":"2009-01-23T22:34:04Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"On Fri, Jan 23, 2009 at 23:32, Jeff King wrote:\n> Really? I have scripts that call clone (usually followed by building the\n> result). Are you proposing that all scripts should \"git init && git\n> remote add && git fetch\"?\n>\n> So I am strongly in favor of telling the difference between failure and\n> emptiness.\n\nA switch then, '--allow-empty'?\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"101702","messageId":"20090123223740.GA11527@coredump.intra.peff.net","threadId":"17315","inReplyTo":"bd6139dc0901231434n6d6af5a6ma8bb12a634d5b3ba@mail.gmail.com","subject":"Re: [PATCH] Allow cloning an empty repository","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-01-23T22:37:41Z","receivedAt":"2009-01-23T22:37:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 23, 2009 at 11:34:04PM +0100, Sverre Rabbelier wrote:\n\n> On Fri, Jan 23, 2009 at 23:32, Jeff King wrote:\n> > Really? I have scripts that call clone (usually followed by building the\n> > result). Are you proposing that all scripts should \"git init && git\n> > remote add && git fetch\"?\n> >\n> > So I am strongly in favor of telling the difference between failure and\n> > emptiness.\n> \n> A switch then, '--allow-empty'?\n\nNo, I don't mind success on cloning an empty repository. But I thought\nthe issue at hand was that for some instances, we would report that we\nsuccessfully created an empty repository, when in fact what happened was\nthat we failed to clone a non-empty repository. And that that was\nfixable, but it was a problem with our code interfaces (which should be\nfixable) and not some fundamental limitation.\n\nOr am I misunderstanding the situation?\n\n-Peff\n"},{"id":"101703","messageId":"bd6139dc0901231439x1e20f06bka90a228a883681ff@mail.gmail.com","threadId":"17315","inReplyTo":"20090123223740.GA11527@coredump.intra.peff.net","subject":"Re: [PATCH] Allow cloning an empty repository","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-01-23T22:39:56Z","receivedAt":"2009-01-23T22:39:56Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"On Fri, Jan 23, 2009 at 23:37, Jeff King wrote:\n> No, I don't mind success on cloning an empty repository. But I thought\n> the issue at hand was that for some instances, we would report that we\n> successfully created an empty repository, when in fact what happened was\n> that we failed to clone a non-empty repository. And that that was\n> fixable, but it was a problem with our code interfaces (which should be\n> fixable) and not some fundamental limitation.\n\nAh, mhh, if that is the case then I'm afraid that's beyond me (at\nleast for now), someone else would have to do said refactoring first\n:(.\n\n> Or am I misunderstanding the situation?\n\nI think you summarized it pretty well, if that is indeed what Junio meant.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"101705","messageId":"20090123230520.GL21473@genesis.frugalware.org","threadId":"17315","inReplyTo":"alpine.DEB.1.00.0901230333060.3586@pacific.mpi-cbg.de","subject":"Re: [PATCH] Allow cloning an empty repository","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2009-01-23T23:05:20Z","receivedAt":"2009-01-23T23:05:20Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Fri, Jan 23, 2009 at 03:42:24AM +0100, Johannes Schindelin wrote:\n> > > I am mostly worried about a failure case (connected but couldn't get \n> > > the refs, or perhaps connection failed to start). If you get a NULL \n> > > in such a case you may end up saying \"oh you cloned a void\" when you \n> > > should say \"nah, such a remote repository does not exist\".\n> > \n> > Yes, this was my concern as well.\n> \n> From what I can see in get_remote_heads(), the native protocols would \n> die(), as would rsync().\n> \n> HTTP transport, however, would not die() on connection errors, from my \n> cursory look.\n\nI'm not familiar with the HTTP code, either, but here is the call stack\nI see:\n\n- builtin-clone calls transport_get_remote_refs()\n- that will call transport->get_refs_list()\n- that will call get_refs_via_curl()\n- that die()s on error, does not use return error()\n\nHave I missed something?\n"},{"id":"101708","messageId":"alpine.DEB.1.00.0901240125190.3586@pacific.mpi-cbg.de","threadId":"17315","inReplyTo":"20090123215559.GA5561@glandium.org","subject":"http fixes, was Re: [PATCH] Allow cloning an empty repository","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-24T00:26:06Z","receivedAt":"2009-01-24T00:26:06Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 23 Jan 2009, Mike Hommey wrote:\n\n> As I said when posting my patch batch, I don't have much time nor \n> motivation to work on this series. But let's make a deal: if someone \n> writes a good enough http test suite, I'll polish the http code.\n\nI already said in my replied to your patch that I will add the http test \nsuite if you fix your patches.\n\nCiao,\nDscho\n"},{"id":"101709","messageId":"alpine.DEB.1.00.0901240127110.3586@pacific.mpi-cbg.de","threadId":"17315","inReplyTo":"20090123223740.GA11527@coredump.intra.peff.net","subject":"Re: [PATCH] Allow cloning an empty repository","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-24T00:28:27Z","receivedAt":"2009-01-24T00:28:27Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 23 Jan 2009, Jeff King wrote:\n\n> But I thought the issue at hand was that for some instances, we would \n> report that we successfully created an empty repository, when in fact \n> what happened was that we failed to clone a non-empty repository. And \n> that that was fixable, but it was a problem with our code interfaces \n> (which should be fixable) and not some fundamental limitation.\n\nIt appears that the HTTP code (at least after the first round of Mike's \npatches) has issues there, yes, but I think it would not be fair to stop \nSverre's patch because of that: the HTTP code needs fixing anyway, and \nthis fix is orthogonal to the empty clones.\n\nCiao,\nDscho\n"},{"id":"101710","messageId":"alpine.DEB.1.00.0901240131570.3586@pacific.mpi-cbg.de","threadId":"17315","inReplyTo":"20090123230520.GL21473@genesis.frugalware.org","subject":"Re: [PATCH] Allow cloning an empty repository","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-24T00:33:04Z","receivedAt":"2009-01-24T00:33:04Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 24 Jan 2009, Miklos Vajna wrote:\n\n> On Fri, Jan 23, 2009 at 03:42:24AM +0100, Johannes Schindelin \n> wrote:\n> > > > I am mostly worried about a failure case (connected but couldn't \n> > > > get the refs, or perhaps connection failed to start). If you get \n> > > > a NULL in such a case you may end up saying \"oh you cloned a void\" \n> > > > when you should say \"nah, such a remote repository does not \n> > > > exist\".\n> > > \n> > > Yes, this was my concern as well.\n> > \n> > From what I can see in get_remote_heads(), the native protocols would \n> > die(), as would rsync().\n> > \n> > HTTP transport, however, would not die() on connection errors, from my \n> > cursory look.\n> \n> I'm not familiar with the HTTP code, either, but here is the call stack \n> I see:\n> \n> - builtin-clone calls transport_get_remote_refs()\n> - that will call transport->get_refs_list()\n> - that will call get_refs_via_curl()\n> - that die()s on error, does not use return error()\n> \n> Have I missed something?\n\nNot really.\n\nOnly after writing my email (and just before sending) did I remember that \nI have Mike's HTTP cleanups applied in my Git checkout. So I was \nanalyzing the wrong code.\n\nThanks for analyzing the right code.\n\nCiao,\nDscho\n"},{"id":"101712","messageId":"bd6139dc0901231642v6bd593d3mfefaca3c419a9f0a@mail.gmail.com","threadId":"17315","inReplyTo":"alpine.DEB.1.00.0901240131570.3586@pacific.mpi-cbg.de","subject":"Re: [PATCH] Allow cloning an empty repository","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-01-24T00:42:30Z","receivedAt":"2009-01-24T00:42:30Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"On Sat, Jan 24, 2009 at 01:33, Johannes Schindelin\n wrote:\n> On Sat, 24 Jan 2009, Miklos Vajna wrote:\n>> I'm not familiar with the HTTP code, either, but here is the call stack\n>> I see:\n>>\n>> - builtin-clone calls transport_get_remote_refs()\n>> - that will call transport->get_refs_list()\n>> - that will call get_refs_via_curl()\n>> - that die()s on error, does not use return error()\n\nWhich means all protocols actually die?\n\n> Only after writing my email (and just before sending) did I remember that\n> I have Mike's HTTP cleanups applied in my Git checkout. So I was\n> analyzing the wrong code.\n\nHmmm, can't blame you considering it was around 4am ;).\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"101736","messageId":"bd6139dc0901240139l76ea28bq2610bf0ac07087b3@mail.gmail.com","threadId":"17315","inReplyTo":"alpine.DEB.1.00.0901240125190.3586@pacific.mpi-cbg.de","subject":"Re: http fixes, was Re: [PATCH] Allow cloning an empty repository","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-01-24T09:39:34Z","receivedAt":"2009-01-24T09:39:34Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"On Sat, Jan 24, 2009 at 01:26, Johannes Schindelin\n wrote:\n> On Fri, 23 Jan 2009, Mike Hommey wrote:\n>> As I said when posting my patch batch, I don't have much time nor\n>> motivation to work on this series. But let's make a deal: if someone\n>> writes a good enough http test suite, I'll polish the http code.\n>\n> I already said in my replied to your patch that I will add the http test\n> suite if you fix your patches.\n\nI think Mike meant that he wants a working test suite before\ncontinuing work on it? As in, what's the point in working on something\nif you don't have a working test-suite to test your solution against\n;).\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"101802","messageId":"20090125004955.GY21473@genesis.frugalware.org","threadId":"17315","inReplyTo":"bd6139dc0901231642v6bd593d3mfefaca3c419a9f0a@mail.gmail.com","subject":"Re: [PATCH] Allow cloning an empty repository","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2009-01-25T00:49:55Z","receivedAt":"2009-01-25T00:49:55Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Sat, Jan 24, 2009 at 01:42:30AM +0100, Sverre Rabbelier wrote:\n> Which means all protocols actually die?\n\nI think so.\n"},{"id":"101806","messageId":"bd6139dc0901241655n66f75a8fn8450b774809fa8e7@mail.gmail.com","threadId":"17315","inReplyTo":"20090125004955.GY21473@genesis.frugalware.org","subject":"Re: [PATCH] Allow cloning an empty repository","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-01-25T00:55:28Z","receivedAt":"2009-01-25T00:55:28Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Sun, Jan 25, 2009 at 01:49, Miklos Vajna wrote:\n> On Sat, Jan 24, 2009 at 01:42:30AM +0100, Sverre Rabbelier wrote:\n>> Which means all protocols actually die?\n>\n> I think so.\n\nJunio, are your concerns address now, since Miklos confirmed that\ntransport_get_remote_refs always dies?\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"101815","messageId":"7vy6x0j7py.fsf@gitster.siamese.dyndns.org","threadId":"17315","inReplyTo":"bd6139dc0901241655n66f75a8fn8450b774809fa8e7@mail.gmail.com","subject":"Re: [PATCH] Allow cloning an empty repository","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-25T05:05:45Z","receivedAt":"2009-01-25T05:05:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sverre Rabbelier writes:\n\n> On Sun, Jan 25, 2009 at 01:49, Miklos Vajna wrote:\n>> On Sat, Jan 24, 2009 at 01:42:30AM +0100, Sverre Rabbelier wrote:\n>>> Which means all protocols actually die?\n>>\n>> I think so.\n>\n> Junio, are your concerns address now, since Miklos confirmed that\n> transport_get_remote_refs always dies?\n\nYeah, it is on 'next' and I've pushed the results out last night, but I\ngot sick and didn't manage to send out \"What's cooking\".\n"},{"id":"101850","messageId":"bd6139dc0901251033g166478d5y2050977cc7beaa5a@mail.gmail.com","threadId":"17315","inReplyTo":"7vy6x0j7py.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Allow cloning an empty repository","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-01-25T18:33:01Z","receivedAt":"2009-01-25T18:33:01Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"On Sun, Jan 25, 2009 at 06:05, Junio C Hamano wrote:\n> Yeah, it is on 'next' and I've pushed the results out last night, but I\n> got sick and didn't manage to send out \"What's cooking\".\n\nOk, awesome! :).\n\n-- \nCheers,\n\nSverre Rabbelier\n"}]}