ci: Add CentOS 7 build

hebasto commented at 9:39 pm on November 29, 2019: member

Arguably, CentOS is the most conservative distro of all the popular ones. Thus, it could be a good way to check the Bitcoin Core compatibility with aged dependencies.

Currently, CentOS 7 has:

Berkeley DB == 4.8.30
Boost == 1.53.0
GCC == 4.8.5
libevent == 2.0.21 < minimum required 2.0.22, but tests passed
MiniUPnPc == 2.0
Python == 3.6.8
qrencode == 3.4.1
Qt == 5.9.7
ZeroMQ == 4.1.4

~~Please note that this PR is based on the bugfix #17634.~~

Also trusty build has been removed for the following reasons:

#17628 (comment):

Maybe it’d make sense to replace Ubuntu Trusty with Centos 7 as the “check ancient backward compatibililty” Travis run. It’s supported until 2024, apparently.

#17635 (review):

Our travis is currently running at its limit and this doesn’t seem like it is adding a lot new coverage compared to the other builds.

Close #17628

hebasto commented at 9:41 pm on November 29, 2019: member

@laanwj’s #17628 (comment):

Maybe it’d make sense to replace Ubuntu Trusty with Centos 7 as the “check ancient backward compatibililty” Travis run. It’s supported until 2024, apparently.

DrahtBot added the label Build system on Nov 29, 2019

DrahtBot added the label GUI on Nov 29, 2019

DrahtBot added the label Tests on Nov 29, 2019

DrahtBot added the label Validation on Nov 29, 2019

DrahtBot commented at 0:08 am on November 30, 2019: member

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#17490 (ci: Add valgrind run by MarcoFalke)
#12134 (Build previous releases and run functional tests by Sjors)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

fanquake removed the label Build system on Nov 30, 2019

fanquake removed the label GUI on Nov 30, 2019

fanquake removed the label Validation on Nov 30, 2019

practicalswift commented at 6:54 am on November 30, 2019: contributor

Concept ACK: diversity in testing is good

in src/qt/recentrequeststablemodel.cpp:216 in 986aee882b outdated

212@@ -214,10 +213,10 @@ void RecentRequestsTableModel::updateDisplayUnit()
213     updateAmountColumnTitle();
214 }
215 
216-bool RecentRequestEntryLessThan::operator()(RecentRequestEntry &left, RecentRequestEntry &right) const
217+bool RecentRequestEntryLessThan::operator()(const RecentRequestEntry& left, const RecentRequestEntry& right) const

paymog commented at 8:45 am on November 30, 2019:

are these changes required for the centos build to pass?

hebasto commented at 8:53 am on November 30, 2019:

Please look #17634.

paymog commented at 8:58 am on November 30, 2019:

Ah got it. I’m still new to how bitcoin is developed but I think it would be useful if this PR didn’t try to merge to master but instead tried to merge into #17634 so that the changes from the PR don’t appear here (but that’s just a stylistic nit).

hebasto commented at 9:16 am on November 30, 2019:

From OP:

Please note that this PR is based on the bugfix #17634.

When/if #17634 is merged this PR will be rebased and commits from #17634 will not appear here.

in src/qt/peertablemodel.cpp:13 in 986aee882b outdated

 9@@ -10,7 +10,7 @@
10 
11 #include <interfaces/node.h>
12 
13-#include <algorithm>
14+#include <utility>

paymog commented at 8:46 am on November 30, 2019:

why the switch from algorithm to utility?

hebasto commented at 8:53 am on November 30, 2019:

Please look #17634.

paymog commented at 8:48 am on November 30, 2019: none

I’m new to the bitcoin codebase but this generally LGTM.

fanquake commented at 2:41 am on December 2, 2019: member

Concept ACK

hebasto force-pushed on Dec 2, 2019

hebasto commented at 4:37 pm on December 2, 2019: member

Rebased after #17634 has been merged.

in .travis.yml:124 in 8f62ec6380 outdated

117@@ -118,6 +118,11 @@ jobs:
118       env: >-
119         FILE_ENV="./ci/test/00_setup_env_i686.sh"
120 
121+    - stage: test
122+      name: 'x86_64 Linux  [GOAL: install]  [CentOS 7]  [no depends, only system libs]'
123+      env: >-
124+        FILE_ENV="./ci/test/00_setup_env_centos.sh"

MarcoFalke commented at 1:34 am on December 3, 2019:

