git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: "Yi, EungJun" <semtlenori@gmail.com>
Subject: [PATCH 4/9] remote-curl: always show friendlier 404 message
Date: Fri, 5 Apr 2013 18:20:43 -0400	[thread overview]
Message-ID: <20130405222043.GD22163@sigill.intra.peff.net> (raw)
In-Reply-To: <20130405221331.GA21209@sigill.intra.peff.net>

When we get an http 404 trying to get the initial list of
refs from the server, we try to be helpful and remind the
user that update-server-info may need to be run. This looks
like:

  $ git clone https://github.com/non/existent
  Cloning into 'existent'...
  fatal: https://github.com/non/existent/info/refs?service=git-upload-pack not found: did you run git update-server-info on the server?

Suggesting update-server-info may be a good suggestion for
users who are in control of the server repo and who are
planning to set up dumb http. But for users of smart http,
and especially users who are not in control of the server
repo, the advice is useless and confusing.

Since most people are expected to use smart http these days,
it does not make sense to keep the update-server-info hint.

We not only drop the mention of update-server-info, but also
show only the main repo URL, not the full "info/refs" and
service parameter. These elements may be useful for
debugging a broken server configuration, but in the majority
of cases, users are not fetching from their own
repositories, but rather from other people's repositories;
they have neither the power nor interest to fix a broken
configuration, and the extra components just make the
message more confusing. Users who do want to debug can and
should use GIT_CURL_VERBOSE to get more complete information
on the actual URLs visited.

Signed-off-by: Jeff King <peff@peff.net>
---
This is obviously a more stringent version of the last patch, and could
just replace it. I think the last one is a no-brainer, because it lets
well-configured sites replace these messages. This one is less obvious,
because we may be hitting some random poorly configured server that
somebody is trying to set up.

Still, if you think about the number of people fetching (against any
server) versus the number of people configuring new servers, I think the
advice to run update-server-info has a vanishingly small chance of being
useful (and IMHO is misleading if you do not know how dumb-http works,
as it makes you think the server is misconfigured, when it is much more
likely to be a user error).

 remote-curl.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/remote-curl.c b/remote-curl.c
index 083efdf..5d9f961 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -209,10 +209,8 @@ static struct discovery* discover_refs(const char *service, int for_push)
 	case HTTP_OK:
 		break;
 	case HTTP_MISSING_TARGET:
-		if (!show_http_message(&type, &buffer))
-			die("repository '%s' not found", url);
-		die("%s not found: did you run git update-server-info on the"
-		    " server?", refs_url);
+		show_http_message(&type, &buffer);
+		die("repository '%s' not found", url);
 	case HTTP_NOAUTH:
 		show_http_message(&type, &buffer);
 		die("Authentication failed");
-- 
1.8.2.rc0.33.gd915649

  parent reply	other threads:[~2013-04-06 16:57 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-05 22:13 [RFC/PATCH 0/9] friendlier http error messages Jeff King
2013-04-05 22:14 ` [PATCH 1/9] http: add HTTP_KEEP_ERROR option Jeff King
2013-04-05 22:17 ` [PATCH 2/9] remote-curl: show server content on http errors Jeff King
2013-04-05 22:17 ` [PATCH 3/9] remote-curl: let servers override http 404 advice Jeff King
2013-04-05 22:20 ` Jeff King [this message]
2013-04-05 22:21 ` [PATCH 5/9] remote-curl: consistently report repo url for http errors Jeff King
2013-04-05 22:21 ` [PATCH 6/9] http: simplify http_error helper function Jeff King
2013-04-05 22:22 ` [PATCH 7/9] http: re-word http error message Jeff King
2013-04-05 22:22 ` [PATCH 8/9] remote-curl: die directly with http error messages Jeff King
2013-04-05 22:22 ` [PATCH 9/9] http: drop http_error function Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130405222043.GD22163@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=semtlenori@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).