Merge branch 'jc/upload-pack-send-symref'
authorJunio C Hamano <gitster@pobox.com>
Wed, 30 Oct 2013 19:10:00 +0000 (12:10 -0700)
committerJunio C Hamano <gitster@pobox.com>
Wed, 30 Oct 2013 19:10:06 +0000 (12:10 -0700)
One long-standing flaw in the pack transfer protocol used by "git
clone" was that there was no way to tell the other end which branch
"HEAD" points at, and the receiving end needed to guess. A new
capability has been defined in the pack protocol to convey this
information so that cloning from a repository with more than one
branches pointing at the same commit where the HEAD is at now
reliably sets the initial branch in the resulting repository.

* jc/upload-pack-send-symref:
t5570: Update for clone-progress-to-stderr branch
t5570: Update for symref capability
clone: test the new HEAD detection logic
connect: annotate refs with their symref information in get_remote_head()
connect.c: make parse_feature_value() static
upload-pack: send non-HEAD symbolic refs
upload-pack: send symbolic ref information as capability
upload-pack.c: do not pass confusing cb_data to mark_our_ref()
t5505: fix "set-head --auto with ambiguous HEAD" test

connect.c
connect.h
t/t5505-remote.sh
t/t5570-git-daemon.sh
t/t5601-clone.sh
upload-pack.c
index 40868610ab1c02d10cc7cdc927fdf97a5e010414..06e88b0705f7fbd138e504920c20c70218121696 100644 (file)
--- a/connect.c
+++ b/connect.c
@@ -7,8 +7,10 @@
 #include "remote.h"
 #include "connect.h"
 #include "url.h"
+#include "string-list.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)
 {
@@ -60,6 +62,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 +124,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,10 +172,13 @@ 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;
 }
 
-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;
 
index 9dff25cad4de82e84343b50490b1b1601103ffac..64fb7dbbee8714c034c5e018b3b1b4f8c29f0ea7 100644 (file)
--- a/connect.h
+++ b/connect.h
@@ -8,6 +8,5 @@ extern int git_connection_is_socket(struct child_process *conn);
 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);
 
 #endif
index 8f6e3922dce2b676221610d83a87c5b36d3bc5b6..ac79dd915da4a520e90fca6a11a239aba6e688d7 100755 (executable)
@@ -160,9 +160,7 @@ cat >test/expect <<EOF
 * remote two
   Fetch URL: ../two
   Push  URL: ../three
-  HEAD branch (remote HEAD is ambiguous, may be one of the following):
-    another
-    master
+  HEAD branch: master
   Local refs configured for 'git push':
     ahead  forces to master  (fast-forwardable)
     master pushes to another (up to date)
@@ -262,16 +260,12 @@ test_expect_success 'set-head --auto' '
        )
 '
 
-cat >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 &&
-               test_must_fail git remote set-head --auto two >output 2>&1 &&
+               git fetch two "refs/heads/*:refs/remotes/two/*" &&
+               git remote set-head --auto two >output 2>&1 &&
+               echo "two/HEAD set to master" >expect &&
                test_i18ncmp expect output
        )
 '
index f01edffa3c0babf76593fbd211f6606ff2036c74..e06146835cdda5b7264a34a24405aedd9f950006 100755 (executable)
@@ -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 &&
@@ -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)
index 8f3cd44d514f3996b62da28f6f83be7b1d786f01..1d1c8755ead4e432744a82f0d7b8ed7dfc8a6510 100755 (executable)
@@ -329,4 +329,15 @@ test_expect_success 'bracketed hostnames are still ssh' '
        expect_ssh myhost:123 src
 '
 
+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
index a6c54e06bb4c4725ace576740912cbb6133e291e..43342ac161952f5c03c7b737231c4c664b7513a7 100644 (file)
@@ -689,6 +689,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"
@@ -697,35 +707,64 @@ 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)
-               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);
+       for_each_namespaced_ref(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;