Re: [RFC] dropping support for ancient versions of curl

[Junio C Hamano]

Jeff King  writes:

> I think I'm leaning towards the very first patch I posted, that assumes
> 7.11.0 and later. And then hold off on the others for a few years. In
> terms of "number of ifdefs removed" we could go further, but I think it
> was the first patch that removes a lot of the really questionable bits
> (like silently ignoring security-related features).

I do not have problems with setting cut-off at 7.11.0; I do not
foresee anybody who actively maintains a port for a platform whose
cURL is older than that version would step up ;-)

Thanks.

Re: [RFC] dropping support for ancient versions of curl

[Jeff King]

On Fri, Apr 07, 2017 at 01:18:30PM +0200, Johannes Schindelin wrote:

> On Thu, 6 Apr 2017, Jeff King wrote:
> 
> > And it's not like people on ancient mission-critical systems get cut
> > off. They can still run the version of Git they were running when their
> > OS went out of support.
> 
> You keep baiting me, so I'll bite, after resisting the urge for so long.

I wasn't going to respond to this, because I didn't feel like the
discussion was going anywhere. But I ran across yet another issue
related to this today that hadn't been mentioned yet.

Your story shows that yes, it's convenient when old libraries are
supported. I don't dispute that. But one of my earlier points is that
this isn't just about maintenance burden (which I agree is not huge);
it's about whether we do a disservice to users to pretend that Git is
even remotely tested with older versions of curl.

For instance, did you know that versions of curl prior to v7.17 rely on
any strings fed via curl_easy_setopt() remaining valid for the lifetime
of the curl handle[1]?

We have some workarounds for this in old code (for example, see the
handling of CURLOPT_PASSWORD in http.c), but a lot of calls have been
added since then. I think there's a very good chance there are
use-after-free bugs when Git is compiled against an older curl.

I'm concerned that we're giving users a false sense of what is
reasonable to compile against.  You can reframe that as a maintenance
question (we _could_ find and fix those bugs), but that changes the
cost/benefit analysis.

[1] 
http://public-inbox.org/git/alpine.deb.2.00.1306180825460.24...@tvnag.unkk.fr/

-Peff

Re: [RFC] dropping support for ancient versions of curl

[Johannes Schindelin]

Hi Peff,

On Thu, 6 Apr 2017, Jeff King wrote:

> And it's not like people on ancient mission-critical systems get cut
> off. They can still run the version of Git they were running when their
> OS went out of support.

You keep baiting me, so I'll bite, after resisting the urge for so long.

Let me share a little story of my own. So that this is not some academic,
theoretical wanking off, but something very tangible, very concrete, and
something very hard to handwave away.

As you know, in my previous job I was working as a scientist. Part of my
job was to support other scientists e.g. when they were using that really
big, really expensive, really advanced and super cool microscope. The
"really advanced" part only applied to the hardware, though, they were
running a super old CentOS, and everything was clamped down pretty good
(as many microscope vendors are wont to do), so there was no way to
upgrade the OS (I do not recall whether the drivers were only available
for this particular version of CentOS, but it would make sense, wouldn't
it, now, as microscope vendors are much more of experts in microscope
hardware than in anything vaguely related to software, even if they like
to pretend to be experts in both).

I did have access to gcc, though. And I did have to get Git working to be
able to install and develop additional software on that machine. It did
take me a while to get things to compile because of ancient libraries
being installed on that machine. Since only trusted people were allowed to
use that machine, security on that box was not really an issue. It did
make work for my colleagues substantially easier once I could develop on
that machine, using Git.

Just imagine how thankful I was that support for these old library
versions was almost working, still, and the one bigger problem I had was
easily solved by a patch that a quick web search found!

This. This is the impact of supporting older library versions, even if
they are long EOL.

Do not get me wrong: I am all for dropping support that is a huge
maintenance burden. You probably do not know the full back story, but let
me tell you what a relief it was to drop Windows XP support in Git for
Windows (and the Cygwin developers can sing several epic, Lord of the
Rings sized war songs about that, too).

At the same time, I want us to do better than all those maintainers out
there who drop backwards-compatibility just for the heck of it, not
because it would be a huge maintenance burden. "Everybody should upgrade,
anyway" is usually the naive excuse for not knowing why users are often
unable to upgrade.

Git is incredibly popular, and part of that is due to us being inclusive
and open and supporting a wide range of platforms. Yes, we can always do a
better job, that is true. We are also doing pretty well, though, and I
think that part of the reason is that we weigh carefully the maintenance
cost against the benefit of our users. Tiny "cleanup" patches that fail to
improve the maintenance burden substantially, and that also make users'
lives harder, are rightfully rejected. A little harder work from us
maintainers goes a long way to make a lot of users a lot happier.

My former self in my former life as a scientist is very thankful for the
consideration that goes into each and every "should we drop supporting
XYZ?" decision.

Very, very thankful.

Ciao,
Dscho

Re: [RFC] dropping support for ancient versions of curl

[Jeff King]

On Thu, Apr 06, 2017 at 06:43:06PM +0200, Tom G. Christensen wrote:

> On 06/04/17 11:21, Jeff King wrote:
> > On Wed, Apr 05, 2017 at 11:33:37AM +0200, Tom G. Christensen wrote:
> > > I don't use the el3 and el4 versions much any more and el5 use will also
> > > drop of now as I'm busy converting machines from el5 to el7.
> > 
> > Thanks for sharing, that's a really interesting data point.
> > 
> > I'm not quite sure what to take away from it, though. Either "yes,
> > somebody really is using Git with antique versions of curl". Or "even
> > the antique people are giving up el4, and it might be reasonable to
> > start requiring curl >= 7.11.0".

[Aside: re-reading my second paragraph above, it sounds like I am
 complaining that your comment wasn't clear. But you were perfectly
 clear. It's just that with your data point I am more conflicted than
 ever].

> I know of no users of the packages that I make available other than myself
> and my work place (only el5 and later, soon just el6 and later).
> 
> There is not currently any patches needed to use git on el5 with curl
> 7.15.5. The only thing not working out of the box in 2.12.2 would be the
> emacs integration.

I think I'm leaning towards the very first patch I posted, that assumes
7.11.0 and later. And then hold off on the others for a few years. In
terms of "number of ifdefs removed" we could go further, but I think it
was the first patch that removes a lot of the really questionable bits
(like silently ignoring security-related features).

> If you must drop support for old curl releases then from my perspective the
> cutoff should be 7.19.7 at the latest since that is what ships with RHEL 6
> and that is still supported by Red Hat.

Yeah, I think 7.19.7 would be the next reasonable cutoff (it shipped in
RHEL6, which is a reasonable standard for long-term support; and it
catches quite a few ifdefs in our code). But I think we can give RHEL5 a
bit more time. It just left its support window a few days ago.

-Peff

Re: [RFC] dropping support for ancient versions of curl

[Tom G. Christensen]

On 06/04/17 11:21, Jeff King wrote:

On Wed, Apr 05, 2017 at 11:33:37AM +0200, Tom G. Christensen wrote:

I don't use the el3 and el4 versions much any more and el5 use will also
drop of now as I'm busy converting machines from el5 to el7.


Thanks for sharing, that's a really interesting data point.

I'm not quite sure what to take away from it, though. Either "yes,
somebody really is using Git with antique versions of curl". Or "even
the antique people are giving up el4, and it might be reasonable to
start requiring curl >= 7.11.0".



I do not know of anyone who actually need to have the latest git on 
their el3 or el4 system, myself included.
I'm not here to champion the inclusion of support for el3 and el4, there 
is no point.
I kept git buildable on those platforms since I could do so with minimal 
effort and good testresults, but this was entirely for my own satisfaction.


I know of no users of the packages that I make available other than 
myself and my work place (only el5 and later, soon just el6 and later).


There is not currently any patches needed to use git on el5 with curl 
7.15.5. The only thing not working out of the box in 2.12.2 would be the 
emacs integration.


If you must drop support for old curl releases then from my perspective 
the cutoff should be 7.19.7 at the latest since that is what ships with 
RHEL 6 and that is still supported by Red Hat.
Other long term supported Linux releases may have different ideas, I 
wouldn't know.


-tgc

Re: [RFC] dropping support for ancient versions of curl

[Jeff King]

On Wed, Apr 05, 2017 at 11:33:37AM +0200, Tom G. Christensen wrote:

