Re: [PATCH v2 0/5] drop support for ancient curl
- From
Jeff King <peff@peff.net>
- Date
- Jul 23, 2021, 10:16 UTC
- Message-ID
- <YPqW8lAcwno3j7Fq@coredump.intra.peff.net>
- In-Reply-To
- <cover-0.5-00000000000-20210721T220402Z-avarab@gmail.com>
On Thu, Jul 22, 2021 at 12:22:11AM +0200, Ævar Arnfjörð Bjarmason wrote:
Show 5 quoted lines
> This series is a re-roll of patches found in Peff's GitHub repo at > jk/no-ancient-curl, which were already-rebased versions of those > patches. His original on-list version had his Signed-off-by, but the > range-diff is against that branch, hence the addition of > Signed-off-by in the range-diff.
Heh, OK. It's a little surprising to see random junk pulled out of my GitHub repo, but in this case I was holding onto them with the intent of eventually resending after more time passed.
So I'm happy to see these cleaned up and posted. I think what's on that branch should be good-ish, in the sense that I've been rebasing it forward as part of my daily routine, and it's part of the build that I use day-to-day. Though apparently I never applied the CURLOPT_POST301 fix. :-/
I know my S-o-b was on the originals to the list, but just to make clear: I am fine with using them on the rebased versions you grabbed.
> I then added a couple of patches on top, one is based on my comments > on the v1 http://lore.kernel.org/git/871sokhoi9.fsf@gmail.com, > i.e. the CURLAUTH_DIGEST_IE and CURLOPT_USE_SSL flags are also > version-based, and we can drop support for curls that don't have them.
Seems reasonable.
Show 5 quoted lines
> I then renamed the ancient CURLOPT_FILE alias to > CURLOPT_WRITEDATA. Incidentally that's how I remembered to dig up this > series, i.e. I tried to search for "CURLOPT_FILE" in API documentation > while reading our HTTP code, but had a hard time finding it, turns out > we were using a very ancient synonym for the preferred name.
This seemed weirdly familiar. Looks like it was part of a series last year, but the trickier parts built on top merited a re-roll that never came:
https://lore.kernel.org/git/20201013191729.2524700-2-smcallis@google.com/
Show 8 quoted lines
> Jeff King (3): > http: drop support for curl < 7.11.1 > http: drop support for curl < 7.16.0 > http: drop support for curl < 7.19.4 > > Ævar Arnfjörð Bjarmason (2): > http: drop support for curl < 7.19.3 and < 7.16.4 (again) > http: rename CURLOPT_FILE to CURLOPT_WRITEDATA
So modulo the commit message tweaks that Junio suggested, this all looks fine. I actually think my original "#error on too-old curl" is still reasonable. Yes, people whose distro has backported all of these features could possibly still use it. But in that case they likely know what's going on and can rip out the #error. It seems much more likely to me that it _won't_ work, and they'll get confused by obscure errors when they try to use an old curl.
But I don't feel too stronlgy about it either way.
-Peff