Re: [PATCH v2 0/5] drop support for ancient curl
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 21, 2021, 22:39 UTC
- Message-ID
- <xmqq35s7cmvi.fsf@gitster.g>
- In-Reply-To
- <cover-0.5-00000000000-20210721T220402Z-avarab@gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> This series simplifies the http code by dropping support for curl > versions older than 7.19.4, released in March 2009.
Thanks.
Will take a look and may have some comments, but I'd prefer to see an Ack from Peff as well.
Show 201 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
>
> http-push.c | 29 +--------
> http-walker.c | 14 +----
> http.c | 169 ++------------------------------------------------
> http.h | 46 --------------
> imap-send.c | 4 --
> remote-curl.c | 11 +---
> 6 files changed, 10 insertions(+), 263 deletions(-)
>
> Range-diff against v1:
> 1: 8793735cc2c ! 1: dcbb6f95652 http: drop support for curl < 7.11.1
> @@ Metadata
> ## Commit message ##
> http: drop support for curl < 7.11.1
>
> - Recent versions of Git will not build with curl older than
> - 7.11.1 due to (at least) two issues:
> + Drop support for this ancient version of curl and simplify the code by
> + allowing us get rid of some "#ifdef"'s.
> +
> + Git will not build with vanilla curl older than 7.11.1 due to (at
> + least) two issues:
>
> - our use of CURLOPT_POSTFIELDSIZE in 37ee680d9b
> (http.postbuffer: allow full range of ssize_t values,
> @@ Commit message
> obvious benefit is that we'll have fewer conditional bits
> cluttering the code.
>
> - But more importantly, we're doing a disservice to users to
> - pretend that Git works with old versions. It's clear that
> - nobody is testing modern Git with such old versions of curl
> - (we've had 3 released versions with the CURLPROTO issue
> - without a report of anyone seeing the breakage in the wild).
> - And there are a lot of subtle ways we could be getting this
> - wrong (for instance, curl prior to 7.17.0 did not copy
> - string arguments to curl_easy_setopt(), which means that
> - using an old copy of curl could produce use-after-free
> - bugs that are not present with more recent versions).
> -
> This patch drops all #ifdefs that reference older versions
> (note that curl's preprocessor macros are in hex, so we're
> looking for 070b01, not 071101).
>
> + Signed-off-by: Jeff King <peff@peff.net>
> + Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> +
> ## http.c ##
> @@
> static struct trace_key trace_curl = TRACE_KEY_INIT(CURL);
> 2: 15638cd1856 ! 2: 1c9f3bc031b http: drop support for curl < 7.16.0
> @@ Metadata
> ## Commit message ##
> http: drop support for curl < 7.16.0
>
> - As discussed in the previous commit, Git is not well-tested
> - with old versions of curl (and in fact since v2.12.0 does
> - not even compile with versions older than 7.19.4). Let's
> - stop pretending we support curl that old and drop any
> - now-obslete #ifdefs.
> + In the last commit we dropped support for curl < 7.11.1, let's
> + continue that and drop support for versions older than 7.16.0. This
> + allows us to get rid of some now-obsolete #ifdefs.
>
> - Choosing 7.16.0 is a somewhat arbitrary cutoff, but:
> + Choosing 7.16.0 is a somewhat arbitrary cutoff:
>
> - 1. it came out in October of 2006, over 10 years ago.
> - Besides being a nice round number, it's a common
> - end-of-life support period, even for conservative
> + 1. It came out in October of 2006, almost 15 years ago.
> + Besides being a nice round number, around 10 years is
> + a common end-of-life support period, even for conservative
> distributions.
>
> - 2. that version introduced the curl_multi interface, which
> + 2. That version introduced the curl_multi interface, which
> gives us a lot of bang for the buck in removing #ifdefs
>
> + RHEL 5 came with curl 7.15.5[1] (released in August 2006). RHEL 5's
> + extended life cycle program ended on 2020-11-30[1]. RHEL 6 comes with
> + curl 7.19.7 (released in November 2009), and RHEL 7 comes with
> + 7.29.0 (released in February 2013).
> +
> + 1. http://lore.kernel.org/git/873e1f31-2a96-5b72-2f20-a5816cad1b51@jupiterrise.com
> +
> + Signed-off-by: Jeff King <peff@peff.net>
> + Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> +
> ## http-push.c ##
> @@ http-push.c: static void curl_setup_http(CURL *curl, const char *url,
> curl_easy_setopt(curl, CURLOPT_INFILE, buffer);
> @@ http.h: void finish_all_active_slots(void);
> void http_init(struct remote *remote, const char *url,
> int proactive_auth);
>
> + ## imap-send.c ##
> +@@ imap-send.c: static int curl_append_msgs_to_imap(struct imap_server_conf *server,
> + if (cred.username) {
> + if (res == CURLE_OK)
> + credential_approve(&cred);
> +-#if LIBCURL_VERSION_NUM >= 0x070d01
> + else if (res == CURLE_LOGIN_DENIED)
> +-#else
> +- else
> +-#endif
> + credential_reject(&cred);
> + }
> +
> +
> ## remote-curl.c ##
> @@ remote-curl.c: static size_t rpc_out(void *ptr, size_t eltsize,
> return avail;
> 3: 335046de7bc ! 3: faae88b7fec http: drop support for curl < 7.19.4
> @@ Metadata
> ## Commit message ##
> http: drop support for curl < 7.19.4
>
> - Since v2.12.0, Git does not compile with versions of curl
> - older than 7.19.4. That version of curl is about 8 years
> - old. This means it may still be used in some distributions
> - with long-running support periods. But the fact that we
> - haven't received a single bug report about the compile-time
> - breakage implies that nobody cares about building recent
> - releases on such platforms.
> + In the last commit we dropped support for curl < 7.16.0, let's
> + continue that and drop support for versions older than 7.19.4. This
> + allows us to simplify the code by getting rid of some "#ifdef"'s.
>
> - As discussed in the previous two commits, this cleans up the
> - code and gives a more realistic signal to users about which
> - versions of Git are actually tested (in particular, this
> - moves us past the potential use-after-free issues with curl
> - older than 7.17.0).
> + Git was broken with vanilla curl < 7.19.4 from v2.12.0 until
> + v2.15.0. Compiling with it was broken by using CURLPROTO_* outside any
> + "#ifdef" in aeae4db174 (http: create function to get curl allowed
> + protocols, 2016-12-14), and fixed in v2.15.0 in f18777ba6ef (http: fix
> + handling of missing CURLPROTO_*, 2017-08-11).
> +
> + It's unclear how much anyone was impacted by that in practice, since
> + as noted in [1] RHEL versions using curl older than that still
> + compiled, because RedHat backported some features. Perhaps other
> + vendors did the same.
> +
> + Still, it's one datapoint indicating that it wasn't in active use at
> + the time. That (the v2.12.0 release) was in Feb 24, 2017, with v2.15.0
> + on Oct 30, 2017, it's now mid-2021.
> +
> + 1. http://lore.kernel.org/git/c8a2716d-76ac-735c-57f9-175ca3acbcb0@jupiterrise.com;
> + followed-up by f18777ba6ef (http: fix handling of missing CURLPROTO_*,
> + 2017-08-11)
> +
> + Signed-off-by: Jeff King <peff@peff.net>
> + Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>
> ## http.c ##
> @@ http.c: static int min_curl_sessions = 1;
> @@ http.c: static void var_override(const char **var, char *value)
> }
>
> static void init_curl_proxy_auth(CURL *result)
> +@@ http.c: void setup_curl_trace(CURL *handle)
> + curl_easy_setopt(handle, CURLOPT_DEBUGDATA, NULL);
> + }
> +
> +-#ifdef CURLPROTO_HTTP
> + static long get_curl_allowed_protocols(int from_user)
> + {
> + long allowed_protocols = 0;
> +@@ http.c: static long get_curl_allowed_protocols(int from_user)
> +
> + return allowed_protocols;
> + }
> +-#endif
> +
> + #if LIBCURL_VERSION_NUM >=0x072f00
> + static int get_curl_http_version_opt(const char *version_string, long *opt)
> @@ http.c: static CURL *get_curl_handle(void)
> }
>
> @@ http.c: static CURL *get_curl_handle(void)
> -#if LIBCURL_VERSION_NUM >= 0x071301
> curl_easy_setopt(result, CURLOPT_POSTREDIR, CURL_REDIR_POST_ALL);
> -#elif LIBCURL_VERSION_NUM >= 0x071101
> - curl_easy_setopt(result, CURLOPT_POST301, 1);
> +- curl_easy_setopt(result, CURLOPT_POST301, 1);
> -#endif
> -#ifdef CURLPROTO_HTTP
> curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS,
> 4: e049f37357a < -: ----------- http: #error on too-old curl
> -: ----------- > 4: 9a30e92520c http: drop support for curl < 7.19.3 and < 7.16.4 (again)
> -: ----------- > 5: 64e510b4a6b http: rename CURLOPT_FILE to CURLOPT_WRITEDATA