> FWIW I maintain freely available updated git packages for RHEL 3, 4, 5, 6
> and 7.
> 
> They can be found here:
> https://jupiterrise.com/blog/jrpms/
> 
> And direct access here:
> https://jupiterrise.com/jrpms/ (for el3,el4,el5)
> https://jupiterrise.com/jrpmsplus/ (for el6, el7)
> 
> They are built against system versions of curl though a few patches are
> required for 7.10.6 (el3) and 7.12.1 (el4) support.
> Patches can be found in the src.rpm, though I can also post them here as
> patch series, they cover more than just curl.
> 
> I don't use the el3 and el4 versions much any more and el5 use will also
> drop of now as I'm busy converting machines from el5 to el7.

Thanks for sharing, that's a really interesting data point.

I'm not quite sure what to take away from it, though. Either "yes,
somebody really is using Git with antique versions of curl". Or "even
the antique people are giving up el4, and it might be reasonable to
start requiring curl >= 7.11.0".

-Peff

Re: [RFC] dropping support for ancient versions of curl

[Jeff King]

On Thu, Apr 06, 2017 at 12:53:01AM +, brian m. carlson wrote:

> > It would be great to have them on-list, as far as I can tell they were
> > never submitted? Is there some time/administrative reason for why
> > you're not submitting them? Some of these are many years old, it would
> > be great to have them on-list for wider review & included so vanilla
> > git works on these platforms.
> 
> I'm very opposed to accepting patches for operating systems that are no
> longer security supported.  Having insecure systems directly or
> indirectly connected to the Internet is a very bad thing, and we
> shouldn't make it easier for people who want to do that.

Hmm. I'm not sure whether I agree with that or not. I certainly wouldn't
want to _encourage_ people to use ancient unpatched systems. But I'm
also not entirely comfortable passing judgements on people's OS choices.
Security isn't a discrete variable, and there are lots of situations
where it makes sense to stick with an old, unpatched system because the
risk of changing it outweighs the risk of it being attacked (think
mission-critical systems sitting behind firewalls).

That said, I don't mind the argument "even the people who made this OS
are no longer supporting it; why should we?". And the response from Todd
seems to reinforce that.

And it's not like people on ancient mission-critical systems get cut
off. They can still run the version of Git they were running when their
OS went out of support.

-Peff

Re: [RFC] dropping support for ancient versions of curl

[Todd Zullinger]

brian m. carlson wrote:

On Wed, Apr 05, 2017 at 12:51:38PM +0200, Ævar Arnfjörð Bjarmason wrote:

On Wed, Apr 5, 2017 at 11:33 AM, Tom G. Christensen  
wrote:
Whoah. So my assumption in 

that nobody was compiling this & thus not reporting failures was 
false. Rather there's an entire community & distribution mechanism 
around patching git for older EL versions, but the patches aren't 
making it upstream.


$ grep -h -e ^Subject -e ^Date *patch 
Date: Tue, 13 Oct 2009 11:34:11 +0200 
Subject: [PATCH 1/7] Make NO_PERL_MAKEMAKER behave more like 
Date: Fri, 13 Jun 2014 11:02:02 +0200 
Subject: [PATCH 2/7] Install man pages when NO_PERL_MAKEMAKER is used 
Date: Mon, 22 Sep 2014 13:42:50 +0200 
Subject: [PATCH 3/7] Allow svnrdump_sim.py to be used with Python 2.2 
Date: Tue, 8 Mar 2016 09:31:56 +0100 
Subject: [PATCH 4/7] Handle missing HTTP_CONNECTCODE in curl < 7.10.7 
Date: Tue, 23 Aug 2016 10:32:51 +0200 
Subject: [PATCH 5/7] Add support for gnupg < 1.4 
Date: Tue, 23 Aug 2016 18:15:13 +0200 
Subject: [PATCH 6/7] Handle missing CURLINFO_SSL_DATA_{IN,OUT} 
Date: Tue, 23 Aug 2016 18:26:54 +0200 
Subject: [PATCH 7/7] Do not use curl_easy_strerror with curl < 7.12.0 
Date: Wed, 2 Feb 2011 21:24:44 -0500 
Subject: [PATCH] Restore vc-git.el for basic compatibility on EL-5 
Date: Mon, 23 Mar 2009 00:03:36 -0400 
Subject: [PATCH] git-cvsimport: Ignore cvsps-2.2b1 Branches: output


Patches can be found in the src.rpm, though I can also post them here as 
patch series, they cover more than just curl.


I don't use the el3 and el4 versions much any more and el5 use will also 
drop of now as I'm busy converting machines from el5 to el7.


It would be great to have them on-list, as far as I can tell they were 
never submitted? Is there some time/administrative reason for why 
you're not submitting them? Some of these are many years old, it would 
be great to have them on-list for wider review & included so vanilla 
git works on these platforms.


The vc-git.el patch which I think came from the Fedora/EPEL package 
for EL-5 was only intended to restore functionality to support the 
older emacs on EL-5.  Now that EL-5 is EOL, it can (thankfully, IMO) 
die off.


The git-csvimport patch looks to be in the same category (assuming it 
was similar to what we included in the EPEL packages for EL-5 
support).  It was only needed (and applied) to support older systems 
and wasn't something worth carrying upstream.


I'm very opposed to accepting patches for operating systems that are no 
longer security supported.  Having insecure systems directly or 
indirectly connected to the Internet is a very bad thing, and we 
shouldn't make it easier for people who want to do that.


I concur.  I (or we, the Fedora/EPEL package maintainers for git) have 
tried to ensure that the packages for the latest Fedora release build 
cleanly for all supported Fedora and EL releases.  I think that is 
useful for those who use CentOS/RHEL and still want to have a recent 
git build.


I don't see any benefit in making it easier for folks to run systems 
that are no longer receiving updates for critical security issues.
It's surely hard enough to support EL-6 as it ages. ;)


CentOS 6 and 7 are still in use and security supported, and I think 
we should support them.  CentOS 5 is EOL, although RHEL 5 is still 
supported in some cases.  RHEL 4 and earlier are definitely out of 
support and we shouldn't support or consider them further.


I know RHEL 5 has some extended support from Red Hat, but EPEL-5 is 
dead.  So from my perspective, supporting EL-5 isn't worth the effort.


--
Todd
~~
Statistics are like a lamp-post to a drunken man - more for leaning on
than illumination.



signature.asc
Description: PGP signature

Re: [RFC] dropping support for ancient versions of curl

[brian m. carlson]

On Wed, Apr 05, 2017 at 12:51:38PM +0200, Ævar Arnfjörð Bjarmason wrote:
> On Wed, Apr 5, 2017 at 11:33 AM, Tom G. Christensen  
> wrote:
> Whoah. So my assumption in
> 
> that nobody was compiling this & thus not reporting failures was
> false. Rather there's an entire community & distribution mechanism
> around patching git for older EL versions, but the patches aren't
> making it upstream.
> 
> $ grep -h -e ^Subject -e ^Date *patch
> Date: Tue, 13 Oct 2009 11:34:11 +0200
> Subject: [PATCH 1/7] Make NO_PERL_MAKEMAKER behave more like
> Date: Fri, 13 Jun 2014 11:02:02 +0200
> Subject: [PATCH 2/7] Install man pages when NO_PERL_MAKEMAKER is used
> Date: Mon, 22 Sep 2014 13:42:50 +0200
> Subject: [PATCH 3/7] Allow svnrdump_sim.py to be used with Python 2.2
> Date: Tue, 8 Mar 2016 09:31:56 +0100
> Subject: [PATCH 4/7] Handle missing HTTP_CONNECTCODE in curl < 7.10.7
> Date: Tue, 23 Aug 2016 10:32:51 +0200
> Subject: [PATCH 5/7] Add support for gnupg < 1.4
> Date: Tue, 23 Aug 2016 18:15:13 +0200
> Subject: [PATCH 6/7] Handle missing CURLINFO_SSL_DATA_{IN,OUT}
> Date: Tue, 23 Aug 2016 18:26:54 +0200
> Subject: [PATCH 7/7] Do not use curl_easy_strerror with curl < 7.12.0
> Date: Wed, 2 Feb 2011 21:24:44 -0500
> Subject: [PATCH] Restore vc-git.el for basic compatibility on EL-5
> Date: Mon, 23 Mar 2009 00:03:36 -0400
> Subject: [PATCH] git-cvsimport: Ignore cvsps-2.2b1 Branches: output
> 
> > Patches can be found in the src.rpm, though I can also post them here as
> > patch series, they cover more than just curl.
> >
> > I don't use the el3 and el4 versions much any more and el5 use will also
> > drop of now as I'm busy converting machines from el5 to el7.
> 
> It would be great to have them on-list, as far as I can tell they were
> never submitted? Is there some time/administrative reason for why
> you're not submitting them? Some of these are many years old, it would
> be great to have them on-list for wider review & included so vanilla
> git works on these platforms.

