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 2/3] http: normalize curl results for dumb loose and alternates fetches
Date: Sun, 24 Mar 2019 08:09:46 -0400 [thread overview]
Message-ID: <20190324120946.GB312@sigill.intra.peff.net> (raw)
In-Reply-To: <20190324120757.GA18684@sigill.intra.peff.net>
If the dumb-http walker encounters a 404 when fetching a loose object,
it then looks at any http-alternates for the object. The 404 check is
implemented by missing_target(), which checks not only the http code,
but also that we got an http error from the CURLcode.
That broke when we stopped using CURLOPT_FAILONERROR in 17966c0a63
(http: avoid disconnecting on 404s for loose objects, 2016-07-11), since
our CURLcode will now be CURLE_OK. As a result, fetching over dumb-http
from a repository with alternates could result in Git printing "Unable
to find abcd1234..." and aborting.
We could probably fix this just by loosening missing_target(). However,
there's other code which looks at the curl result, and it would have to
be tweaked as well. Instead, let's just normalize the result the same
way the smart-http code does.
There's a similar case in processing the alternates (where we failover
from "info/http-alternates" to "info/alternates"). We'll give it the
same treatment.
After this patch, we should be hitting all code paths that need this
normalization (notably absent here is the http_pack_request path, but it
does not use FAILONERROR, nor missing_target()).
Signed-off-by: Jeff King <peff@peff.net>
---
http-walker.c | 8 ++++++++
t/t5550-http-fetch-dumb.sh | 16 ++++++++++++++++
2 files changed, 24 insertions(+)
diff --git a/http-walker.c b/http-walker.c
index 8ae5d76c6a..745436921d 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -98,6 +98,11 @@ static void process_object_response(void *callback_data)
process_http_object_request(obj_req->req);
obj_req->state = COMPLETE;
+ normalize_curl_result(&obj_req->req->curl_result,
+ obj_req->req->http_code,
+ obj_req->req->errorstr,
+ sizeof(obj_req->req->errorstr));
+
/* Use alternates if necessary */
if (missing_target(obj_req->req)) {
fetch_alternates(walker, alt->base);
@@ -208,6 +213,9 @@ static void process_alternates_response(void *callback_data)
char *data;
int i = 0;
+ normalize_curl_result(&slot->curl_result, slot->http_code,
+ curl_errorstr, sizeof(curl_errorstr));
+
if (alt_req->http_specific) {
if (slot->curl_result != CURLE_OK ||
!alt_req->buffer->len) {
diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh
index 6d7d88ccc9..694b77c855 100755
--- a/t/t5550-http-fetch-dumb.sh
+++ b/t/t5550-http-fetch-dumb.sh
@@ -408,5 +408,21 @@ test_expect_success 'print HTTP error when any intermediate redirect throws erro
test_i18ngrep "unable to access.*/redir-to/502" stderr
'
+test_expect_success 'fetching via http alternates works' '
+ parent=$HTTPD_DOCUMENT_ROOT_PATH/alt-parent.git &&
+ git init --bare "$parent" &&
+ git -C "$parent" --work-tree=. commit --allow-empty -m foo &&
+ git -C "$parent" update-server-info &&
+ commit=$(git -C "$parent" rev-parse HEAD) &&
+
+ child=$HTTPD_DOCUMENT_ROOT_PATH/alt-child.git &&
+ git init --bare "$child" &&
+ echo "../../alt-parent.git/objects" >"$child/objects/info/alternates" &&
+ git -C "$child" update-ref HEAD $commit &&
+ git -C "$child" update-server-info &&
+
+ git -c http.followredirects=true clone "$HTTPD_URL/dumb/alt-child.git"
+'
+
stop_httpd
test_done
--
2.21.0.705.g64cb90f133
next prev parent reply other threads:[~2019-03-24 12:09 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 ` Jeff King [this message]
2019-03-24 12:13 ` [PATCH 3/3] http: use normalize_curl_result() instead of manual conversion 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
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=20190324120946.GB312@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).