[v4,0/5] drop support for ancient curl

Message

Ævar Arnfjörð Bjarmason July 30, 2021, 5:59 p.m. UTC

Per the feature creep feedback on v3 this v4 ejects the two new
changes new in v3. The below range-diff is against v2, not v3.

I dug into the 7.16.4 v.s. 7.17.0 documentation issue and found that
it's bug in curl's docs, for which I submitted a patch.

I considered keeping
<[e-mail]>, but
sequencing it in made the range diff quite a bit larger, so per the
feature creep feedback I ejected it too. Junio: Perhaps you'd like to
cherry-pick it on top too, or it can be dug up post-release.

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.17.0 (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:  dcbb6f95652 ! 1:  6bd41764a54 http: drop support for curl < 7.11.1
    @@ Commit message
         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,
    -        2017-04-11). This field was introduced in curl 7.11.1.
    -
    -      - our use of CURLPROTO_* outside any #ifdef in aeae4db174
    -        (http: create function to get curl allowed protocols,
    -        2016-12-14). These were introduced in curl 7.19.4.
    +    Git will not build with vanilla curl older than 7.11.1 due our use of
    +    CURLOPT_POSTFIELDSIZE in 37ee680d9b
    +    (http.postbuffer: allow full range of ssize_t values,
    +    2017-04-11). This field was introduced in curl 7.11.1.
     
         We could solve these compilation problems with more #ifdefs,
         but it's not worth the trouble. Version 7.11.1 came out in
    -    March of 2004, over 13 years ago. Let's declare that too old
    +    March of 2004, over 17 years ago. Let's declare that too old
         and drop any existing ifdefs that go further back. One
         obvious benefit is that we'll have fewer conditional bits
         cluttering the code.
2:  1c9f3bc031b = 2:  fb308258e2b http: drop support for curl < 7.16.0
3:  faae88b7fec ! 3:  fba5560a3ba http: drop support for curl < 7.19.4
    @@ Commit message
         http: drop support for curl < 7.19.4
     
         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
    +    continue that and drop support for versions older than 7.19.3. This
         allows us to simplify the code by getting rid of some "#ifdef"'s.
     
         Git was broken with vanilla curl < 7.19.4 from v2.12.0 until
4:  9a30e92520c ! 4:  42d1c72ff7e http: drop support for curl < 7.19.3 and < 7.16.4 (again)
    @@ Metadata
     Author: Ævar Arnfjörð Bjarmason <[e-mail]>
     
      ## Commit message ##
    -    http: drop support for curl < 7.19.3 and < 7.16.4 (again)
    +    http: drop support for curl < 7.19.3 and < 7.17.0 (again)
     
         Remove the conditional use of CURLAUTH_DIGEST_IE and
         CURLOPT_USE_SSL. These two have been split from earlier simpler checks
         against LIBCURL_VERSION_NUM for ease of review.
     
         The CURLAUTH_DIGEST_IE flag was added in n 7.19.3[1], and
    -    CURLOPT_USE_SSL in 7.16.4[2], as noted in [2] it was then renamed from
    -    the older CURLOPT_FTP_SSL.
    +    CURLOPT_USE_SSL in 7.17.0[2][3], as noted in [2] it was then renamed
    +    from the older CURLOPT_FTP_SSL.
    +
    +    The documentation[2] currently claims that it was introduced in
    +    7.16.4, but the symbols-in-versions file correctly states
    +    7.17.0[3].
    +
    +    I've submitted an upstream
    +    patch (<[e-mail]>) to the
    +    curl-library mailing list fix the documentation.
     
         1. https://curl.se/libcurl/c/CURLOPT_HTTPAUTH.html
         2. https://curl.se/libcurl/c/CURLOPT_USE_SSL.html
    +    3. https://github.com/curl/curl/blob/master/docs/libcurl/symbols-in-versions
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <[e-mail]>
     
5:  64e510b4a6b = 5:  e34ab1d1f65 http: rename CURLOPT_FILE to CURLOPT_WRITEDATA

Comments

Junio C Hamano July 30, 2021, 7:03 p.m. UTC | #1

Ævar Arnfjörð Bjarmason  <[e-mail]> writes:

> Per the feature creep feedback on v3 this v4 ejects the two new
> changes new in v3. The below range-diff is against v2, not v3.
>
> I dug into the 7.16.4 v.s. 7.17.0 documentation issue and found that
> it's bug in curl's docs, for which I submitted a patch.
>
> I considered keeping
> <[e-mail]>, but
> sequencing it in made the range diff quite a bit larger, so per the
> feature creep feedback I ejected it too. Junio: Perhaps you'd like to
> cherry-pick it on top too, or it can be dug up post-release.

Either is fine; it is absolutely correct but "who cares" low impact
patch in the shorter term.  We'd eventually want to have it (and
something in the spirit of "have a central place we know what the
current state of our cURL dependency is", too) for the longer-term,
but they are not in urgent need anyway.

Thanks for working on it.

>     +    The documentation[2] currently claims that it was introduced in
>     +    7.16.4, but the symbols-in-versions file correctly states
>     +    7.17.0[3].
>     +
>     +    I've submitted an upstream
>     +    patch (<[e-mail]>) to the
>     +    curl-library mailing list fix the documentation.

I am not sure how to get to the patch, but I suspect you might be
misreading "up to X", which is different from "before X".  Once cURL
mailing list confirms my suspicion, we would need to come back and
update this patch again.

>          1. https://curl.se/libcurl/c/CURLOPT_HTTPAUTH.html
>          2. https://curl.se/libcurl/c/CURLOPT_USE_SSL.html
>     +    3. https://github.com/curl/curl/blob/master/docs/libcurl/symbols-in-versions
>      
>          Signed-off-by: Ævar Arnfjörð Bjarmason <[e-mail]>
>      
> 5:  64e510b4a6b = 5:  e34ab1d1f65 http: rename CURLOPT_FILE to CURLOPT_WRITEDATA