I'm very opposed to accepting patches for operating systems that are no
longer security supported.  Having insecure systems directly or
indirectly connected to the Internet is a very bad thing, and we
shouldn't make it easier for people who want to do that.

CentOS 6 and 7 are still in use and security supported, and I think we
should support them.  CentOS 5 is EOL, although RHEL 5 is still
supported in some cases.  RHEL 4 and earlier are definitely out of
support and we shouldn't support or consider them further.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

Re: [RFC] dropping support for ancient versions of curl

[Tom G. Christensen]

On 05/04/17 12:51, Ævar Arnfjörð Bjarmason wrote:

On Wed, Apr 5, 2017 at 11:33 AM, Tom G. Christensen  
wrote:
Whoah. So my assumption in

that nobody was compiling this & thus not reporting failures was
false. Rather there's an entire community & distribution mechanism
around patching git for older EL versions, but the patches aren't
making it upstream.



The community as I know it consists of me and EPEL5 (now dead and archived).
The packages that I build are probably only used by me and the company I 
work for as they are not exactly easy to find via search engines.
EPEL5 supported git 1.8.3.2 but the Fedora git specfile still contains 
all the infrastructure though I cannot know if it actually got used with 
anything later than 1.8.3.2.


I don't know of anyone that actually *needs* to use the latest git on 
RHEL < 5, myself included. I kept the support for RHEL < 5 because I 
could and it was good fun to tinker with.


Also I should say that testresults are good, no problems there except a 
few small nits as revealed in the specfile:

%if %{?el4:1}0
# These tests fail with subversion 1.1.4
export GIT_SKIP_TESTS="t9140.4"
%ifarch x86_64
# These tests fail with subversion 1.1.4 but only on x86_64
export GIT_SKIP_TESTS="$GIT_SKIP_TESTS t9106.7 t9106.8 t9106.9 t9106.10 
t9137.4 t9164.5 t9164.6 t9164.7 t9164.8"

%endif
%endif
%if %{?el3:1}%{?el4:1}0
# not ok 6 - url high-bit escapes
export GIT_SKIP_TESTS="$GIT_SKIP_TESTS t0110.6"
# not ok 32 - ref name 'heads/foo' is invalid
export GIT_SKIP_TESTS="$GIT_SKIP_TESTS t1402.32"
%endif
%if %{?el3:1}0
# t7800 failed 17 among 56 test(s)
export GIT_SKIP_TESTS="$GIT_SKIP_TESTS t7800"
%endif

It's been a little while since I did a build without those exclusions 
but I doubt much has changed.



$ grep -h -e ^Subject -e ^Date *patch
Date: Tue, 13 Oct 2009 11:34:11 +0200
Subject: [PATCH 1/7] Make NO_PERL_MAKEMAKER behave more like
Date: Fri, 13 Jun 2014 11:02:02 +0200
Subject: [PATCH 2/7] Install man pages when NO_PERL_MAKEMAKER is used
Date: Mon, 22 Sep 2014 13:42:50 +0200
Subject: [PATCH 3/7] Allow svnrdump_sim.py to be used with Python 2.2
Date: Tue, 8 Mar 2016 09:31:56 +0100
Subject: [PATCH 4/7] Handle missing HTTP_CONNECTCODE in curl < 7.10.7
Date: Tue, 23 Aug 2016 10:32:51 +0200
Subject: [PATCH 5/7] Add support for gnupg < 1.4
Date: Tue, 23 Aug 2016 18:15:13 +0200
Subject: [PATCH 6/7] Handle missing CURLINFO_SSL_DATA_{IN,OUT}
Date: Tue, 23 Aug 2016 18:26:54 +0200
Subject: [PATCH 7/7] Do not use curl_easy_strerror with curl < 7.12.0


All original work done by me.


Date: Wed, 2 Feb 2011 21:24:44 -0500
Subject: [PATCH] Restore vc-git.el for basic compatibility on EL-5
Date: Mon, 23 Mar 2009 00:03:36 -0400
Subject: [PATCH] git-cvsimport: Ignore cvsps-2.2b1 Branches: output



These two I can't claim credit for. They are lifted verbatim from 
Fedora/EPEL and as the headers reveal they were created by Todd Zullinger.
I won't submit them for inclusion since I am not familiar with nor a 
user of the parts they touch.



Patches can be found in the src.rpm, though I can also post them here as
patch series, they cover more than just curl.

I don't use the el3 and el4 versions much any more and el5 use will also
drop of now as I'm busy converting machines from el5 to el7.


It would be great to have them on-list, as far as I can tell they were
never submitted? Is there some time/administrative reason for why
you're not submitting them?


Well I recently took the time to clean them up with the intention of 
maybe finally submitting them but I never got that far.


The first one in the series I actually submitted many years ago but it 
was ultimately rejected.


I've submitted a few patches over the years to support older RHEL 
releases and some of them ended up being included.



Some of these are many years old, it would
be great to have them on-list for wider review & included so vanilla
git works on these platforms.



I just posted them now. The series was made against the v2.12.2 tag.

-tgc

Re: [RFC] dropping support for ancient versions of curl

[Ævar Arnfjörð Bjarmason]

On Wed, Apr 5, 2017 at 11:33 AM, Tom G. Christensen  
wrote:
> On 04/04/17 04:54, Jeff King wrote:
>>
>> A nearby thread raised the question of whether we can rely on a version
>> of libcurl that contains a particular feature. The version in question
>> is curl 7.11.1, which came out in March 2004.
>>
>> My feeling is that this is old enough to stop caring about. Which means
>> we can drop some of the #ifdefs that clutter the HTTP code (and there's
>> a patch at the end of this mail dropping support for everything older
>> than 7.11.1). But that made wonder: how old is too old? I think it's
>> nice that we don't force people to upgrade to the latest version of
>> curl. But at some point, if you are running a 13-year-old version of
>> libcurl, how likely are you to run a brand new version of Git? :)
>>
>
> FWIW I maintain freely available updated git packages for RHEL 3, 4, 5, 6
> and 7.
>
> They can be found here:
> https://jupiterrise.com/blog/jrpms/
>
> And direct access here:
> https://jupiterrise.com/jrpms/ (for el3,el4,el5)
> https://jupiterrise.com/jrpmsplus/ (for el6, el7)
>
> They are built against system versions of curl though a few patches are
> required for 7.10.6 (el3) and 7.12.1 (el4) support.

Whoah. So my assumption in

that nobody was compiling this & thus not reporting failures was
false. Rather there's an entire community & distribution mechanism
around patching git for older EL versions, but the patches aren't
making it upstream.

$ grep -h -e ^Subject -e ^Date *patch
Date: Tue, 13 Oct 2009 11:34:11 +0200
Subject: [PATCH 1/7] Make NO_PERL_MAKEMAKER behave more like
Date: Fri, 13 Jun 2014 11:02:02 +0200
Subject: [PATCH 2/7] Install man pages when NO_PERL_MAKEMAKER is used
Date: Mon, 22 Sep 2014 13:42:50 +0200
Subject: [PATCH 3/7] Allow svnrdump_sim.py to be used with Python 2.2
Date: Tue, 8 Mar 2016 09:31:56 +0100
Subject: [PATCH 4/7] Handle missing HTTP_CONNECTCODE in curl < 7.10.7
Date: Tue, 23 Aug 2016 10:32:51 +0200
Subject: [PATCH 5/7] Add support for gnupg < 1.4
Date: Tue, 23 Aug 2016 18:15:13 +0200
Subject: [PATCH 6/7] Handle missing CURLINFO_SSL_DATA_{IN,OUT}
Date: Tue, 23 Aug 2016 18:26:54 +0200
Subject: [PATCH 7/7] Do not use curl_easy_strerror with curl < 7.12.0
Date: Wed, 2 Feb 2011 21:24:44 -0500
Subject: [PATCH] Restore vc-git.el for basic compatibility on EL-5
Date: Mon, 23 Mar 2009 00:03:36 -0400
Subject: [PATCH] git-cvsimport: Ignore cvsps-2.2b1 Branches: output

