Re: [RFC] dropping support for ancient versions of curl
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
> 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
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 voi