Re: [PATCH 0/4] dropping support for older curl

From: "Tom G. Christensen" <tgc@jupiterrise.com>
To: Jeff King <peff@peff.net>
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>, git@vger.kernel.org
Subject: Re: [PATCH 0/4] dropping support for older curl
Date: Fri, 11 Aug 2017 00:23:42 +0200	[thread overview]
Message-ID: <fbd7e636-0087-9c2b-746f-e2413c6d2133@jupiterrise.com> (raw)
In-Reply-To: <20170810213236.dej4ibsag2lxf5w2@sigill.intra.peff.net>

On 10/08/17 23:32, Jeff King wrote:
> On Thu, Aug 10, 2017 at 10:33:18PM +0200, Tom G. Christensen wrote:
> 
>>> You've totally ignored the argument I made back then[1], and which I
>>> reiterated in this thread. So I'll say it one more time: the more
>>> compelling reason is not the #ifdefs, but the fact that the older
>>> versions are totally untested.
>>
>> Perhaps you forgot but I stated in the original thread that I build RPMS for
>> RHEL/CentOS 3, 4, 5, 6 and 7. I still do and I run the testsuite every
>> single time.
> 
> I didn't forget. I actually double-checked the patches you sent at the
> time, but I didn't see one for the CURLPROTO issue. And indeed, it is
> still broken for me:
> 
>    $ cd /path/to/curl/repo
>    $ git checkout curl-7_15_5
>    $ ./buildconf && ./configure --prefix=/tmp/foo && make install
>    $ cd /path/to/git
>    $ git checkout v2.14.0
>    $ make CURLDIR=/tmp/foo V=1 http.o
>    gcc -o http.o -c -MF ./.depend/http.o.d -MQ http.o -MMD -MP   -g -O0 -Wall -Werror -Wdeclaration-after-statement -Wpointer-arith -Wstrict-prototypes -Wvla -Wold-style-declaration -Wold-style-definition -Wno-error -Wno-cpp -Wno-unused-value -Wno-strict-prototypes  -I. -DUSE_LIBPCRE1 -DHAVE_ALLOCA_H -I/tmp/foo/include -DUSE_CURL_FOR_IMAP_SEND -DNO_GETTEXT -DSHA1_DC -DSHA1DC_NO_STANDARD_INCLUDES -DSHA1DC_INIT_SAFE_HASH_DEFAULT=0 -DSHA1DC_CUSTOM_INCLUDE_SHA1_C="\"cache.h\"" -DSHA1DC_CUSTOM_TRAILING_INCLUDE_SHA1_C="\"sha1dc_git.c\"" -DSHA1DC_CUSTOM_TRAILING_INCLUDE_SHA1_H="\"sha1dc_git.h\"" -DSHA1DC_CUSTOM_INCLUDE_UBC_CHECK_C="\"git-compat-util.h\""  -DHAVE_PATHS_H -DHAVE_DEV_TTY -DHAVE_CLOCK_GETTIME -DHAVE_CLOCK_MONOTONIC -DHAVE_GETDELIM  -DFREAD_READS_DIRECTORIES -DNO_STRLCPY -DSHELL_PATH='
>   "/bin/sh"' -DPAGER_ENV='"LESS=FRX LV=-c"'  http.c
>    http.c: In function ‘get_curl_allowed_protocols’:
>    http.c:685:24: error: ‘CURLPROTO_HTTP’ undeclared (first use in this function); did you mean ‘CURLPROXY_HTTP’?
>       allowed_protocols |= CURLPROTO_HTTP;
>                            ^~~~~~~~~~~~~~
>                            CURLPROXY_HTTP
>    [and so on]
> 
>> I just built a pristine 2.14.0 on CentOS 5 with curl 7.15.5. No problems at
>> all neither with building nor with running the testsuite.
> 
> As you can see, this does not compile for me. What's going on?
>
The call site for get_curl_allowed_protocols() in http.c is still 
protected by an #if:
#if LIBCURL_VERSION_NUM >= 0x071304
         curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS,
                          get_curl_allowed_protocols(0));
         curl_easy_setopt(result, CURLOPT_PROTOCOLS,
                          get_curl_allowed_protocols(-1));
#else
         warning("protocol restrictions not applied to curl redirects 
because\n"
                 "your curl version is too old (>= 7.19.4)");
#endif

> I don't see how it could work, as CURLPROTO_HTTP is not defined at all
> in that version of curl. 

Indeed but the #if will handle that.

> Can you please double-check that you're
> building against the correct version of curl, and that you are building
> the HTTP parts of Git (which _are_ optional, and the test suite will
> pass without them).
> 

I use a mock buildroot and there is no other curl than the vendor 
supplied 7.15.5 installed:
# pwd
/var/lib/mock/jrpms-el5-x86_64/root
# find . -name 'curlver.h'
./usr/include/curl/curlver.h
# grep LIBCURL_VERSION_NUM ./usr/include/curl/curlver.h
    parsing and comparions by programs. The LIBCURL_VERSION_NUM define will
#define LIBCURL_VERSION_NUM 0x070f05
#

[root@c5-32bit-01 ~]# rpm -q git
git-2.14.1-1.el5.jr
[root@c5-32bit-01 ~]# ldd /usr/libexec/git-core/git-http-fetch |grep libcurl
         libcurl.so.3 => /usr/lib/libcurl.so.3 (0x001e7000)
[root@c5-32bit-01 ~]# rpm -qf /usr/lib/libcurl.so.3
curl-7.15.5-17.el5_9
[root@c5-32bit-01 ~]# git --version
git version 2.14.1
[root@c5-32bit-01 ~]# git clone 
https://github.com/tgc/tgcware-for-solaris.git
Cloning into 'tgcware-for-solaris'...
warning: protocol restrictions not applied to curl redirects because
your curl version is too old (>= 7.19.4)
remote: Counting objects: 2793, done.
remote: Total 2793 (delta 0), reused 0 (delta 0), pack-reused 2793
Receiving objects: 100% (2793/2793), 780.88 KiB | 639.00 KiB/s, done.
Resolving deltas: 100% (1233/1233), done.
[root@c5-32bit-01 ~]#

<snip>

> I also won't claim any absolutes. I think we all agree this is a
> cost/benefit tradeoff. But there are a lot of options for building on a
> very old system. For instance, building without http if you don't need
> it. Or building a more recent libcurl (and even linking statically for
> simplicity).
> 

Of course that is always an option but it does complicate things.

> I'd find arguments against the latter more compelling if recent curl
> were hard to compile on old systems. I don't know whether that's the
> case (certainly on a modern system, it's much easier to get newer
> versions of curl to compile than older ones).
> 

I have no experience with building curl on older Linux systems. I know 
that I can build it on old Solaris releases but that is not quite the 
same since there I am also building against recent versions of curls 
dependecies (openssl etc.).

>> Also FWIW Red Hat continues to support RHEL 5 with the Extended Life-cycle
>> Support program until 2020-11-30.
> 
> I saw that, too. But as I understand it, they provide no code updates:
> no bugfixes and no security updates. They just promise to answer the
> phone and help you with troubleshooting. It's possible my perception is
> wrong, though; I'm certainly not one of their customers.
> 

I am refering to the Extended Life-cycle Support product (ELS), which 
promises:
"the ELS Add-On delivers certain critical-impact security fixes and 
selected urgent priority bug fixes and troubleshooting for the last 
minor release"

The full description is here:
https://access.redhat.com/support/policy/updates/errata#Extended_Life_Cycle_Phase

-tgc