> Patches can be found in the src.rpm, though I can also post them here as
> patch series, they cover more than just curl.
>
> I don't use the el3 and el4 versions much any more and el5 use will also
> drop of now as I'm busy converting machines from el5 to el7.

It would be great to have them on-list, as far as I can tell they were
never submitted? Is there some time/administrative reason for why
you're not submitting them? Some of these are many years old, it would
be great to have them on-list for wider review & included so vanilla
git works on these platforms.

Re: [RFC] dropping support for ancient versions of curl

[Tom G. Christensen]

On 04/04/17 04:54, Jeff King wrote:

A nearby thread raised the question of whether we can rely on a version
of libcurl that contains a particular feature. The version in question
is curl 7.11.1, which came out in March 2004.

My feeling is that this is old enough to stop caring about. Which means
we can drop some of the #ifdefs that clutter the HTTP code (and there's
a patch at the end of this mail dropping support for everything older
than 7.11.1). But that made wonder: how old is too old? I think it's
nice that we don't force people to upgrade to the latest version of
curl. But at some point, if you are running a 13-year-old version of
libcurl, how likely are you to run a brand new version of Git? :)



FWIW I maintain freely available updated git packages for RHEL 3, 4, 5, 
6 and 7.


They can be found here:
https://jupiterrise.com/blog/jrpms/

And direct access here:
https://jupiterrise.com/jrpms/ (for el3,el4,el5)
https://jupiterrise.com/jrpmsplus/ (for el6, el7)

They are built against system versions of curl though a few patches are 
required for 7.10.6 (el3) and 7.12.1 (el4) support.
Patches can be found in the src.rpm, though I can also post them here as 
patch series, they cover more than just curl.


I don't use the el3 and el4 versions much any more and el5 use will also 
drop of now as I'm busy converting machines from el5 to el7.


-tgc

Re: [RFC] dropping support for ancient versions of curl

[Jeff King]

On Wed, Apr 05, 2017 at 10:49:47AM +0200, Johannes Schindelin wrote:

> Let's reiterate that we are talking about some #ifdef's here that are a
> tiny maintenance burden. That may have a bug here and there, easily fixed.

Forget the maintenance cost for a moment. My concern is that we are
doing users a disservice by shipping broken and untested code without
them (or us) realizing it. The compile failure is the _best_ case,
because they know there's a bug to be fixed. A build that quietly fails
to enforce security properties is actively dangerous, and the user would
potentially be better off with an #error.

> Also, maybe, just maybe, there are more pressing issues than removing a
> couple lines here and there? This discussion vaguely reminds me of the
> opening statement of https://en.wikipedia.org/wiki/Law_of_triviality...
> Just saying'...

It's not just removing a couple of lines. It's remembering to check and
#ifdef new lines that get added, too (this conversation started because
of review on another patch which failed to do so). Is our attitude "add
it and when somebody with an ancient curl complains and provides a
patch, we'll #ifdef it"?

-Peff

Re: [RFC] dropping support for ancient versions of curl

[Johannes Schindelin]

Hi Stefan,

On Tue, 4 Apr 2017, Stefan Beller wrote:

> On Tue, Apr 4, 2017 at 3:46 PM, Johannes Schindelin
>  wrote:
> >
> > On Tue, 4 Apr 2017, Brandon Williams wrote:
> >
> >> I'm all for seeing a patch like this applied.  I agree that we can't
> >> expect the world to be running the most up-to-date version of curl
> >> but we should be able to select some "oldest" version we will support
> >> which can be bumped up every couple of years.
> >>
> >> I mean, ensuring that you are running with an up-to-date version of
> >> curl is really important when it comes to all of the security fixes
> >> that have been made in each revision.
> >
> > I am not in the business of dictating to others what software they
> > have to run. I am in the business of maintaining Git for Windows. And
> > part of that job is to drag along code that is maybe not the most
> > elegant, but works.
> >
> > The patch in question resolves such a wart. Sure, it would be a
> > cleanup.  Is it a huge maintenance burden to keep those few #ifdef's,
> > though?  Absolutely not.
> 
> Keeping them around is the easy part, the hard part is promising to
> users that the software you maintain is as good as your reputation, when
> e.g.  we find out that certain #ifdefs don't even compile.  (See Frank
> Gevaerts answer)

>From that point of view, we should drop support for platforms as soon as
we have a bug in our code that lets the build fail: there is little
difference between the #ifdef's that let Git build with older dependencies
and #ifdef's that let Git build on platforms other than Linux.

By that reasoning, we would have dropped FreeBSD support a gazillion
times. Or for that matter, Windows support.

I obviously disagree with this stance.

> Initially I thought I had a similar stance as you ("A well written line
> of code is cheap") but I kept quiet, as I do not have a lot of
> experience with dealing "old" Software.

I am speaking from my point of view, of course, maintaining Git for
Windows. Of course I would love to drop support for old MSys. But that's
not going to happen because at least one active contributor has a vested
interest in keeping it going.

But even then, it sometimes takes a while until any breakages get fixed.
Prematurely removing a build option "because it has been broken for a
couple of major versions" would make it infinitely harder for (often
overworked) contributors to fix the breakage.

Anyway, this whole discussion took way more effort from my side than to
maintain a couple of slightly stale #ifdef's. From
https://xkcd.com/1205/'s point of view, I should have ignored the
"let's rip out stuff just because we can" patch.

I just thought that we care a bit more about contributors' experience.

> Maybe the git community would want to take a look at the kernel
> community (or other similar communities), how they solve the "long term
> stable" problem of computer science.

If we want to follow the example of a inclusive community that values
contributors' time and effort, maybe we should look elsewhere?

I vividly remember hearing Greg KH's statement at Git Merge 2016 "we
deliberately waste contributors' time". Very vividly. Very, very vividly.

> So I would propose to take this patch, but be prepared to revert it in case
> some user yells.

It would be much nicer to contributors (who are likely not subscribed to
the Git mailing list, or at least do not read all the mails coming in on
that channel) if they could simply imitate the surrounding #ifdef's and
make that tiny patch that adds a guard around an unused static function.

If you require them to revert a patch first that reinstates an almost
working state with an old dependency, you expect them to know that there
was such a patch in the first place, and to dig it up.

Let's reiterate that we are talking about some #ifdef's here that are a
tiny maintenance burden. That may have a bug here and there, easily fixed.

Again, I am sure that we can provide a much better contributors'
experience.

Also, maybe, just maybe, there are more pressing issues than removing a
couple lines here and there? This discussion vaguely reminds me of the
opening statement of https://en.wikipedia.org/wiki/Law_of_triviality...
Just saying'...

Ciao,
Dscho

Re: [RFC] dropping support for ancient versions of curl

[Brandon Williams]

On 04/05, Johannes Schindelin wrote:
> Hi Brandon,
> 
> On Tue, 4 Apr 2017, Brandon Williams wrote:
> 
> > I'm all for seeing a patch like this applied.  I agree that we can't
> > expect the world to be running the most up-to-date version of curl but
> > we should be able to select some "oldest" version we will support which
> > can be bumped up every couple of years.  
> > 
> > I mean, ensuring that you are running with an up-to-date version of curl
> > is really important when it comes to all of the security fixes that have
> > been made in each revision.
> 
> I am not in the business of dictating to others what software they have to
> run. I am in the business of maintaining Git for Windows. And part of that
> job is to drag along code that is maybe not the most elegant, but works.
> 
> The patch in question resolves such a wart. Sure, it would be a cleanup.
> Is it a huge maintenance burden to keep those few #ifdef's, though?
> Absolutely not.

haha, very true.  Of course I'm stating my opinion based on my limited
experience and work environment.

-- 
Brandon Williams

Re: [RFC] dropping support for ancient versions of curl

[Stefan Beller]

On Tue, Apr 4, 2017 at 3:46 PM, Johannes Schindelin
 wrote:
> Hi Brandon,
>
> On Tue, 4 Apr 2017, Brandon Williams wrote:
>
>> I'm all for seeing a patch like this applied.  I agree that we can't
>> expect the world to be running the most up-to-date version of curl but
>> we should be able to select some "oldest" version we will support which
>> can be bumped up every couple of years.
>>
>> I mean, ensuring that you are running with an up-to-date version of curl
>> is really important when it comes to all of the security fixes that have
>> been made in each revision.
>
> I am not in the business of dictating to others what software they have to
> run. I am in the business of maintaining Git for Windows. And part of that
> job is to drag along code that is maybe not the most elegant, but works.
>
> The patch in question resolves such a wart. Sure, it would be a cleanup.
> Is it a huge maintenance burden to keep those few #ifdef's, though?
> Absolutely not.

