{"thread":{"id":"34955","subject":"[PATCH v3 0/7] Removing the guesswork of HEAD in \"clone\"","startedAt":"2013-09-18T05:14:16Z","lastAt":"2013-09-18T05:14:23Z","messageCount":8,"participants":["Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":7},"messages":[{"id":"227821","messageId":"1379481263-29903-1-git-send-email-gitster@pobox.com","threadId":"34955","inReplyTo":null,"subject":"[PATCH v3 0/7] Removing the guesswork of HEAD in \"clone\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-18T05:14:16Z","receivedAt":"2013-09-18T05:14:16Z","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 \"symref\", and allows\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\nCredits for re-igniting the ember with an earlier patch series goes\nto Andreas Krey.\n\n * The test-fix in [PATCH 1/7] is new this round.\n\n * The main patch to upload-pack.c [PATCH 3/7] has the fix I earlier\n sent. The capability was called \"sym\" in the previous one, but\n it spells out \"symref\" in this round.\n\n * The patch on the receiving end [PATCH 6/7] now comes with an\n update to a test that was fixed in [PATCH 1/7].\n\nThis round seems to pass all the test, and the code is fairly\nstraight-forward, so it may be ready for at least 'pu' if not\n'next'.\n\nThe series is to be applied on top of v1.8.4; between there and the\n'master', there is some code reorganization to create connect.h out\nof cache.h which may cause patch conflict, but it should be trivial\nto fix when merging it up (queued as an evil merge near the tip of\nthe 'pu' branch).\n\n\nJunio C Hamano (7):\n t5505: fix \"set-head --auto with ambiguous HEAD\" test\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\n get_remote_head()\n clone: test the new HEAD detection logic\n\n cache.h | 1 -\n connect.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-\n t/t5505-remote.sh | 16 +++++---------\n t/t5601-clone.sh | 11 ++++++++++\n upload-pack.c | 51 ++++++++++++++++++++++++++++++++++++++------\n 5 files changed, 123 insertions(+), 19 deletions(-)\n\n-- \n1.8.4-585-g8d1dcaf\n"},{"id":"227822","messageId":"1379481263-29903-2-git-send-email-gitster@pobox.com","threadId":"34955","inReplyTo":"1379481263-29903-1-git-send-email-gitster@pobox.com","subject":"[PATCH v3 1/7] t5505: fix \"set-head --auto with ambiguous HEAD\" test","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-18T05:14:17Z","receivedAt":"2013-09-18T05:14:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When two or more branches point at the same commit and HEAD is\npointing at one of them, without the symref extension, there is no\nway to remotely tell which one of these branches HEAD points at.\nThe test in question attempts to make sure that this situation is\ndiagnosed and results in a failure.\n\nHowever, even if there _were_ a way to reliably tell which branch\nthe HEAD points at, \"set-head --auto\" would fail if there is no\nremote tracking branch. Make sure that this test does not fail\nfor that \"wrong\" reason.\n\nSigned-off-by: Junio C Hamano \n---\n\n * New in this round.\n\n t/t5505-remote.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex 8f6e392..197d3f7 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -271,6 +271,7 @@ EOF\n test_expect_success 'set-head --auto fails w/multiple HEADs' '\n \t(\n \t\tcd test &&\n+\t\tgit fetch two \"refs/heads/*:refs/remotes/two/*\" &&\n \t\ttest_must_fail git remote set-head --auto two >output 2>&1 &&\n \t\ttest_i18ncmp expect output\n \t)\n-- \n1.8.4-585-g8d1dcaf\n"},{"id":"227828","messageId":"1379481263-29903-3-git-send-email-gitster@pobox.com","threadId":"34955","inReplyTo":"1379481263-29903-1-git-send-email-gitster@pobox.com","subject":"[PATCH v3 2/7] upload-pack.c: do not pass confusing cb_data to mark_our_ref()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-18T05:14:18Z","receivedAt":"2013-09-18T05:14:18Z","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":"227827","messageId":"1379481263-29903-4-git-send-email-gitster@pobox.com","threadId":"34955","inReplyTo":"1379481263-29903-1-git-send-email-gitster@pobox.com","subject":"[PATCH v3 3/7] upload-pack: send symbolic ref information as capability","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-18T05:14:19Z","receivedAt":"2013-09-18T05:14:19Z","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 capability \"symref=HEAD:refs/heads/master\", let the sender to\ntell the receiver what symbolic ref points at what ref.\n\nThis capability can be repeated more than once to represent symbolic\nrefs other than HEAD, 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..979fc8e 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, \" symref=%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\tfor_each_namespaced_ref(send_ref, NULL);\n+\t\thead_ref_namespaced(send_ref, &symref);\n+\t\tfor_each_namespaced_ref(send_ref, &symref);\n \t\tpacket_flush(1);\n \t} else {\n \t\thead_ref_namespaced(mark_our_ref, NULL);\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":"227824","messageId":"1379481263-29903-5-git-send-email-gitster@pobox.com","threadId":"34955","inReplyTo":"1379481263-29903-1-git-send-email-gitster@pobox.com","subject":"[PATCH v3 4/7] upload-pack: send non-HEAD symbolic refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-18T05:14:20Z","receivedAt":"2013-09-18T05:14:20Z","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 979fc8e..2826909 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":"227823","messageId":"1379481263-29903-6-git-send-email-gitster@pobox.com","threadId":"34955","inReplyTo":"1379481263-29903-1-git-send-email-gitster@pobox.com","subject":"[PATCH v3 5/7] connect.c: make parse_feature_value() static","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-18T05:14:21Z","receivedAt":"2013-09-18T05:14:21Z","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":"227826","messageId":"1379481263-29903-7-git-send-email-gitster@pobox.com","threadId":"34955","inReplyTo":"1379481263-29903-1-git-send-email-gitster@pobox.com","subject":"[PATCH v3 6/7] connect: annotate refs with their symref information in get_remote_head()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-18T05:14:22Z","receivedAt":"2013-09-18T05:14:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"By doing this, clients of upload-pack can now reliably tell what ref\na symbolic ref points at; the updated test in t5505 used to expect\nfailure due to the ambiguity and made sure we give diagnostics, but\nwe no longer need to be so pessimistic. Make sure we correctly learn\nwhich branch HEAD points at from the other side instead.\n\nSigned-off-by: Junio C Hamano \n---\n connect.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++++\n t/t5505-remote.sh | 15 ++++----------\n 2 files changed, 64 insertions(+), 11 deletions(-)\n\ndiff --git a/connect.c b/connect.c\nindex e4c7ae6..553a80c 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 \"symref\" */\n+\t/* e.g. \"symref=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 \"symref=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/* \"symref=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, \"symref\", &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 \ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex 197d3f7..ac79dd9 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -160,9 +160,7 @@ cat >test/expect <test/expect <<\\EOF\n-error: Multiple remote HEAD branches. Please choose one explicitly with:\n- git remote set-head two another\n- git remote set-head two master\n-EOF\n-\n-test_expect_success 'set-head --auto fails w/multiple HEADs' '\n+test_expect_success 'set-head --auto has no problem w/multiple HEADs' '\n \t(\n \t\tcd test &&\n \t\tgit fetch two \"refs/heads/*:refs/remotes/two/*\" &&\n-\t\ttest_must_fail git remote set-head --auto two >output 2>&1 &&\n+\t\tgit remote set-head --auto two >output 2>&1 &&\n+\t\techo \"two/HEAD set to master\" >expect &&\n \t\ttest_i18ncmp expect output\n \t)\n '\n-- \n1.8.4-585-g8d1dcaf\n"},{"id":"227825","messageId":"1379481263-29903-8-git-send-email-gitster@pobox.com","threadId":"34955","inReplyTo":"1379481263-29903-1-git-send-email-gitster@pobox.com","subject":"[PATCH v3 7/7] clone: test the new HEAD detection logic","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-18T05:14:23Z","receivedAt":"2013-09-18T05:14:23Z","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"}]}