Re: [PATCH] Revert retry request without query when info/refs?query fails

2012-09-20 Thread Junio C Hamano
Shawn O. Pearce spea...@spearce.org writes:

 This reverts commit 703e6e76a14825e5b0c960d525f34e607154b4f7.

 Retrying without the query parameter was added as a workaround
 for a single broken HTTP server at git.debian.org[1]. The server
 was misconfigured to route every request with a query parameter
 into gitweb.cgi. Admins fixed the server's configuration within
 16 hours of the bug report to the Git mailing list, but we still
 patched Git with this fallback and have been paying for it since.

As the consequence of the above, the only two things we know about
the servers in the wild are (1) a misconfiguration that requires
this retry was once made, so it is not very unlikely others did the
same misconfiguration, and (2) those unknown number of servers have
been happily serving the current clients because the workaround
patch have been hiding the misconfiguration ever since.

But as long as the failure diagnosis from updated clients that
revert this workaround is sufficient to allow such misconfigured
servers, I think it is OK.  We might see a large number of small
people having to run around and fix the configuration as a fallout,
though.

 Most Git hosting services configure the smart HTTP protocol and the
 retry logic confuses users when there is a transient HTTP error as
 Git dropped the real error from the smart HTTP request. Removing the
 retry makes root causes easier to identify.

Does that hold true also for dumb only small people installations?
They are the ones that need more help than the large installations
staffed sufficiently and run smart http gateway.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Revert retry request without query when info/refs?query fails

2012-09-20 Thread Jeff King
On Wed, Sep 19, 2012 at 11:29:34PM -0700, Junio C Hamano wrote:

 Shawn O. Pearce spea...@spearce.org writes:
 
  This reverts commit 703e6e76a14825e5b0c960d525f34e607154b4f7.
 
  Retrying without the query parameter was added as a workaround
  for a single broken HTTP server at git.debian.org[1]. The server
  was misconfigured to route every request with a query parameter
  into gitweb.cgi. Admins fixed the server's configuration within
  16 hours of the bug report to the Git mailing list, but we still
  patched Git with this fallback and have been paying for it since.
 
 As the consequence of the above, the only two things we know about
 the servers in the wild are (1) a misconfiguration that requires
 this retry was once made, so it is not very unlikely others did the
 same misconfiguration, and (2) those unknown number of servers have
 been happily serving the current clients because the workaround
 patch have been hiding the misconfiguration ever since.

The misconfiguration was pretty wild in this case. I'd be much more
worried about stupidly non-compliant servers that will not serve
foo/bar when asked for foo/bar?key=value.

 But as long as the failure diagnosis from updated clients that
 revert this workaround is sufficient to allow such misconfigured
 servers, I think it is OK.  We might see a large number of small
 people having to run around and fix the configuration as a fallout,
 though.

I think Shawn's revert is the right thing to do. But it is not complete
without the manual workaround. I'm putting that patch together now and
should have it out in a few minutes.

  Most Git hosting services configure the smart HTTP protocol and the
  retry logic confuses users when there is a transient HTTP error as
  Git dropped the real error from the smart HTTP request. Removing the
  retry makes root causes easier to identify.
 
 Does that hold true also for dumb only small people installations?
 They are the ones that need more help than the large installations
 staffed sufficiently and run smart http gateway.

For the most part, yes. They will get a useful error out of the smart
request if there is a transient error, the repo does not exist, etc.
The real fallout is the people who are hitting a broken or misconfigured
server and may get a confusing error code (in the one case we know
about, it was a 404, but it really could be anything, depending on the
exact nature of the misconfiguration).

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html