Merge branch 'hv/remote-end-hung-up'
authorJunio C Hamano <gitster@pobox.com>
Thu, 5 Jul 2012 06:40:11 +0000 (23:40 -0700)
committerJunio C Hamano <gitster@pobox.com>
Thu, 5 Jul 2012 06:40:12 +0000 (23:40 -0700)
When we get disconnected while expecting a response from the remote
side because authentication failed, we issued an error message "The
remote side hung up unexpectedly."

Give hint that it may be a permission problem in the message when we
can reasonably suspect it.

* hv/remote-end-hung-up:
remove the impression of unexpectedness when access is denied

1  2 
connect.c
t/t5512-ls-remote.sh
diff --combined connect.c
index 41b7400aa92d7c07db269de9658b9f961db9becf,fe6e9214b29cc0d53accabcd3d88be53ed0b0129..55a85adea94c87c912280079ee13afd6926901aa
+++ b/connect.c
@@@ -22,7 -22,7 +22,7 @@@ static int check_ref(const char *name, 
        len -= 5;
  
        /* REF_NORMAL means that we don't want the magic fake tag refs */
 -      if ((flags & REF_NORMAL) && check_ref_format(name) < 0)
 +      if ((flags & REF_NORMAL) && check_refname_format(name, 0))
                return 0;
  
        /* REF_HEADS means that we want regular branch heads */
@@@ -49,13 -49,26 +49,25 @@@ static void add_extra_have(struct extra
        extra->nr++;
  }
  
+ static void die_initial_contact(int got_at_least_one_head)
+ {
+       if (got_at_least_one_head)
+               die("The remote end hung up upon initial contact");
+       else
+               die("Could not read from remote repository.\n\n"
+                   "Please make sure you have the correct access rights\n"
+                   "and the repository exists.");
+ }
  /*
   * Read all the refs from the other end
   */
  struct ref **get_remote_heads(int in, struct ref **list,
 -                            int nr_match, char **match,
                              unsigned int flags,
                              struct extra_have_objects *extra_have)
  {
+       int got_at_least_one_head = 0;
        *list = NULL;
        for (;;) {
                struct ref *ref;
                char *name;
                int len, name_len;
  
-               len = packet_read_line(in, buffer, sizeof(buffer));
+               len = packet_read(in, buffer, sizeof(buffer));
+               if (len < 0)
+                       die_initial_contact(got_at_least_one_head);
                if (!len)
                        break;
                if (buffer[len-1] == '\n')
  
                if (!check_ref(name, name_len, flags))
                        continue;
 -              if (nr_match && !path_match(name, nr_match, match))
 -                      continue;
                ref = alloc_ref(buffer + 41);
                hashcpy(ref->old_sha1, old_sha1);
                *list = ref;
                list = &ref->next;
+               got_at_least_one_head = 1;
        }
        return list;
  }
  
  int server_supports(const char *feature)
  {
 -      return server_capabilities &&
 -              strstr(server_capabilities, feature) != NULL;
 +      return !!parse_feature_request(server_capabilities, feature);
  }
  
 -int path_match(const char *path, int nr, char **match)
 +const char *parse_feature_request(const char *feature_list, const char *feature)
  {
 -      int i;
 -      int pathlen = strlen(path);
 -
 -      for (i = 0; i < nr; i++) {
 -              char *s = match[i];
 -              int len = strlen(s);
 -
 -              if (!len || len > pathlen)
 -                      continue;
 -              if (memcmp(path + pathlen - len, s, len))
 -                      continue;
 -              if (pathlen > len && path[pathlen - len - 1] != '/')
 -                      continue;
 -              *s = 0;
 -              return (i + 1);
 +      int len;
 +
 +      if (!feature_list)
 +              return NULL;
 +
 +      len = strlen(feature);
 +      while (*feature_list) {
 +              const char *found = strstr(feature_list, feature);
 +              if (!found)
 +                      return NULL;
 +              if ((feature_list == found || isspace(found[-1])) &&
 +                  (!found[len] || isspace(found[len]) || found[len] == '='))
 +                      return found;
 +              feature_list = found + 1;
        }
 -      return 0;
 +      return NULL;
  }
  
  enum protocol {
@@@ -170,15 -191,6 +186,15 @@@ static void get_host_and_port(char **ho
        }
  }
  
 +static void enable_keepalive(int sockfd)
 +{
 +      int ka = 1;
 +
 +      if (setsockopt(sockfd, SOL_SOCKET, SO_KEEPALIVE, &ka, sizeof(ka)) < 0)
 +              fprintf(stderr, "unable to set SO_KEEPALIVE on socket: %s\n",
 +                      strerror(errno));
 +}
 +
  #ifndef NO_IPV6
  
  static const char *ai_name(const struct addrinfo *ai)
@@@ -243,8 -255,6 +259,8 @@@ static int git_tcp_connect_sock(char *h
        if (sockfd < 0)
                die("unable to connect to %s:\n%s", host, error_message.buf);
  
 +      enable_keepalive(sockfd);
 +
        if (flags & CONNECT_VERBOSE)
                fprintf(stderr, "done.\n");
  
   */
  static int git_tcp_connect_sock(char *host, int flags)
  {
 -      int sockfd = -1, saved_errno = 0;
 +      struct strbuf error_message = STRBUF_INIT;
 +      int sockfd = -1;
        const char *port = STR(DEFAULT_GIT_PORT);
        char *ep;
        struct hostent *he;
                fprintf(stderr, "done.\nConnecting to %s (port %s) ... ", host, port);
  
        for (cnt = 0, ap = he->h_addr_list; *ap; ap++, cnt++) {
 -              sockfd = socket(he->h_addrtype, SOCK_STREAM, 0);
 -              if (sockfd < 0) {
 -                      saved_errno = errno;
 -                      continue;
 -              }
 -
                memset(&sa, 0, sizeof sa);
                sa.sin_family = he->h_addrtype;
                sa.sin_port = htons(nport);
                memcpy(&sa.sin_addr, *ap, he->h_length);
  
 -              if (connect(sockfd, (struct sockaddr *)&sa, sizeof sa) < 0) {
 -                      saved_errno = errno;
 -                      fprintf(stderr, "%s[%d: %s]: errno=%s\n",
 +              sockfd = socket(he->h_addrtype, SOCK_STREAM, 0);
 +              if ((sockfd < 0) ||
 +                  connect(sockfd, (struct sockaddr *)&sa, sizeof sa) < 0) {
 +                      strbuf_addf(&error_message, "%s[%d: %s]: errno=%s\n",
                                host,
                                cnt,
                                inet_ntoa(*(struct in_addr *)&sa.sin_addr),
 -                              strerror(saved_errno));
 -                      close(sockfd);
 +                              strerror(errno));
 +                      if (0 <= sockfd)
 +                              close(sockfd);
                        sockfd = -1;
                        continue;
                }
        }
  
        if (sockfd < 0)
 -              die("unable to connect a socket (%s)", strerror(saved_errno));
 +              die("unable to connect to %s:\n%s", host, error_message.buf);
 +
 +      enable_keepalive(sockfd);
  
        if (flags & CONNECT_VERBOSE)
                fprintf(stderr, "done.\n");
@@@ -536,7 -547,7 +552,7 @@@ struct child_process *git_connect(int f
         * Add support for ssh port: ssh://host.xy:<port>/...
         */
        if (protocol == PROTO_SSH && host != url)
 -              port = get_port(host);
 +              port = get_port(end);
  
        if (protocol == PROTO_GIT) {
                /* These underlying connection commands die() if they
@@@ -627,3 -638,47 +643,3 @@@ int finish_connect(struct child_proces
        free(conn);
        return code;
  }
 -
 -char *git_getpass(const char *prompt)
 -{
 -      const char *askpass;
 -      struct child_process pass;
 -      const char *args[3];
 -      static struct strbuf buffer = STRBUF_INIT;
 -
 -      askpass = getenv("GIT_ASKPASS");
 -      if (!askpass)
 -              askpass = askpass_program;
 -      if (!askpass)
 -              askpass = getenv("SSH_ASKPASS");
 -      if (!askpass || !(*askpass)) {
 -              char *result = getpass(prompt);
 -              if (!result)
 -                      die_errno("Could not read password");
 -              return result;
 -      }
 -
 -      args[0] = askpass;
 -      args[1] = prompt;
 -      args[2] = NULL;
 -
 -      memset(&pass, 0, sizeof(pass));
 -      pass.argv = args;
 -      pass.out = -1;
 -
 -      if (start_command(&pass))
 -              exit(1);
 -
 -      strbuf_reset(&buffer);
 -      if (strbuf_read(&buffer, pass.out, 20) < 0)
 -              die("failed to read password from %s\n", askpass);
 -
 -      close(pass.out);
 -
 -      if (finish_command(&pass))
 -              exit(1);
 -
 -      strbuf_setlen(&buffer, strcspn(buffer.buf, "\r\n"));
 -
 -      return buffer.buf;
 -}
diff --combined t/t5512-ls-remote.sh
index 6764d511ce08cf62da2c257bf5fe50cabaf2f03c,2ab66d6e3383097079406d1aeab91737567d2875..d16e5d384a8966bc04e9fde6e92bd41818526aab
@@@ -5,6 -5,7 +5,6 @@@ test_description='git ls-remote
  . ./test-lib.sh
  
  test_expect_success setup '
 -
        >file &&
        git add file &&
        test_tick &&
        ) >expected.all &&
  
        git remote add self "$(pwd)/.git"
 -
  '
  
  test_expect_success 'ls-remote --tags .git' '
 -
        git ls-remote --tags .git >actual &&
        test_cmp expected.tag actual
 -
  '
  
  test_expect_success 'ls-remote .git' '
 -
        git ls-remote .git >actual &&
        test_cmp expected.all actual
 -
  '
  
  test_expect_success 'ls-remote --tags self' '
 -
        git ls-remote --tags self >actual &&
        test_cmp expected.tag actual
 -
  '
  
  test_expect_success 'ls-remote self' '
 -
        git ls-remote self >actual &&
        test_cmp expected.all actual
 -
  '
  
  test_expect_success 'dies when no remote specified and no default remotes found' '
 -
        test_must_fail git ls-remote
 -
  '
  
  test_expect_success 'use "origin" when no remote specified' '
 -
        URL="$(pwd)/.git" &&
        echo "From $URL" >exp_err &&
  
  
        test_cmp exp_err actual_err &&
        test_cmp expected.all actual
 -
  '
  
  test_expect_success 'suppress "From <url>" with -q' '
 -
        git ls-remote -q 2>actual_err &&
        test_must_fail test_cmp exp_err actual_err
 -
  '
  
  test_expect_success 'use branch.<name>.remote if possible' '
 -
        #
        # Test that we are indeed using branch.<name>.remote, not "origin", even
        # though the "origin" remote has been set.
        git ls-remote 2>actual_err >actual &&
        test_cmp exp_err actual_err &&
        test_cmp exp actual
 -
  '
  
 -cat >exp <<EOF
 -fatal: 'refs*master' does not appear to be a git repository
 -fatal: Could not read from remote repository.
 -
 -Please make sure you have the correct access rights
 -and the repository exists.
 -EOF
  test_expect_success 'confuses pattern as remote when no remote specified' '
-       fatal: The remote end hung up unexpectedly
 +      cat >exp <<-\EOF &&
 +      fatal: '\''refs*master'\'' does not appear to be a git repository
++      fatal: Could not read from remote repository.
++
++      Please make sure you have the correct access rights
++      and the repository exists.
 +      EOF
        #
-       # Do not expect "git ls-remote <pattern>" to work; ls-remote, correctly,
-       # confuses <pattern> for <remote>. Although ugly, this behaviour is akin
-       # to the confusion of refspecs for remotes by git-fetch and git-push,
-       # eg:
-       #
-       #   $ git fetch branch
-       #
+       # Do not expect "git ls-remote <pattern>" to work; ls-remote needs
+       # <remote> if you want to feed <pattern>, just like you cannot say
+       # fetch <branch>.
        # We could just as easily have used "master"; the "*" emphasizes its
        # role as a pattern.
        test_must_fail git ls-remote refs*master >actual 2>&1 &&
        test_cmp exp actual
 -
  '
  
  test_expect_success 'die with non-2 for wrong repository even with --exit-code' '