diff mbox series

[v3,5/7] http: drop support for curl < 7.18.0 (again)

Message ID patch-v3-5.7-b857a9ef7b1-20210730T092843Z-avarab@gmail.com (mailing list archive)
State New
Headers show
Series drop support for ancient curl, improve version checks | expand

Commit Message

Ævar Arnfjörð Bjarmason July 30, 2021, 9:31 a.m. UTC
In a preceding commit we dropped support for curl < 7.19.4, so we can
drop support for this non-obvious dependency on curl < 7.18.0.

It's non-obvious because in curl's hex version notation 0x071800 is
version 7.24.0, *not* 7.18.0, so at a glance this patch looks
incorrect.

But it's correct, because the existing version check being removed
here is wrong. The check guards use of the following curl defines:

    CURLPROXY_SOCKS4                7.10
    CURLPROXY_SOCKS4A               7.18.0
    CURLPROXY_SOCKS5                7.10
    CURLPROXY_SOCKS5_HOSTNAME       7.18.0

I.e. the oldest version that has these is in fact 7.18.0, not
7.24.0. That we were checking 7.24.0 is just an mistake in
6d7afe07f29 (remote-http(s): support SOCKS proxies, 2015-10-26),
i.e. its author confusing base 10 and base 16.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 http.c | 2 --
 1 file changed, 2 deletions(-)

Comments

Junio C Hamano July 30, 2021, 4:22 p.m. UTC | #1
Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> In a preceding commit we dropped support for curl < 7.19.4, so we can
> drop support for this non-obvious dependency on curl < 7.18.0.
>
> It's non-obvious because in curl's hex version notation 0x071800 is
> version 7.24.0, *not* 7.18.0, so at a glance this patch looks
> incorrect.
>
> But it's correct, because the existing version check being removed
> here is wrong. The check guards use of the following curl defines:
>
>     CURLPROXY_SOCKS4                7.10
>     CURLPROXY_SOCKS4A               7.18.0
>     CURLPROXY_SOCKS5                7.10
>     CURLPROXY_SOCKS5_HOSTNAME       7.18.0
>
> I.e. the oldest version that has these is in fact 7.18.0, not
> 7.24.0. That we were checking 7.24.0 is just an mistake in
> 6d7afe07f29 (remote-http(s): support SOCKS proxies, 2015-10-26),
> i.e. its author confusing base 10 and base 16.

Heh, not just the author but everybody who reviewed it may have been
confused that the version number string was binary coded decimal.

Nicely found.

> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  http.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/http.c b/http.c
> index e9446850a62..477bf591141 100644
> --- a/http.c
> +++ b/http.c
> @@ -927,7 +927,6 @@ static CURL *get_curl_handle(void)
>  		 */
>  		curl_easy_setopt(result, CURLOPT_PROXY, "");
>  	} else if (curl_http_proxy) {
> -#if LIBCURL_VERSION_NUM >= 0x071800
>  		if (starts_with(curl_http_proxy, "socks5h"))
>  			curl_easy_setopt(result,
>  				CURLOPT_PROXYTYPE, CURLPROXY_SOCKS5_HOSTNAME);
> @@ -940,7 +939,6 @@ static CURL *get_curl_handle(void)
>  		else if (starts_with(curl_http_proxy, "socks"))
>  			curl_easy_setopt(result,
>  				CURLOPT_PROXYTYPE, CURLPROXY_SOCKS4);
> -#endif
>  #if LIBCURL_VERSION_NUM >= 0x073400
>  		else if (starts_with(curl_http_proxy, "https")) {
>  			curl_easy_setopt(result, CURLOPT_PROXYTYPE, CURLPROXY_HTTPS);
diff mbox series

Patch

diff --git a/http.c b/http.c
index e9446850a62..477bf591141 100644
--- a/http.c
+++ b/http.c
@@ -927,7 +927,6 @@  static CURL *get_curl_handle(void)
 		 */
 		curl_easy_setopt(result, CURLOPT_PROXY, "");
 	} else if (curl_http_proxy) {
-#if LIBCURL_VERSION_NUM >= 0x071800
 		if (starts_with(curl_http_proxy, "socks5h"))
 			curl_easy_setopt(result,
 				CURLOPT_PROXYTYPE, CURLPROXY_SOCKS5_HOSTNAME);
@@ -940,7 +939,6 @@  static CURL *get_curl_handle(void)
 		else if (starts_with(curl_http_proxy, "socks"))
 			curl_easy_setopt(result,
 				CURLOPT_PROXYTYPE, CURLPROXY_SOCKS4);
-#endif
 #if LIBCURL_VERSION_NUM >= 0x073400
 		else if (starts_with(curl_http_proxy, "https")) {
 			curl_easy_setopt(result, CURLOPT_PROXYTYPE, CURLPROXY_HTTPS);