{"thread":{"id":"16526","subject":"[PATCH 4/5] upload-pack: implement protocol extension \"symbolic-ref\"","startedAt":"2008-11-30T09:57:28Z","lastAt":"2008-12-01T14:03:41Z","messageCount":11,"participants":["Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"96766","messageId":"1228039053-31099-1-git-send-email-gitster@pobox.com","threadId":"16526","inReplyTo":null,"subject":"[PATCH 5/5] clone: test the new HEAD detection logic","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-30T09:57:28Z","receivedAt":"2008-11-30T09:57:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This demonstrates the new HEAD detection mechanism based on the\nsymbolic-ref protocol extension.\n\nSigned-off-by: Junio C Hamano \n---\n t/t5601-clone.sh | 11 +++++++++++\n 1 files changed, 11 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t5601-clone.sh b/t/t5601-clone.sh\nindex 78a3fa6..6d4665b 100755\n--- a/t/t5601-clone.sh\n+++ b/t/t5601-clone.sh\n@@ -125,4 +125,15 @@ test_expect_success 'clone to destination with extra trailing /' '\n \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-3 &&\n+\ttest \"z$( cd target-3 && git symbolic-ref HEAD )\" = zrefs/heads/another\n+\n+'\n+\n test_done\n-- \n1.6.0.4.850.g6bd829\n"},{"id":"96763","messageId":"1228039053-31099-2-git-send-email-gitster@pobox.com","threadId":"16526","inReplyTo":"1228039053-31099-1-git-send-email-gitster@pobox.com","subject":"[PATCH 4/5] upload-pack: implement protocol extension \"symbolic-ref\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-30T09:57:29Z","receivedAt":"2008-11-30T09:57:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This extends the fetch-pack protocol to allow the receiving end to ask\nwhich actual ref each symbolic ref points at.\n\nAlthough the new capability is advertised on the first available ref in\nthe same way as the other extensions, the way to trigger this extension\nfrom the receiving end is not by adding it in the first \"want\" line as\nusual. Instead, the receiving end sends a \"symbolic-ref\" request packet\nbefore the usual sequence of \"want\" lines.\n\nThis is unfortunate because it forces an extra round trip (receiving end\nsends a \"please tell me symbolic-ref\" packet, and then upload side sends\n\"here are the requested information\" packet), but it has to be implemented\nthis way because (1) ls-remote may need to ask for this information, in\nwhich case there is no \"want\" to be sent; and (2) the transport API\ninsists that transport_get_remote_refs() returns the final list, and does\nnot allow augmenting what was initially obtained from the call to it by\nlater calls to transport_fetch_refs() easily.\n\nIt also is unfortunate that with this change on the server side, older\nclients running \"ls-remote\" without actually downloading anything will\ntrigger \"The remote end hung up unexpectedly\" error on the uploading side,\nwhich is annoying even though it is benign. You can observe it by applying\nonly this patch but not the patch to the receiving end and running t5601\nunder \"sh -x\".\n\nSigned-off-by: Junio C Hamano \n---\n upload-pack.c | 43 +++++++++++++++++++++++++++++++++++++++----\n 1 files changed, 39 insertions(+), 4 deletions(-)\n\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 4029019..a925f69 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -494,6 +494,23 @@ static void exchange_shallows(int depth, struct object_array *shallows)\n \t\t}\n }\n \n+static int one_symref_info(const char *refname, const unsigned char *sha1, int flag, void *cb_data)\n+{\n+\tif ((flag & REF_ISSYMREF)) {\n+\t\tunsigned char dummy[20];\n+\t\tconst char *target = resolve_ref(refname, dummy, 0, NULL);\n+\t\tpacket_write(1, \"symref %s %s\", refname, target);\n+\t}\n+\treturn 0;\n+}\n+\n+static void send_symref_info(void)\n+{\n+\thead_ref(one_symref_info, NULL);\n+\tfor_each_ref(one_symref_info, NULL);\n+\tpacket_flush(1);\n+}\n+\n static void receive_needs(void)\n {\n \tstruct object_array shallows = {0, 0, NULL};\n@@ -502,11 +519,29 @@ static void receive_needs(void)\n \n \tif (debug_fd)\n \t\twrite_in_full(debug_fd, \"#S\\n\", 3);\n-\tfor (;;) {\n+\n+\t/*\n+\t * This is very unfortunate, but the \"transport\" abstraction\n+\t * is screwed up and insists that getting the list of refs\n+\t * to finish before actually sending the \"needs\" list from\n+\t * the client end.\n+\t */\n+\tlen = packet_read_line(0, line, sizeof(line));\n+\tif (len) {\n+\t\tif (!prefixcmp(line, \"symbolic-ref\")) {\n+\t\t\treset_timeout();\n+\t\t\tsend_symref_info();\n+\t\t\tlen = 0;\n+\t\t}\n+\t}\n+\tfor (;; len = 0) {\n \t\tstruct object *o;\n \t\tunsigned char sha1_buf[20];\n-\t\tlen = packet_read_line(0, line, sizeof(line));\n-\t\treset_timeout();\n+\n+\t\tif (!len) {\n+\t\t\tlen = packet_read_line(0, line, sizeof(line));\n+\t\t\treset_timeout();\n+\t\t}\n \t\tif (!len)\n \t\t\tbreak;\n \t\tif (debug_fd)\n@@ -577,7 +612,7 @@ static void receive_needs(void)\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-\t\t\" side-band-64k ofs-delta shallow no-progress\"\n+\t\t\" side-band-64k ofs-delta shallow no-progress symbolic-ref\"\n \t\t\" include-tag\";\n \tstruct object *o = parse_object(sha1);\n \n-- \n1.6.0.4.850.g6bd829\n"},{"id":"96764","messageId":"1228039053-31099-3-git-send-email-gitster@pobox.com","threadId":"16526","inReplyTo":"1228039053-31099-2-git-send-email-gitster@pobox.com","subject":"[PATCH 3/5] clone: find the current branch more explicitly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-30T09:57:30Z","receivedAt":"2008-11-30T09:57:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This makes \"git clone\" use the symbolic-ref protocol extension to find the\nref the HEAD is pointing at (when available), so that it can point at the\ncurrent branch that is not 'master' but happens to point at the same\ncommit as 'master'. IOW, immediately after doing the following:\n\n $ git checkout -b another master\n\na clone made out of that repository will check out 'another' branch, not\n'master'.\n\nSigned-off-by: Junio C Hamano \n---\n builtin-clone.c | 24 +++++++++++++++++++-----\n connect.c | 36 ++++++++++++++++++++++++++++++++++++\n 2 files changed, 55 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex 2feac9c..a8b8d56 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -299,13 +299,27 @@ static const struct ref *locate_head(const struct ref *refs,\n \tconst struct ref *remote_head = NULL;\n \tconst struct ref *remote_master = NULL;\n \tconst struct ref *r;\n-\tfor (r = refs; r; r = r->next)\n-\t\tif (!strcmp(r->name, \"HEAD\"))\n+\n+\tfor (r = refs; r; r = r->next) {\n+\t\tif (!strcmp(r->name, \"HEAD\")) {\n \t\t\tremote_head = r;\n+\t\t\tbreak;\n+\t\t}\n+\t}\n \n-\tfor (r = mapped_refs; r; r = r->next)\n-\t\tif (!strcmp(r->name, \"refs/heads/master\"))\n-\t\t\tremote_master = r;\n+\tif (remote_head && remote_head->symref) {\n+\t\tfor (r = mapped_refs; r; r = r->next)\n+\t\t\tif (!strcmp(r->name, remote_head->symref)) {\n+\t\t\t\tremote_master = r;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t}\n+\n+\tif (!remote_master) {\n+\t\tfor (r = mapped_refs; r; r = r->next)\n+\t\t\tif (!strcmp(r->name, \"refs/heads/master\"))\n+\t\t\t\tremote_master = r;\n+\t}\n \n \tif (remote_head_p)\n \t\t*remote_head_p = remote_head;\ndiff --git a/connect.c b/connect.c\nindex 402fbe6..40b43b4 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -48,6 +48,39 @@ static void add_extra_have(struct extra_have_objects *extra, unsigned char *sha1\n \textra->nr++;\n }\n \n+static void receive_symref(int fd[2], struct ref *refs)\n+{\n+\tchar line[1024];\n+\tint len;\n+\n+\tpacket_write(fd[1], \"symbolic-ref\");\n+\twhile ((len = packet_read_line(fd[0], line, sizeof(line)))) {\n+\t\tif (!prefixcmp(line, \"symref \")) {\n+\t\t\tstruct ref *sym;\n+\t\t\tchar *symref = line + 7;\n+\t\t\tchar *target = strchr(symref, ' ');\n+\t\t\tif (!target)\n+\t\t\t\tdie(\"Malformed symref line %s\", line);\n+\t\t\t*target++ = '\\0';\n+\t\t\tsym = find_ref_by_name(refs, symref);\n+\t\t\tif (!sym) {\n+\t\t\t\t/*\n+\t\t\t\t * NEEDSWORK: perhaps create the symref ref\n+\t\t\t\t * that is still unborn and queue it?\n+\t\t\t\t */\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t\tif (sym->symref)\n+\t\t\t\tdie(\"symref line says %s points at %s \"\n+\t\t\t\t \"but earlier it said it points at %s\",\n+\t\t\t\t symref, target, sym->symref);\n+\t\t\tsym->symref = xstrdup(target);\n+\t\t\tcontinue;\n+\t\t}\n+\t\tdie(\"expected symref, got %s\", line);\n+\t}\n+}\n+\n /*\n * Read all the refs from the other end\n */\n@@ -56,6 +89,7 @@ struct ref **get_remote_heads(int fd[2], struct ref **list,\n \t\t\t unsigned int flags,\n \t\t\t struct extra_have_objects *extra_have)\n {\n+\tstruct ref **original_list = list;\n \t*list = NULL;\n \tfor (;;) {\n \t\tstruct ref *ref;\n@@ -98,6 +132,8 @@ struct ref **get_remote_heads(int fd[2], struct ref **list,\n \t\t*list = ref;\n \t\tlist = &ref->next;\n \t}\n+\tif (server_supports(\"symbolic-ref\"))\n+\t\treceive_symref(fd, *original_list);\n \treturn list;\n }\n \n-- \n1.6.0.4.850.g6bd829\n"},{"id":"96765","messageId":"1228039053-31099-4-git-send-email-gitster@pobox.com","threadId":"16526","inReplyTo":"1228039053-31099-3-git-send-email-gitster@pobox.com","subject":"[PATCH 2/5] get_remote_heads(): do not assume that the conversation is one-way","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-30T09:57:31Z","receivedAt":"2008-11-30T09:57:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This is the first step to extend the fetch-pack protocol so that we can\ntransfer symref information to accurately clone from a repository whose\nHEAD points at a branch that is different from its 'master' branch but\nhappens to point at the same commit.\n\nThe old code assumed that the discovery of remote refs can be done by\nreceiving side only listening to what the other side said, which makes it\nimpossible for it to tell the other side to enable the protocol extension.\nThis passes both ends of the socket to the function to allow the receiving\nend to talk back to the sending end.\n\nSigned-off-by: Junio C Hamano \n---\n builtin-fetch-pack.c | 2 +-\n builtin-send-pack.c | 7 ++++---\n cache.h | 2 +-\n connect.c | 4 ++--\n transport.c | 4 ++--\n 5 files changed, 10 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c\nindex 372bfa2..6927410 100644\n--- a/builtin-fetch-pack.c\n+++ b/builtin-fetch-pack.c\n@@ -735,7 +735,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n \tconn = git_connect(fd, (char *)dest, args.uploadpack,\n \t\t\t args.verbose ? CONNECT_VERBOSE : 0);\n \tif (conn) {\n-\t\tget_remote_heads(fd[0], &ref, 0, NULL, 0, NULL);\n+\t\tget_remote_heads(fd, &ref, 0, NULL, 0, NULL);\n \n \t\tref = fetch_pack(&args, fd, conn, ref, dest, nr_heads, heads, NULL);\n \t\tclose(fd[0]);\ndiff --git a/builtin-send-pack.c b/builtin-send-pack.c\nindex a9fdbf9..23cdd48 100644\n--- a/builtin-send-pack.c\n+++ b/builtin-send-pack.c\n@@ -386,7 +386,7 @@ static int refs_pushed(struct ref *ref)\n \treturn 0;\n }\n \n-static int do_send_pack(int in, int out, struct remote *remote, const char *dest, int nr_refspec, const char **refspec)\n+static int do_send_pack(int fd[2], struct remote *remote, const char *dest, int nr_refspec, const char **refspec)\n {\n \tstruct ref *ref;\n \tint new_refs;\n@@ -395,6 +395,7 @@ static int do_send_pack(int in, int out, struct remote *remote, const char *dest\n \tint expect_status_report = 0;\n \tint flags = MATCH_REFS_NONE;\n \tint ret;\n+\tint in = fd[0], out = fd[1];\n \tstruct extra_have_objects extra_have;\n \n \tmemset(&extra_have, 0, sizeof(extra_have));\n@@ -404,7 +405,7 @@ static int do_send_pack(int in, int out, struct remote *remote, const char *dest\n \t\tflags |= MATCH_REFS_MIRROR;\n \n \t/* No funny business with the matcher */\n-\tremote_tail = get_remote_heads(in, &remote_refs, 0, NULL, REF_NORMAL,\n+\tremote_tail = get_remote_heads(fd, &remote_refs, 0, NULL, REF_NORMAL,\n \t\t\t\t &extra_have);\n \tget_local_heads();\n \n@@ -665,7 +666,7 @@ int send_pack(struct send_pack_args *my_args,\n \tverify_remote_names(nr_heads, heads);\n \n \tconn = git_connect(fd, dest, args.receivepack, args.verbose ? CONNECT_VERBOSE : 0);\n-\tret = do_send_pack(fd[0], fd[1], remote, dest, nr_heads, heads);\n+\tret = do_send_pack(fd, remote, dest, nr_heads, heads);\n \tclose(fd[0]);\n \t/* do_send_pack always closes fd[1] */\n \tret |= finish_connect(conn);\ndiff --git a/cache.h b/cache.h\nindex 487e5e1..2bee869 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -752,7 +752,7 @@ struct extra_have_objects {\n \tint nr, alloc;\n \tunsigned char (*array)[20];\n };\n-extern struct ref **get_remote_heads(int in, struct ref **list, int nr_match, char **match, unsigned int flags, struct extra_have_objects *);\n+extern struct ref **get_remote_heads(int fd[2], struct ref **list, int nr_match, char **match, unsigned int flags, struct extra_have_objects *);\n extern int server_supports(const char *feature);\n \n extern struct packed_git *parse_pack_index(unsigned char *sha1);\ndiff --git a/connect.c b/connect.c\nindex 584e04c..402fbe6 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -51,7 +51,7 @@ static void add_extra_have(struct extra_have_objects *extra, unsigned char *sha1\n /*\n * Read all the refs from the other end\n */\n-struct ref **get_remote_heads(int in, struct ref **list,\n+struct ref **get_remote_heads(int fd[2], struct ref **list,\n \t\t\t int nr_match, char **match,\n \t\t\t unsigned int flags,\n \t\t\t struct extra_have_objects *extra_have)\n@@ -64,7 +64,7 @@ struct ref **get_remote_heads(int in, struct ref **list,\n \t\tchar *name;\n \t\tint len, name_len;\n \n-\t\tlen = packet_read_line(in, buffer, sizeof(buffer));\n+\t\tlen = packet_read_line(fd[0], buffer, sizeof(buffer));\n \t\tif (!len)\n \t\t\tbreak;\n \t\tif (buffer[len-1] == '\\n')\ndiff --git a/transport.c b/transport.c\nindex 56831c5..72b3a76 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -617,7 +617,7 @@ static struct ref *get_refs_via_connect(struct transport *transport)\n \tstruct ref *refs;\n \n \tconnect_setup(transport);\n-\tget_remote_heads(data->fd[0], &refs, 0, NULL, 0, NULL);\n+\tget_remote_heads(data->fd, &refs, 0, NULL, 0, NULL);\n \n \treturn refs;\n }\n@@ -650,7 +650,7 @@ static int fetch_refs_via_pack(struct transport *transport,\n \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\tget_remote_heads(data->fd, &refs_tmp, 0, NULL, 0, NULL);\n \t}\n \n \trefs = fetch_pack(&args, data->fd, data->conn,\n-- \n1.6.0.4.850.g6bd829\n"},{"id":"96767","messageId":"1228039053-31099-5-git-send-email-gitster@pobox.com","threadId":"16526","inReplyTo":"1228039053-31099-4-git-send-email-gitster@pobox.com","subject":"[PATCH 1/5] upload-pack.c: refactor receive_needs()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-30T09:57:32Z","receivedAt":"2008-11-30T09:57:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"At the end of the function it had a block of \"shallow fetch\" support code\nthat assumed it will be the last protocol extension ever by returning\nearly. Move the bulky code away into a separate function to make it\nmaintainable.\n\nSigned-off-by: Junio C Hamano \n---\n upload-pack.c | 97 ++++++++++++++++++++++++++++++---------------------------\n 1 files changed, 51 insertions(+), 46 deletions(-)\n\ndiff --git a/upload-pack.c b/upload-pack.c\nindex e5adbc0..4029019 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -444,6 +444,56 @@ static int get_common_commits(void)\n \t}\n }\n \n+static void exchange_shallows(int depth, struct object_array *shallows)\n+{\n+\tif (depth == 0 && shallows->nr == 0)\n+\t\treturn;\n+\tif (depth > 0) {\n+\t\tstruct commit_list *result, *backup;\n+\t\tint i;\n+\t\tbackup = result = get_shallow_commits(&want_obj, depth,\n+\t\t\tSHALLOW, NOT_SHALLOW);\n+\t\twhile (result) {\n+\t\t\tstruct object *object = &result->item->object;\n+\t\t\tif (!(object->flags & (CLIENT_SHALLOW|NOT_SHALLOW))) {\n+\t\t\t\tpacket_write(1, \"shallow %s\",\n+\t\t\t\t\t\tsha1_to_hex(object->sha1));\n+\t\t\t\tregister_shallow(object->sha1);\n+\t\t\t}\n+\t\t\tresult = result->next;\n+\t\t}\n+\t\tfree_commit_list(backup);\n+\t\tfor (i = 0; i < shallows->nr; i++) {\n+\t\t\tstruct object *object = shallows->objects[i].item;\n+\t\t\tif (object->flags & NOT_SHALLOW) {\n+\t\t\t\tstruct commit_list *parents;\n+\t\t\t\tpacket_write(1, \"unshallow %s\",\n+\t\t\t\t\tsha1_to_hex(object->sha1));\n+\t\t\t\tobject->flags &= ~CLIENT_SHALLOW;\n+\t\t\t\t/* make sure the real parents are parsed */\n+\t\t\t\tunregister_shallow(object->sha1);\n+\t\t\t\tobject->parsed = 0;\n+\t\t\t\tif (parse_commit((struct commit *)object))\n+\t\t\t\t\tdie(\"invalid commit\");\n+\t\t\t\tparents = ((struct commit *)object)->parents;\n+\t\t\t\twhile (parents) {\n+\t\t\t\t\tadd_object_array(&parents->item->object,\n+\t\t\t\t\t\t\tNULL, &want_obj);\n+\t\t\t\t\tparents = parents->next;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t\t/* make sure commit traversal conforms to client */\n+\t\t\tregister_shallow(object->sha1);\n+\t\t}\n+\t\tpacket_flush(1);\n+\t} else\n+\t\tif (shallows->nr > 0) {\n+\t\t\tint i;\n+\t\t\tfor (i = 0; i < shallows->nr; i++)\n+\t\t\t\tregister_shallow(shallows->objects[i].item->sha1);\n+\t\t}\n+}\n+\n static void receive_needs(void)\n {\n \tstruct object_array shallows = {0, 0, NULL};\n@@ -520,52 +570,7 @@ static void receive_needs(void)\n \t}\n \tif (debug_fd)\n \t\twrite_in_full(debug_fd, \"#E\\n\", 3);\n-\tif (depth == 0 && shallows.nr == 0)\n-\t\treturn;\n-\tif (depth > 0) {\n-\t\tstruct commit_list *result, *backup;\n-\t\tint i;\n-\t\tbackup = result = get_shallow_commits(&want_obj, depth,\n-\t\t\tSHALLOW, NOT_SHALLOW);\n-\t\twhile (result) {\n-\t\t\tstruct object *object = &result->item->object;\n-\t\t\tif (!(object->flags & (CLIENT_SHALLOW|NOT_SHALLOW))) {\n-\t\t\t\tpacket_write(1, \"shallow %s\",\n-\t\t\t\t\t\tsha1_to_hex(object->sha1));\n-\t\t\t\tregister_shallow(object->sha1);\n-\t\t\t}\n-\t\t\tresult = result->next;\n-\t\t}\n-\t\tfree_commit_list(backup);\n-\t\tfor (i = 0; i < shallows.nr; i++) {\n-\t\t\tstruct object *object = shallows.objects[i].item;\n-\t\t\tif (object->flags & NOT_SHALLOW) {\n-\t\t\t\tstruct commit_list *parents;\n-\t\t\t\tpacket_write(1, \"unshallow %s\",\n-\t\t\t\t\tsha1_to_hex(object->sha1));\n-\t\t\t\tobject->flags &= ~CLIENT_SHALLOW;\n-\t\t\t\t/* make sure the real parents are parsed */\n-\t\t\t\tunregister_shallow(object->sha1);\n-\t\t\t\tobject->parsed = 0;\n-\t\t\t\tif (parse_commit((struct commit *)object))\n-\t\t\t\t\tdie(\"invalid commit\");\n-\t\t\t\tparents = ((struct commit *)object)->parents;\n-\t\t\t\twhile (parents) {\n-\t\t\t\t\tadd_object_array(&parents->item->object,\n-\t\t\t\t\t\t\tNULL, &want_obj);\n-\t\t\t\t\tparents = parents->next;\n-\t\t\t\t}\n-\t\t\t}\n-\t\t\t/* make sure commit traversal conforms to client */\n-\t\t\tregister_shallow(object->sha1);\n-\t\t}\n-\t\tpacket_flush(1);\n-\t} else\n-\t\tif (shallows.nr > 0) {\n-\t\t\tint i;\n-\t\t\tfor (i = 0; i < shallows.nr; i++)\n-\t\t\t\tregister_shallow(shallows.objects[i].item->sha1);\n-\t\t}\n+\texchange_shallows(depth, &shallows);\n \tfree(shallows.objects);\n }\n \n-- \n1.6.0.4.850.g6bd829\n"},{"id":"96768","messageId":"1228039053-31099-6-git-send-email-gitster@pobox.com","threadId":"16526","inReplyTo":"1228039053-31099-5-git-send-email-gitster@pobox.com","subject":"[PATCH 0/5] Detecting HEAD more reliably while cloning","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-30T09:57:33Z","receivedAt":"2008-11-30T09:57:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This is a \"works but is unsatisfactory from my acceptance standard\" WIP\nfor review and improvements. It tries to introduce a protocol extension\nthat lets \"git clone\" discover where the HEAD points at more reliably.\n\nThe current code has to guess, because the original protocol tells what\nobject each ref points at but does not talk about which other ref a\nsymbolic ref points at. The implication of this is that if you prepare\nanother branch that points at your master, like this:\n\n\t$ git checkout -b another master\n\nand keep that other branch checked out (and in sync with 'master'), a\nclone made from such a repository may incorrectly have its HEAD pointing\nat 'master', not 'another'.\n\nHere are the five patches to remedy the problem.\n\n [PATCH 1/5] upload-pack.c: refactor receive_needs()\n [PATCH 2/5] get_remote_heads(): do not assume that the conversation is one-way\n [PATCH 3/5] clone: find the current branch more explicitly\n [PATCH 4/5] upload-pack: implement protocol extension \"symbolic-ref\"\n [PATCH 5/5] clone: test the new HEAD detection logic\n\nThe first one is a mere clean-up and is not absolutely necessary. The\nsecond one is a preparatory step and it is needed for later steps (it by\nitself does not change any behaviour).\n\nThe third one and the fourth one implement the receiver and and the sender\nend respectively. It is better to test these applying each of them\nindependently on top of the second one and then merge the result, so that\nwhat happens during the transition period during which old client talks to\nnew server (and vice versa) can be tested. The new feature is activated\nonly when the updated client talks to the new server, so the test appears\nat the end, as a separate patch.\n\nIn other words, after storing these five patches in five separate files,\nyou would build this history (on top of 'master'):\n\n git am 1 2 3\n git reset --hard HEAD^ 4---M---5\n git am 4 / /\n git merge HEAD@{2} ---1---2---3\n git am 5\n\nThe reason I say it is unsatisfactory is mostly because the protocol\nextension for this is very hard in the face of ls-remote which receives\nwhat the upload-pack says but disconnects without saying anything after\nthat. The upload-pack side needs to check if the receiver wants to\ntrigger protocol extension, but reading from the socket when talking to an\nold client will trigger an error message from it, although it is actually\nharmless. When git-clone runs locally, you can actually observe the error\nmessage arising from this issue, by running t5601 after applying 1 2 and 4\nbut not 3 (i.e. the state after \"git am 4\" in the above sequence) under\n\"sh -x\". We could trivially fix this by giving an extra parameter to\npacket_read_line() and safe_read() to tell them that it is Ok if the other\nend gives them an EOF if we wanted to, but I left the visible-but-harmless\nbreakage as is to illustrate the issue for this round.\n\n builtin-clone.c | 24 +++++++--\n builtin-fetch-pack.c | 2 +-\n builtin-send-pack.c | 7 ++-\n cache.h | 2 +-\n connect.c | 40 ++++++++++++++-\n t/t5601-clone.sh | 11 ++++\n transport.c | 4 +-\n upload-pack.c | 140 ++++++++++++++++++++++++++++++++------------------\n 8 files changed, 166 insertions(+), 64 deletions(-)\n"},{"id":"96769","messageId":"7vabbhttq0.fsf@gitster.siamese.dyndns.org","threadId":"16526","inReplyTo":"1228039053-31099-6-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 0/5] Detecting HEAD more reliably while cloning","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-30T10:04:55Z","receivedAt":"2008-11-30T10:04:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"You may have noticed that the new git-send-email reversed the order of six\npatch files (one cover and five patches) I gave from the command line.\nPlease consider this series as a bug report ;-)\n\nI think the bug is that \"pop @ARGV\" should read \"shift @ARGV\" or something\nsilly and trivial like that, but it is getting late, so I won't debug\ntonight.\n"},{"id":"96790","messageId":"20081130180214.GA10375@coredump.intra.peff.net","threadId":"16526","inReplyTo":"1228039053-31099-2-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 4/5] upload-pack: implement protocol extension \"symbolic-ref\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-11-30T18:02:14Z","receivedAt":"2008-11-30T18:02:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Nov 30, 2008 at 01:57:29AM -0800, Junio C Hamano wrote:\n\n> Although the new capability is advertised on the first available ref in\n> the same way as the other extensions, the way to trigger this extension\n> from the receiving end is not by adding it in the first \"want\" line as\n> usual. Instead, the receiving end sends a \"symbolic-ref\" request packet\n> before the usual sequence of \"want\" lines.\n> \n> This is unfortunate because it forces an extra round trip (receiving end\n> sends a \"please tell me symbolic-ref\" packet, and then upload side sends\n> \"here are the requested information\" packet), but it has to be implemented\n> this way because (1) ls-remote may need to ask for this information, in\n> which case there is no \"want\" to be sent; and (2) the transport API\n> insists that transport_get_remote_refs() returns the final list, and does\n> not allow augmenting what was initially obtained from the call to it by\n> later calls to transport_fetch_refs() easily.\n\nHrm. For (1), could we allow either interaction method? IOW, allow\nrequesting a symref on the first want line, _or_ by separate \"symbolic\nref\" packet? That would allow clients who are using \"want\" to\npiggy-back the symref request as an optimization, but not restrict those\nthat just want to ask for it?\n\nNot being too familiar with the transport code, I can't speak to (2).\nBut it would be sad to see an internal API shortcoming that we have\n_now_ stick us with a crappy protocol _forever_. We can fix the API, but\nonce the protocol is in the wild, it becomes much harder to change.\n\n> It also is unfortunate that with this change on the server side, older\n> clients running \"ls-remote\" without actually downloading anything will\n> trigger \"The remote end hung up unexpectedly\" error on the uploading side,\n> which is annoying even though it is benign. You can observe it by applying\n> only this patch but not the patch to the receiving end and running t5601\n> under \"sh -x\".\n\nAnd obviously this wouldn't go away with the proposal above, since we'd\nstill have to be looking for the \"tell me this symbolic ref\" packet. But\nthe solution you outlined in 0/5 sounded sane to me (and I think it\ndefinitely needs to be addressed).\n\n-Peff\n"},{"id":"96791","messageId":"20081130181011.GB10375@coredump.intra.peff.net","threadId":"16526","inReplyTo":"1228039053-31099-3-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 3/5] clone: find the current branch more explicitly","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-11-30T18:10:11Z","receivedAt":"2008-11-30T18:10:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Nov 30, 2008 at 01:57:30AM -0800, Junio C Hamano wrote:\n\n> +\t\t\tsym = find_ref_by_name(refs, symref);\n> +\t\t\tif (!sym) {\n> +\t\t\t\t/*\n> +\t\t\t\t * NEEDSWORK: perhaps create the symref ref\n> +\t\t\t\t * that is still unborn and queue it?\n> +\t\t\t\t */\n> +\t\t\t\tcontinue;\n> +\t\t\t}\n\nI don't see any reason not to create the pointer to an unborn branch. I\nthink it would be nice eventually to allow cloning an empty repository\n(i.e., to facilitate:\n\n mkdir foo &&\n (cd foo && git --bare init) &&\n git clone foo bar &&\n cd bar &&\n echo content >file && git add file && git commit -m content &&\n git push\n\n) which people have asked for from time to time.\n\n> +\t\t\tif (sym->symref)\n> +\t\t\t\tdie(\"symref line says %s points at %s \"\n> +\t\t\t\t \"but earlier it said it points at %s\",\n> +\t\t\t\t symref, target, sym->symref);\n\nI wonder if there would ever be a use for sending multiple symref\npackets for the same ref. IOW, should we \"be liberal in what we accept\"\nhere and just choose some sane behavior (like picking the first or last\nto be sent), and allow room for expansion there.\n\nI can't actually think of a possible use for that, but nor can I think\nof any particular reason to die here (save for detecting bugs in\nupload-pack :) ). But just consider that we will be stuck with clients\nwith the \"die\" behavior forever.\n\n-Peff\n"},{"id":"96807","messageId":"7vzljgsiyn.fsf@gitster.siamese.dyndns.org","threadId":"16526","inReplyTo":"7vabbhttq0.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 0/5] Detecting HEAD more reliably while cloning","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-12-01T02:54:56Z","receivedAt":"2008-12-01T02:54:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> You may have noticed that the new git-send-email reversed the order of six\n> patch files (one cover and five patches) I gave from the command line.\n> Please consider this series as a bug report ;-)\n>\n> I think the bug is that \"pop @ARGV\" should read \"shift @ARGV\" or something\n> silly and trivial like that, but it is getting late, so I won't debug\n> tonight.\n\nPerhaps this is a good enough fix? Very lightly tested.\n\n-- >8 --\nsend-email: do not reverse the command line arguments\n\nThe loop picks elements from @ARGV one by one, sifts them into arguments\nmeant for format-patch and the script itself, and pushes them to @files\nand @rev_list_opts arrays. Pick elements from @ARGV starting at the\nbeginning using shift, instead of at the end using pop, as push appends\nthem to the end of the array.\n\nSigned-off-by: Junio C Hamano \n---\n git-send-email.perl | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git c/git-send-email.perl w/git-send-email.perl\nindex 7508f8f..45beb9c 100755\n--- c/git-send-email.perl\n+++ w/git-send-email.perl\n@@ -421,7 +421,7 @@ EOF\n # Now that all the defaults are set, process the rest of the command line\n # arguments and collect up the files that need to be processed.\n my @rev_list_opts;\n-while (my $f = pop @ARGV) {\n+while (defined(my $f = shift @ARGV)) {\n \tif ($f eq \"--\") {\n \t\tpush @rev_list_opts, \"--\", @ARGV;\n \t\t@ARGV = ();\n"},{"id":"96844","messageId":"7vy6z0q9fm.fsf@gitster.siamese.dyndns.org","threadId":"16526","inReplyTo":"20081130180214.GA10375@coredump.intra.peff.net","subject":"Re: [PATCH 4/5] upload-pack: implement protocol extension \"symbolic-ref\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-12-01T14:03:41Z","receivedAt":"2008-12-01T14:03:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Sun, Nov 30, 2008 at 01:57:29AM -0800, Junio C Hamano wrote:\n> ...\n>> This is unfortunate because it forces an extra round trip (receiving end\n>> sends a \"please tell me symbolic-ref\" packet, and then upload side sends\n>> \"here are the requested information\" packet), but it has to be implemented\n>> this way because (1) ls-remote may need to ask for this information, in\n>> which case there is no \"want\" to be sent; and (2) the transport API\n>> insists that transport_get_remote_refs() returns the final list, and does\n>> not allow augmenting what was initially obtained from the call to it by\n>> later calls to transport_fetch_refs() easily.\n>\n> Hrm. For (1), could we allow either interaction method? IOW, allow\n> requesting a symref on the first want line, _or_ by separate \"symbolic\n> ref\" packet? That would allow clients who are using \"want\" to\n> piggy-back the symref request as an optimization, but not restrict those\n> that just want to ask for it?\n\nI think I found another hole in the protocol that we can use to carry the\n\"which branch does HEAD points at\" information in a backward compatible\nway, and it would be a much less intrusive although more sneaky way. And\nit would not have to suffer from the above issues at all.\n\nA patchset follows shortly...\n"}]}