git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Eric Wong <e@80x24.org>
Cc: Wolfgang Denk <wd@denx.de>,
	Heinrich Schuchardt <xypron.glpk@gmx.de>,
	git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
	Takahiro Akashi <takahiro.akashi@linaro.org>
Subject: [PATCH 3/3] http: use normalize_curl_result() instead of manual conversion
Date: Sun, 24 Mar 2019 08:13:16 -0400	[thread overview]
Message-ID: <20190324121316.GC312@sigill.intra.peff.net> (raw)
In-Reply-To: <20190324120757.GA18684@sigill.intra.peff.net>

When we switched off CURLOPT_FAILONERROR in 17966c0a63 (http: avoid
disconnecting on 404s for loose objects, 2016-07-11), the fetch_object()
function started manually handling 404's. Since we now have
normalize_curl_result() for use elsewhere, we can use it here as well,
shortening the code.

Note that we lose the check for http/https in the URL here. None of the
other result-normalizing code paths bother with this. Looking at
missing_target(), which checks specifically for an FTP-specific CURLcode
and "http" code 550, it seems likely that git-over-ftp has been subtly
broken since 17966c0a63. This patch does nothing to fix that, but nor
should it make anything worse (in fact, it may be slightly better
because we'll actually recognize an error as such, rather than assuming
CURLE_OK means we actually got some data).

Signed-off-by: Jeff King <peff@peff.net>
---
I can't bring myself to care too much about whether git-over-ftp works
with alternates, but if somebody wants to dig into it, be my guest. I
suspect other bits may be broken, too, as we check http_code in several
places and assume it has some sensible http-based number in it (notably,
for 401 triggering authentication).

It may not even work at all. I didn't try (and I'd be kind of surprised
if somebody is actually using it in practice). I'm content to let it
bit-rot unless somebody using it shows up to report a bug.

(I'd also be fine with officially deprecating it. But then I kind of
feel the same way about the dumb-http code).

 http-walker.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/http-walker.c b/http-walker.c
index 745436921d..48b1b3a193 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -526,17 +526,8 @@ static int fetch_object(struct walker *walker, unsigned char *sha1)
 		req->localfile = -1;
 	}
 
-	/*
-	 * we turned off CURLOPT_FAILONERROR to avoid losing a
-	 * persistent connection and got CURLE_OK.
-	 */
-	if (req->http_code >= 300 && req->curl_result == CURLE_OK &&
-			(starts_with(req->url, "http://") ||
-			 starts_with(req->url, "https://"))) {
-		req->curl_result = CURLE_HTTP_RETURNED_ERROR;
-		xsnprintf(req->errorstr, sizeof(req->errorstr),
-			  "HTTP request failed");
-	}
+	normalize_curl_result(&req->curl_result, req->http_code,
+			      req->errorstr, sizeof(req->errorstr));
 
 	if (obj_req->state == ABORTED) {
 		ret = error("Request for %s aborted", hex);
-- 
2.21.0.705.g64cb90f133

      parent reply	other threads:[~2019-03-24 12:13 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <7e4a2f1d-2b3a-eb83-d3f1-9ac63d68991b@gmx.de>
     [not found] ` <20190322005107.GL9937@linaro.org>
2019-03-22  6:02   ` [BUG] Cloning with git HEAD fails for some repositories Heinrich Schuchardt
2019-03-22  7:12     ` Jeff King
2019-03-22  8:21       ` Wolfgang Denk
2019-03-22  9:31         ` Jeff King
2019-03-22 16:50           ` Eric Wong
2019-03-22 17:42             ` Heinrich Schuchardt
2019-03-22 18:09               ` Eric Wong
2019-03-22 18:41                 ` Heinrich Schuchardt
2019-03-22 20:35                   ` Eric Wong
2019-03-23  8:53             ` Jeff King
2019-03-24 12:07               ` [PATCH 0/3] fix dumb-http fetch with alternates Jeff King
2019-03-24 12:08                 ` [PATCH 1/3] http: factor out curl result code normalization Jeff King
2019-03-24 12:09                 ` [PATCH 2/3] http: normalize curl results for dumb loose and alternates fetches Jeff King
2019-03-24 12:13                 ` Jeff King [this message]

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20190324121316.GC312@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=e@80x24.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=takahiro.akashi@linaro.org \
    --cc=wd@denx.de \
    --cc=xypron.glpk@gmx.de \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).