mbox series

[v3,0/9] post-v2.33 "drop support for ancient curl" follow-up

Message ID cover-v3-0.9-00000000000-20210911T092751Z-avarab@gmail.com (mailing list archive)
Headers show
Series post-v2.33 "drop support for ancient curl" follow-up | expand

Message

Ævar Arnfjörð Bjarmason Sept. 11, 2021, 9:34 a.m. UTC
This is a follow-up to the already-integrated topic for dropping
support for older curl versions submitted before the v2.33 release[1].

See
https://lore.kernel.org/git/cover-v2-0.8-00000000000-20210910T105523Z-avarab@gmail.com/
for v2.

This has relatively minor changes against v2. This addresses Jeff
King's comments about the INSTALL phrasing, I split up the more
general improvements into another commit.

The CURL_SOCKOPT_OK is now defined in terms of LIBCURL_VERSION_NUM
like everything else.

I did not re-arrange the macros as suggested by Junio in
http://lore.kernel.org/git/xmqqr1dwtlt1.fsf@gitster.g

Ævar Arnfjörð Bjarmason (9):
  INSTALL: don't mention the "curl" executable at all
  INSTALL: reword and copy-edit the "libcurl" section
  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           |  15 +++---
 Makefile          |  11 +---
 git-curl-compat.h | 128 ++++++++++++++++++++++++++++++++++++++++++++++
 http.c            |  35 ++++++-------
 imap-send.c       |   2 +-
 5 files changed, 157 insertions(+), 34 deletions(-)
 create mode 100644 git-curl-compat.h

Range-diff against v2:
 1:  ac11cf8cfd1 !  1:  7b771aa70ef INSTALL: don't mention the "curl" executable at all
    @@ INSTALL: Issues of note:
      	- "libcurl" library is used by git-http-fetch, git-fetch, and, if
     -	  the curl version >= 7.34.0, for git-imap-send.  You might also
     -	  want the "curl" executable for debugging purposes. If you do not
    --	  use http:// or https:// repositories, and do not want to put
    --	  patches into an IMAP mailbox, you do not have to have them
    --	  (use NO_CURL).
    -+	  the curl version >= 7.34.0, for git-imap-send.
    -+
    -+	  If you do not use http:// or https:// repositories, and do
    -+	  not want to put patches into an IMAP mailbox, you do not
    -+	  have to have them (use NO_CURL).
    - 
    - 	- "expat" library; git-http-push uses it for remote lock
    - 	  management over DAV.  Similar to "curl" above, this is optional
    ++	  the curl version >= 7.34.0, for git-imap-send. If you do not
    + 	  use http:// or https:// repositories, and do not want to put
    + 	  patches into an IMAP mailbox, you do not have to have them
    + 	  (use NO_CURL).
 -:  ----------- >  2:  3b0119958a3 INSTALL: reword and copy-edit the "libcurl" section
 2:  4b653cee2d3 !  3:  dce6520a5c9 INSTALL: mention that we need libcurl 7.19.4 or newer to build
    @@ Commit message
     
      ## INSTALL ##
     @@ INSTALL: Issues of note:
    - 	- "libcurl" library is used by git-http-fetch, git-fetch, and, if
    - 	  the curl version >= 7.34.0, for git-imap-send.
    + 	  not need that functionality, use NO_CURL to build without
    + 	  it.
      
    -+	  Git version "7.19.4" of "libcurl" or later to build. This
    -+	  version requirement may be bumped in the future.
    ++	  Git requires version "7.19.4" or later of "libcurl", to
    ++	  build (without NO_CURL). This version requirement may be
    ++	  bumped in the future.
     +
    - 	  If you do not use http:// or https:// repositories, and do
    - 	  not want to put patches into an IMAP mailbox, you do not
    - 	  have to have them (use NO_CURL).
    + 	- "expat" library; git-http-push uses it for remote lock
    + 	  management over DAV.  Similar to "curl" above, this is optional
    + 	  (with NO_EXPAT).
 3:  76c2aa6e78d =  4:  98cdb7c35a9 Makefile: drop support for curl < 7.9.8 (again)
 4:  e73a9ff1780 =  5:  7919debfd89 http: drop support for curl < 7.18.0 (again)
 5:  2567b888c3d =  6:  67bc1992762 http: correct version check for CURL_HTTP_VERSION_2
 6:  397d54a1352 =  7:  db7d6029dda http: correct curl version check for CURLOPT_PINNEDPUBLICKEY
 7:  8e57a8409c5 =  8:  e2e53cbfba1 http: centralize the accounting of libcurl dependencies
 8:  465ab33ebda !  9:  4bdec34a545 http: don't hardcode the value of CURL_SOCKOPT_OK
    @@ git-curl-compat.h
      
     +/**
     + * 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
    ++#if LIBCURL_VERSION_NUM < 0x071505
     +#define CURL_SOCKOPT_OK 0
     +#endif
     +

Comments

Jeff King Sept. 11, 2021, 2:46 p.m. UTC | #1
On Sat, Sep 11, 2021 at 11:34:14AM +0200, Ævar Arnfjörð Bjarmason wrote:

> This has relatively minor changes against v2. This addresses Jeff
> King's comments about the INSTALL phrasing, I split up the more
> general improvements into another commit.
> 
> The CURL_SOCKOPT_OK is now defined in terms of LIBCURL_VERSION_NUM
> like everything else.

Thanks, this version looks good to me.

Probably not worth a re-roll, but maybe worth fixing up while applying:

>  2:  4b653cee2d3 !  3:  dce6520a5c9 INSTALL: mention that we need libcurl 7.19.4 or newer to build
>     @@ Commit message
>      
>       ## INSTALL ##
>      @@ INSTALL: Issues of note:
>     - 	- "libcurl" library is used by git-http-fetch, git-fetch, and, if
>     - 	  the curl version >= 7.34.0, for git-imap-send.
>     + 	  not need that functionality, use NO_CURL to build without
>     + 	  it.
>       
>     -+	  Git version "7.19.4" of "libcurl" or later to build. This
>     -+	  version requirement may be bumped in the future.
>     ++	  Git requires version "7.19.4" or later of "libcurl", to
>     ++	  build (without NO_CURL). This version requirement may be
>     ++	  bumped in the future.

The comma after libcurl (before "to build") is extraneous (and IMHO
makes the sentence harder to read).

-Peff
Junio C Hamano Sept. 12, 2021, 7:01 p.m. UTC | #2
Jeff King <peff@peff.net> writes:

> Probably not worth a re-roll, but maybe worth fixing up while applying:
>
>>  2:  4b653cee2d3 !  3:  dce6520a5c9 INSTALL: mention that we need libcurl 7.19.4 or newer to build
>>     @@ Commit message
>>      
>>       ## INSTALL ##
>>      @@ INSTALL: Issues of note:
>>     - 	- "libcurl" library is used by git-http-fetch, git-fetch, and, if
>>     - 	  the curl version >= 7.34.0, for git-imap-send.
>>     + 	  not need that functionality, use NO_CURL to build without
>>     + 	  it.
>>       
>>     -+	  Git version "7.19.4" of "libcurl" or later to build. This
>>     -+	  version requirement may be bumped in the future.
>>     ++	  Git requires version "7.19.4" or later of "libcurl", to
>>     ++	  build (without NO_CURL). This version requirement may be
>>     ++	  bumped in the future.
>
> The comma after libcurl (before "to build") is extraneous (and IMHO
> makes the sentence harder to read).

Yes.  Also it would make it easier to follow if the parentheses
around "without NO_CURL" are removed, I would think.