From a4dfee06806df64b7a44866f67f15b54567ec722 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 17 Sep 2013 21:45:34 -0700 Subject: 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 diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh index 8f6e392..197d3f7 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 ) -- cgit v0.10.2-6-g49f6 From a4d695de0dbaf41505cb6db44c6bb8290980398e Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 17 Sep 2013 16:03:32 -0700 Subject: 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 diff --git a/upload-pack.c b/upload-pack.c index 127e59a..a6e107f 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) -- cgit v0.10.2-6-g49f6 From 7171d8c15f919c760d52f814a0aee1b1253385b1 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 17 Sep 2013 16:17:33 -0700 Subject: 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 diff --git a/upload-pack.c b/upload-pack.c index a6e107f..979fc8e 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; -- cgit v0.10.2-6-g49f6 From 5e7dcad771cb873e278a0571b46910d7c32e2f6c Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 17 Sep 2013 16:21:33 -0700 Subject: 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 diff --git a/upload-pack.c b/upload-pack.c index 979fc8e..2826909 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(); -- cgit v0.10.2-6-g49f6 From 5d54cffc36f654721302a717ff12fb317c1b494a Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 17 Sep 2013 16:29:28 -0700 Subject: connect.c: make parse_feature_value() static Signed-off-by: Junio C Hamano diff --git a/cache.h b/cache.h index 85b544f..2c853ba 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 a0783d4..e4c7ae6 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; -- cgit v0.10.2-6-g49f6 From a45b5f0552eec3c8800edae44e071cf1d647bf96 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 17 Sep 2013 19:10:31 -0700 Subject: 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 diff --git a/connect.c b/connect.c index e4c7ae6..553a80c 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 197d3f7..ac79dd9 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 ) ' -- cgit v0.10.2-6-g49f6 From 8b2772220927a67d75948dd8758a406979505623 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 6 Sep 2013 17:57:53 +0200 Subject: clone: test the new HEAD detection logic Signed-off-by: Junio C Hamano diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh index 0629149..ccda6df 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 -- cgit v0.10.2-6-g49f6 From 2ecb573bb3aa9b409e2a74d82bee560ef77b101b Mon Sep 17 00:00:00 2001 From: Brian Gernhardt Date: Mon, 21 Oct 2013 13:54:11 -0400 Subject: 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 diff --git a/t/t5570-git-daemon.sh b/t/t5570-git-daemon.sh index f01edff..dc55e51 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 && -- cgit v0.10.2-6-g49f6 From 360a3261a471ee59760b0743bbb27d3a60849ae2 Mon Sep 17 00:00:00 2001 From: Brian Gernhardt Date: Mon, 21 Oct 2013 13:54:12 -0400 Subject: 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 diff --git a/t/t5570-git-daemon.sh b/t/t5570-git-daemon.sh index dc55e51..e061468 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) -- cgit v0.10.2-6-g49f6