Keeping them around is the easy part, the hard part is promising to users
that the software you maintain is as good as your reputation, when e.g.
we find out that certain #ifdefs don't even compile.
(See Frank Gevaerts answer)

So from my point of view it ought to be easier to maintain software that
is fully compiled and tested by a lot of people, and not have a long tail
of niche features that are not well tested.

Initially I thought I had a similar stance as you ("A well written line of code
is cheap") but I kept quiet, as I do not have a lot of experience with dealing
"old" Software.

Maybe the git community would want to take a look at the kernel community
(or other similar communities), how they solve the "long term stable" problem
of computer science.

And there are different things to be considered:
(1) the kernel community has "stable" maintainer(s) that are not the same
as the maintainer of the bleeding edge branch. So I would expect that these
maintainers have expertise in "dealing with old stuff, particular from
$DATE_RANGE".
(2) one of the kernels rules is "don't break user space". However sometimes
they do remove code[1]. And then their policy seemed to be: Wait until someone
shows up and we can revert the removal.

[1] http://lkml.iu.edu/hypermail/linux/kernel/1212.1/01152.html

So I would propose to take this patch, but be prepared to revert it in case
some user yells.

Thanks,
Stefan

Re: [RFC] dropping support for ancient versions of curl

[Johannes Schindelin]

Hi Brandon,

On Tue, 4 Apr 2017, Brandon Williams wrote:

> I'm all for seeing a patch like this applied.  I agree that we can't
> expect the world to be running the most up-to-date version of curl but
> we should be able to select some "oldest" version we will support which
> can be bumped up every couple of years.  
> 
> I mean, ensuring that you are running with an up-to-date version of curl
> is really important when it comes to all of the security fixes that have
> been made in each revision.

I am not in the business of dictating to others what software they have to
run. I am in the business of maintaining Git for Windows. And part of that
job is to drag along code that is maybe not the most elegant, but works.

The patch in question resolves such a wart. Sure, it would be a cleanup.
Is it a huge maintenance burden to keep those few #ifdef's, though?
Absolutely not.

Ciao,
Johannes

Re: [RFC] dropping support for ancient versions of curl

[Jeff King]

On Tue, Apr 04, 2017 at 04:06:46PM +0200, Ævar Arnfjörð Bjarmason wrote:

> > But a couple of #ifdef's? C'mon, man, we can carry this *without sweat*
> > indefinitely ;-)
> 
> I don't really care about applying this patch, but I wouldn't mind
> seeing it applied.
> 
> I just wanted to clarify the counteractive point that it's not unusual
> for some (particularly corporate) environments to be compiling fresh
> upstream releases of some software against really ancient versions of
> other upstream libraries.
> 
> But as Frank Gevaerts's reply (thanks!) which came after your reply
> points out, this code has already been broken since v2.12.0, so it's
> rarely used enough that nobody's reported being unable to compile git
> 2.12.0 on e.g. CentOS 5 >2 months since release.

Yeah, this is exactly the kind of thing I was wondering about (but was
too lazy to actually build an ancient version of curl -- thank you,
Frank).

In this case it was a compile error, which was obvious. I'm much more
worried about subtle interactions, or the fact that some of the ifdefs
are around security features that get ignored. In some cases we at least
issue a warning, but not always. E.g., we silently ignore
http.sslcapath.  Depending what you're using it for that could fail
closed (if you were trying to add a CA) or open (if you were trying to
restrict to a specific CA).

-Peff

Re: [RFC] dropping support for ancient versions of curl

[Brandon Williams]

On 04/04, Ævar Arnfjörð Bjarmason wrote:
> On Tue, Apr 4, 2017 at 1:54 PM, Johannes Schindelin
>  wrote:
> > Hi,
> >
> > On Tue, 4 Apr 2017, Ævar Arnfjörð Bjarmason wrote:
> >
> >> I think it's completely fine to include your patch as-is. At some
> >> point we need to pass the burden of dealing with these old software
> >> versions, saying that you should use a <10 year old library isn't
> >> unreasonable. Anyone packaging new git on RHEL5 or derivatives can
> >> just package a newer libcurl as well.
> >
> > But how much maintenance burden is it, really? Is the continued use of
> > those #ifdef's really worth this much discussion, let alone applying a
> > patch that may break users who have so far been happy?
> >
> > It would be a different thing if we had to have hacks to support old cURL
> > versions, where we need to ship entire >10kB source files that tap into
> > internal data structures that may, or may not have changed. Such a hack, I
> > would be happy to discuss when we could possibly remove it.
> >
> > But a couple of #ifdef's? C'mon, man, we can carry this *without sweat*
> > indefinitely ;-)
> 
> I don't really care about applying this patch, but I wouldn't mind
> seeing it applied.
> 
> I just wanted to clarify the counteractive point that it's not unusual
> for some (particularly corporate) environments to be compiling fresh
> upstream releases of some software against really ancient versions of
> other upstream libraries.
> 
> But as Frank Gevaerts's reply (thanks!) which came after your reply
> points out, this code has already been broken since v2.12.0, so it's
> rarely used enough that nobody's reported being unable to compile git
> 2.12.0 on e.g. CentOS 5 >2 months since release.
> 
> I think this is a stronger argument for removing stuff like this. At
> some point we're shipping code nobody's tested in combination with the
> rest of our code. This can easily becomes a source of bugs as someone
> e.g. compiling a new git on co5 becomes literally the first person to
> ever test some new combination of codepaths we've added around mostly
> unused ifdefs.

I'm all for seeing a patch like this applied.  I agree that we can't
expect the world to be running the most up-to-date version of curl but
we should be able to select some "oldest" version we will support which
can be bumped up every couple of years.  

I mean, ensuring that you are running with an up-to-date version of curl
is really important when it comes to all of the security fixes that have
been made in each revision.

-- 
Brandon Williams

Re: [RFC] dropping support for ancient versions of curl

[Ævar Arnfjörð Bjarmason]

On Tue, Apr 4, 2017 at 1:54 PM, Johannes Schindelin
 wrote:
> Hi,
>
> On Tue, 4 Apr 2017, Ævar Arnfjörð Bjarmason wrote:
>
>> I think it's completely fine to include your patch as-is. At some
>> point we need to pass the burden of dealing with these old software
>> versions, saying that you should use a <10 year old library isn't
>> unreasonable. Anyone packaging new git on RHEL5 or derivatives can
>> just package a newer libcurl as well.
>
> But how much maintenance burden is it, really? Is the continued use of
> those #ifdef's really worth this much discussion, let alone applying a
> patch that may break users who have so far been happy?
>
> It would be a different thing if we had to have hacks to support old cURL
> versions, where we need to ship entire >10kB source files that tap into
> internal data structures that may, or may not have changed. Such a hack, I
> would be happy to discuss when we could possibly remove it.
>
> But a couple of #ifdef's? C'mon, man, we can carry this *without sweat*
> indefinitely ;-)

I don't really care about applying this patch, but I wouldn't mind
seeing it applied.

I just wanted to clarify the counteractive point that it's not unusual
for some (particularly corporate) environments to be compiling fresh
upstream releases of some software against really ancient versions of
other upstream libraries.

But as Frank Gevaerts's reply (thanks!) which came after your reply
points out, this code has already been broken since v2.12.0, so it's
rarely used enough that nobody's reported being unable to compile git
2.12.0 on e.g. CentOS 5 >2 months since release.

I think this is a stronger argument for removing stuff like this. At
some point we're shipping code nobody's tested in combination with the
rest of our code. This can easily becomes a source of bugs as someone
e.g. compiling a new git on co5 becomes literally the first person to
ever test some new combination of codepaths we've added around mostly
unused ifdefs.

Re: [RFC] dropping support for ancient versions of curl

[Frank Gevaerts]

On Mon, Apr 03, 2017 at 10:54:38PM -0400, Jeff King wrote:
> A nearby thread raised the question of whether we can rely on a version
> of libcurl that contains a particular feature. The version in question
> is curl 7.11.1, which came out in March 2004.

I had a quick look at the 7.11.1 support, and I found that current git
actually doesn't build with anything older than curl 7.19.4. The issue
is aeae4db174, which moved some stuff to a helper function but didn't
copy the corresponding #ifdefs.

With that issue fixed, I could build with versions back to 7.12.2, which
is the oldest curl version I could get to build on my modern system.
Note that while I could build with those versions, I didn't actually
check if the result worked.

