From a4dfee06806df64b7a44866f67f15b54567ec722 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 17 Sep 2013 21:45:34 -0700 Subject: [PATCH 1/9] t5505: fix "set-head --auto with ambiguous HEAD" test When two or more branches point at the same commit and HEAD is pointing at one of them, without the symref extension, there is no way to remotely tell which one of these branches HEAD points at. The test in question attempts to make sure that this situation is diagnosed and results in a failure. However, even if there _were_ a way to reliably tell which branch the HEAD points at, "set-head --auto" would fail if there is no remote tracking branch. Make sure that this test does not fail for that "wrong" reason. Signed-off-by: Junio C Hamano --- t/t5505-remote.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh index 8f6e3922dc..197d3f763d 100755 --- a/t/t5505-remote.sh +++ b/t/t5505-remote.sh @@ -271,6 +271,7 @@ EOF test_expect_success 'set-head --auto fails w/multiple HEADs' ' ( cd test && + git fetch two "refs/heads/*:refs/remotes/two/*" && test_must_fail git remote set-head --auto two >output 2>&1 && test_i18ncmp expect output ) From a4d695de0dbaf41505cb6db44c6bb8290980398e Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 17 Sep 2013 16:03:32 -0700 Subject: [PATCH 2/9] upload-pack.c: do not pass confusing cb_data to mark_our_ref() The callee does not use cb_data, and the caller is an intermediate function in a callchain that later wants to use the cb_data for its own use. Clarify the code by breaking the dataflow explicitly by not passing cb_data down to mark_our_ref(). Signed-off-by: Junio C Hamano --- upload-pack.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/upload-pack.c b/upload-pack.c index 127e59a603..a6e107f0b1 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -742,7 +742,7 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo const char *refname_nons = strip_namespace(refname); unsigned char peeled[20]; - if (mark_our_ref(refname, sha1, flag, cb_data)) + if (mark_our_ref(refname, sha1, flag, NULL)) return 0; if (capabilities) From 7171d8c15f919c760d52f814a0aee1b1253385b1 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 17 Sep 2013 16:17:33 -0700 Subject: [PATCH 3/9] upload-pack: send symbolic ref information as capability One long-standing flaw in the pack transfer protocol was that there was no way to tell the other end which branch "HEAD" points at. With a capability "symref=HEAD:refs/heads/master", let the sender to tell the receiver what symbolic ref points at what ref. This capability can be repeated more than once to represent symbolic refs other than HEAD, such as "refs/remotes/origin/HEAD"). Add an infrastructure to collect symbolic refs, format them as extra capabilities and put it on the wire. For now, just send information on the "HEAD" and nothing else. Signed-off-by: Junio C Hamano --- upload-pack.c | 48 +++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 43 insertions(+), 5 deletions(-) diff --git a/upload-pack.c b/upload-pack.c index a6e107f0b1..979fc8eae3 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -734,6 +734,16 @@ static int mark_our_ref(const char *refname, const unsigned char *sha1, int flag return 0; } +static void format_symref_info(struct strbuf *buf, struct string_list *symref) +{ + struct string_list_item *item; + + if (!symref->nr) + return; + for_each_string_list_item(item, symref) + strbuf_addf(buf, " symref=%s:%s", item->string, (char *)item->util); +} + static int send_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data) { static const char *capabilities = "multi_ack thin-pack side-band" @@ -745,32 +755,60 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo if (mark_our_ref(refname, sha1, flag, NULL)) return 0; - if (capabilities) - packet_write(1, "%s %s%c%s%s%s agent=%s\n", + if (capabilities) { + struct strbuf symref_info = STRBUF_INIT; + + format_symref_info(&symref_info, cb_data); + packet_write(1, "%s %s%c%s%s%s%s agent=%s\n", sha1_to_hex(sha1), refname_nons, 0, capabilities, allow_tip_sha1_in_want ? " allow-tip-sha1-in-want" : "", stateless_rpc ? " no-done" : "", + symref_info.buf, git_user_agent_sanitized()); - else + strbuf_release(&symref_info); + } else { packet_write(1, "%s %s\n", sha1_to_hex(sha1), refname_nons); + } capabilities = NULL; if (!peel_ref(refname, peeled)) packet_write(1, "%s %s^{}\n", sha1_to_hex(peeled), refname_nons); return 0; } +static int find_symref(const char *refname, const unsigned char *sha1, int flag, + void *cb_data) +{ + const char *symref_target; + struct string_list_item *item; + unsigned char unused[20]; + + if ((flag & REF_ISSYMREF) == 0) + return 0; + symref_target = resolve_ref_unsafe(refname, unused, 0, &flag); + if (!symref_target || (flag & REF_ISSYMREF) == 0) + die("'%s' is a symref but it is not?", refname); + item = string_list_append(cb_data, refname); + item->util = xstrdup(symref_target); + return 0; +} + static void upload_pack(void) { + struct string_list symref = STRING_LIST_INIT_DUP; + + head_ref_namespaced(find_symref, &symref); + if (advertise_refs || !stateless_rpc) { reset_timeout(); - head_ref_namespaced(send_ref, NULL); - for_each_namespaced_ref(send_ref, NULL); + head_ref_namespaced(send_ref, &symref); + for_each_namespaced_ref(send_ref, &symref); packet_flush(1); } else { head_ref_namespaced(mark_our_ref, NULL); for_each_namespaced_ref(mark_our_ref, NULL); } + string_list_clear(&symref, 1); if (advertise_refs) return; From 5e7dcad771cb873e278a0571b46910d7c32e2f6c Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 17 Sep 2013 16:21:33 -0700 Subject: [PATCH 4/9] upload-pack: send non-HEAD symbolic refs With the same mechanism as used to tell where "HEAD" points at to the other end, we can tell the target of other symbolic refs as well. Signed-off-by: Junio C Hamano --- upload-pack.c | 1 + 1 file changed, 1 insertion(+) diff --git a/upload-pack.c b/upload-pack.c index 979fc8eae3..2826909eba 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -798,6 +798,7 @@ static void upload_pack(void) struct string_list symref = STRING_LIST_INIT_DUP; head_ref_namespaced(find_symref, &symref); + for_each_namespaced_ref(find_symref, &symref); if (advertise_refs || !stateless_rpc) { reset_timeout(); From 5d54cffc36f654721302a717ff12fb317c1b494a Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 17 Sep 2013 16:29:28 -0700 Subject: [PATCH 5/9] connect.c: make parse_feature_value() static Signed-off-by: Junio C Hamano --- cache.h | 1 - connect.c | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cache.h b/cache.h index 85b544f38d..2c853ba856 100644 --- a/cache.h +++ b/cache.h @@ -1098,7 +1098,6 @@ extern struct ref **get_remote_heads(int in, char *src_buf, size_t src_len, extern int server_supports(const char *feature); extern int parse_feature_request(const char *features, const char *feature); extern const char *server_feature_value(const char *feature, int *len_ret); -extern const char *parse_feature_value(const char *feature_list, const char *feature, int *len_ret); extern struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_path); diff --git a/connect.c b/connect.c index a0783d4867..e4c7ae6e72 100644 --- a/connect.c +++ b/connect.c @@ -8,6 +8,7 @@ #include "url.h" static char *server_capabilities; +static const char *parse_feature_value(const char *, const char *, int *); static int check_ref(const char *name, int len, unsigned int flags) { @@ -116,7 +117,7 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len, return list; } -const char *parse_feature_value(const char *feature_list, const char *feature, int *lenp) +static const char *parse_feature_value(const char *feature_list, const char *feature, int *lenp) { int len; From a45b5f0552eec3c8800edae44e071cf1d647bf96 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 17 Sep 2013 19:10:31 -0700 Subject: [PATCH 6/9] connect: annotate refs with their symref information in get_remote_head() By doing this, clients of upload-pack can now reliably tell what ref a symbolic ref points at; the updated test in t5505 used to expect failure due to the ambiguity and made sure we give diagnostics, but we no longer need to be so pessimistic. Make sure we correctly learn which branch HEAD points at from the other side instead. Signed-off-by: Junio C Hamano --- connect.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++ t/t5505-remote.sh | 15 ++++-------- 2 files changed, 64 insertions(+), 11 deletions(-) diff --git a/connect.c b/connect.c index e4c7ae6e72..553a80c734 100644 --- a/connect.c +++ b/connect.c @@ -6,6 +6,7 @@ #include "run-command.h" #include "remote.h" #include "url.h" +#include "string-list.h" static char *server_capabilities; static const char *parse_feature_value(const char *, const char *, int *); @@ -60,6 +61,61 @@ static void die_initial_contact(int got_at_least_one_head) "and the repository exists."); } +static void parse_one_symref_info(struct string_list *symref, const char *val, int len) +{ + char *sym, *target; + struct string_list_item *item; + + if (!len) + return; /* just "symref" */ + /* e.g. "symref=HEAD:refs/heads/master" */ + sym = xmalloc(len + 1); + memcpy(sym, val, len); + sym[len] = '\0'; + target = strchr(sym, ':'); + if (!target) + /* just "symref=something" */ + goto reject; + *(target++) = '\0'; + if (check_refname_format(sym, REFNAME_ALLOW_ONELEVEL) || + check_refname_format(target, REFNAME_ALLOW_ONELEVEL)) + /* "symref=bogus:pair */ + goto reject; + item = string_list_append(symref, sym); + item->util = target; + return; +reject: + free(sym); + return; +} + +static void annotate_refs_with_symref_info(struct ref *ref) +{ + struct string_list symref = STRING_LIST_INIT_DUP; + const char *feature_list = server_capabilities; + + while (feature_list) { + int len; + const char *val; + + val = parse_feature_value(feature_list, "symref", &len); + if (!val) + break; + parse_one_symref_info(&symref, val, len); + feature_list = val + 1; + } + sort_string_list(&symref); + + for (; ref; ref = ref->next) { + struct string_list_item *item; + item = string_list_lookup(&symref, ref->name); + if (!item) + continue; + ref->symref = xstrdup((char *)item->util); + } + string_list_clear(&symref, 0); +} + /* * Read all the refs from the other end */ @@ -67,6 +123,7 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len, struct ref **list, unsigned int flags, struct extra_have_objects *extra_have) { + struct ref **orig_list = list; int got_at_least_one_head = 0; *list = NULL; @@ -114,6 +171,9 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len, list = &ref->next; got_at_least_one_head = 1; } + + annotate_refs_with_symref_info(*orig_list); + return list; } diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh index 197d3f763d..ac79dd915d 100755 --- a/t/t5505-remote.sh +++ b/t/t5505-remote.sh @@ -160,9 +160,7 @@ cat >test/expect <test/expect <<\EOF -error: Multiple remote HEAD branches. Please choose one explicitly with: - git remote set-head two another - git remote set-head two master -EOF - -test_expect_success 'set-head --auto fails w/multiple HEADs' ' +test_expect_success 'set-head --auto has no problem w/multiple HEADs' ' ( cd test && git fetch two "refs/heads/*:refs/remotes/two/*" && - test_must_fail git remote set-head --auto two >output 2>&1 && + git remote set-head --auto two >output 2>&1 && + echo "two/HEAD set to master" >expect && test_i18ncmp expect output ) ' From 8b2772220927a67d75948dd8758a406979505623 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 6 Sep 2013 17:57:53 +0200 Subject: [PATCH 7/9] clone: test the new HEAD detection logic Signed-off-by: Junio C Hamano --- t/t5601-clone.sh | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh index 0629149edd..ccda6dfb4c 100755 --- a/t/t5601-clone.sh +++ b/t/t5601-clone.sh @@ -285,4 +285,15 @@ test_expect_success NOT_MINGW,NOT_CYGWIN 'clone local path foo:bar' ' git clone "./foo:bar" foobar ' +test_expect_success 'clone from a repository with two identical branches' ' + + ( + cd src && + git checkout -b another master + ) && + git clone src target-11 && + test "z$( cd target-11 && git symbolic-ref HEAD )" = zrefs/heads/another + +' + test_done From 2ecb573bb3aa9b409e2a74d82bee560ef77b101b Mon Sep 17 00:00:00 2001 From: Brian Gernhardt Date: Mon, 21 Oct 2013 13:54:11 -0400 Subject: [PATCH 8/9] t5570: Update for symref capability git-daemon now uses the symref capability to send the correct HEAD reference, so the test for that in t5570 now passes. Signed-off-by: Brian Gernhardt Signed-off-by: Junio C Hamano --- t/t5570-git-daemon.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t5570-git-daemon.sh b/t/t5570-git-daemon.sh index f01edffa3c..dc55e51c18 100755 --- a/t/t5570-git-daemon.sh +++ b/t/t5570-git-daemon.sh @@ -37,7 +37,7 @@ test_expect_success 'fetch changes via git protocol' ' test_cmp file clone/file ' -test_expect_failure 'remote detects correct HEAD' ' +test_expect_success 'remote detects correct HEAD' ' git push public master:other && (cd clone && git remote set-head -d origin && From 360a3261a471ee59760b0743bbb27d3a60849ae2 Mon Sep 17 00:00:00 2001 From: Brian Gernhardt Date: Mon, 21 Oct 2013 13:54:12 -0400 Subject: [PATCH 9/9] t5570: Update for clone-progress-to-stderr branch git clone now reports its progress to standard error, which throws off t5570. Using test_i18ngrep instead of test_cmp allows the test to be more flexible by only looking for the expected error and ignoring any other output from the program. Signed-off-by: Brian Gernhardt Signed-off-by: Junio C Hamano --- t/t5570-git-daemon.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/t/t5570-git-daemon.sh b/t/t5570-git-daemon.sh index dc55e51c18..e06146835c 100755 --- a/t/t5570-git-daemon.sh +++ b/t/t5570-git-daemon.sh @@ -122,8 +122,7 @@ test_remote_error() fi test_must_fail git "$cmd" "$GIT_DAEMON_URL/$repo" "$@" 2>output && - echo "fatal: remote error: $msg: /$repo" >expect && - test_cmp expect output + test_i18ngrep "fatal: remote error: $msg: /$repo" output && ret=$? chmod +x "$GIT_DAEMON_DOCUMENT_ROOT_PATH/repo.git" (exit $ret)