Re: [PATCH v4 2/5] http: drop support for curl < 7.16.0

From: Jeff King <peff@peff.net>
Date: Sep 11, 2021, 14:32 UTC
Message-ID: <YTy+GH0I2KPnBCUY@coredump.intra.peff.net>
In-Reply-To: <1b18309a-93e8-60cc-1bd3-61857b1da819@gmail.com>

On Sat, Sep 11, 2021 at 12:28:25AM +0200, Andrei Rybak wrote:

Show 21 quoted lines

> >   enum http_follow_config http_follow_config = HTTP_FOLLOW_INITIAL;
> > -#if LIBCURL_VERSION_NUM >= 0x071700
> > -/* Use CURLOPT_KEYPASSWD as is */
> > -#elif LIBCURL_VERSION_NUM >= 0x070903
> > -#define CURLOPT_KEYPASSWD CURLOPT_SSLKEYPASSWD
> > -#else
> > -#define CURLOPT_KEYPASSWD CURLOPT_SSLCERTPASSWD
> > -#endif
> 
> In 0x071700, 0x17 == 23, so it seems that this chain of `#if`s
> distinguishes between three categories of curl versions:
> 
>   1. version < 7.9.3
>   2. 7.9.3 <= version < 7.23.0
>   3. 7.23.0 <= version
> 
> So it seems that this patch [1] drops support for curl < 7.23.0, while
> the subject line claims "drop support for curl < 7.16.0".
> 
> [1]: already in branch master as commit 013c7e2b07 (http: drop support
>      for curl < 7.16.0, 2021-07-30)

Yes, you're right, though there's a little more going on.

The same issue was raised in the original thread starting here:

  https://lore.kernel.org/git/xmqq1sok7i82.fsf@gitster.mtv.corp.google.com/

but unfortunately that seems to have been missed when the patches were reposted more recently. :(

There's further discussion there, though. It turns out this was yet another decimal/hex confusion, and the correct version is actually 7.17.0. So it breaks things from 7.9.3 <= version < 7.17.0. But later in the series we bump the minimum to 7.19.4 anyway.

So the patch is indeed wrong, but the series overall ends up correct.

-Peff