| Message ID | cover-v2-0.8-00000000000-20210910T105523Z-avarab@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | post-v2.33 "drop support for ancient curl" follow-up | expand |
On Fri, Sep 10, 2021 at 01:04:25PM +0200, Ævar Arnfjörð Bjarmason wrote: > Version 1 of this had a really bad bug where we'd effectively make all > supported curl versions act like 7.19.4, i.e. the oldest supported > version except for a couple of our supported features. This is because > most of the things checked with the "ifdef" checks are enum fields, > not macros. So basically the "devil's advocate" Jeff King pointed out > in [2] was already the case. Oops! Oops. :) Well, I'm glad I mentioned it, then (I really was just playing devil's advocate). It does seem a little scary that we'd compile without all of these features and not even notice it. I guess it's hard to test for those features when we don't know for sure that our libcurl supports them. We could perhaps make the tests optional, but how do we set up the prereqs? Version checks based on "curl --version" or even "pkg-config --modversion libcurl" seems error-prone. The curl binary might not match the library we used, or the user might have given us a specific libcurl to use rather than the pkg-config version. We could teach Git to tell us which features it thinks curl supports, but that's depending on the very thing we're trying to test. We could have Git output the LIBCURL_VERSION_NUM it saw at build time, but then we're just implementing the same "if version > X, we have this feature" logic now in the test suite. So I dunno. I do not have any clever solution that would have caught this automatically without creating an even bigger maintenance headache. (Though for other reasons, it might be nice to report the curl version from "git version --build-options". This is a bit tricky because we avoid linking at libcurl at all in the main binary. Definitely orthogonal to your series, anyway). > This means that anyone on an older distro with backported features > will need to -DGIT_CURL_HAVE_* if their version of curl supports some > of these via a backport, not ideal, but an acceptable trade-off. If we > cared about this we could have some "detect-curl" script similar to my > proposed "detect-compiler"[3] (or another homegrown autoconf > replacement). I think that's acceptable. This should be rare, and they can always set CFLAGS appropriately. -Peff
On Fri, Sep 10 2021, Jeff King wrote: > On Fri, Sep 10, 2021 at 01:04:25PM +0200, Ævar Arnfjörð Bjarmason wrote: > >> Version 1 of this had a really bad bug where we'd effectively make all >> supported curl versions act like 7.19.4, i.e. the oldest supported >> version except for a couple of our supported features. This is because >> most of the things checked with the "ifdef" checks are enum fields, >> not macros. So basically the "devil's advocate" Jeff King pointed out >> in [2] was already the case. Oops! > > Oops. :) > > Well, I'm glad I mentioned it, then (I really was just playing devil's > advocate). It does seem a little scary that we'd compile without all of > these features and not even notice it. > > I guess it's hard to test for those features when we don't know for sure > that our libcurl supports them. We could perhaps make the tests > optional, but how do we set up the prereqs? > > Version checks based on "curl --version" or even "pkg-config > --modversion libcurl" seems error-prone. The curl binary might not match > the library we used, or the user might have given us a specific libcurl > to use rather than the pkg-config version. We could teach Git to tell us > which features it thinks curl supports, but that's depending on the very > thing we're trying to test. We could have Git output the > LIBCURL_VERSION_NUM it saw at build time, but then we're just > implementing the same "if version > X, we have this feature" logic now > in the test suite. > > So I dunno. I do not have any clever solution that would have caught > this automatically without creating an even bigger maintenance headache. Yeah, ideally we'd have tests for these optional features, and we'd then just add them to GIT-BUILD-OPTIONS and skip dedicated tests appropriately, then it would be more visible to see those tests skipped. Presumably someone testing this would run "make test" against a glob that includes *curl*. But that's not easy to do in practice since the flags are either not visible from the outside in terms of behavior, or if they are it's all something that requires proxies, SSL etc, which we don't test currently. We can and should test that, but requires e.g. extending lib-httpd.sh to start apache with ssl support, or maybe we could do it more easily with an optional stunnel or something... > (Though for other reasons, it might be nice to report the curl version > from "git version --build-options". This is a bit tricky because we > avoid linking at libcurl at all in the main binary. Definitely > orthogonal to your series, anyway). We could always ship a git-version-curl and shell out to it, or embed the version curl-config --vernum gave us at compile time via Makefile -> -DGIT_CURL_VERSION. That version could be changed at runtime. We could call that a feature. It's --build-options, not --runtime-libraries :)
On Fri, Sep 10, 2021 at 05:08:32PM +0200, Ævar Arnfjörð Bjarmason wrote: > > So I dunno. I do not have any clever solution that would have caught > > this automatically without creating an even bigger maintenance headache. > > Yeah, ideally we'd have tests for these optional features, and we'd then > just add them to GIT-BUILD-OPTIONS and skip dedicated tests > appropriately, then it would be more visible to see those tests > skipped. Presumably someone testing this would run "make test" against a > glob that includes *curl*. > > But that's not easy to do in practice since the flags are either not > visible from the outside in terms of behavior, or if they are it's all > something that requires proxies, SSL etc, which we don't test currently. > > We can and should test that, but requires e.g. extending lib-httpd.sh to > start apache with ssl support, or maybe we could do it more easily with > an optional stunnel or something... Yeah, even after we get the right version/feature flags into the test suite, most of these are pretty obscure and hard to test. I definitely think that's not something we should worry about for this series. > > (Though for other reasons, it might be nice to report the curl version > > from "git version --build-options". This is a bit tricky because we > > avoid linking at libcurl at all in the main binary. Definitely > > orthogonal to your series, anyway). > > We could always ship a git-version-curl and shell out to it, or embed > the version curl-config --vernum gave us at compile time via Makefile -> > -DGIT_CURL_VERSION. Yeah, I was thinking "git remote-curl --curl-version" or something. Let's punt on it for now, though. I think this series is getting close to ready, and this is all only semi-related. -Peff
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > Version 1 of this had a really bad bug where we'd effectively make all > supported curl versions act like 7.19.4, i.e. the oldest supported > version except for a couple of our supported features. This is because > most of the things checked with the "ifdef" checks are enum fields, > not macros. So basically the "devil's advocate" Jeff King pointed out > in [2] was already the case. Oops! Wow. Thanks for bothering to actually test ;-) Because we learned that "#ifdef CURL_SOME_FEATURE_WE_WANT" is not a generally applicable way to conditionally build for various features and we'd need to switch on the version numbers, it is now clear (at least to me) that the central registry approach would be a direction to go. > In this v2 we're instead checking LIBCURL_VERSION_NUM consistently, > even in those cases where we are checking things that are defined via > macros. Nice. > ++ * For each X symbol we need from curl we define our own > ++ * GIT_CURL_HAVE_X. If multiple similar symbols with the same prefix > ++ * were defined in the same version we pick one and check for that name. > + * > + * Keep any symbols in date order of when their support was > + * introduced, oldest first, in the official version of cURL library. > @@ git-curl-compat.h (new) > +/** > + * CURLOPT_TCP_KEEPALIVE was added in 7.25.0, released in March 2012. > + */ > -+#ifdef CURLOPT_TCP_KEEPALIVE > ++#if LIBCURL_VERSION_NUM >= 0x071900 > +#define GITCURL_HAVE_CURLOPT_TCP_KEEPALIVE 1 > +#endif What we have in the posted patch is perfectly OK and serviceable, but this organization somewhat surprised me. Instead of one #if...#endif block per a feature macro, I expected to see a sequence of /* 7.34.0 (0x072200) - Dec 2013 */ #if 0x072200 < VERSION #define HAVE_FOO 1 #define HAVE_BAR 1 ... #endif /* 7.25.0 (0x071900) - March 2012 */ #if 0x071900 < VERSION #define HAVE_BAZ 1 #define HAVE_QUX 1 ... #endif because it would make it more clear which features can now be unconditionally used when we raise the cut-off point for the oldest supported version if we group these entries along the versions. Thanks.
On September 10, 2021 12:53 PM, Junio C Hamano wrote: >To: Ævar Arnfjörð Bjarmason <avarab@gmail.com> >Cc: git@vger.kernel.org; Jeff King <peff@peff.net>; brian m . carlson <sandals@crustytoothpaste.net>; Bagas Sanjaya ><bagasdotme@gmail.com> >Subject: Re: [PATCH v2 0/8] post-v2.33 "drop support for ancient curl" follow-up > >Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > >> Version 1 of this had a really bad bug where we'd effectively make all >> supported curl versions act like 7.19.4, i.e. the oldest supported >> version except for a couple of our supported features. This is because >> most of the things checked with the "ifdef" checks are enum fields, >> not macros. So basically the "devil's advocate" Jeff King pointed out >> in [2] was already the case. Oops! > >Wow. Thanks for bothering to actually test ;-) > >Because we learned that "#ifdef CURL_SOME_FEATURE_WE_WANT" is not a generally applicable way to conditionally build for various >features and we'd need to switch on the version numbers, it is now clear (at least to me) that the central registry approach would be a >direction to go. > >> In this v2 we're instead checking LIBCURL_VERSION_NUM consistently, >> even in those cases where we are checking things that are defined via >> macros. > >Nice. > >> ++ * For each X symbol we need from curl we define our own >> ++ * GIT_CURL_HAVE_X. If multiple similar symbols with the same prefix >> ++ * were defined in the same version we pick one and check for that name. >> + * >> + * Keep any symbols in date order of when their support was >> + * introduced, oldest first, in the official version of cURL library. >> @@ git-curl-compat.h (new) >> +/** >> + * CURLOPT_TCP_KEEPALIVE was added in 7.25.0, released in March 2012. >> + */ >> -+#ifdef CURLOPT_TCP_KEEPALIVE >> ++#if LIBCURL_VERSION_NUM >= 0x071900 >> +#define GITCURL_HAVE_CURLOPT_TCP_KEEPALIVE 1 >> +#endif > >What we have in the posted patch is perfectly OK and serviceable, but this organization somewhat surprised me. > >Instead of one #if...#endif block per a feature macro, I expected to see a sequence of > > /* 7.34.0 (0x072200) - Dec 2013 */ > #if 0x072200 < VERSION > #define HAVE_FOO 1 > #define HAVE_BAR 1 > ... > #endif > > /* 7.25.0 (0x071900) - March 2012 */ > #if 0x071900 < VERSION > #define HAVE_BAZ 1 > #define HAVE_QUX 1 > ... > #endif > > >because it would make it more clear which features can now be unconditionally used when we raise the cut-off point for the oldest >supported version if we group these entries along the versions. > >Thanks. Not that it might matter much, but both NonStop platforms are now on curl 7.78.0 and we are deploying git under OpenSSL 3 this week or next. So any objections I might previously have had are now fortunately really moot. Cheers, -Randall
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > This is a follow-up to the already-integrated topic for dropping > support for older curl versions submitted before the v2.33 release[1]. To which commit are these expected to apply? I am having trouble wiggling 5 and 7 in. Thanks.
Junio C Hamano <gitster@pobox.com> writes: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > >> This is a follow-up to the already-integrated topic for dropping >> support for older curl versions submitted before the v2.33 release[1]. > > To which commit are these expected to apply? I am having trouble > wiggling 5 and 7 in. Nevermind. I seem to be getting garbage from "b4 am", possibly an operator error. I've seen "b4 am" getting confused on a thread with multiple versions (like the config-based-hook thing where the mixture of Emily's v9 and your v5 for base in the same huge thread seem to confuse it).
On Fri, Sep 10 2021, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > >> Version 1 of this had a really bad bug where we'd effectively make all >> supported curl versions act like 7.19.4, i.e. the oldest supported >> version except for a couple of our supported features. This is because >> most of the things checked with the "ifdef" checks are enum fields, >> not macros. So basically the "devil's advocate" Jeff King pointed out >> in [2] was already the case. Oops! > > Wow. Thanks for bothering to actually test ;-) FWIW I'd tested that this worked for CURL_SOCKOPT_OK for v1, but there I happened to pick one that *is* a macro. Then I also tested GIT_CURL_HAVE_CURLOPT_PINNEDPUBLICKEY, which isn't, but the way I wrote the code would have thrown a warning/error about the unused variable, except in v1 I managed to scew that up in the last patch, so we unconditionally used the relevant variable either way...
On Fri, Sep 10 2021, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > >> This is a follow-up to the already-integrated topic for dropping >> support for older curl versions submitted before the v2.33 release[1]. > > To which commit are these expected to apply? I am having trouble > wiggling 5 and 7 in. I based this on top of the current master (8463beaeb69) and these apply cleanly to me on top of that: vm git (master $>) 0 $ git reset --hard @{u} HEAD is now at 8463beaeb69 The fourth batch vm git (master $=) 0 $ git am 000*patch Applying: INSTALL: don't mention the "curl" executable at all Applying: INSTALL: mention that we need libcurl 7.19.4 or newer to build Applying: Makefile: drop support for curl < 7.9.8 (again) Applying: http: drop support for curl < 7.18.0 (again) Applying: http: correct version check for CURL_HTTP_VERSION_2 Applying: http: correct curl version check for CURLOPT_PINNEDPUBLICKEY Applying: http: centralize the accounting of libcurl dependencies Applying: http: don't hardcode the value of CURL_SOCKOPT_OK So, weird, I can't imagine why they wouldn't apply...
On Fri, Sep 10, 2021 at 10:32:52AM -0700, Junio C Hamano wrote: > >> This is a follow-up to the already-integrated topic for dropping > >> support for older curl versions submitted before the v2.33 release[1]. > > > > To which commit are these expected to apply? I am having trouble > > wiggling 5 and 7 in. > > Nevermind. I seem to be getting garbage from "b4 am", possibly an > operator error. I've seen "b4 am" getting confused on a thread with > multiple versions (like the config-based-hook thing where the > mixture of Emily's v9 and your v5 for base in the same huge thread > seem to confuse it). Yeah, sorry, the thread is not great for figuring out which patch goes where (there are two v2 versions, one from July and one from today, and a v4 from July that b4 considers the latest). The only solution I can offer is doing "b4 mbox" first, then deleting everything you don't want, and then doing "b4 am -m that.mbox". I could add extra heuristics that also looks at series age, but I'm worried that the "what you get is what you (probably) meant" logic will just get more and more tortured. -K
Konstantin Ryabitsev <konstantin@linuxfoundation.org> writes: > The only solution I can offer is doing "b4 mbox" first, then deleting > everything you don't want, and then doing "b4 am -m that.mbox". Thanks. Marking the articles I want in GNUS (I read via nntp://lore) and downloading works just fine ;-)
This is a follow-up to the already-integrated topic for dropping support for older curl versions submitted before the v2.33 release[1]. Version 1 of this had a really bad bug where we'd effectively make all supported curl versions act like 7.19.4, i.e. the oldest supported version except for a couple of our supported features. This is because most of the things checked with the "ifdef" checks are enum fields, not macros. So basically the "devil's advocate" Jeff King pointed out in [2] was already the case. Oops! In this v2 we're instead checking LIBCURL_VERSION_NUM consistently, even in those cases where we are checking things that are defined via macros. This means that anyone on an older distro with backported features will need to -DGIT_CURL_HAVE_* if their version of curl supports some of these via a backport, not ideal, but an acceptable trade-off. If we cared about this we could have some "detect-curl" script similar to my proposed "detect-compiler"[3] (or another homegrown autoconf replacement). This also corrects commit messages, removes already-dead code from the Makefile that we'd missed, and mentions the oldest supported version in the INSTALL document. The part where we left behind a potentially warning "ssl_pinnedkey" variable is also gone, although due to another bug in v1 we'd unconditionally use it (for config) with the "centralize the accounting" change there. 1. https://lore.kernel.org/git/cover-v4-0.5-00000000000-20210730T175650Z-avarab@gmail.com/ [1] 2. http://lore.kernel.org/git/YTkPfyAYTU4ZgRgb@coredump.intra.peff.net 3. https://lore.kernel.org/git/87bl6aypke.fsf@evledraar.gmail.com/ Ævar Arnfjörð Bjarmason (8): INSTALL: don't mention the "curl" executable at all INSTALL: mention that we need libcurl 7.19.4 or newer to build Makefile: drop support for curl < 7.9.8 (again) http: drop support for curl < 7.18.0 (again) http: correct version check for CURL_HTTP_VERSION_2 http: correct curl version check for CURLOPT_PINNEDPUBLICKEY http: centralize the accounting of libcurl dependencies http: don't hardcode the value of CURL_SOCKOPT_OK INSTALL | 13 +++-- Makefile | 11 +--- git-curl-compat.h | 133 ++++++++++++++++++++++++++++++++++++++++++++++ http.c | 35 ++++++------ imap-send.c | 2 +- 5 files changed, 161 insertions(+), 33 deletions(-) create mode 100644 git-curl-compat.h Range-diff against v1: -: ----------- > 1: ac11cf8cfd1 INSTALL: don't mention the "curl" executable at all -: ----------- > 2: 4b653cee2d3 INSTALL: mention that we need libcurl 7.19.4 or newer to build -: ----------- > 3: 76c2aa6e78d Makefile: drop support for curl < 7.9.8 (again) 1: 3ffa2f491dd = 4: e73a9ff1780 http: drop support for curl < 7.18.0 (again) 3: d8192164937 ! 5: 2567b888c3d http: correct version check for CURL_HTTP_VERSION_2_0 @@ Metadata Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com> ## Commit message ## - http: correct version check for CURL_HTTP_VERSION_2_0 + http: correct version check for CURL_HTTP_VERSION_2 In d73019feb44 (http: add support selecting http version, 2018-11-08) - a dependency was added on CURL_HTTP_VERSION_2_0, but this feature was + a dependency was added on CURL_HTTP_VERSION_2, but this feature was introduced in curl version 7.43.0, not 7.47.0, as the incorrect version check led us to believe. @@ Commit message dependency on CURL_HTTP_VERSION_2TLS, but the corresponding version check was not corrected. + The newest symbol we depend on is CURL_HTTP_VERSION_2. It was added in + 7.33.0, but the CURL_HTTP_VERSION_2 alias we used was added in + 7.47.0. So we could support an even older version here, but let's just + correct the checked version. + 1. https://lore.kernel.org/git/pull.69.git.gitgitgadget@gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> 2: 511534ce17a ! 6: 397d54a1352 http: correct curl version check for CURLOPT_PINNEDPUBLICKEY @@ Commit message but the relevant code depended on CURLOPT_PINNEDPUBLICKEY, introduced in 7.39.0. - Let's also remove the macro check before we declare the ssl_pinnedkey - variable, the pattern for other such variables is to declare the - static variable unconditionally, we just may not use it on older - versions. This reduces macro verbosity. - - The reduction in verbosity comes at the small cost of issuing a - warning about the unused variable if this code is compiled with curl - versions older than 7.39.0. I think that's an acceptable trade-off, - anyone compiling a new git with a 2014-era toolchain likely has at - least other warning that'll have prompted them not to use -Werror, and - if not maybe this'll prompt them to compile their new git with a more - modern libcurl. - Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> ## http.c ## @@ http.c: static struct { static const char *ssl_capath; static const char *curl_no_proxy; -#if LIBCURL_VERSION_NUM >= 0x072c00 ++#if LIBCURL_VERSION_NUM >= 0x072700 static const char *ssl_pinnedkey; --#endif + #endif static const char *ssl_cainfo; - static long curl_low_speed_limit = -1; - static long curl_low_speed_time = -1; @@ http.c: static int http_options(const char *var, const char *value, void *cb) } 4: 47b513a261b ! 7: 8e57a8409c5 http: centralize the accounting of libcurl dependencies @@ Commit message As discussed in 644de29e220 (http: drop support for curl < 7.19.4, 2021-07-30) checking against LIBCURL_VERSION_NUM isn't as reliable as - checking specific defines in curl, as some distros have been known to - backport features. Furthermore as shown in the preceding commit doing - these version checks makes for hard to read and possibly buggy code, - as shown by the bug fixed there where we were conflating base 10 for - base 16 when comparing the version. + checking specific symbols present in curl, as some distros have been + known to backport features. - Let's instead add a new git-curl-compat.h header that'll keep track of - these dependencies. Following this pattern will also make it much - easier to track when we should deprecate curl versions in the future, - as we just did post-v2.33 e48a623dea0 (Merge branch - 'ab/http-drop-old-curl', 2021-08-24). + However, while some of the curl_easy_setopt() arguments we rely on are + macros, others are enum, and we can't assume that those that are + macros won't change into enums in the future. + + So we're still going to have to check LIBCURL_VERSION_NUM, but by + doing that in one central place and using a macro definition of our + own, anyone who's backporting features can define it themselves, and + thus have access to more modern curl features that they backported, + even if they didn't bump the LIBCURL_VERSION_NUM. + + More importantly, as shown in a preceding commit doing these version + checks makes for hard to read and possibly buggy code, as shown by the + bug fixed there where we were conflating base 10 for base 16 when + comparing the version. + + By doing them all in one place we'll hopefully reduce the chances of + such future mistakes, furthermore it now becomes easier to see at a + glance what the oldest supported version is, which makes it easier to + reason about any future deprecation similar to the recent + e48a623dea0 (Merge branch 'ab/http-drop-old-curl', 2021-08-24). Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> @@ git-curl-compat.h (new) @@ +#ifndef GIT_CURL_COMPAT_H +#define GIT_CURL_COMPAT_H ++#include <curl/curl.h> + +/** + * This header centralizes the declaration of our libcurl dependencies @@ git-curl-compat.h (new) + * inform decisions about removing support for older libcurl in the + * future. + * ++ * The oldest supported version of curl is documented in the "INSTALL" ++ * document. ++ * + * The source of truth for what versions have which symbols is + * https://github.com/curl/curl/blob/master/docs/libcurl/symbols-in-versions; + * the release dates are taken from curl.git (at + * https://github.com/curl/curl/). + * -+ * For each X symbol we need from curl we check if it exists and -+ * declare our own GIT_CURL_HAVE_X, or if it's for both X and Y -+ * GIT_CURL_HAVE_X_and_Y, where the "Y" in "X_and_Y" is only the part -+ * of the symbol name that "X" and "Y" don't have in common. -+ * -+ * We avoid comparisons against LIBCURL_VERSION_NUM, enterprise -+ * distros have been known to backport symbols to their older curl -+ * versions. ++ * For each X symbol we need from curl we define our own ++ * GIT_CURL_HAVE_X. If multiple similar symbols with the same prefix ++ * were defined in the same version we pick one and check for that name. + * + * Keep any symbols in date order of when their support was + * introduced, oldest first, in the official version of cURL library. @@ git-curl-compat.h (new) +/** + * CURLOPT_TCP_KEEPALIVE was added in 7.25.0, released in March 2012. + */ -+#ifdef CURLOPT_TCP_KEEPALIVE ++#if LIBCURL_VERSION_NUM >= 0x071900 +#define GITCURL_HAVE_CURLOPT_TCP_KEEPALIVE 1 +#endif + @@ git-curl-compat.h (new) +/** + * CURLOPT_LOGIN_OPTIONS was added in 7.34.0, released in December + * 2013. ++ * ++ * If we start requiring 7.34.0 we might also be able to remove the ++ * code conditional on USE_CURL_FOR_IMAP_SEND in imap-send.c, see ++ * 1e16b255b95 (git-imap-send: use libcurl for implementation, ++ * 2014-11-09) and the check it added for "072200" in the Makefile. ++ + */ -+#ifdef CURLOPT_LOGIN_OPTIONS ++#if LIBCURL_VERSION_NUM >= 0x072200 +#define GIT_CURL_HAVE_CURLOPT_LOGIN_OPTIONS 1 +#endif + @@ git-curl-compat.h (new) + * CURL_SSLVERSION_TLSv1_[012] was added in 7.34.0, released in + * December 2013. + */ -+#if defined(CURL_SSLVERSION_TLSv1_0) && \ -+ defined(CURL_SSLVERSION_TLSv1_1) && \ -+ defined(CURL_SSLVERSION_TLSv1_2) -+#define GIT_CURL_HAVE_CURL_SSLVERSION_TLSv1_0_and_1_and_2 ++#if LIBCURL_VERSION_NUM >= 0x072200 ++#define GIT_CURL_HAVE_CURL_SSLVERSION_TLSv1_0 +#endif + +/** + * CURLOPT_PINNEDPUBLICKEY was added in 7.39.0, released in November + * 2014. + */ -+#ifdef CURLOPT_PINNEDPUBLICKEY ++#if LIBCURL_VERSION_NUM >= 0x072c00 +#define GIT_CURL_HAVE_CURLOPT_PINNEDPUBLICKEY 1 +#endif + +/** + * CURL_HTTP_VERSION_2 was added in 7.43.0, released in June 2015. ++ * ++ * The CURL_HTTP_VERSION_2 alias (but not CURL_HTTP_VERSION_2_0) has ++ * always been a macro, not an enum field (checked on curl version ++ * 7.78.0) + */ -+#ifdef CURL_HTTP_VERSION_2 ++#if LIBCURL_VERSION_NUM >= 0x072b00 +#define GIT_CURL_HAVE_CURL_HTTP_VERSION_2 1 +#endif + +/** + * CURLSSLOPT_NO_REVOKE was added in 7.44.0, released in August 2015. ++ * ++ * The CURLSSLOPT_NO_REVOKE is, has always been a macro, not an enum ++ * field (checked on curl version 7.78.0) + */ -+#ifdef CURLSSLOPT_NO_REVOKE ++#if LIBCURL_VERSION_NUM >= 0x072c00 +#define GIT_CURL_HAVE_CURLSSLOPT_NO_REVOKE 1 +#endif + +/** + * CURLOPT_PROXY_CAINFO was added in 7.52.0, released in August 2017. + */ -+#ifdef CURLOPT_PROXY_CAINFO ++#if LIBCURL_VERSION_NUM >= 0x073400 +#define GIT_CURL_HAVE_CURLOPT_PROXY_CAINFO 1 +#endif + @@ git-curl-compat.h (new) + * CURLOPT_PROXY_{KEYPASSWD,SSLCERT,SSLKEY} was added in 7.52.0, + * released in August 2017. + */ -+#if defined(CURLOPT_PROXY_KEYPASSWD) && \ -+ defined(CURLOPT_PROXY_SSLCERT) && \ -+ defined(CURLOPT_PROXY_SSLKEY) -+#define GIT_CURL_HAVE_CURLOPT_PROXY_KEYPASSWD_and_SSLCERT_and_SSLKEY 1 ++#if LIBCURL_VERSION_NUM >= 0x073400 ++#define GIT_CURL_HAVE_CURLOPT_PROXY_KEYPASSWD 1 +#endif + +/** + * CURL_SSLVERSION_TLSv1_3 was added in 7.53.0, released in February + * 2017. + */ -+#ifdef CURL_SSLVERSION_TLSv1_3 ++#if LIBCURL_VERSION_NUM >= 0x073400 +#define GIT_CURL_HAVE_CURL_SSLVERSION_TLSv1_3 1 +#endif + @@ git-curl-compat.h (new) + * CURLSSLSET_{NO_BACKENDS,OK,TOO_LATE,UNKNOWN_BACKEND} were added in + * 7.56.0, released in September 2017. + */ -+#if defined(CURLSSLSET_NO_BACKENDS) && \ -+ defined(CURLSSLSET_OK) && \ -+ defined(CURLSSLSET_TOO_LATE) && \ -+ defined(CURLSSLSET_UNKNOWN_BACKEND) -+#define GIT_CURL_HAVE_CURLSSLSET_NO_BACKENDS_and_OK_and_TOO_LATE_and_UNKNOWN_BACKEND 1 ++#if LIBCURL_VERSION_NUM >= 0x073800 ++#define GIT_CURL_HAVE_CURLSSLSET_NO_BACKENDS +#endif + +#endif @@ http.c: static struct { { "sslv3", CURL_SSLVERSION_SSLv3 }, { "tlsv1", CURL_SSLVERSION_TLSv1 }, -#if LIBCURL_VERSION_NUM >= 0x072200 -+#if GIT_CURL_HAVE_CURL_SSLVERSION_TLSv1_0_AND_1_AND_2 ++#ifdef GIT_CURL_HAVE_CURL_SSLVERSION_TLSv1_0 { "tlsv1.0", CURL_SSLVERSION_TLSv1_0 }, { "tlsv1.1", CURL_SSLVERSION_TLSv1_1 }, { "tlsv1.2", CURL_SSLVERSION_TLSv1_2 }, #endif -#if LIBCURL_VERSION_NUM >= 0x073400 -+#if GIT_CURL_HAVE_CURL_SSLVERSION_TLSv1_3 ++#ifdef GIT_CURL_HAVE_CURL_SSLVERSION_TLSv1_3 { "tlsv1.3", CURL_SSLVERSION_TLSv1_3 }, #endif }; + static const char *ssl_key; + static const char *ssl_capath; + static const char *curl_no_proxy; +-#if LIBCURL_VERSION_NUM >= 0x072700 ++#ifdef GIT_CURL_HAVE_CURLOPT_PINNEDPUBLICKEY + static const char *ssl_pinnedkey; + #endif + static const char *ssl_cainfo; @@ http.c: static int http_options(const char *var, const char *value, void *cb) } if (!strcmp("http.pinnedpubkey", var)) { -#if LIBCURL_VERSION_NUM >= 0x072700 -- return git_config_pathname(&ssl_pinnedkey, var, value); --#else -+#ifndef GIT_CURL_HAVE_CURLOPT_PINNEDPUBLICKEY ++#ifdef GIT_CURL_HAVE_CURLOPT_PINNEDPUBLICKEY + return git_config_pathname(&ssl_pinnedkey, var, value); + #else warning(_("Public key pinning not supported with cURL < 7.39.0")); -- return 0; - #endif -+ return git_config_pathname(&ssl_pinnedkey, var, value); - } - - if (!strcmp("http.extraheader", var)) { @@ http.c: static int has_cert_password(void) return 1; } -#if LIBCURL_VERSION_NUM >= 0x073400 -+#ifdef GIT_CURL_HAVE_CURLOPT_PROXY_KEYPASSWD_and_SSLCERT_and_SSLKEY ++#ifdef GIT_CURL_HAVE_CURLOPT_PROXY_KEYPASSWD static int has_proxy_cert_password(void) { if (http_proxy_ssl_cert == NULL || proxy_ssl_cert_password_required != 1) @@ http.c: static CURL *get_curl_handle(void) curl_easy_setopt(result, CURLOPT_PROXYTYPE, CURLPROXY_SOCKS4); -#if LIBCURL_VERSION_NUM >= 0x073400 -+#ifdef GIT_CURL_HAVE_CURLOPT_PROXY_KEYPASSWD_and_SSLCERT_and_SSLKEY ++#ifdef GIT_CURL_HAVE_CURLOPT_PROXY_KEYPASSWD else if (starts_with(curl_http_proxy, "https")) { curl_easy_setopt(result, CURLOPT_PROXYTYPE, CURLPROXY_HTTPS); @@ http.c: void http_init(struct remote *remote, const char *url, int proactive_aut string_list_clear(&config.vars, 1); -#if LIBCURL_VERSION_NUM >= 0x073800 -+#ifdef GIT_CURL_HAVE_CURLSSLSET_NO_BACKENDS_and_OK_and_TOO_LATE_and_UNKNOWN_BACKEND ++#ifdef GIT_CURL_HAVE_CURLSSLSET_NO_BACKENDS if (http_ssl_backend) { const curl_ssl_backend **backends; struct strbuf buf = STRBUF_INIT; 5: 4f42c0e48b0 ! 8: 465ab33ebda http: don't hardcode the value of CURL_SOCKOPT_OK @@ Commit message ## git-curl-compat.h ## @@ - * GIT_CURL_HAVE_X_and_Y, where the "Y" in "X_and_Y" is only the part - * of the symbol name that "X" and "Y" don't have in common. + * GIT_CURL_HAVE_X. If multiple similar symbols with the same prefix + * were defined in the same version we pick one and check for that name. * + * We may also define a missing CURL_* symbol to its known value, if + * doing so is sufficient to add support for it to older versions that + * don't have it. + * - * We avoid comparisons against LIBCURL_VERSION_NUM, enterprise - * distros have been known to backport symbols to their older curl - * versions. -@@ + * Keep any symbols in date order of when their support was * introduced, oldest first, in the official version of cURL library. */ +/** + * CURL_SOCKOPT_OK was added in 7.21.5, released in April 2011. ++ * ++ * This should be safe as CURL_SOCKOPT_OK has always been a macro, not ++ * an enum field (checked on curl version 7.78.0, released on July 19, ++ * 2021). Even if that were to change the value of "0" for "OK" is ++ * unlikely to change. + */ +#ifndef CURL_SOCKOPT_OK +#define CURL_SOCKOPT_OK 0