{"thread":{"id":"34865","subject":"[PATCH 0/3] Unconfuse git clone when two branches at are HEAD.","startedAt":"2013-09-06T15:52:04Z","lastAt":"2013-09-18T04:36:17Z","messageCount":32,"participants":["Andreas Krey","Philip Oakley","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"226943","messageId":"20130906155204.GE12966@inner.h.apk.li","threadId":"34865","inReplyTo":null,"subject":"[PATCH 0/3] Unconfuse git clone when two branches at are HEAD.","fromName":"Andreas Krey","fromEmail":"a.krey@gmx.de","sentAt":"2013-09-06T15:52:04Z","receivedAt":"2013-09-06T15:52:04Z","isPatch":true,"sender":{"key":"a.krey@gmx.de","avatar":"https://avatars.githubusercontent.com/u/37810?v=4"},"body":"Ok, here are some patches that make git actually\ncheck out the current remote branch when cloning.\n\nUp to now this failed when there were two branches that pointed to \nthe HEAD commit of the remote repo, and git clone would sometimes\nchoose the wrong one as the HEAD ref isn't transmitted in all\ntransport.\n\nStuff the HEAD ref into the capability list (assuming refs are clean \nenough to do that w/o escaping), and read them out on the other\nside. All other things were thankfully already in place.\n\nTwo of the patches have Junio in the From as they are essentially his.\n\nAndreas\n"},{"id":"226945","messageId":"20130906155608.GF12966@inner.h.apk.li","threadId":"34865","inReplyTo":"20130906155204.GE12966@inner.h.apk.li","subject":"[PATCH 1/3] upload-pack: send the HEAD information","fromName":"Andreas Krey","fromEmail":"a.krey@gmx.de","sentAt":"2013-09-06T15:56:08Z","receivedAt":"2013-09-06T15:56:08Z","isPatch":true,"sender":{"key":"a.krey@gmx.de","avatar":"https://avatars.githubusercontent.com/u/37810?v=4"},"body":"From: Junio C Hamano \n\nThis implements the server side of protocol extension to show which branch\nthe HEAD points at. The information is sent as a capability symref=.\n\nSigned-off-by: Junio C Hamano \nSigned-off-by: Andreas Krey \n---\n upload-pack.c | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 127e59a..390d1ec 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -745,13 +745,17 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo\n \tif (mark_our_ref(refname, sha1, flag, cb_data))\n \t\treturn 0;\n \n-\tif (capabilities)\n-\t\tpacket_write(1, \"%s %s%c%s%s%s agent=%s\\n\",\n+\tif (capabilities) {\n+\t\tunsigned char dummy[20];\n+\t\tconst char *target = resolve_ref_unsafe(\"HEAD\", dummy, 0, NULL);\n+\t\tpacket_write(1, \"%s %s%c%s%s%s%s%s agent=%s\\n\",\n \t\t\t sha1_to_hex(sha1), refname_nons,\n \t\t\t 0, capabilities,\n \t\t\t allow_tip_sha1_in_want ? \" allow-tip-sha1-in-want\" : \"\",\n \t\t\t stateless_rpc ? \" no-done\" : \"\",\n+\t\t\t target ? \" symref=\" : \"\", target ? target : 0,\n \t\t\t git_user_agent_sanitized());\n+\t}\n \telse\n \t\tpacket_write(1, \"%s %s\\n\", sha1_to_hex(sha1), refname_nons);\n \tcapabilities = NULL;\n-- \n1.8.3.1.485.g9704416.dirty\n"},{"id":"226944","messageId":"20130906155655.GG12966@inner.h.apk.li","threadId":"34865","inReplyTo":"20130906155204.GE12966@inner.h.apk.li","subject":"[PATCH 2/3] connect.c: save symref info from server capabilities","fromName":"Andreas Krey","fromEmail":"a.krey@gmx.de","sentAt":"2013-09-06T15:56:55Z","receivedAt":"2013-09-06T15:56:55Z","isPatch":true,"sender":{"key":"a.krey@gmx.de","avatar":"https://avatars.githubusercontent.com/u/37810?v=4"},"body":"Signed-off-by: Andreas Krey \n---\n connect.c | 11 +++++++++--\n 1 file changed, 9 insertions(+), 2 deletions(-)\n\ndiff --git a/connect.c b/connect.c\nindex a0783d4..98c4868 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -72,8 +72,8 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,\n \tfor (;;) {\n \t\tstruct ref *ref;\n \t\tunsigned char old_sha1[20];\n-\t\tchar *name;\n-\t\tint len, name_len;\n+\t\tchar *name, *symref;\n+\t\tint len, name_len, symref_len;\n \t\tchar *buffer = packet_buffer;\n \n \t\tlen = packet_read(in, &src_buf, &src_len,\n@@ -94,9 +94,12 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,\n \t\tname = buffer + 41;\n \n \t\tname_len = strlen(name);\n+\t\tsymref = 0;\n \t\tif (len != name_len + 41) {\n \t\t\tfree(server_capabilities);\n \t\t\tserver_capabilities = xstrdup(name + name_len + 1);\n+\t\t\tsymref = parse_feature_value(server_capabilities,\n+\t\t\t\t\t\t \"symref\", &symref_len);\n \t\t}\n \n \t\tif (extra_have &&\n@@ -108,6 +111,10 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,\n \t\tif (!check_ref(name, name_len, flags))\n \t\t\tcontinue;\n \t\tref = alloc_ref(buffer + 41);\n+\t\tif (symref) {\n+\t\t\tref->symref = xcalloc(symref_len + 1, 1);\n+\t\t\tstrncpy(ref->symref, symref, symref_len);\n+\t\t}\n \t\thashcpy(ref->old_sha1, old_sha1);\n \t\t*list = ref;\n \t\tlist = &ref->next;\n-- \n1.8.3.1.485.g9704416.dirty\n"},{"id":"226946","messageId":"20130906155753.GH12966@inner.h.apk.li","threadId":"34865","inReplyTo":"20130906155204.GE12966@inner.h.apk.li","subject":"[PATCH 3/3] clone: test the new HEAD detection logic","fromName":"Andreas Krey","fromEmail":"a.krey@gmx.de","sentAt":"2013-09-06T15:57:53Z","receivedAt":"2013-09-06T15:57:53Z","isPatch":true,"sender":{"key":"a.krey@gmx.de","avatar":"https://avatars.githubusercontent.com/u/37810?v=4"},"body":"From: Junio C Hamano \n\nSigned-off-by: Junio C Hamano \nSigned-off-by: Andreas Krey \n---\n t/t5601-clone.sh | 11 +++++++++++\n 1 file changed, 11 insertions(+)\n\ndiff --git a/t/t5601-clone.sh b/t/t5601-clone.sh\nindex 0629149..ccda6df 100755\n--- a/t/t5601-clone.sh\n+++ b/t/t5601-clone.sh\n@@ -285,4 +285,15 @@ test_expect_success NOT_MINGW,NOT_CYGWIN 'clone local path foo:bar' '\n \tgit clone \"./foo:bar\" foobar\n '\n \n+test_expect_success 'clone from a repository with two identical branches' '\n+\n+\t(\n+\t\tcd src &&\n+\t\tgit checkout -b another master\n+\t) &&\n+\tgit clone src target-11 &&\n+\ttest \"z$( cd target-11 && git symbolic-ref HEAD )\" = zrefs/heads/another\n+\n+'\n+\n test_done\n-- \n1.8.3.1.485.g9704416.dirty\n"},{"id":"226956","messageId":"6649DD0E3B6B4CE59D330217786B6B05@PhilipOakley","threadId":"34865","inReplyTo":"20130906155204.GE12966@inner.h.apk.li","subject":"Re: [PATCH 0/3] Unconfuse git clone when two branches at are HEAD.","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2013-09-06T17:29:16Z","receivedAt":"2013-09-06T17:29:16Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Andreas Krey\" \n> Ok, here are some patches that make git actually\n> check out the current remote branch when cloning.\n>\n> Up to now this failed when there were two branches that pointed to\n> the HEAD commit of the remote repo, and git clone would sometimes\n> choose the wrong one as the HEAD ref isn't transmitted in all\n> transport.\n>\n> Stuff the HEAD ref into the capability list (assuming refs are clean\n> enough to do that w/o escaping), and read them out on the other\n> side. All other things were thankfully already in place.\n>\n> Two of the patches have Junio in the From as they are essentially his.\n>\n> Andreas\n> --\n\nDoes this have any impact on the alleged bug in `git bundle --all` \n(which can then be cloned from) where the current HEAD ref wasn't \nincluded in the bundle? Or am I mis-remembering?\n\nPhilip \n"},{"id":"226967","messageId":"xmqqsixhyhan.fsf@gitster.dls.corp.google.com","threadId":"34865","inReplyTo":"20130906155608.GF12966@inner.h.apk.li","subject":"Re: [PATCH 1/3] upload-pack: send the HEAD information","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-06T17:46:24Z","receivedAt":"2013-09-06T17:46:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andreas Krey writes:\n\n> From: Junio C Hamano \n>\n> This implements the server side of protocol extension to show which branch\n> the HEAD points at. The information is sent as a capability symref=.\n>\n> Signed-off-by: Junio C Hamano \n> Signed-off-by: Andreas Krey \n> ---\n> upload-pack.c | 8 ++++++--\n> 1 file changed, 6 insertions(+), 2 deletions(-)\n>\n> diff --git a/upload-pack.c b/upload-pack.c\n> index 127e59a..390d1ec 100644\n> --- a/upload-pack.c\n> +++ b/upload-pack.c\n> @@ -745,13 +745,17 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo\n> \tif (mark_our_ref(refname, sha1, flag, cb_data))\n> \t\treturn 0;\n> \n> -\tif (capabilities)\n> -\t\tpacket_write(1, \"%s %s%c%s%s%s agent=%s\\n\",\n> +\tif (capabilities) {\n> +\t\tunsigned char dummy[20];\n> +\t\tconst char *target = resolve_ref_unsafe(\"HEAD\", dummy, 0, NULL);\n> +\t\tpacket_write(1, \"%s %s%c%s%s%s%s%s agent=%s\\n\",\n> \t\t\t sha1_to_hex(sha1), refname_nons,\n> \t\t\t 0, capabilities,\n> \t\t\t allow_tip_sha1_in_want ? \" allow-tip-sha1-in-want\" : \"\",\n> \t\t\t stateless_rpc ? \" no-done\" : \"\",\n> +\t\t\t target ? \" symref=\" : \"\", target ? target : 0,\n> \t\t\t git_user_agent_sanitized());\n> +\t}\n> \telse\n> \t\tpacket_write(1, \"%s %s\\n\", sha1_to_hex(sha1), refname_nons);\n> \tcapabilities = NULL;\n\nI think it is perfectly fine to expose _only_ HEAD now, and wait\nuntil we find a good reason that we should send this information for\nother symbolic refs in the repository.\n\nHowever, because we already anticipate that we may find such a good\nreason later, on-the-wire format should be prepared to support such\nlater enhancement. I think sending\n\n\tsymref=HEAD:refs/heads/master\n\nis probably one good way to do so, as Peff suggested in that old\nthread ($gmane/102070; note that back then this wasn't suggested as\na proper capability so the exact format he suggests in the message\nis different). Then we could later add advertisements for other\nsymbolic refs if we find it necessary to do so, e.g.\n\n\tsymref=HEAD:refs/heads/master\n symref=refs/remotes/origin/HEAD:refs/remotes/origin/master\n\n(all on one line together with other capabilities separated with a\nSP in between).\n"},{"id":"226968","messageId":"xmqqob85ygt8.fsf@gitster.dls.corp.google.com","threadId":"34865","inReplyTo":"20130906155655.GG12966@inner.h.apk.li","subject":"Re: [PATCH 2/3] connect.c: save symref info from server capabilities","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-06T17:56:51Z","receivedAt":"2013-09-06T17:56:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andreas Krey writes:\n\n> Signed-off-by: Andreas Krey \n> ---\n> connect.c | 11 +++++++++--\n> 1 file changed, 9 insertions(+), 2 deletions(-)\n>\n> diff --git a/connect.c b/connect.c\n> index a0783d4..98c4868 100644\n> --- a/connect.c\n> +++ b/connect.c\n> @@ -72,8 +72,8 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,\n> \tfor (;;) {\n> \t\tstruct ref *ref;\n> \t\tunsigned char old_sha1[20];\n> -\t\tchar *name;\n> -\t\tint len, name_len;\n> +\t\tchar *name, *symref;\n> +\t\tint len, name_len, symref_len;\n> \t\tchar *buffer = packet_buffer;\n> \n> \t\tlen = packet_read(in, &src_buf, &src_len,\n> @@ -94,9 +94,12 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,\n> \t\tname = buffer + 41;\n> \n> \t\tname_len = strlen(name);\n> +\t\tsymref = 0;\n> \t\tif (len != name_len + 41) {\n> \t\t\tfree(server_capabilities);\n> \t\t\tserver_capabilities = xstrdup(name + name_len + 1);\n> +\t\t\tsymref = parse_feature_value(server_capabilities,\n> +\t\t\t\t\t\t \"symref\", &symref_len);\n> \t\t}\n> \t\tif (extra_have &&\n> @@ -108,6 +111,10 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,\n> \t\tif (!check_ref(name, name_len, flags))\n> \t\t\tcontinue;\n> \t\tref = alloc_ref(buffer + 41);\n> +\t\tif (symref) {\n> +\t\t\tref->symref = xcalloc(symref_len + 1, 1);\n> +\t\t\tstrncpy(ref->symref, symref, symref_len);\n> +\t\t}\n> \t\thashcpy(ref->old_sha1, old_sha1);\n> \t\t*list = ref;\n> \t\tlist = &ref->next;\n\n\nThis looks utterly wrong. HEAD may happen to be the first ref that\nis advertised and hence capability list typically comes on it, but\nthat does not necessarily have to be the case from the protocol's\ncorrectness point of view.\n\nI think this function should do this instead.\n\n - inside the loop, collect the \"symref=...\" capabilities;\n\n - after the loop, look at the \"symref=...\" capabilities, and\n among the refs the loop added to the *list, find the \"HEAD\"\n ref and set its ->symref to point at an appropirate ref.\n"},{"id":"226971","messageId":"xmqqfvthyfui.fsf@gitster.dls.corp.google.com","threadId":"34865","inReplyTo":"6649DD0E3B6B4CE59D330217786B6B05@PhilipOakley","subject":"Re: [PATCH 0/3] Unconfuse git clone when two branches at are HEAD.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-06T18:17:41Z","receivedAt":"2013-09-06T18:17:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Philip Oakley\" writes:\n\n> Does this have any impact on the alleged bug in `git bundle --all`\n> (which can then be cloned from) where the current HEAD ref wasn't\n> included in the bundle? Or am I mis-remembering?\n\nNot \"current HEAD ref\", but \"git clone\" will fail to check out from\na bundle that does not include HEAD ref (it is easy to just say\n\"reset --hard master\" or whatever after it, though).\n\nI think I suggested to update \"git bundle\" to include HEAD when\nthere is no HEAD specified some time ago, but I do not think anybody\nwas interested, so this may be a non-issue.\n"},{"id":"226978","messageId":"20130906192515.GI12966@inner.h.apk.li","threadId":"34865","inReplyTo":"xmqqob85ygt8.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 2/3] connect.c: save symref info from server capabilities","fromName":"Andreas Krey","fromEmail":"a.krey@gmx.de","sentAt":"2013-09-06T19:25:15Z","receivedAt":"2013-09-06T19:25:15Z","isPatch":true,"sender":{"key":"a.krey@gmx.de","avatar":"https://avatars.githubusercontent.com/u/37810?v=4"},"body":"On Fri, 06 Sep 2013 10:56:51 +0000, Junio C Hamano wrote:\n> Andreas Krey writes:\n> \n...\n> > +\t\tif (symref) {\n> > +\t\t\tref->symref = xcalloc(symref_len + 1, 1);\n> > +\t\t\tstrncpy(ref->symref, symref, symref_len);\n> > +\t\t}\n...\n> \n> This looks utterly wrong. HEAD may happen to be the first ref that\n> is advertised and hence capability list typically comes on it, but\n> that does not necessarily have to be the case from the protocol's\n> correctness point of view.\n\nOk, then I misunderstood that part. I thought we'd be going to\nput the symref (if any) into 'capabilities' on the respective ref,\nbut putting them all in one capability list sounds saner all in all.\n\n> I think this function should do this instead.\n> \n> - inside the loop, collect the \"symref=...\" capabilities;\n> \n> - after the loop, look at the \"symref=...\" capabilities, and\n> among the refs the loop added to the *list, find the \"HEAD\"\n> ref and set its ->symref to point at an appropirate ref.\n\nFair game. There goes the parse_feature_value; will have to iterate\nanother way (or look them (\"symref=#{name}:\") up instead of collecting\nthem into a hash beforehand).\n\nCan I assume that the only capability list is always on the\nfirst ref sent (as it is now)?\n\n(And besides, is there something more suitable for the \n xcalloc/strncpy combination?)\n\nAndreas\n\n-- \n\"Totally trivial. Famous last words.\"\nFrom: Linus Torvalds \nDate: Fri, 22 Jan 2010 07:29:21 -0800\n"},{"id":"226980","messageId":"20130906192918.GJ12966@inner.h.apk.li","threadId":"34865","inReplyTo":"xmqqsixhyhan.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 1/3] upload-pack: send the HEAD information","fromName":"Andreas Krey","fromEmail":"a.krey@gmx.de","sentAt":"2013-09-06T19:29:18Z","receivedAt":"2013-09-06T19:29:18Z","isPatch":true,"sender":{"key":"a.krey@gmx.de","avatar":"https://avatars.githubusercontent.com/u/37810?v=4"},"body":"On Fri, 06 Sep 2013 10:46:24 +0000, Junio C Hamano wrote:\n> Andreas Krey writes:\n> \n...\n> reason later, on-the-wire format should be prepared to support such\n> later enhancement. I think sending\n> \n> \tsymref=HEAD:refs/heads/master\n\nMirco-bikeshed: Should that possibly be\n\n symref:HEAD=refs/heads/master\n\nas then 'symref:HEAD' is a regular capability key?\n\nAndreas\n\n-- \n\"Totally trivial. Famous last words.\"\nFrom: Linus Torvalds \nDate: Fri, 22 Jan 2010 07:29:21 -0800\n"},{"id":"226981","messageId":"xmqqvc2dwx64.fsf@gitster.dls.corp.google.com","threadId":"34865","inReplyTo":"20130906192515.GI12966@inner.h.apk.li","subject":"Re: [PATCH 2/3] connect.c: save symref info from server capabilities","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-06T19:46:27Z","receivedAt":"2013-09-06T19:46:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andreas Krey writes:\n\n> Can I assume that the only capability list is always on the\n> first ref sent (as it is now)?\n\nThe capability list _could_ be sent more than once, and the\nreceiving end is prepared to accept such a stream. Everything\nlearned from an older capability list needs to be forgot and only\nthe last one is honored, I think.\n"},{"id":"226982","messageId":"xmqqr4d1wwtd.fsf@gitster.dls.corp.google.com","threadId":"34865","inReplyTo":"20130906192918.GJ12966@inner.h.apk.li","subject":"Re: [PATCH 1/3] upload-pack: send the HEAD information","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-06T19:54:06Z","receivedAt":"2013-09-06T19:54:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andreas Krey writes:\n\n> On Fri, 06 Sep 2013 10:46:24 +0000, Junio C Hamano wrote:\n>> Andreas Krey writes:\n>> \n> ...\n>> reason later, on-the-wire format should be prepared to support such\n>> later enhancement. I think sending\n>> \n>> \tsymref=HEAD:refs/heads/master\n>\n> Mirco-bikeshed: Should that possibly be\n>\n> symref:HEAD=refs/heads/master\n>\n> as then 'symref:HEAD' is a regular capability key?\n\nI doubt that is a good change.\n\nWe haven't needed (and have refrained from adding) any capability\nwith an unknown name. The variable part should go to the value\nportion of the token.\n"},{"id":"226999","messageId":"94A71512041A4F9BB402474DB385E310@PhilipOakley","threadId":"34865","inReplyTo":"xmqqfvthyfui.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 0/3] Unconfuse git clone when two branches at are HEAD.","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2013-09-06T23:19:39Z","receivedAt":"2013-09-06T23:19:39Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Junio C Hamano\" \n> \"Philip Oakley\" writes:\n>\n>> Does this have any impact on the alleged bug in `git bundle --all`\n>> (which can then be cloned from) where the current HEAD ref wasn't\n>> included in the bundle? Or am I mis-remembering?\n>\n> Not \"current HEAD ref\", but \"git clone\" will fail to check out from\n> a bundle that does not include HEAD ref (it is easy to just say\n> \"reset --hard master\" or whatever after it, though).\n>\n> I think I suggested to update \"git bundle\" to include HEAD when\n> there is no HEAD specified some time ago, but I do not think anybody\n> was interested, so this may be a non-issue.\n>\nJust had a quick look at a very quick test repo (10 objects, 2 branches) \nand the bundle file does contain the HEAD ref, but again it has the two \nref/heads/* are better than one problem, in that the clone from the \nbundle checks out master, whilst the source repo has feature checked \nout.\n\nPhilip \n"},{"id":"227022","messageId":"xmqqwqmsvdfh.fsf@gitster.dls.corp.google.com","threadId":"34865","inReplyTo":"94A71512041A4F9BB402474DB385E310@PhilipOakley","subject":"Re: [PATCH 0/3] Unconfuse git clone when two branches at are HEAD.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-07T15:50:26Z","receivedAt":"2013-09-07T15:50:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Philip Oakley\" writes:\n\n> From: \"Junio C Hamano\" \n>> \"Philip Oakley\" writes:\n>>\n>>> Does this have any impact on the alleged bug in `git bundle --all`\n>>> (which can then be cloned from) where the current HEAD ref wasn't\n>>> included in the bundle? Or am I mis-remembering?\n>>\n>> Not \"current HEAD ref\", but \"git clone\" will fail to check out from\n>> a bundle that does not include HEAD ref (it is easy to just say\n>> \"reset --hard master\" or whatever after it, though).\n>>\n>> I think I suggested to update \"git bundle\" to include HEAD when\n>> there is no HEAD specified some time ago, but I do not think anybody\n>> was interested, so this may be a non-issue.\n>>\n> Just had a quick look at a very quick test repo (10 objects, 2\n> branches) and the bundle file does contain the HEAD ref, but again it\n> has the two ref/heads/* are better than one problem, in that the clone\n> from the bundle checks out master, whilst the source repo has feature\n> checked out.\n\nI do not think the bundle header records symref any differently from\nother refs, so a HEAD that points at a commit that is at the tip of\nmore than one ref needs to be guessed at the extraction end, just\nlike the network-transfer case discussed in this thread.\n\nBut this thread is not about updating the current bundle format to a\nnew one, so any of the updates proposed in these patches will not\naffect it.\n"},{"id":"227027","messageId":"531DBE1FF66D4356AEE6AEE5C2FE9389@PhilipOakley","threadId":"34865","inReplyTo":"xmqqwqmsvdfh.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 0/3] Unconfuse git clone when two branches at are HEAD.","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2013-09-07T19:19:20Z","receivedAt":"2013-09-07T19:19:20Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Junio C Hamano\" \nSent: Saturday, September 07, 2013 4:50 PM\n> \"Philip Oakley\" writes:\n>> From: \"Junio C Hamano\" \n>>> \"Philip Oakley\" writes:\n>>>\n>>>> Does this have any impact on the alleged bug in `git bundle --all`\n>>>> (which can then be cloned from) where the current HEAD ref wasn't\n>>>> included in the bundle? Or am I mis-remembering?\n>>>\n>>> Not \"current HEAD ref\", but \"git clone\" will fail to check out from\n>>> a bundle that does not include HEAD ref (it is easy to just say\n>>> \"reset --hard master\" or whatever after it, though).\n>>>\n>>> I think I suggested to update \"git bundle\" to include HEAD when\n>>> there is no HEAD specified some time ago, but I do not think anybody\n>>> was interested, so this may be a non-issue.\n>>>\n>> Just had a quick look at a very quick test repo (10 objects, 2\n>> branches) and the bundle file does contain the HEAD ref, but again it\n>> has the two ref/heads/* are better than one problem, in that the \n>> clone\n>> from the bundle checks out master, whilst the source repo has feature\n>> checked out.\n>\n> I do not think the bundle header records symref any differently from\n> other refs, so a HEAD that points at a commit that is at the tip of\n> more than one ref needs to be guessed at the extraction end, just\n> like the network-transfer case discussed in this thread.\n>\n> But this thread is not about updating the current bundle format to a\n> new one, so any of the updates proposed in these patches will not\n> affect it.\n> --\nI was having a quick look at the different bundle/clone routes and tried \nout (on 1.8.1.msysgit.1) the following script to see the differences \n(probably word wrap damaged):\n\n---\ncd /c/ # if on Windows to be at the top of c:/\nmkdir gitBundleTest1\ncd gitBundleTest1\ngit init\necho AAA >a.txt\ngit add a.txt\ngit commit -mfirst\ngit checkout -b feature\ngit checkout -b zulu # does this, alphabetically after master, change \nanything?\ngit status # observe on 'feature' branch\n# one repo, one file, one commit, two branches\n\n# test the bundle - clone transfer\ngit bundle create Repo.bundle --all\ngit clone Repo.bundle ../gitBundleTest2\ncd ../gitBundleTest2\ngit status # observe on wrong branch\n\n# back to original repo\ncd ../gitBundleTest1\n# test the direct clone transfer\ngit clone . ../gitBundleTest3\ncd ../gitBundleTest3\ngit status # observe on wrong branch again\n\n# back to top level (wherever that is on Msys Windows ;-)\ncd ..\npwd\n# test the git protocol clone transfer\n# it's file:// followed by abolute path /path/to/dir so ...\n# but note msys windows /c/\ngit clone file:///c/gitBundleTest1 ./gitBundleTest4\ncd ./gitBundleTest4\ngit status # observe on wrong branch again\n\ncd ~ # return home\n---\n\nWhat I observed was that all the clones had the same HEAD problem, which \nI think comes from clone.c: guess_remote_head().\n\nWhen I looked in the Repo.bundle file I saw the refs/heads/* listed in \nalphabetic order followed by HEAD, all with the same sha1 (in this \ncase), followed by PACK and then the binary data.\n\nMy quick look at clone.c suggested to me that there would be a lot of \ncommonality between the bundle data stream and the transport streams \n(identical?), and it was just a case of adding into the bundle data the \nsame HEAD symref indication that would solve the normal clone problem \n(including backward compatibility). Is that a reasonable assesssment?\n\nPhilip\n"},{"id":"227082","messageId":"20130908071359.GJ14019@sigill.intra.peff.net","threadId":"34865","inReplyTo":"xmqqsixhyhan.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 1/3] upload-pack: send the HEAD information","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-09-08T07:13:59Z","receivedAt":"2013-09-08T07:13:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Sep 06, 2013 at 10:46:24AM -0700, Junio C Hamano wrote:\n\n> I think it is perfectly fine to expose _only_ HEAD now, and wait\n> until we find a good reason that we should send this information for\n> other symbolic refs in the repository.\n\nYeah, I agree with that.\n\n> However, because we already anticipate that we may find such a good\n> reason later, on-the-wire format should be prepared to support such\n> later enhancement. I think sending\n> \n> \tsymref=HEAD:refs/heads/master\n> \n> is probably one good way to do so, as Peff suggested in that old\n> thread ($gmane/102070; note that back then this wasn't suggested as\n> a proper capability so the exact format he suggests in the message\n> is different). Then we could later add advertisements for other\n> symbolic refs if we find it necessary to do so, e.g.\n> \n> \tsymref=HEAD:refs/heads/master\n> symref=refs/remotes/origin/HEAD:refs/remotes/origin/master\n> \n> (all on one line together with other capabilities separated with a\n> SP in between).\n\nIt somehow feels a little weird to me that we would output the\ninformation about refs/foo on the HEAD line. A few possible issues (and\nI am playing devil's advocate to some degree here):\n\n 1. What if we have a large number of symrefs? Would we run afoul of\n pkt-line length limits?\n\n 2. What's the impact of having to display all symrefs on the first\n line, before we output other refs? Right now we can just stream out\n refs as we read them, but we would have to make two passes (and/or\n cache them all) to find all of the symrefs before we start\n outputting. Will the extra latency ever matter?\n\nWhat do you think about teaching git to read extra data after \"\\0\" for\n_every_ ref line? And then ref advertisement might look something like:\n\n HEAD\\0multi_ack thin-pack ... symref=refs/heads/master\\n\n refs/heads/master\\n\n refs/heads/my-alias\\0symref=refs/heads/master\n\nThat would leave us future room for more ref annotations if we should\nwant them, and I think (but haven't tested) that existing receivers\nshould ignore everything after the NUL.\n\n-Peff\n"},{"id":"227099","messageId":"20130908072243.GK14019@sigill.intra.peff.net","threadId":"34865","inReplyTo":"20130908071359.GJ14019@sigill.intra.peff.net","subject":"Re: [PATCH 1/3] upload-pack: send the HEAD information","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-09-08T07:22:43Z","receivedAt":"2013-09-08T07:22:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Sep 08, 2013 at 03:13:59AM -0400, Jeff King wrote:\n\n> What do you think about teaching git to read extra data after \"\\0\" for\n> _every_ ref line? And then ref advertisement might look something like:\n> \n> HEAD\\0multi_ack thin-pack ... symref=refs/heads/master\\n\n> refs/heads/master\\n\n> refs/heads/my-alias\\0symref=refs/heads/master\n> \n> That would leave us future room for more ref annotations if we should\n> want them, and I think (but haven't tested) that existing receivers\n> should ignore everything after the NUL.\n\nMeh, elsewhere you said:\n\n The capability list _could_ be sent more than once, and the\n receiving end is prepared to accept such a stream. Everything\n learned from an older capability list needs to be forgot and only\n the last one is honored, I think.\n\nand I think you are right. We simply keep a copy of the string the\nserver sent, and when we see a new one, we free the old and replace it.\nSo each subsequent ref would have to re-send the whole capability string\n(only if it is a symref, but still, it is somewhat ugly).\n\n-Peff\n"},{"id":"227279","messageId":"xmqqli351u7r.fsf@gitster.dls.corp.google.com","threadId":"34865","inReplyTo":"20130908071359.GJ14019@sigill.intra.peff.net","subject":"Re: [PATCH 1/3] upload-pack: send the HEAD information","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-08T17:27:50Z","receivedAt":"2013-09-08T17:27:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> It somehow feels a little weird to me that we would output the\n> information about refs/foo on the HEAD line.\n\nI see that you realized why the above is not the case in the\ndownthread; the capability list is not about describing HEAD. The\nlist happens to be on the line for HEAD merely because HEAD is the\nfirst ref.\n\nUnfortunately, the \"read and replace capability if we see one\" (not\n\"read and update capability with newly discovered one on a second\nand subsequent capability list\") is a restriction imposed by existing\nreader side code that are deployed on the wild, so we need to stick\nto it until we revamp the protocol in a backward incompatible way\n(which has been discussed a few times; websearch for \"who speaks\nfirst\" in the list archive).\n"},{"id":"227143","messageId":"xmqqk3ir6wu3.fsf@gitster.dls.corp.google.com","threadId":"34865","inReplyTo":"531DBE1FF66D4356AEE6AEE5C2FE9389@PhilipOakley","subject":"Re: [PATCH 0/3] Unconfuse git clone when two branches at are HEAD.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-08T17:35:00Z","receivedAt":"2013-09-08T17:35:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Philip Oakley\" writes:\n\n> What I observed was that all the clones had the same HEAD problem,\n> which I think comes from clone.c: guess_remote_head().\n\nYes. They share \"having to guess\" property because their data\nsource does not tell them.\n\n> My quick look at clone.c suggested to me that there would be a lot of\n> commonality between the bundle data stream and the transport streams\n> (identical?), and it was just a case of adding into the bundle data\n> the same HEAD symref indication that would solve the normal clone\n> problem (including backward compatibility). Is that a reasonable\n> assesssment?\n\nYou need to find a hole in the existing readers to stick the new\ninformation in a way that do not break existing readers but allow\nupdated readers to extract that information. That is exactly what\nwe did when we added the protocol capability. I do not offhand\nthink an equivalent hole exists in the bundle file format.\n"},{"id":"227158","messageId":"5425F66B510F423EA685BCEF40EF8FA7@PhilipOakley","threadId":"34865","inReplyTo":"xmqqk3ir6wu3.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 0/3] Unconfuse git clone when two branches at are HEAD.","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2013-09-08T21:00:24Z","receivedAt":"2013-09-08T21:00:24Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Junio C Hamano\" \nSent: Sunday, September 08, 2013 6:35 PM\n> \"Philip Oakley\" writes:\n>\n>> What I observed was that all the clones had the same HEAD problem,\n>> which I think comes from clone.c: guess_remote_head().\n>\n> Yes. They share \"having to guess\" property because their data\n> source does not tell them.\n>\n>> My quick look at clone.c suggested to me that there would be a lot of\n>> commonality between the bundle data stream and the transport streams\n>> (identical?), and it was just a case of adding into the bundle data\n>> the same HEAD symref indication that would solve the normal clone\n>> problem (including backward compatibility). Is that a reasonable\n>> assesssment?\n>\n> You need to find a hole in the existing readers to stick the new\n> information in a way that do not break existing readers but allow\n> updated readers to extract that information. That is exactly what\n> we did when we added the protocol capability. I do not offhand\n> think an equivalent hole exists in the bundle file format.\n> --\n\nI've been rummaging about as to options.\n\nOne is to extend the ref format such that\n refs/heads/Test:HEAD\nwould be considered a valid indicator of a symref relationship (i.e. \nusing the typical 'colon' style). It would be appended after the regular \nrefs, so all the existing refs are still transported.\n\nThe point is that while it produces an error, it doesn't stop the \ncloning, and the error message\n \"error: * Ignoring funny ref 'refs/remotes/origin/Test:HEAD' locally\"\ngives a pretty clear statement of intent to those with older versions of \ngit.\n\nAnother alternative is to add an additional name space (e.g.)\n refs/remotes/origin/HEAD/Test\nwhich would simply be an extra directory layer that reflects where the \nHEAD should have been. Though this namespace example has the D/F \nconflict.\n\nPhilip\n"},{"id":"227231","messageId":"xmqqr4cy5a2z.fsf@gitster.dls.corp.google.com","threadId":"34865","inReplyTo":"5425F66B510F423EA685BCEF40EF8FA7@PhilipOakley","subject":"Re: [PATCH 0/3] Unconfuse git clone when two branches at are HEAD.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-09T14:44:04Z","receivedAt":"2013-09-09T14:44:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Philip Oakley\" writes:\n\n> One is to extend the ref format such that\n> refs/heads/Test:HEAD\n> would be considered a valid indicator of a symref relationship\n> (i.e. using the typical 'colon' style). It would be appended after the\n> regular refs, so all the existing refs are still transported.\n>\n> The point is that while it produces an error, it doesn't stop the\n> cloning, and the error message\n> \"error: * Ignoring funny ref 'refs/remotes/origin/Test:HEAD' locally\"\n> gives a pretty clear statement of intent to those with older versions\n> of git.\n\nCute. If it does not stop any of these:\n\n git ls-remote such.bundle\n\tgit clone such.bundle\n git fetch such.bundle\n\tgit fetch such.bundle master ;# if 'master' branch is in it\n git ls-remote such.bundle\n git ls-remote such.bundle master ;# if 'master' branch is in it\n\neven if some of them may give error messages, I think that may be a\nworkable escape hatch.\n\n> Another alternative is to add an additional name space (e.g.)\n> refs/remotes/origin/HEAD/Test\n> which would simply be an extra directory layer that reflects where the\n> HEAD should have been. Though this namespace example has the D/F\n> conflict.\n\nI'd rather not go this route. Allowing refs/heads/master and local\nbranches that forked from it in refs/heads/master/{a,b,c,...} could\nbe a potentially useful future enhancement, and this approach will\nclose the door for it.\n"},{"id":"227235","messageId":"20130909160817.GN12966@inner.h.apk.li","threadId":"34865","inReplyTo":"xmqqr4cy5a2z.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 0/3] Unconfuse git clone when two branches at are HEAD.","fromName":"Andreas Krey","fromEmail":"a.krey@gmx.de","sentAt":"2013-09-09T16:08:17Z","receivedAt":"2013-09-09T16:08:17Z","isPatch":true,"sender":{"key":"a.krey@gmx.de","avatar":"https://avatars.githubusercontent.com/u/37810?v=4"},"body":"On Mon, 09 Sep 2013 07:44:04 +0000, Junio C Hamano wrote:\n...\n> I'd rather not go this route. Allowing refs/heads/master and local\n> branches that forked from it in refs/heads/master/{a,b,c,...} could\n> be a potentially useful future enhancement,\n\nWant! We're currently going the route of naming the branches\n\n master\n feature/master\n feature/subfeature/master\n\nto allow feature/subfeature/topic, and feature/subfeature in the first place.\n\n(Other hierarchy separator candidates were ugly, shell-unwieldy, already\ncommonly used within branch names, or illegal.)\n\nAndreas\n\n-- \n\"Totally trivial. Famous last words.\"\nFrom: Linus Torvalds \nDate: Fri, 22 Jan 2010 07:29:21 -0800\n"},{"id":"227270","messageId":"CBB3BFF2B74D4A768238512FBF1CD6E7@PhilipOakley","threadId":"34865","inReplyTo":"xmqqr4cy5a2z.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 0/3] Unconfuse git clone when two branches at are HEAD.","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2013-09-09T22:20:41Z","receivedAt":"2013-09-09T22:20:41Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Junio C Hamano\" \nSent: Monday, September 09, 2013 3:44 PM\n> \"Philip Oakley\" writes:\n>\n>> One is to extend the ref format such that\n>> refs/heads/Test:HEAD\n>> would be considered a valid indicator of a symref relationship\n>> (i.e. using the typical 'colon' style). It would be appended after \n>> the\n>> regular refs, so all the existing refs are still transported.\n>>\n>> The point is that while it produces an error, it doesn't stop the\n>> cloning, and the error message\n>> \"error: * Ignoring funny ref 'refs/remotes/origin/Test:HEAD' locally\"\n>> gives a pretty clear statement of intent to those with older versions\n>> of git.\n>\n> Cute. If it does not stop any of these:\n>\n> git ls-remote such.bundle\n> git clone such.bundle\n> git fetch such.bundle\n> git fetch such.bundle master ;# if 'master' branch is in it\n> git ls-remote such.bundle\n> git ls-remote such.bundle master ;# if 'master' branch is in it\n>\n> even if some of them may give error messages, I think that may be a\n> workable escape hatch.\n>\nThese look to work OK. Not sure If I've properly covered all the \noptions.\n\nA nice feature is that ls-remote will find the fake ref !\n\n$ git ls-remote /c/gitBundleTest1/RepoHEADnomaster.bundle Test:HEAD\n41ccb18028d1cb6516251e94cef1cd5cb3f0bcb5 refs/heads/Test:HEAD\n\nIt's only the clone that barfs (so far) (which could be 'fixed').\n\nObviously, if it can be made to work, one check would be that the two \nrefs (HEAD and refs/heads/wherever) point to the came commit before \ngenerating the HEAD symref.\n\nPhilip \n"},{"id":"227271","messageId":"xmqqk3ip3a36.fsf@gitster.dls.corp.google.com","threadId":"34865","inReplyTo":"CBB3BFF2B74D4A768238512FBF1CD6E7@PhilipOakley","subject":"Re: [PATCH 0/3] Unconfuse git clone when two branches at are HEAD.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-09T22:26:53Z","receivedAt":"2013-09-09T22:26:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Philip Oakley\" writes:\n\n> These look to work OK. Not sure If I've properly covered all the\n> options.\n>\n> A nice feature is that ls-remote will find the fake ref !\n>\n> $ git ls-remote /c/gitBundleTest1/RepoHEADnomaster.bundle Test:HEAD\n> 41ccb18028d1cb6516251e94cef1cd5cb3f0bcb5 refs/heads/Test:HEAD\n>\n> It's only the clone that barfs (so far) (which could be 'fixed').\n\nYou cannot 'fix' the ones deployed in the wild, but I think saying\n\"funny ref\" and not aborting is a good compromise. The updated\ndocumentation for bundle can mention that error message and explain\nthat the older version of \"clone\" will say that in what situation.\n\nI didn't notice it the first time, but I think the above would read\nbetter and match what has been discussed on the on-the-wire format\nto swap the order, i.e. use \"HEAD:refs/heads/Test\" to mean \"HEAD is\na symref that points at the Test branch\".\n\n> Obviously, if it can be made to work, one check would be that the two\n> refs (HEAD and refs/heads/wherever) point to the came commit before\n> generating the HEAD symref.\n\nYes, that is a sensible check.\n"},{"id":"227812","messageId":"1379471489-26280-1-git-send-email-gitster@pobox.com","threadId":"34865","inReplyTo":"20130906155608.GF12966@inner.h.apk.li","subject":"[PATCH v2 0/6] Removing the guesswork of HEAD in \"clone\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-18T02:31:23Z","receivedAt":"2013-09-18T02:31:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This reworks the old idea from 2008 ($gmane/102039) to teach\nupload-pack to say where symbolic refs are pointing at in the\ninitial ref advertisement as a new capability \"sym\", and allow\n\"git clone\" to take advantage of that knowledge when deciding what\nbranch to point at with the HEAD of the newly created repository.\n\nThanks go to Andreas Krey for reigniting the ember in a patch series\na few weeks ago.\n\nI did not do anything more than just compile it once; all the bugs\nin this round are mine (it is all new code after all).\n\n\nJunio C Hamano (6):\n upload-pack.c: do not pass confusing cb_data to mark_our_ref()\n upload-pack: send symbolic ref information as capability\n upload-pack: send non-HEAD symbolic refs\n connect.c: make parse_feature_value() static\n connect: annotate refs with their symref information in get_remote_head()\n clone: test the new HEAD detection logic\n\n cache.h | 1 -\n connect.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-\n t/t5601-clone.sh | 11 ++++++++++\n upload-pack.c | 51 +++++++++++++++++++++++++++++++++++++++------\n 4 files changed, 118 insertions(+), 8 deletions(-)\n\n-- \n1.8.4-585-g8d1dcaf\n"},{"id":"227813","messageId":"1379471489-26280-2-git-send-email-gitster@pobox.com","threadId":"34865","inReplyTo":"1379471489-26280-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 1/6] upload-pack.c: do not pass confusing cb_data to mark_our_ref()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-18T02:31:24Z","receivedAt":"2013-09-18T02:31:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The callee does not use cb_data, and the caller is an intermediate\nfunction in a callchain that later wants to use the cb_data for its\nown use. Clarify the code by breaking the dataflow explicitly by\nnot passing cb_data down to mark_our_ref().\n\nSigned-off-by: Junio C Hamano \n---\n upload-pack.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 127e59a..a6e107f 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -742,7 +742,7 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo\n \tconst char *refname_nons = strip_namespace(refname);\n \tunsigned char peeled[20];\n \n-\tif (mark_our_ref(refname, sha1, flag, cb_data))\n+\tif (mark_our_ref(refname, sha1, flag, NULL))\n \t\treturn 0;\n \n \tif (capabilities)\n-- \n1.8.4-585-g8d1dcaf\n"},{"id":"227814","messageId":"1379471489-26280-3-git-send-email-gitster@pobox.com","threadId":"34865","inReplyTo":"1379471489-26280-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 2/6] upload-pack: send symbolic ref information as capability","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-18T02:31:25Z","receivedAt":"2013-09-18T02:31:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"One long-standing flaw in the pack transfer protocol was that there\nwas no way to tell the other end which branch \"HEAD\" points at.\nWith a new \"sym\" capability (e.g. \"sym=HEAD:refs/heads/master\"; can\nbe repeated more than once to represent symbolic refs other than\nHEAD, such as \"refs/remotes/origin/HEAD\").\n\nAdd an infrastructure to collect symbolic refs, format them as extra\ncapabilities and put it on the wire. For now, just send information\non the \"HEAD\" and nothing else.\n\nSigned-off-by: Junio C Hamano \n---\n upload-pack.c | 48 +++++++++++++++++++++++++++++++++++++++++++-----\n 1 file changed, 43 insertions(+), 5 deletions(-)\n\ndiff --git a/upload-pack.c b/upload-pack.c\nindex a6e107f..53958b9 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -734,6 +734,16 @@ static int mark_our_ref(const char *refname, const unsigned char *sha1, int flag\n \treturn 0;\n }\n \n+static void format_symref_info(struct strbuf *buf, struct string_list *symref)\n+{\n+\tstruct string_list_item *item;\n+\n+\tif (!symref->nr)\n+\t\treturn;\n+\tfor_each_string_list_item(item, symref)\n+\t\tstrbuf_addf(buf, \" sym=%s:%s\", item->string, (char *)item->util);\n+}\n+\n static int send_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)\n {\n \tstatic const char *capabilities = \"multi_ack thin-pack side-band\"\n@@ -745,32 +755,60 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo\n \tif (mark_our_ref(refname, sha1, flag, NULL))\n \t\treturn 0;\n \n-\tif (capabilities)\n-\t\tpacket_write(1, \"%s %s%c%s%s%s agent=%s\\n\",\n+\tif (capabilities) {\n+\t\tstruct strbuf symref_info = STRBUF_INIT;\n+\n+\t\tformat_symref_info(&symref_info, cb_data);\n+\t\tpacket_write(1, \"%s %s%c%s%s%s%s agent=%s\\n\",\n \t\t\t sha1_to_hex(sha1), refname_nons,\n \t\t\t 0, capabilities,\n \t\t\t allow_tip_sha1_in_want ? \" allow-tip-sha1-in-want\" : \"\",\n \t\t\t stateless_rpc ? \" no-done\" : \"\",\n+\t\t\t symref_info.buf,\n \t\t\t git_user_agent_sanitized());\n-\telse\n+\t\tstrbuf_release(&symref_info);\n+\t} else {\n \t\tpacket_write(1, \"%s %s\\n\", sha1_to_hex(sha1), refname_nons);\n+\t}\n \tcapabilities = NULL;\n \tif (!peel_ref(refname, peeled))\n \t\tpacket_write(1, \"%s %s^{}\\n\", sha1_to_hex(peeled), refname_nons);\n \treturn 0;\n }\n \n+static int find_symref(const char *refname, const unsigned char *sha1, int flag,\n+\t\t void *cb_data)\n+{\n+\tconst char *symref_target;\n+\tstruct string_list_item *item;\n+\tunsigned char unused[20];\n+\n+\tif ((flag & REF_ISSYMREF) == 0)\n+\t\treturn 0;\n+\tsymref_target = resolve_ref_unsafe(refname, unused, 0, &flag);\n+\tif (!symref_target || (flag & REF_ISSYMREF) == 0)\n+\t\tdie(\"'%s' is a symref but it is not?\", refname);\n+\titem = string_list_append(cb_data, refname);\n+\titem->util = xstrdup(symref_target);\n+\treturn 0;\n+}\n+\n static void upload_pack(void)\n {\n+\tstruct string_list symref = STRING_LIST_INIT_DUP;\n+\n+\thead_ref_namespaced(find_symref, &symref);\n+\n \tif (advertise_refs || !stateless_rpc) {\n \t\treset_timeout();\n-\t\thead_ref_namespaced(send_ref, NULL);\n+\t\thead_ref_namespaced(send_ref, &symref);\n \t\tfor_each_namespaced_ref(send_ref, NULL);\n \t\tpacket_flush(1);\n \t} else {\n-\t\thead_ref_namespaced(mark_our_ref, NULL);\n+\t\thead_ref_namespaced(mark_our_ref, &symref);\n \t\tfor_each_namespaced_ref(mark_our_ref, NULL);\n \t}\n+\tstring_list_clear(&symref, 1);\n \tif (advertise_refs)\n \t\treturn;\n \n-- \n1.8.4-585-g8d1dcaf\n"},{"id":"227815","messageId":"1379471489-26280-4-git-send-email-gitster@pobox.com","threadId":"34865","inReplyTo":"1379471489-26280-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 3/6] upload-pack: send non-HEAD symbolic refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-18T02:31:26Z","receivedAt":"2013-09-18T02:31:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"With the same mechanism as used to tell where \"HEAD\" points at to\nthe other end, we can tell the target of other symbolic refs as\nwell.\n\nSigned-off-by: Junio C Hamano \n---\n upload-pack.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 53958b9..7ca6154 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -798,6 +798,7 @@ static void upload_pack(void)\n \tstruct string_list symref = STRING_LIST_INIT_DUP;\n \n \thead_ref_namespaced(find_symref, &symref);\n+\tfor_each_namespaced_ref(find_symref, &symref);\n \n \tif (advertise_refs || !stateless_rpc) {\n \t\treset_timeout();\n-- \n1.8.4-585-g8d1dcaf\n"},{"id":"227816","messageId":"1379471489-26280-5-git-send-email-gitster@pobox.com","threadId":"34865","inReplyTo":"1379471489-26280-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 4/6] connect.c: make parse_feature_value() static","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-18T02:31:27Z","receivedAt":"2013-09-18T02:31:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Signed-off-by: Junio C Hamano \n---\n cache.h | 1 -\n connect.c | 3 ++-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 85b544f..2c853ba 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1098,7 +1098,6 @@ extern struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,\n extern int server_supports(const char *feature);\n extern int parse_feature_request(const char *features, const char *feature);\n extern const char *server_feature_value(const char *feature, int *len_ret);\n-extern const char *parse_feature_value(const char *feature_list, const char *feature, int *len_ret);\n \n extern struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_path);\n \ndiff --git a/connect.c b/connect.c\nindex a0783d4..e4c7ae6 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -8,6 +8,7 @@\n #include \"url.h\"\n \n static char *server_capabilities;\n+static const char *parse_feature_value(const char *, const char *, int *);\n \n static int check_ref(const char *name, int len, unsigned int flags)\n {\n@@ -116,7 +117,7 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,\n \treturn list;\n }\n \n-const char *parse_feature_value(const char *feature_list, const char *feature, int *lenp)\n+static const char *parse_feature_value(const char *feature_list, const char *feature, int *lenp)\n {\n \tint len;\n \n-- \n1.8.4-585-g8d1dcaf\n"},{"id":"227818","messageId":"1379471489-26280-6-git-send-email-gitster@pobox.com","threadId":"34865","inReplyTo":"1379471489-26280-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 5/6] connect: annotate refs with their symref information in get_remote_head()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-18T02:31:28Z","receivedAt":"2013-09-18T02:31:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Signed-off-by: Junio C Hamano \n---\n connect.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 60 insertions(+)\n\ndiff --git a/connect.c b/connect.c\nindex e4c7ae6..a53ef6d 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -6,6 +6,7 @@\n #include \"run-command.h\"\n #include \"remote.h\"\n #include \"url.h\"\n+#include \"string-list.h\"\n \n static char *server_capabilities;\n static const char *parse_feature_value(const char *, const char *, int *);\n@@ -60,6 +61,61 @@ static void die_initial_contact(int got_at_least_one_head)\n \t\t \"and the repository exists.\");\n }\n \n+static void parse_one_symref_info(struct string_list *symref, const char *val, int len)\n+{\n+\tchar *sym, *target;\n+\tstruct string_list_item *item;\n+\n+\tif (!len)\n+\t\treturn; /* just \"sym\" */\n+\t/* e.g. \"sym=HEAD:refs/heads/master\" */\n+\tsym = xmalloc(len + 1);\n+\tmemcpy(sym, val, len);\n+\tsym[len] = '\\0';\n+\ttarget = strchr(sym, ':');\n+\tif (!target)\n+\t\t/* just \"sym=something\" */\n+\t\tgoto reject;\n+\t*(target++) = '\\0';\n+\tif (check_refname_format(sym, REFNAME_ALLOW_ONELEVEL) ||\n+\t check_refname_format(target, REFNAME_ALLOW_ONELEVEL))\n+\t\t/* \"sym=bogus:pair */\n+\t\tgoto reject;\n+\titem = string_list_append(symref, sym);\n+\titem->util = target;\n+\treturn;\n+reject:\n+\tfree(sym);\n+\treturn;\n+}\n+\n+static void annotate_refs_with_symref_info(struct ref *ref)\n+{\n+\tstruct string_list symref = STRING_LIST_INIT_DUP;\n+\tconst char *feature_list = server_capabilities;\n+\n+\twhile (feature_list) {\n+\t\tint len;\n+\t\tconst char *val;\n+\n+\t\tval = parse_feature_value(feature_list, \"sym\", &len);\n+\t\tif (!val)\n+\t\t\tbreak;\n+\t\tparse_one_symref_info(&symref, val, len);\n+\t\tfeature_list = val + 1;\n+\t}\n+\tsort_string_list(&symref);\n+\n+\tfor (; ref; ref = ref->next) {\n+\t\tstruct string_list_item *item;\n+\t\titem = string_list_lookup(&symref, ref->name);\n+\t\tif (!item)\n+\t\t\tcontinue;\n+\t\tref->symref = xstrdup((char *)item->util);\n+\t}\n+\tstring_list_clear(&symref, 0);\n+}\n+\n /*\n * Read all the refs from the other end\n */\n@@ -67,6 +123,7 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,\n \t\t\t struct ref **list, unsigned int flags,\n \t\t\t struct extra_have_objects *extra_have)\n {\n+\tstruct ref **orig_list = list;\n \tint got_at_least_one_head = 0;\n \n \t*list = NULL;\n@@ -114,6 +171,9 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,\n \t\tlist = &ref->next;\n \t\tgot_at_least_one_head = 1;\n \t}\n+\n+\tannotate_refs_with_symref_info(*orig_list);\n+\n \treturn list;\n }\n \n-- \n1.8.4-585-g8d1dcaf\n"},{"id":"227817","messageId":"1379471489-26280-7-git-send-email-gitster@pobox.com","threadId":"34865","inReplyTo":"1379471489-26280-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 6/6] clone: test the new HEAD detection logic","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-18T02:31:29Z","receivedAt":"2013-09-18T02:31:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Signed-off-by: Junio C Hamano \n---\n t/t5601-clone.sh | 11 +++++++++++\n 1 file changed, 11 insertions(+)\n\ndiff --git a/t/t5601-clone.sh b/t/t5601-clone.sh\nindex 0629149..ccda6df 100755\n--- a/t/t5601-clone.sh\n+++ b/t/t5601-clone.sh\n@@ -285,4 +285,15 @@ test_expect_success NOT_MINGW,NOT_CYGWIN 'clone local path foo:bar' '\n \tgit clone \"./foo:bar\" foobar\n '\n \n+test_expect_success 'clone from a repository with two identical branches' '\n+\n+\t(\n+\t\tcd src &&\n+\t\tgit checkout -b another master\n+\t) &&\n+\tgit clone src target-11 &&\n+\ttest \"z$( cd target-11 && git symbolic-ref HEAD )\" = zrefs/heads/another\n+\n+'\n+\n test_done\n-- \n1.8.4-585-g8d1dcaf\n"},{"id":"227820","messageId":"xmqq8uyuwxtq.fsf@gitster.dls.corp.google.com","threadId":"34865","inReplyTo":"1379471489-26280-3-git-send-email-gitster@pobox.com","subject":"Re: [PATCH v2 2/6] upload-pack: send symbolic ref information as capability","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-18T04:36:17Z","receivedAt":"2013-09-18T04:36:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> static void upload_pack(void)\n> {\n> +\tstruct string_list symref = STRING_LIST_INIT_DUP;\n> +\n> +\thead_ref_namespaced(find_symref, &symref);\n> +\n> \tif (advertise_refs || !stateless_rpc) {\n> \t\treset_timeout();\n> -\t\thead_ref_namespaced(send_ref, NULL);\n> +\t\thead_ref_namespaced(send_ref, &symref);\n> \t\tfor_each_namespaced_ref(send_ref, NULL);\n\nThis one was trying to be too clever; HEAD may be pointing at an\nunborn branch in which case head_ref_namespaced() will not emit\nthe capability bit, so the second line also needs to be\n\n\tfor_each_namespaced_ref(send_ref, &symref);\n"}]}