Hi,
On Mon, Sep 7, 2009 at 12:59 PM, Junio C Hamano<gitster@xxxxxxxxx> wrote:
> Before 39dc52c (http: use new http API in fetch_index(), 2009-06-06), we
> used to run the slot by hand and checked results.curl_request against
> CURLE_OK. Everything else was an error.
>
> With the updated code, that all went to http_get_strbuf() which in turn
> calls http_request() that does the same thing, and the function returns
> HTTP_OK only when it gets CURLE_OK, but now it says MISSING_TARGET when we
> ask for an idx file we think exists in the repository but the server says
> it doesn't, and all other errors will be ignored with this patch.
We should only be interested in the MISSING_TARGET error, because it
tells us that the pack file is indeed not available. We aren't
interested in other errors, like being unable to perform the request
(HTTP_START_FAILED), or, say, a 401 (Unauthorized) error, or even a
500; we simply move along and we tell the user we couldn't perform the
check.
> If this codepath is what was broken by github returning 500 [*1*], the
> client before 39dc52c would have failed the same way. I do not think
> loosening error checking like this is a real solution, but I may be
> reading the patch incorrectly.
You're right to say that git before 39dc52c would have failed. It did,
but no one had the chance to break anything, because 39dc52c was part
of my http patch series that only went wild in v1.6.4.
We can trace this back to 48188c2 ("http-walker: verify remote
packs"), which copied the "feature" from http-push.c to http-walker.c.
Before that, if you tried fetching a pack with http-push.c from a
500-prone server, you would also experience this.
--
Cheers,
Ray Chuan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html