I'd say it's going to be increasingly likely with time that similar
issues will crop up for such old versions of dependencies.

Frank

diff --git a/http.c b/http.c
index 96d84bbed..8c782a086 100644
--- a/http.c
+++ b/http.c
@@ -674,6 +674,7 @@ void setup_curl_trace(CURL *handle)
curl_easy_setopt(handle, CURLOPT_DEBUGDATA, NULL);
 }
 
+#if LIBCURL_VERSION_NUM >= 0x071304
 static long get_curl_allowed_protocols(int from_user)
 {
long allowed_protocols = 0;
@@ -689,6 +690,7 @@ static long get_curl_allowed_protocols(int from_user)
 
return allowed_protocols;
 }
+#endif
 
 static CURL *get_curl_handle(void)
 {

-- 
"Debugging is twice as hard as writing the code in the first place.
Therefore, if you write the code as cleverly as possible, you are,
by definition, not smart enough to debug it." - Brian W. Kernighan

Re: [RFC] dropping support for ancient versions of curl

[Johannes Schindelin]

Hi,

On Tue, 4 Apr 2017, Ævar Arnfjörð Bjarmason wrote:

> I think it's completely fine to include your patch as-is. At some
> point we need to pass the burden of dealing with these old software
> versions, saying that you should use a <10 year old library isn't
> unreasonable. Anyone packaging new git on RHEL5 or derivatives can
> just package a newer libcurl as well.

But how much maintenance burden is it, really? Is the continued use of
those #ifdef's really worth this much discussion, let alone applying a
patch that may break users who have so far been happy?

It would be a different thing if we had to have hacks to support old cURL
versions, where we need to ship entire >10kB source files that tap into
internal data structures that may, or may not have changed. Such a hack, I
would be happy to discuss when we could possibly remove it.

But a couple of #ifdef's? C'mon, man, we can carry this *without sweat*
indefinitely ;-)

Ciao,
Dscho

Re: [RFC] dropping support for ancient versions of curl

[Ævar Arnfjörð Bjarmason]

On Tue, Apr 4, 2017 at 10:33 AM, Jeff King  wrote:
> On Tue, Apr 04, 2017 at 10:17:51AM +0200, Ævar Arnfjörð Bjarmason wrote:
>
>> On Tue, Apr 4, 2017 at 4:54 AM, Jeff King  wrote:
>> > My feeling is that this is old enough to stop caring about. Which means
>> > we can drop some of the #ifdefs that clutter the HTTP code (and there's
>> > a patch at the end of this mail dropping support for everything older
>> > than 7.11.1). But that made wonder: how old is too old? I think it's
>> > nice that we don't force people to upgrade to the latest version of
>> > curl. But at some point, if you are running a 13-year-old version of
>> > libcurl, how likely are you to run a brand new version of Git? :)
>> >
>> > If we declared 7.16.0 as a cutoff, we could unconditionally define
>> > USE_CURL_MULTI, which gets rid of quite a few messy ifdefs.
>>
>> I don't object to this patch, but just as a general comment, in
>> enterprise-y environments using an old OS (e.g. CentOS 5/6) & then
>> compiling some selected packages like git based on OS libraries is
>> quite common.
>>
>> E.g. at work we're running git 2.12.0 compiled against CentOS 6
>> libraries, which has curl 7.20.1, released on
>> Apr 14 2010. Not so long ago we were still running CentOS 5 which
>> comes with 7.15.5 released in Aug 7 2006 which would break with your
>> patch.
>>
>> Whether we support that is another question, I think it's reasonable
>> to say that if you're compiling git on such an old system you also
>> need to compile a libcurl instead of using the OS version. I just
>> wanted to point out that this *does* happen, someone is going to be
>> compiling new git releases on CentOS 5 & will be hit by this.
>
> Thanks, that's a very useful bit of data. According to:
>
>   https://access.redhat.com/support/policy/updates/errata
>
> RHEL5 (which as I understand it basically the same as CentOS 5 in terms
> of packages).

Yeah CentOS is just s/RHEL/CentOS/g across the RHEL-provided source
packages. It's a way to use RHEL without paying for (or getting) RHEL
support.

> ended its 10-year support cycle just a few days ago. That
> would perhaps make 7.20.1 a reasonable cutoff.

FWIW these cut-offs don't have a lot to do with how long some
installations run things like RHEL5 or CO5 actually run. RHEL5's EOP3
phase ended days ago, but the End of Extended Life-cycle Support goes
until late 2020, and for anyone running the CentOS flavors these dates
matter very little, since they're about how long RedHat is supporting
it, and if you don't have RedHat support in the first place...

> I dunno. We could also just punt on the whole thing. It was nice to see
> a bunch of #ifdefs go away, but the code they're protecting is actually
> not that complicated. Most of them do not have an #else at all, and we
> just silently skip features on old versions.
>
> I think it might be nice to declare a "too old" version, though, just so
> we can stop adding _new_ ifdefs. Maybe 7.11.1 is that version now, and
> in another few years we can bump to 7.16.0. :)

I think it's completely fine to include your patch as-is. At some
point we need to pass the burden of dealing with these old software
versions, saying that you should use a <10 year old library isn't
unreasonable. Anyone packaging new git on RHEL5 or derivatives can
just package a newer libcurl as well.

Re: [RFC] dropping support for ancient versions of curl

[Jeff King]

On Tue, Apr 04, 2017 at 10:17:51AM +0200, Ævar Arnfjörð Bjarmason wrote:

> On Tue, Apr 4, 2017 at 4:54 AM, Jeff King  wrote:
> > My feeling is that this is old enough to stop caring about. Which means
> > we can drop some of the #ifdefs that clutter the HTTP code (and there's
> > a patch at the end of this mail dropping support for everything older
> > than 7.11.1). But that made wonder: how old is too old? I think it's
> > nice that we don't force people to upgrade to the latest version of
> > curl. But at some point, if you are running a 13-year-old version of
> > libcurl, how likely are you to run a brand new version of Git? :)
> >
> > If we declared 7.16.0 as a cutoff, we could unconditionally define
> > USE_CURL_MULTI, which gets rid of quite a few messy ifdefs.
> 
> I don't object to this patch, but just as a general comment, in
> enterprise-y environments using an old OS (e.g. CentOS 5/6) & then
> compiling some selected packages like git based on OS libraries is
> quite common.
> 
> E.g. at work we're running git 2.12.0 compiled against CentOS 6
> libraries, which has curl 7.20.1, released on
> Apr 14 2010. Not so long ago we were still running CentOS 5 which
> comes with 7.15.5 released in Aug 7 2006 which would break with your
> patch.
> 
> Whether we support that is another question, I think it's reasonable
> to say that if you're compiling git on such an old system you also
> need to compile a libcurl instead of using the OS version. I just
> wanted to point out that this *does* happen, someone is going to be
> compiling new git releases on CentOS 5 & will be hit by this.

Thanks, that's a very useful bit of data. According to:

  https://access.redhat.com/support/policy/updates/errata

RHEL5 (which as I understand it basically the same as CentOS 5 in terms
of packages) ended its 10-year support cycle just a few days ago. That
would perhaps make 7.20.1 a reasonable cutoff.

I dunno. We could also just punt on the whole thing. It was nice to see
a bunch of #ifdefs go away, but the code they're protecting is actually
not that complicated. Most of them do not have an #else at all, and we
just silently skip features on old versions.

I think it might be nice to declare a "too old" version, though, just so
we can stop adding _new_ ifdefs. Maybe 7.11.1 is that version now, and
in another few years we can bump to 7.16.0. :)

-Peff

Re: [RFC] dropping support for ancient versions of curl

[Ævar Arnfjörð Bjarmason]

On Tue, Apr 4, 2017 at 4:54 AM, Jeff King  wrote:
> My feeling is that this is old enough to stop caring about. Which means
> we can drop some of the #ifdefs that clutter the HTTP code (and there's
> a patch at the end of this mail dropping support for everything older
> than 7.11.1). But that made wonder: how old is too old? I think it's
> nice that we don't force people to upgrade to the latest version of
> curl. But at some point, if you are running a 13-year-old version of
> libcurl, how likely are you to run a brand new version of Git? :)
>
> If we declared 7.16.0 as a cutoff, we could unconditionally define
> USE_CURL_MULTI, which gets rid of quite a few messy ifdefs.