Junio C Hamano July 30, 2021, 7:50 p.m. UTC | #2

Junio C Hamano <[e-mail]> writes:

>>     +    The documentation[2] currently claims that it was introduced in
>>     +    7.16.4, but the symbols-in-versions file correctly states
>>     +    7.17.0[3].
>>     +
>>     +    I've submitted an upstream
>>     +    patch (<[e-mail]>) to the
>>     +    curl-library mailing list fix the documentation.
>
> I am not sure how to get to the patch, but I suspect you might be
> misreading "up to X", which is different from "before X".  Once cURL
> mailing list confirms my suspicion, we would need to come back and
> update this patch again.

Ah, I found it at https://curl.se/mail/lib-2021-07/0058.html

Nobody seems to have responded yet, but I do think you are
misreading what "X was known under a different name Y up to 7.16.4"
means.  These places do not say "before 7.16.4", which would have
implied that as of 7.16.4 you would be able to use X (not Y).

But because "up to" is "less than or equal to but not more than"
(e.g https://dictionary.cambridge.org/us/dictionary/english/up-to),
what applies to 7.16.3 also applies to 7.16.4, but not to 7.17.0.
IOW, the feature X was known as Y when 7.16.4 was current, so our
use of X would not have worked with that exact version.  We would
have needed to wait until the next version (7.17.0).

Junio C Hamano July 30, 2021, 10:49 p.m. UTC | #3

Junio C Hamano <[e-mail]> writes:

> Junio C Hamano <[e-mail]> writes:
>
>>>     +    The documentation[2] currently claims that it was introduced in
>>>     +    7.16.4, but the symbols-in-versions file correctly states
>>>     +    7.17.0[3].
>>>     +
>>>     +    I've submitted an upstream
>>>     +    patch (<[e-mail]>) to the
>>>     +    curl-library mailing list fix the documentation.
>>
>> I am not sure how to get to the patch, but I suspect you might be
>> misreading "up to X", which is different from "before X".  Once cURL
>> mailing list confirms my suspicion, we would need to come back and
>> update this patch again.
>
> Ah, I found it at https://curl.se/mail/lib-2021-07/0058.html
>
> Nobody seems to have responded yet, but I do think you are
> misreading what "X was known under a different name Y up to 7.16.4"
> means.  These places do not say "before 7.16.4", which would have
> implied that as of 7.16.4 you would be able to use X (not Y).
>
> But because "up to" is "less than or equal to but not more than"
> (e.g https://dictionary.cambridge.org/us/dictionary/english/up-to),
> what applies to 7.16.3 also applies to 7.16.4, but not to 7.17.0.
> IOW, the feature X was known as Y when 7.16.4 was current, so our
> use of X would not have worked with that exact version.  We would
> have needed to wait until the next version (7.17.0).

It seems that Daniel has exactly the same reaction as I did.

    https://curl.se/mail/lib-2021-07/0059.html

So, let's fix the log message for [4/5] on our end again.

-- >8 --
http: drop support for curl < 7.19.3 and < 7.17.0 (again)

Remove the conditional use of CURLAUTH_DIGEST_IE and
CURLOPT_USE_SSL. These two have been split from earlier simpler
checks against LIBCURL_VERSION_NUM for ease of review.

According to

  https://github.com/curl/curl/blob/master/docs/libcurl/symbols-in-versions

the CURLAUTH_DIGEST_IE flag became available in 7.19.3, and
CURLOPT_USE_SSL in 7.17.0.

Signed-off-by: Ævar Arnfjörð Bjarmason <[e-mail]>
Signed-off-by: Junio C Hamano <[e-mail]>

diff --git a/http.c b/http.c
index 1f0d7664d3..e9446850a6 100644
--- a/http.c
+++ b/http.c
@@ -120,9 +120,7 @@ static int http_auth_methods_restricted;
 /* Modes for which empty_auth cannot actually help us. */
 static unsigned long empty_auth_useless =
 	CURLAUTH_BASIC
-#ifdef CURLAUTH_DIGEST_IE
 	| CURLAUTH_DIGEST_IE
-#endif
 	| CURLAUTH_DIGEST;
 
 static struct curl_slist *pragma_header;
@@ -893,10 +891,8 @@ static CURL *get_curl_handle(void)
 	if (curl_ftp_no_epsv)
 		curl_easy_setopt(result, CURLOPT_FTP_USE_EPSV, 0);
 
-#ifdef CURLOPT_USE_SSL
 	if (curl_ssl_try)
 		curl_easy_setopt(result, CURLOPT_USE_SSL, CURLUSESSL_TRY);
-#endif
 
 	/*
 	 * CURL also examines these variables as a fallback; but we need to query
diff --git a/http.h b/http.h
index 19f19dbe74..3db5a0cf32 100644
--- a/http.h
+++ b/http.h
@@ -12,15 +12,6 @@
 
 #define DEFAULT_MAX_REQUESTS 5
 
-/*
- * CURLOPT_USE_SSL was known as CURLOPT_FTP_SSL up to 7.16.4,
- * and the constants were known as CURLFTPSSL_*
-*/
-#if !defined(CURLOPT_USE_SSL) && defined(CURLOPT_FTP_SSL)
-#define CURLOPT_USE_SSL CURLOPT_FTP_SSL
-#define CURLUSESSL_TRY CURLFTPSSL_TRY
-#endif
-
 struct slot_results {
 	CURLcode curl_result;
 	long http_code;