nit: This should run on any architecture

0        FILE_ENV="./ci/test/00_setup_env_native_centos.sh"

hebasto commented at 5:46 am on December 3, 2019:

Done.

MarcoFalke approved

hebasto force-pushed on Dec 3, 2019

laanwj commented at 9:28 am on December 3, 2019: member

I think you should do a depends build here.

While the binary (and the tests) must run on CentOS 7 (this was a constraint on the libc version in #17538), it doesn’t need to build with the ancient library versions provided there: CentOS 7 does not put a restriction on our minimum dependency versions.

laanwj commented at 9:33 am on December 3, 2019: member

This also implicitly restricts the minimum supported g++ version to 4.8.5, complicating #16684 in the future. But I think we can worry about that then (maybe there’s a way to install a newer compiler, no need to hold this up now).

hebasto commented at 12:44 pm on December 3, 2019: member

I think you should do a depends build here.

I have encountered a libevent build error on Travis (the local build is ok). Going to debug…

MarcoFalke commented at 2:06 pm on December 3, 2019: member

But then, what is the point of using CentOs 7 to get all the old packages (of boost, etc) when a depends build is done anyway?

laanwj commented at 2:16 pm on December 3, 2019: member

I don’t know. Some of the package versions make sense, for Qt even recent (5.9.7 is from last year), but In the case of libevent the version is absurdly old.

MarcoFalke commented at 2:20 pm on December 3, 2019: member

We mostly had issues with compiling against older versions of boost (and qt). If the trusty run is eventually removed, we should have something else to run against an old version of boost.

laanwj commented at 2:25 pm on December 3, 2019: member

yes, the boost version is OK; and I don’t think we can do a depends build without boost at this point

aj on IRC commented something about “software collections”, could this be used to install a newer version of libevent only?

hebasto commented at 2:32 pm on December 3, 2019: member

aj on IRC commented something about “software collections”, could this be used to install a newer version of libevent only?

https://www.softwarecollections.org/en/scls/?search=libevent

… nothing.

MarcoFalke commented at 2:54 pm on December 3, 2019: member

ACK d88782e3fc897cf1fe63eaa2255ac93fb222f711

MarcoFalke commented at 3:07 pm on December 3, 2019: member

A nice follow up (either a new commit or pr) would be to adjust the readme and the list of packages to only the needed ones: boost-thread boost-test boost-filesystem boost-chrono instead of the entirety of boost.

hebasto commented at 3:15 pm on December 3, 2019: member

A nice follow up (either a new commit or pr) would be to adjust the readme and the list of packages to only the needed ones: boost-thread boost-test boost-filesystem boost-chrono instead of the entirety of boost.

CentOS / Fedora has only one boost-devel package. There is no boost-{thread|test|filesystem|chrono}-devel packages, unfortunately.

promag commented at 7:58 am on December 6, 2019: member

ACK d88782e3fc897cf1fe63eaa2255ac93fb222f711.

in .travis.yml:124 in d88782e3fc outdated

117@@ -118,6 +118,11 @@ jobs:
118       env: >-
119         FILE_ENV="./ci/test/00_setup_env_i686.sh"
120 
121+    - stage: test
122+      name: 'x86_64 Linux  [GOAL: install]  [CentOS 7]  [no depends, only system libs]'
123+      env: >-
124+        FILE_ENV="./ci/test/00_setup_env_native_centos.sh"

MarcoFalke commented at 12:34 pm on December 6, 2019:

Our travis is currently running at its limit and this doesn’t seem like it is adding a lot new coverage compared to the other builds. What about only running it in cirrus instead of travis?

hebasto commented at 1:25 pm on December 6, 2019:

Could CentOS build be a replacement for Trusty build on Travis?

MarcoFalke commented at 1:30 pm on December 6, 2019:

Sounds good

hebasto commented at 1:32 pm on December 6, 2019:

Should Trusty build be removed from Travis in this PR?

MarcoFalke commented at 1:34 pm on December 6, 2019:

Yeah, I think this has been the plan: #17628 (comment)

hebasto commented at 7:59 pm on December 6, 2019:

@emilengler’s 5423bd3474581a479b244cb9b599cbefafc64618 has been cherry-picked.

in ci/test/00_setup_env_native_centos.sh:11 in d88782e3fc outdated

 6+
 7+export LC_ALL=C.UTF-8
 8+
 9+export DOCKER_NAME_TAG=centos:7
10+export DOCKER_PACKAGES="gcc-c++ libtool make git python3"
11+export PACKAGES="boost-devel libevent-devel libdb4-devel libdb4-cxx-devel miniupnpc-devel zeromq-devel qt5-qtbase-devel qt5-qttools-devel qrencode-devel"

MarcoFalke commented at 1:31 pm on December 6, 2019:

Please install python3-zmq, if available. See https://travis-ci.org/bitcoin/bitcoin/jobs/619963005#L3914

hebasto commented at 7:57 pm on December 6, 2019:

Done.

MarcoFalke commented at 7:59 pm on December 6, 2019:

And https://apps.fedoraproject.org/packages/python3-zmq

hebasto commented at 8:08 pm on December 6, 2019:

https://centos.pkgs.org/7/epel-x86_64/python36-zmq-14.7.0-8.el7.x86_64.rpm.html

MarcoFalke commented at 8:25 pm on December 6, 2019:

Ah, please add it then.

hebasto commented at 8:27 pm on December 6, 2019:

Already:

https://github.com/bitcoin/bitcoin/blob/7f3ae224685efaeb6fe714de90e8871d12e55f34/ci/test/00_setup_env_native_centos.sh#L10

hebasto commented at 8:29 pm on December 6, 2019:

https://travis-ci.org/bitcoin/bitcoin/jobs/621765674

MarcoFalke renamed this:
~~build: Add Travis CentOS 7 build~~
ci: Add CentOS 7 build
on Dec 6, 2019

ci: Add CentOS 7 build 7f3ae22468

ci: Remove trusty build

Co-authored-by: Emil Engler <me@emilengler.com>

711e0449cf

hebasto force-pushed on Dec 6, 2019

MarcoFalke commented at 4:32 am on December 7, 2019: member

ACK 711e0449cf4a0f15cabe0d64094e3add24ad44b0 🚠

Signature:

 0-----BEGIN PGP SIGNED MESSAGE-----
 1Hash: SHA512
 2
 3ACK 711e0449cf4a0f15cabe0d64094e3add24ad44b0 🚠
 4-----BEGIN PGP SIGNATURE-----
 5
 6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
 7pUgl0gv/fWA9rxwx5f/Yr178u4iXUkmIRtbazRKORuGeAEPzOoWCG1hpOWMMUaX+
 8UUqNn9hEPu5J9lLfZdPItdVLyjKRTFu6Ch2iLmRstrn9Nev6q3wwKNnY6vjNVNdd
 9KZAJVnA11eEGIcfhhMlTCy7+E4IQB+mI8/+HL/KmHnckhaVfTLBp5ZEt0MUJf/GT
108lQ+/qjr0uUd6dCt6TsqpEitdbA5F8TTGENT9z49AnlDNdFgFjnoI4NMALvwftnw
11r+aqwBprafaKyJrl1zcnr1e+2O33kD884+NAINfoVHMQ6VKKQyg9mTqa1yzrLoQu
129qkk1qwxaxL1+XHbux1gyRXQc8J7SnBy7qZg2tOpB1QshaC9CYgukIX540uEFJab
13xVitKe0MZLnwIbTx8CATN90bm+/EcmiCmJHunPPWv8A4oJKwPDlIeqIFT4UsfzWy
14bDWM4UW3U/0PS+guH1Y8gNIPCUd+J1ECuaHjFbYjeBrnbvK1Wz0sZX8vBE2HfpPN
154gDSSFSh
16=UKBg
17-----END PGP SIGNATURE-----

Timestamp of file with hash 6113c2488002e62c4f29f31002293d1c4fcd8b73206b423cb8ff1244d50cc142 -

MarcoFalke referenced this in commit fc0da027e8 on Dec 7, 2019

MarcoFalke merged this on Dec 7, 2019

MarcoFalke closed this on Dec 7, 2019

hebasto deleted the branch on Dec 7, 2019

sidhujag referenced this in commit 0bfc992d17 on Dec 7, 2019

elichai commented at 12:42 pm on December 8, 2019: contributor

ACK After merge. I actually think this can help us more confidently bump the C++ version in the future (probably will require bumping to CentOS 8)

sidhujag referenced this in commit 352aa1f19d on Nov 10, 2020

MarcoFalke locked this on Dec 16, 2021

ci: Add CentOS 7 build #17635

Conflicts