git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tay Ray Chuan <rctay89@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, msysgit@googlegroups.com
Subject: Re: Issue 323 in msysgit: Can't clone over http
Date: Mon, 7 Sep 2009 13:53:04 +0800	[thread overview]
Message-ID: <be6fef0d0909062253p1b86628et8a9f979952eebb00@mail.gmail.com> (raw)
In-Reply-To: <7v8wgrbb9e.fsf@alter.siamese.dyndns.org>

Hi,

On Mon, Sep 7, 2009 at 12:59 PM, Junio C Hamano<gitster@pobox.com> 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

  reply	other threads:[~2009-09-07  5:53 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <0016e6470f36315b8a0472bc75a8@google.com>
2009-09-04 10:25 ` Issue 323 in msysgit: Can't clone over http Junio C Hamano
2009-09-04 13:29 ` Tay Ray Chuan
2009-09-07  4:59   ` Junio C Hamano
2009-09-07  5:53     ` Tay Ray Chuan [this message]
2009-09-07  7:10       ` Junio C Hamano
2009-09-07  8:18         ` Junio C Hamano
2009-09-07  9:27         ` Tay Ray Chuan
2009-09-07 19:06           ` Junio C Hamano
2009-09-08 12:57             ` Jakub Narebski
2009-09-08 13:18               ` Tay Ray Chuan
2009-09-08 13:10             ` Tay Ray Chuan
2009-09-09 12:33             ` Tay Ray Chuan
2009-09-09 19:08               ` Junio C Hamano
2009-09-11  8:54               ` Junio C Hamano
2009-09-12 10:01                 ` Tay Ray Chuan
2009-09-12 17:31                   ` Junio C Hamano

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=be6fef0d0909062253p1b86628et8a9f979952eebb00@mail.gmail.com \
    --to=rctay89@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=msysgit@googlegroups.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).