| Message ID | 20241010235621.738239-10-sandals@crustytoothpaste.net (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Update versions of libcurl and Perl | expand |
On Thu, Oct 10, 2024 at 11:56:17PM +0000, brian m. carlson wrote: > libcurl 7.56.0 was released in September 2017, which is over seven years > ago, and no major operating system vendor is still providing security > support for it. Debian 10, which is out of mainstream security support, > has supported a newer version, and Ubuntu 20.04 and RHEL 8, which are > still in support, also have a newer version. > > Remove the check for this version and use this functionality > unconditionally. > > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> > --- > git-curl-compat.h | 8 -------- > http.c | 2 -- > 2 files changed, 10 deletions(-) > > diff --git a/git-curl-compat.h b/git-curl-compat.h > index 65ba1ee0f8..703756ba85 100644 > --- a/git-curl-compat.h > +++ b/git-curl-compat.h > @@ -28,14 +28,6 @@ > * introduced, oldest first, in the official version of cURL library. > */ > > -/** > - * CURLSSLSET_{NO_BACKENDS,OK,TOO_LATE,UNKNOWN_BACKEND} were added in > - * 7.56.0, released in September 2017. > - */ > -#if LIBCURL_VERSION_NUM >= 0x073800 > -#define GIT_CURL_HAVE_CURLSSLSET_NO_BACKENDS > -#endif > - > /** > * Versions before curl 7.66.0 (September 2019) required manually setting the > * transfer-encoding for a streaming POST; after that this is handled > diff --git a/http.c b/http.c > index c5fdf1cd4c..4d59f11ad2 100644 > --- a/http.c > +++ b/http.c > @@ -1275,7 +1275,6 @@ void http_init(struct remote *remote, const char *url, int proactive_auth) > free(normalized_url); > string_list_clear(&config.vars, 1); > > -#ifdef GIT_CURL_HAVE_CURLSSLSET_NO_BACKENDS > if (http_ssl_backend) { > const curl_ssl_backend **backends; > struct strbuf buf = STRBUF_INIT; > @@ -1300,7 +1299,6 @@ void http_init(struct remote *remote, const char *url, int proactive_auth) > break; /* Okay! */ > } > } > -#endif > > if (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK) > die("curl_global_init failed"); > I wonder whether we want to have something like the below patch to give people a better error message in case they have a version that is too old now. Other than that I agree with the sentiment of this patch series. Supporting ancient dependency versions that aren't used by any still-supported and available distro doesn't feel sensible to me, and scenarios like this are why we have introduced the platform support policy in the first place. Patrick diff --git a/git-curl-compat.h b/git-curl-compat.h index e1d0bdd2735..d65b5f55126 100644 --- a/git-curl-compat.h +++ b/git-curl-compat.h @@ -143,4 +143,8 @@ #define GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR 1 #endif +#if LIBCURL_VERSION_NUM < 0x073d00 +# error "Your version of curl is too old. You need to have at least curl 7.61.0" +#endif + #endif
On Fri, Oct 11, 2024 at 08:48:51AM +0200, Patrick Steinhardt wrote: > I wonder whether we want to have something like the below patch to give > people a better error message in case they have a version that is too > old now. > [...] > +#if LIBCURL_VERSION_NUM < 0x073d00 > +# error "Your version of curl is too old. You need to have at least curl 7.61.0" > +#endif IIRC we ran into some interesting situations in the past where some distros had older versions that had backported some features. So Git would continue to compile, even though it was not technically the version we said was needed. And a patch like the one above would break those systems, even they'd otherwise be OK. Now possibly that is a little bit insane and not something we should worry about. I don't have good examples of what kinds of things got backported, but searching the archive for LIBCURL_VERSION_NUM and "backport" yielded this: https://lore.kernel.org/git/4d29d43d458f61c6dabca093f591ad8698ca2ceb.1502462884.git.tgc@jupiterrise.com/ and I seem to recall most of the discussion of this was around that author and RHEL/EPEL. -Peff
On Fri, Oct 11, 2024 at 03:33:26AM -0400, Jeff King wrote: > On Fri, Oct 11, 2024 at 08:48:51AM +0200, Patrick Steinhardt wrote: > > > I wonder whether we want to have something like the below patch to give > > people a better error message in case they have a version that is too > > old now. > > [...] > > +#if LIBCURL_VERSION_NUM < 0x073d00 > > +# error "Your version of curl is too old. You need to have at least curl 7.61.0" > > +#endif > > IIRC we ran into some interesting situations in the past where some > distros had older versions that had backported some features. So Git > would continue to compile, even though it was not technically the > version we said was needed. And a patch like the one above would break > those systems, even they'd otherwise be OK. > > Now possibly that is a little bit insane and not something we should > worry about. I don't have good examples of what kinds of things got > backported, but searching the archive for LIBCURL_VERSION_NUM and > "backport" yielded this: > > https://lore.kernel.org/git/4d29d43d458f61c6dabca093f591ad8698ca2ceb.1502462884.git.tgc@jupiterrise.com/ > > and I seem to recall most of the discussion of this was around that > author and RHEL/EPEL. Huh, interesting, thanks for the context! I'm not really sure whether we really should worry about such weird backports all that much. But in any case I'm okay with not pursuing the error. Patrick
Patrick Steinhardt <ps@pks.im> writes: >> > I wonder whether we want to have something like the below patch to give >> > people a better error message in case they have a version that is too >> > old now. >> > [...] >> > +#if LIBCURL_VERSION_NUM < 0x073d00 >> > +# error "Your version of curl is too old. You need to have at least curl 7.61.0" >> > +#endif >> >> IIRC we ran into some interesting situations in the past where some >> distros had older versions that had backported some features. So Git >> would continue to compile, even though it was not technically the >> version we said was needed. And a patch like the one above would break >> those systems, even they'd otherwise be OK. >> >> Now possibly that is a little bit insane and not something we should >> worry about. I don't have good examples of what kinds of things got >> backported, but searching the archive for LIBCURL_VERSION_NUM and >> "backport" yielded this: >> >> https://lore.kernel.org/git/4d29d43d458f61c6dabca093f591ad8698ca2ceb.1502462884.git.tgc@jupiterrise.com/ >> >> and I seem to recall most of the discussion of this was around that >> author and RHEL/EPEL. > > Huh, interesting, thanks for the context! I'm not really sure whether we > really should worry about such weird backports all that much. But in any > case I'm okay with not pursuing the error. Yup, the runtime die() would work it around for such versions of libcURL with silent backports. The message should be made _("localizable"), though. Thanks.
diff --git a/git-curl-compat.h b/git-curl-compat.h index 65ba1ee0f8..703756ba85 100644 --- a/git-curl-compat.h +++ b/git-curl-compat.h @@ -28,14 +28,6 @@ * introduced, oldest first, in the official version of cURL library. */ -/** - * CURLSSLSET_{NO_BACKENDS,OK,TOO_LATE,UNKNOWN_BACKEND} were added in - * 7.56.0, released in September 2017. - */ -#if LIBCURL_VERSION_NUM >= 0x073800 -#define GIT_CURL_HAVE_CURLSSLSET_NO_BACKENDS -#endif - /** * Versions before curl 7.66.0 (September 2019) required manually setting the * transfer-encoding for a streaming POST; after that this is handled diff --git a/http.c b/http.c index c5fdf1cd4c..4d59f11ad2 100644 --- a/http.c +++ b/http.c @@ -1275,7 +1275,6 @@ void http_init(struct remote *remote, const char *url, int proactive_auth) free(normalized_url); string_list_clear(&config.vars, 1); -#ifdef GIT_CURL_HAVE_CURLSSLSET_NO_BACKENDS if (http_ssl_backend) { const curl_ssl_backend **backends; struct strbuf buf = STRBUF_INIT; @@ -1300,7 +1299,6 @@ void http_init(struct remote *remote, const char *url, int proactive_auth) break; /* Okay! */ } } -#endif if (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK) die("curl_global_init failed");
libcurl 7.56.0 was released in September 2017, which is over seven years ago, and no major operating system vendor is still providing security support for it. Debian 10, which is out of mainstream security support, has supported a newer version, and Ubuntu 20.04 and RHEL 8, which are still in support, also have a newer version. Remove the check for this version and use this functionality unconditionally. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> --- git-curl-compat.h | 8 -------- http.c | 2 -- 2 files changed, 10 deletions(-)