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

1  2 
connect.c
connect.h
t/t5601-clone.sh
upload-pack.c
diff --combined connect.c
index 40868610ab1c02d10cc7cdc927fdf97a5e010414,553a80c734dc2a4420ee8c0db4491edead53b947..06e88b0705f7fbd138e504920c20c70218121696
+++ b/connect.c
@@@ -5,10 -5,11 +5,12 @@@
  #include "refs.h"
  #include "run-command.h"
  #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 -61,61 +62,61 @@@ static void die_initial_contact(int got
                    "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 +124,7 @@@ struct ref **get_remote_heads(int in, c
                              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;
                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;
  
@@@ -552,7 -612,7 +613,7 @@@ struct child_process *git_connect(int f
        path = strchr(end, c);
        if (path && !has_dos_drive_prefix(end)) {
                if (c == ':') {
 -                      if (path < strchrnul(host, '/')) {
 +                      if (host != url || path < strchrnul(host, '/')) {
                                protocol = PROTO_SSH;
                                *path++ = '\0';
                        } else /* '/' in the host part, assume local path */
diff --combined connect.h
index 9dff25cad4de82e84343b50490b1b1601103ffac,0000000000000000000000000000000000000000..64fb7dbbee8714c034c5e018b3b1b4f8c29f0ea7
mode 100644,000000..100644
--- /dev/null
+++ b/connect.h
@@@ -1,13 -1,0 +1,12 @@@
- extern const char *parse_feature_value(const char *feature_list, const char *feature, int *len_ret);
 +#ifndef CONNECT_H
 +#define CONNECT_H
 +
 +#define CONNECT_VERBOSE       (1u << 0)
 +extern struct child_process *git_connect(int fd[2], const char *url, const char *prog, int flags);
 +extern int finish_connect(struct child_process *conn);
 +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);
 +
 +#endif
diff --combined t/t5601-clone.sh
index 8f3cd44d514f3996b62da28f6f83be7b1d786f01,f8e5a9a4ae1051216e4ef55c8a46c99686e45b5c..1d1c8755ead4e432744a82f0d7b8ed7dfc8a6510
@@@ -280,53 -280,20 +280,64 @@@ test_expect_success 'clone checking ou
        test_cmp fetch.expected fetch.actual
  '
  
 +test_expect_success 'setup ssh wrapper' '
 +      write_script "$TRASH_DIRECTORY/ssh-wrapper" <<-\EOF &&
 +      echo >>"$TRASH_DIRECTORY/ssh-output" "ssh: $*" &&
 +      # throw away all but the last argument, which should be the
 +      # command
 +      while test $# -gt 1; do shift; done
 +      eval "$1"
 +      EOF
 +
 +      GIT_SSH="$TRASH_DIRECTORY/ssh-wrapper" &&
 +      export GIT_SSH &&
 +      export TRASH_DIRECTORY
 +'
 +
 +clear_ssh () {
 +      >"$TRASH_DIRECTORY/ssh-output"
 +}
 +
 +expect_ssh () {
 +      {
 +              case "$1" in
 +              none)
 +                      ;;
 +              *)
 +                      echo "ssh: $1 git-upload-pack '$2'"
 +              esac
 +      } >"$TRASH_DIRECTORY/ssh-expect" &&
 +      (cd "$TRASH_DIRECTORY" && test_cmp ssh-expect ssh-output)
 +}
 +
 +test_expect_success 'cloning myhost:src uses ssh' '
 +      clear_ssh &&
 +      git clone myhost:src ssh-clone &&
 +      expect_ssh myhost src
 +'
 +
  test_expect_success NOT_MINGW,NOT_CYGWIN 'clone local path foo:bar' '
 +      clear_ssh &&
        cp -R src "foo:bar" &&
 -      git clone "./foo:bar" foobar
 +      git clone "./foo:bar" foobar &&
 +      expect_ssh none
 +'
 +
 +test_expect_success 'bracketed hostnames are still ssh' '
 +      clear_ssh &&
 +      git clone "[myhost:123]:src" ssh-bracket-clone &&
 +      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
diff --combined upload-pack.c
index a6c54e06bb4c4725ace576740912cbb6133e291e,2826909eba69977caa783340c6a3a5290670e015..43342ac161952f5c03c7b737231c4c664b7513a7
@@@ -10,7 -10,6 +10,7 @@@
  #include "revision.h"
  #include "list-objects.h"
  #include "run-command.h"
 +#include "connect.h"
  #include "sigchain.h"
  #include "version.h"
  #include "string-list.h"
@@@ -41,7 -40,6 +41,7 @@@ static struct object_array have_obj
  static struct object_array want_obj;
  static struct object_array extra_edge_obj;
  static unsigned int timeout;
 +static int keepalive = 5;
  /* 0 for no sideband,
   * otherwise maximum packet size (up to 65520 bytes).
   */
@@@ -70,28 -68,87 +70,28 @@@ static ssize_t send_client_data(int fd
        return sz;
  }
  
 -static FILE *pack_pipe = NULL;
 -static void show_commit(struct commit *commit, void *data)
 -{
 -      if (commit->object.flags & BOUNDARY)
 -              fputc('-', pack_pipe);
 -      if (fputs(sha1_to_hex(commit->object.sha1), pack_pipe) < 0)
 -              die("broken output pipe");
 -      fputc('\n', pack_pipe);
 -      fflush(pack_pipe);
 -      free(commit->buffer);
 -      commit->buffer = NULL;
 -}
 -
 -static void show_object(struct object *obj,
 -                      const struct name_path *path, const char *component,
 -                      void *cb_data)
 -{
 -      show_object_with_name(pack_pipe, obj, path, component);
 -}
 -
 -static void show_edge(struct commit *commit)
 -{
 -      fprintf(pack_pipe, "-%s\n", sha1_to_hex(commit->object.sha1));
 -}
 -
 -static int do_rev_list(int in, int out, void *user_data)
 -{
 -      int i;
 -      struct rev_info revs;
 -
 -      pack_pipe = xfdopen(out, "w");
 -      init_revisions(&revs, NULL);
 -      revs.tag_objects = 1;
 -      revs.tree_objects = 1;
 -      revs.blob_objects = 1;
 -      if (use_thin_pack)
 -              revs.edge_hint = 1;
 -
 -      for (i = 0; i < want_obj.nr; i++) {
 -              struct object *o = want_obj.objects[i].item;
 -              /* why??? */
 -              o->flags &= ~UNINTERESTING;
 -              add_pending_object(&revs, o, NULL);
 -      }
 -      for (i = 0; i < have_obj.nr; i++) {
 -              struct object *o = have_obj.objects[i].item;
 -              o->flags |= UNINTERESTING;
 -              add_pending_object(&revs, o, NULL);
 -      }
 -      setup_revisions(0, NULL, &revs, NULL);
 -      if (prepare_revision_walk(&revs))
 -              die("revision walk setup failed");
 -      mark_edges_uninteresting(revs.commits, &revs, show_edge);
 -      if (use_thin_pack)
 -              for (i = 0; i < extra_edge_obj.nr; i++)
 -                      fprintf(pack_pipe, "-%s\n", sha1_to_hex(
 -                                      extra_edge_obj.objects[i].item->sha1));
 -      traverse_commit_list(&revs, show_commit, show_object, NULL);
 -      fflush(pack_pipe);
 -      fclose(pack_pipe);
 -      return 0;
 -}
 -
  static void create_pack_file(void)
  {
 -      struct async rev_list;
        struct child_process pack_objects;
        char data[8193], progress[128];
        char abort_msg[] = "aborting due to possible repository "
                "corruption on the remote side.";
        int buffered = -1;
        ssize_t sz;
 -      const char *argv[10];
 -      int arg = 0;
 +      const char *argv[12];
 +      int i, arg = 0;
 +      FILE *pipe_fd;
 +      char *shallow_file = NULL;
  
 -      argv[arg++] = "pack-objects";
 -      if (!shallow_nr) {
 -              argv[arg++] = "--revs";
 -              if (use_thin_pack)
 -                      argv[arg++] = "--thin";
 +      if (shallow_nr) {
 +              shallow_file = setup_temporary_shallow();
 +              argv[arg++] = "--shallow-file";
 +              argv[arg++] = shallow_file;
        }
 +      argv[arg++] = "pack-objects";
 +      argv[arg++] = "--revs";
 +      if (use_thin_pack)
 +              argv[arg++] = "--thin";
  
        argv[arg++] = "--stdout";
        if (!no_progress)
        if (start_command(&pack_objects))
                die("git upload-pack: unable to fork git-pack-objects");
  
 -      if (shallow_nr) {
 -              memset(&rev_list, 0, sizeof(rev_list));
 -              rev_list.proc = do_rev_list;
 -              rev_list.out = pack_objects.in;
 -              if (start_async(&rev_list))
 -                      die("git upload-pack: unable to fork git-rev-list");
 -      }
 -      else {
 -              FILE *pipe_fd = xfdopen(pack_objects.in, "w");
 -              int i;
 -
 -              for (i = 0; i < want_obj.nr; i++)
 -                      fprintf(pipe_fd, "%s\n",
 -                              sha1_to_hex(want_obj.objects[i].item->sha1));
 -              fprintf(pipe_fd, "--not\n");
 -              for (i = 0; i < have_obj.nr; i++)
 -                      fprintf(pipe_fd, "%s\n",
 -                              sha1_to_hex(have_obj.objects[i].item->sha1));
 -              fprintf(pipe_fd, "\n");
 -              fflush(pipe_fd);
 -              fclose(pipe_fd);
 -      }
 -
 +      pipe_fd = xfdopen(pack_objects.in, "w");
 +
 +      for (i = 0; i < want_obj.nr; i++)
 +              fprintf(pipe_fd, "%s\n",
 +                      sha1_to_hex(want_obj.objects[i].item->sha1));
 +      fprintf(pipe_fd, "--not\n");
 +      for (i = 0; i < have_obj.nr; i++)
 +              fprintf(pipe_fd, "%s\n",
 +                      sha1_to_hex(have_obj.objects[i].item->sha1));
 +      for (i = 0; i < extra_edge_obj.nr; i++)
 +              fprintf(pipe_fd, "%s\n",
 +                      sha1_to_hex(extra_edge_obj.objects[i].item->sha1));
 +      fprintf(pipe_fd, "\n");
 +      fflush(pipe_fd);
 +      fclose(pipe_fd);
  
        /* We read from pack_objects.err to capture stderr output for
         * progress bar, and pack_objects.out to capture the pack data.
        while (1) {
                struct pollfd pfd[2];
                int pe, pu, pollsize;
 +              int ret;
  
                reset_timeout();
  
                if (!pollsize)
                        break;
  
 -              if (poll(pfd, pollsize, -1) < 0) {
 +              ret = poll(pfd, pollsize, 1000 * keepalive);
 +              if (ret < 0) {
                        if (errno != EINTR) {
                                error("poll failed, resuming: %s",
                                      strerror(errno));
                        if (sz < 0)
                                goto fail;
                }
 +
 +              /*
 +               * We hit the keepalive timeout without saying anything; send
 +               * an empty message on the data sideband just to let the other
 +               * side know we're still working on it, but don't have any data
 +               * yet.
 +               *
 +               * If we don't have a sideband channel, there's no room in the
 +               * protocol to say anything, so those clients are just out of
 +               * luck.
 +               */
 +              if (!ret && use_sideband) {
 +                      static const char buf[] = "0005\1";
 +                      write_or_die(1, buf, 5);
 +              }
        }
  
        if (finish_command(&pack_objects)) {
                error("git upload-pack: git-pack-objects died with error.");
                goto fail;
        }
 -      if (shallow_nr && finish_async(&rev_list))
 -              goto fail;      /* error was already reported */
 +      if (shallow_file) {
 +              if (*shallow_file)
 +                      unlink(shallow_file);
 +              free(shallow_file);
 +      }
  
        /* flush the data */
        if (0 <= buffered) {
@@@ -689,6 -734,16 +689,16 @@@ static int mark_our_ref(const char *ref
        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"
        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;
  
@@@ -740,11 -824,6 +779,11 @@@ static int upload_pack_config(const cha
  {
        if (!strcmp("uploadpack.allowtipsha1inwant", var))
                allow_tip_sha1_in_want = git_config_bool(var, value);
 +      else if (!strcmp("uploadpack.keepalive", var)) {
 +              keepalive = git_config_int(var, value);
 +              if (!keepalive)
 +                      keepalive = -1;
 +      }
        return parse_hide_refs_config(var, value, "uploadpack");
  }