Il giorno mar 15 mar 2022 alle ore 16:47 Junio C Hamano <gitster@xxxxxxxxx> ha scritto: > > Elia Pinto <gitter.spiros@xxxxxxxxx> writes: > > > This file was produced from a modified version of symbols.pl > > (https://github.com/curl/curl/blob/master/docs/libcurl/symbols.pl) and > > by manually adding the previous comments describing the dates of release > > of some curl versions not currently reported in the symbols-in-versions. > > > > To do this the symbols are listed in the order defined in the file > > symbols-in-versions rather than as they were previously inserted based > > on release dates. > > > > Most of these symbols are not used by git today. However, inserting > > them all starting from an automatic tool makes it largely unnecessary > > to update this file and therefore reduces the possibility > > of introducing possible errors in the future. > > > > Signed-off-by: Elia Pinto <gitter.spiros@xxxxxxxxx> > > --- > > git-curl-compat.h | 2944 +++++++++++++++++++++++++++++++++++++++++++-- > > 1 file changed, 2875 insertions(+), 69 deletions(-) > > Hmmm. While the basic idea may be sound, I do not think this alone > would be sufficient out of the box. > > > diff --git a/git-curl-compat.h b/git-curl-compat.h > > index 56a83b6bbd..dc9086710a 100644 > > --- a/git-curl-compat.h > > +++ b/git-curl-compat.h > > @@ -24,25 +24,1228 @@ > > * doing so is sufficient to add support for it to older versions that > > * don't have it. > > * > > - * 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. > > - */ > > -#if LIBCURL_VERSION_NUM < 0x071505 > > -#define CURL_SOCKOPT_OK 0 > > -#endif > > Even though you define GIT_HAVE_CURL_SOCKOPT_OK macro below, the > existing codebase is not expecting to handle the lack of > CURL_SOCKOPT_OK by checking the macro. Instead, it expects the > macro to be defined to be 0 for older versions, exactly like this. > > And you do not give a replacement definition. > > > -#if LIBCURL_VERSION_NUM >= 0x071900 > > -#define GITCURL_HAVE_CURLOPT_TCP_KEEPALIVE 1 > > -#endif > > This is an unfortunate oddball that lacks _ between GIT and CURL. > > $ git grep -e '#define ' --and --not -e GIT_CURL_ git-curl-compat.h > > -#if LIBCURL_VERSION_NUM >= 0x071900 > > -#define GITCURL_HAVE_CURLOPT_TCP_KEEPALIVE 1 > > -#endif > > You define GIT_CURL_HAVE_CURLOPT_TCP_KEEPALIVE but naturally, > existing callers do not pay attention to it. They care about the > oddball variant GITCURL_HAVE_CURLOPT_TCP_KEEPALIVE instead. > > Luckily, these two are the only irregulars, it seems. > > $ git grep -e '#define ' --and --not -e GIT_CURL_ git-curl-compat.h > git-curl-compat.h:#define CURL_SOCKOPT_OK 0 > git-curl-compat.h:#define GITCURL_HAVE_CURLOPT_TCP_KEEPALIVE 1 > > So, I think you would need a bit of preliminary clean-up patch > inserted before this automated compatibility layer, perhaps like > this. Hi In the end I did not understand if you think it is worthwhile that i do a reroll of the patch, making a series of patches that put all the preliminary patches like the one you reported. Thank you all > > --- >8 --- > Subject: curl: streamline conditional compilation > > Earlier we introduced git-curl-compat.h that defines bunch of > GIT_CURL_HAVE_X where X is a feature of cURL library we care about, > to make it easily manageable to conditionally compile code against > the version of cURL library we are given. > > There however are two oddball macros. Instead of checking > GIT_CURL_HAVE_CURL_SOCKOPT_OK and using a fallback definition for > CURL_SOCKOPT_OK macro, we just defined CURL_SOCKOPT_OK to a safe > value when compiling against an old version that lack the symbol. > Also, the macro to check CURLOPT_TCP_KEEPALIVE (alone) was named > GITCURL_HAVE_CURLOPT_TCP_KEEPALIVE. > > Introduce GIT_CURL_HAVE_CURL_SOCKOPT_OK and define it for the > versions of cURL where we used to use our fallback definition for > CURL_SOCKOPT_OK, and use the fallback definition based on the new > GIT_CURL_HAVE_CURL_SOCKOPT_OK symbol at its sole use site. > > To better conform the naming convention of other symbols, rename > GITCURL_HAVE_CURLOPT_TCP_KEEPALIVE to GIT_CURL_HAVE_CURL_SOCKOPT_OK > and update its sole use site. > > After this, conditional compilation with cURL library is all > controlled uniformly with GIT_CURL_HAVE_X mechanism. > > Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx> > --- > git-curl-compat.h | 2 +- > http.c | 5 ++++- > 2 files changed, 5 insertions(+), 2 deletions(-) > > diff --git c/git-curl-compat.h w/git-curl-compat.h > index 56a83b6bbd..802d11e7be 100644 > --- c/git-curl-compat.h > +++ w/git-curl-compat.h > @@ -39,7 +39,7 @@ > * CURLOPT_TCP_KEEPALIVE was added in 7.25.0, released in March 2012. > */ > #if LIBCURL_VERSION_NUM >= 0x071900 > -#define GITCURL_HAVE_CURLOPT_TCP_KEEPALIVE 1 > +#define GIT_CURL_HAVE_CURLOPT_TCP_KEEPALIVE 1 > #endif > > > diff --git c/http.c w/http.c > index 2f67fbb33c..8d9a66b5ca 100644 > --- c/http.c > +++ w/http.c > @@ -517,7 +517,7 @@ static int has_proxy_cert_password(void) > } > #endif > > -#ifdef GITCURL_HAVE_CURLOPT_TCP_KEEPALIVE > +#ifdef GIT_CURL_HAVE_CURLOPT_TCP_KEEPALIVE > static void set_curl_keepalive(CURL *c) > { > curl_easy_setopt(c, CURLOPT_TCP_KEEPALIVE, 1); > @@ -537,6 +537,9 @@ static int sockopt_callback(void *client, curl_socket_t fd, curlsocktype type) > if (rc < 0) > warning_errno("unable to set SO_KEEPALIVE on socket"); > > +#ifndef GIT_CURL_HAVE_CURL_SOCKOPT_OK > +#define CURL_SOCKOPT_OK 0 > +#endif > return CURL_SOCKOPT_OK; > } > >
