Re: [PATCH v6] http.postbuffer: allow full range of ssize_t values

From: Jeff King <peff@peff.net>
Date: Apr 11, 2017, 19:41 UTC
Message-ID: <20170411194127.cfy2omkdwhbtkn63@sigill.intra.peff.net>
In-Reply-To: <20170411182740.GO8741@aiede.mtv.corp.google.com>

On Tue, Apr 11, 2017 at 11:27:40AM -0700, Jonathan Nieder wrote:

Show 25 quoted lines

> David Turner wrote:
> 
> > Unfortunately, in order to push some large repos where a server does
> > not support chunked encoding, the http postbuffer must sometimes
> > exceed two gigabytes.  On a 64-bit system, this is OK: we just malloc
> > a larger buffer.
> >
> > This means that we need to use CURLOPT_POSTFIELDSIZE_LARGE to set the
> > buffer size.
> >
> > Signed-off-by: David Turner <dturner@twosigma.com>
> > ---
> >  cache.h       |  1 +
> >  config.c      | 17 +++++++++++++++++
> >  http.c        |  6 ++++--
> >  http.h        |  2 +-
> >  remote-curl.c | 12 +++++++++---
> >  5 files changed, 32 insertions(+), 6 deletions(-)
> 
> The only unresolved issue was whether we can count on curl being new
> enough for CURLOPT_POSTFIELDSIZE_LARGE to be present.  I say
> "unresolved" but it is resolved in my mind since git doesn't build and
> pass tests with such old versions of curl --- what's unresolved is
> formalizing what the oldest curl version is that we want to support.
> And that doesn't need to hold this patch hostage.

It could build on older curl with a minor fix; the regression is in v2.12. So if we did want to continue to support the same versions of curl we did in v2.11, we could apply that fix and then we _would_ care about #ifdef-ing this.

That isn't my preferred route; just pointing out that if the "oldest curl" question isn't settled, that could still be relevant to this patch. It doesn't have to be held hostage to the fix, but we should be aware we are digging the hole deeper.

-Peff