I don't object to this patch, but just as a general comment, in
enterprise-y environments using an old OS (e.g. CentOS 5/6) & then
compiling some selected packages like git based on OS libraries is
quite common.

E.g. at work we're running git 2.12.0 compiled against CentOS 6
libraries, which has curl 7.20.1, released on
Apr 14 2010. Not so long ago we were still running CentOS 5 which
comes with 7.15.5 released in Aug 7 2006 which would break with your
patch.

Whether we support that is another question, I think it's reasonable
to say that if you're compiling git on such an old system you also
need to compile a libcurl instead of using the OS version. I just
wanted to point out that this *does* happen, someone is going to be
compiling new git releases on CentOS 5 & will be hit by this.

Re: [RFC] dropping support for ancient versions of curl

[Jessie Hernandez]

> On Mon, Apr 03, 2017 at 10:54:38PM -0400, Jeff King wrote:
>
>> If we declared 7.16.0 as a cutoff, we could unconditionally define
>> USE_CURL_MULTI, which gets rid of quite a few messy ifdefs.
>
> That version came out 11 years ago. Here's what that patch would look
> like (on top of my other one, but note I missed one older ifdef in the
> last one that's included here).
>
> And we could go further. There are a number of ifdefs around 7.19.1 (Nov
> 2008), 7.22 (Sep 2011). A 5-year cutoff puts us at 7.24. That's getting
> close enough that I'd probably start looking at what long-term distros
> like RHEL are still shipping and supporting.

Hi Peff,

I am all for making code simpler and dropping ancient versions.
I think we have to look at about the 7.19.1 mark.

I can confirm on my Centos 6.8 machine and on RHEL 6.8 on the client side
curl 7.19.7 [1] is used. I do not know what other distros are using.


Jessie

>
> -Peff
>
> ---
>  Documentation/config.txt |  3 +-
>  http-push.c  | 23 --
>  http-walker.c| 12 
>  http.c   | 57 +--
>  http.h   | 18 ---
>  remote-curl.c|  4 ---
>  6 files changed, 2 insertions(+), 115 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 475e874d5..2b04c1777 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1942,8 +1942,7 @@ http.maxRequests::
>  http.minSessions::
>   The number of curl sessions (counted across slots) to be kept across
>   requests. They will not be ended with curl_easy_cleanup() until
> - http_cleanup() is invoked. If USE_CURL_MULTI is not defined, this
> - value will be capped at 1. Defaults to 1.
> + http_cleanup() is invoked. Defaults to 1.
>
>  http.postBuffer::
>   Maximum size in bytes of the buffer used by smart HTTP
> diff --git a/http-push.c b/http-push.c
> index f0e3108f7..40146cdd6 100644
> --- a/http-push.c
> +++ b/http-push.c
> @@ -198,10 +198,8 @@ static void curl_setup_http(CURL *curl, const char
> *url,
>   curl_easy_setopt(curl, CURLOPT_INFILE, buffer);
>   curl_easy_setopt(curl, CURLOPT_INFILESIZE, buffer->buf.len);
>   curl_easy_setopt(curl, CURLOPT_READFUNCTION, fread_buffer);
> -#ifndef NO_CURL_IOCTL
>   curl_easy_setopt(curl, CURLOPT_IOCTLFUNCTION, ioctl_buffer);
>   curl_easy_setopt(curl, CURLOPT_IOCTLDATA, buffer);
> -#endif
>   curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_fn);
>   curl_easy_setopt(curl, CURLOPT_NOBODY, 0);
>   curl_easy_setopt(curl, CURLOPT_CUSTOMREQUEST, custom_req);
> @@ -244,8 +242,6 @@ static void process_response(void *callback_data)
>   finish_request(request);
>  }
>
> -#ifdef USE_CURL_MULTI
> -
>  static void start_fetch_loose(struct transfer_request *request)
>  {
>   struct active_request_slot *slot;
> @@ -295,7 +291,6 @@ static void start_mkcol(struct transfer_request
> *request)
>   request->url = NULL;
>   }
>  }
> -#endif
>
>  static void start_fetch_packed(struct transfer_request *request)
>  {
> @@ -600,7 +595,6 @@ static void finish_request(struct transfer_request
> *request)
>   }
>  }
>
> -#ifdef USE_CURL_MULTI
>  static int is_running_queue;
>  static int fill_active_slot(void *unused)
>  {
> @@ -624,7 +618,6 @@ static int fill_active_slot(void *unused)
>   }
>   return 0;
>  }
> -#endif
>
>  static void get_remote_object_list(unsigned char parent);
>
> @@ -653,10 +646,8 @@ static void add_fetch_request(struct object *obj)
>   request->next = request_queue_head;
>   request_queue_head = request;
>
> -#ifdef USE_CURL_MULTI
>   fill_active_slots();
>   step_active_slots();
> -#endif
>  }
>
>  static int add_send_request(struct object *obj, struct remote_lock *lock)
> @@ -691,10 +682,8 @@ static int add_send_request(struct object *obj,
> struct remote_lock *lock)
>   request->next = request_queue_head;
>   request_queue_head = request;
>
> -#ifdef USE_CURL_MULTI
>   fill_active_slots();
>   step_active_slots();
> -#endif
>
>   return 1;
>  }
> @@ -1673,21 +1662,15 @@ static int delete_remote_branch(const char
> *pattern, int force)
>
>  static void run_request_queue(void)
>  {
> -#ifdef USE_CURL_MULTI
>   is_running_queue = 1;
>   fill_active_slots();
>   add_fill_function(NULL, fill_active_slot);
> -#endif
>   do {
>   finish_all_active_slots();
> -#ifdef USE_CURL_MULTI
>   fill_active_slots();
> -#endif
>   } while (request_queue_head && !aborted);
>
> -#ifdef USE_CURL_MULTI
>   is_running_queue = 0;
> -#endif
>  }
>
>  int cmd_main(int argc, const char **argv)
> @@ -1763,10 +1746,6 @@ int cmd_main(int argc, const char **argv)
>   break;
>   }
>
> -#ifndef USE_CURL_MULTI
> - die("git-push is not available for http/https repository when not
> compiled with USE_CURL_MULTI");
> -#endif

Re: [RFC] dropping support for ancient versions of curl

[Jeff King]

On Mon, Apr 03, 2017 at 10:54:38PM -0400, Jeff King wrote:

> If we declared 7.16.0 as a cutoff, we could unconditionally define
> USE_CURL_MULTI, which gets rid of quite a few messy ifdefs.

That version came out 11 years ago. Here's what that patch would look
like (on top of my other one, but note I missed one older ifdef in the
last one that's included here).

And we could go further. There are a number of ifdefs around 7.19.1 (Nov
2008), 7.22 (Sep 2011). A 5-year cutoff puts us at 7.24. That's getting
close enough that I'd probably start looking at what long-term distros
like RHEL are still shipping and supporting.

-Peff

---
 Documentation/config.txt |  3 +-
 http-push.c  | 23 --
 http-walker.c| 12 
 http.c   | 57 +--
 http.h   | 18 ---
 remote-curl.c|  4 ---
 6 files changed, 2 insertions(+), 115 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 475e874d5..2b04c1777 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1942,8 +1942,7 @@ http.maxRequests::
 http.minSessions::
The number of curl sessions (counted across slots) to be kept across
requests. They will not be ended with curl_easy_cleanup() until
-   http_cleanup() is invoked. If USE_CURL_MULTI is not defined, this
-   value will be capped at 1. Defaults to 1.
+   http_cleanup() is invoked. Defaults to 1.
 
 http.postBuffer::
Maximum size in bytes of the buffer used by smart HTTP
diff --git a/http-push.c b/http-push.c
index f0e3108f7..40146cdd6 100644
--- a/http-push.c
+++ b/http-push.c
@@ -198,10 +198,8 @@ static void curl_setup_http(CURL *curl, const char *url,
curl_easy_setopt(curl, CURLOPT_INFILE, buffer);
curl_easy_setopt(curl, CURLOPT_INFILESIZE, buffer->buf.len);
curl_easy_setopt(curl, CURLOPT_READFUNCTION, fread_buffer);
-#ifndef NO_CURL_IOCTL
curl_easy_setopt(curl, CURLOPT_IOCTLFUNCTION, ioctl_buffer);
curl_easy_setopt(curl, CURLOPT_IOCTLDATA, buffer);
-#endif
curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_fn);
curl_easy_setopt(curl, CURLOPT_NOBODY, 0);
curl_easy_setopt(curl, CURLOPT_CUSTOMREQUEST, custom_req);
@@ -244,8 +242,6 @@ static void process_response(void *callback_data)
finish_request(request);
 }
 
-#ifdef USE_CURL_MULTI
-
 static void start_fetch_loose(struct transfer_request *request)
 {
struct active_request_slot *slot;
@@ -295,7 +291,6 @@ static void start_mkcol(struct transfer_request *request)
request->url = NULL;
}
 }
