Re: [PATCH] add http.maxReceiveSpeed to limit git-receive-pack receiving speed

From: Jeff King <peff@peff.net>
Date: Aug 20, 2021, 18:11 UTC
Message-ID: <YR/wbWhei0PPLSgX@coredump.intra.peff.net>
In-Reply-To: <xmqq4kbl2sre.fsf@gitster.g>

On Thu, Aug 19, 2021 at 11:28:53AM -0700, Junio C Hamano wrote:

Show 7 quoted lines

> > https protocol, and only supports libcurl 7.15.5 and above.
> 
> We are likely be raising the floor versions of libcURL to 7.16.0 or
> even 7.19.4 soonish.  It probably would make it easier to allow it
> unconditionally (otherwise you'd probably need to implement error or
> warning messages when configuration is given but the libcURL version
> used is too old, etc.).

Yeah, I agree that if we can drop the conditional totally, we should.

If we do need to make it conditional, I think there was a preference for shifting to checking the actual option constants, like:

  #ifdef CURLOPT_MAX_RECV_SPEED_LARGE
  ...ok, we have it...
  #endif

The rest of your comments all seemed quite reasonable to me.

As a general feature, IMHO speed-limiting is best done outside of application-level tools like Git (and instead done via proxies or kernel network config). But since we are not building the feature ourselves, but rather just plugging our config to curl's feature, I don't have any problem with it here.

-Peff