-#endif
 
 static void start_fetch_packed(struct transfer_request *request)
 {
@@ -600,7 +595,6 @@ static void finish_request(struct transfer_request *request)
}
 }
 
-#ifdef USE_CURL_MULTI
 static int is_running_queue;
 static int fill_active_slot(void *unused)
 {
@@ -624,7 +618,6 @@ static int fill_active_slot(void *unused)
}
return 0;
 }
-#endif
 
 static void get_remote_object_list(unsigned char parent);
 
@@ -653,10 +646,8 @@ static void add_fetch_request(struct object *obj)
request->next = request_queue_head;
request_queue_head = request;
 
-#ifdef USE_CURL_MULTI
fill_active_slots();
step_active_slots();
-#endif
 }
 
 static int add_send_request(struct object *obj, struct remote_lock *lock)
@@ -691,10 +682,8 @@ static int add_send_request(struct object *obj, struct 
remote_lock *lock)
request->next = request_queue_head;
request_queue_head = request;
 
-#ifdef USE_CURL_MULTI
fill_active_slots();
step_active_slots();
-#endif
 
return 1;
 }
@@ -1673,21 +1662,15 @@ static int delete_remote_branch(const char *pattern, 
int force)
 
 static void run_request_queue(void)
 {
-#ifdef USE_CURL_MULTI
is_running_queue = 1;
fill_active_slots();
add_fill_function(NULL, fill_active_slot);
-#endif
do {
finish_all_active_slots();
-#ifdef USE_CURL_MULTI
fill_active_slots();
-#endif
} while (request_queue_head && !aborted);
 
-#ifdef USE_CURL_MULTI
is_running_queue = 0;
-#endif
 }
 
 int cmd_main(int argc, const char **argv)
@@ -1763,10 +1746,6 @@ int cmd_main(int argc, const char **argv)
break;
}
 
-#ifndef USE_CURL_MULTI
-   die("git-push is not available for http/https repository when not 
compiled with USE_CURL_MULTI");
-#endif
-
if (!repo->url)
usage(http_push_usage);
 
@@ -1779,9 +1758,7 @@ int cmd_main(int argc, const char **argv)
 
http_init(NULL, repo->url, 1);
 
-#ifdef USE_CURL_MULTI
is_running_queue = 0;
-#endif
 
/* Verify DAV compliance/lock support */
if (!locking_available()) {
diff --git a/http-walker.c b/http-walker.c
index ee049cb13..b5b8e03b0 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -119,7 +119,6 @@ static 

[RFC] dropping support for ancient versions of curl

[Jeff King]

A nearby thread raised the question of whether we can rely on a version
of libcurl that contains a particular feature. The version in question
is curl 7.11.1, which came out in March 2004.

My feeling is that this is old enough to stop caring about. Which means
we can drop some of the #ifdefs that clutter the HTTP code (and there's
a patch at the end of this mail dropping support for everything older
than 7.11.1). But that made wonder: how old is too old? I think it's
nice that we don't force people to upgrade to the latest version of
curl. But at some point, if you are running a 13-year-old version of
libcurl, how likely are you to run a brand new version of Git? :)

If we declared 7.16.0 as a cutoff, we could unconditionally define
USE_CURL_MULTI, which gets rid of quite a few messy ifdefs.

-Peff

---
 http.c| 51 ---
 http.h| 11 
 remote-curl.c |  3 ---
 3 files changed, 65 deletions(-)

diff --git a/http.c b/http.c
index 8d94e2c63..7a81f0b68 100644
--- a/http.c
+++ b/http.c
@@ -12,19 +12,11 @@
 #include "transport.h"
 
 static struct trace_key trace_curl = TRACE_KEY_INIT(CURL);
-#if LIBCURL_VERSION_NUM >= 0x070a08
 long int git_curl_ipresolve = CURL_IPRESOLVE_WHATEVER;
-#else
-long int git_curl_ipresolve;
-#endif
 int active_requests;
 int http_is_verbose;
 size_t http_post_buffer = 16 * LARGE_PACKET_MAX;
 
-#if LIBCURL_VERSION_NUM >= 0x070a06
-#define LIBCURL_CAN_HANDLE_AUTH_ANY
-#endif
-
 static int min_curl_sessions = 1;
 static int curl_session_count;
 #ifdef USE_CURL_MULTI
@@ -57,12 +49,8 @@ static struct {
{ "tlsv1.2", CURL_SSLVERSION_TLSv1_2 },
 #endif
 };
-#if LIBCURL_VERSION_NUM >= 0x070903
 static const char *ssl_key;
-#endif
-#if LIBCURL_VERSION_NUM >= 0x070908
 static const char *ssl_capath;
-#endif
 #if LIBCURL_VERSION_NUM >= 0x072c00
 static const char *ssl_pinnedkey;
 #endif
@@ -81,9 +69,7 @@ static struct {
{ "digest", CURLAUTH_DIGEST },
{ "negotiate", CURLAUTH_GSSNEGOTIATE },
{ "ntlm", CURLAUTH_NTLM },
-#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
{ "anyauth", CURLAUTH_ANY },
-#endif
/*
 * CURLAUTH_DIGEST_IE has no corresponding command-line option in
 * curl(1) and is not included in CURLAUTH_ANY, so we leave it out
@@ -123,7 +109,6 @@ enum http_follow_config http_follow_config = 
HTTP_FOLLOW_INITIAL;
 
 static struct credential cert_auth = CREDENTIAL_INIT;
 static int ssl_cert_password_required;
-#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
 static unsigned long http_auth_methods = CURLAUTH_ANY;
 static int http_auth_methods_restricted;
 /* Modes for which empty_auth cannot actually help us. */
@@ -133,7 +118,6 @@ static unsigned long empty_auth_useless =
| CURLAUTH_DIGEST_IE
 #endif
| CURLAUTH_DIGEST;
-#endif
 
 static struct curl_slist *pragma_header;
 static struct curl_slist *no_pragma_header;
@@ -207,12 +191,8 @@ static void finish_active_slot(struct active_request_slot 
*slot)
if (slot->results != NULL) {
slot->results->curl_result = slot->curl_result;
slot->results->http_code = slot->http_code;
-#if LIBCURL_VERSION_NUM >= 0x070a08
curl_easy_getinfo(slot->curl, CURLINFO_HTTPAUTH_AVAIL,
  >results->auth_avail);
-#else
-   slot->results->auth_avail = 0;
-#endif
 
curl_easy_getinfo(slot->curl, CURLINFO_HTTP_CONNECTCODE,
>results->http_connectcode);
@@ -272,14 +252,10 @@ static int http_options(const char *var, const char 
*value, void *cb)
return git_config_string(_version, var, value);
if (!strcmp("http.sslcert", var))
return git_config_string(_cert, var, value);
-#if LIBCURL_VERSION_NUM >= 0x070903
if (!strcmp("http.sslkey", var))
return git_config_string(_key, var, value);
-#endif
-#if LIBCURL_VERSION_NUM >= 0x070908
if (!strcmp("http.sslcapath", var))
return git_config_pathname(_capath, var, value);
-#endif
if (!strcmp("http.sslcainfo", var))
return git_config_pathname(_cainfo, var, value);
if (!strcmp("http.sslcertpasswordprotected", var)) {
@@ -398,12 +374,6 @@ static int curl_empty_auth_enabled(void)
if (curl_empty_auth >= 0)
return curl_empty_auth;
 
-#ifndef LIBCURL_CAN_HANDLE_AUTH_ANY
-   /*
-* Our libcurl is too old to do AUTH_ANY in the first place;
-* just default to turning the feature off.
-*/
-#else
/*
 * In the automatic case, kick in the empty-auth
 * hack as long as we would potentially try some
@@ -416,7 +386,6 @@ static int curl_empty_auth_enabled(void)
if (http_auth_methods_restricted &&
(http_auth_methods & ~empty_auth_useless))
return 1;
-#endif
return 0;
 }
 
@@ -487,7 +456,6 @@ static void init_curl_proxy_auth(CURL *result)
 
var_override(_proxy_authmethod,