* [Tarantool-patches] [PATCH v1] build: added centos 8 @ 2019-10-29 10:23 Alexander V. Tikhonov 2019-10-31 13:09 ` Alexander Turenko 0 siblings, 1 reply; 3+ messages in thread From: Alexander V. Tikhonov @ 2019-10-29 10:23 UTC (permalink / raw) To: Alexander Turenko; +Cc: tarantool-patches, tarantool-patches Added CentOS 8 image build, ready for Packpack use. Found that CentOS 8 completely moved to the use of python3 while Tarantool testing still uses python2, also found that CentOS 8 changed naming of the python packages to python2. To make these changes workable with Tarantool build added use of the python2 packages instead of python. Found that old packages like python-gevent and python-greenlet were completely removed from CentOS 8. To fix it the sources if these packages were downloaded from https://cbs.centos.org/ and rebuilt, to make them available the binaries were saved at the PackageCloud packpack backport repository. Met the issue with app-tap/pwd.test.lua which was allready fixed in local commit 7732daded14f86a314d44486e306e17bbc6ab293 (lua: treat ENOENT as success in getpwall/getgrall), but it wasn't merged into trunk - decided to block temporary the test till the correct fix will be created with the new issue created: https://github.com/tarantool/tarantool/issues/4592 Added CentOS 8 into regular testing at gitlab-ci. Closes #4543 --- Github: https://github.com/tarantool/tarantool/tree/avtikhon/gh-4543-centos8 Issue: https://github.com/tarantool/tarantool/issues/4543 .gitlab-ci.yml | 6 ++++++ rpm/tarantool.spec | 11 ++++++++++- test/app-tap/pwd.skipcond | 7 +++++++ 3 files changed, 23 insertions(+), 1 deletion(-) create mode 100644 test/app-tap/pwd.skipcond diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 431730b67..655e68cdc 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -152,6 +152,12 @@ centos_7: OS: 'el' DIST: '7' +centos_8: + <<: *deploy_test_definition + variables: + OS: 'el' + DIST: '8' + fedora_28: <<: *deploy_test_definition variables: diff --git a/rpm/tarantool.spec b/rpm/tarantool.spec index 10daf1458..5930f79d7 100644 --- a/rpm/tarantool.spec +++ b/rpm/tarantool.spec @@ -15,7 +15,9 @@ BuildRequires: gcc-c++ >= 4.5 BuildRequires: coreutils BuildRequires: sed BuildRequires: readline-devel +%if (0%{?fedora} > 0 || 0%{?rhel} <= 7) BuildRequires: libyaml-devel +%endif BuildRequires: openssl-devel BuildRequires: libicu-devel #BuildRequires: msgpuck-devel @@ -64,12 +66,19 @@ BuildRequires: libunwind-devel %endif # For tests -%if (0%{?fedora} >= 22 || 0%{?rhel} >= 7) +%if (0%{?fedora} >= 22 || 0%{?rhel} == 7) BuildRequires: python >= 2.7 BuildRequires: python-six >= 1.9.0 BuildRequires: python-gevent >= 1.0 BuildRequires: python-yaml >= 3.0.9 %endif +%if (0%{?rhel} >= 8) +BuildRequires: python2 >= 2.7 +BuildRequires: python2-six >= 1.9.0 +BuildRequires: python2-greenlet +BuildRequires: python2-gevent >= 1.0 +BuildRequires: python2-yaml >= 3.0.9 +%endif Name: tarantool # ${major}.${major}.${minor}.${patch}, e.g. 1.6.8.175 diff --git a/test/app-tap/pwd.skipcond b/test/app-tap/pwd.skipcond new file mode 100644 index 000000000..b001fb82c --- /dev/null +++ b/test/app-tap/pwd.skipcond @@ -0,0 +1,7 @@ +import subprocess + +# Disabled at CentOS 8 build due to issue #4592. +if subprocess.check_output(["rpm --eval '%{centos_ver}'"], shell=True).strip() == '8': + self.skip = 1 + +# vim: set ft=python: -- 2.17.1 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Tarantool-patches] [PATCH v1] build: added centos 8 2019-10-29 10:23 [Tarantool-patches] [PATCH v1] build: added centos 8 Alexander V. Tikhonov @ 2019-10-31 13:09 ` Alexander Turenko 2019-10-31 15:09 ` [Tarantool-patches] [tarantool-patches] " Alexander Tikhonov 0 siblings, 1 reply; 3+ messages in thread From: Alexander Turenko @ 2019-10-31 13:09 UTC (permalink / raw) To: Alexander V. Tikhonov; +Cc: tarantool-patches, tarantool-patches Alexander, your mistakes are often the same as ones in other patches and often the same that was already seen on previous review iterations: I think you need some kind of check-list to perform it before sending a patch. Please, organize your work in the way that will allow you to don't miss review comments. I run build and tests and tests segfaults very often. I didn't investigate the crashes, but Vlad says they can be related to https://github.com/tarantool/tarantool/issues/4597 I'll wait for a fix, because I'm unable to test your patch when master crashes randomly. I tried to install the resulting package inside ususal centos:8 docker image and got the following error: | # yum install /pwd/build/tarantool-2.3.0.191-1.el8.x86_64.rpm | Error: | Problem: conflicting requests | - nothing provides libunwind-x86_64.so.8()(64bit) needed by tarantool-2.3.0.191-1.el8.x86_64 | - nothing provides libunwind.so.8()(64bit) needed by tarantool-2.3.0.191-1.el8.x86_64 | (try to add '--skip-broken' to skip uninstallable packages or '--nobest' to use not only best candidate packages) It seems libunwind was moved from base repository to epel. I think we should not depend on libunwind (and disable related features for now), but maybe build it statically and bundle into tarantool in the future (if possible). See other comments below. WBR, Alexander Turenko. > build: added centos 8 Use imperative mood. > > Added CentOS 8 image build, ready for Packpack use. Let me cite my previous review comment: | I don't got the first sentence. Packpack provides build images and a | tool to use them. It is not something that a user need to use | tarantool. https://lists.tarantool.org/pipermail/tarantool-patches/2019-October/012046.html > > Found that CentOS 8 completely moved to the use of python3 > while Tarantool testing still uses python2, also found that > CentOS 8 changed naming of the python packages to python2. > To make these changes workable with Tarantool build added > use of the python2 packages instead of python. This can be said simpler: CentOS 8 uses 'python2-' prefix for Python 2 modules rather the 'python-'. > > Found that old packages like python-gevent and python-greenlet > were completely removed from CentOS 8. To fix it the sources > if these packages were downloaded from https://cbs.centos.org/ > and rebuilt, to make them available the binaries were saved > at the PackageCloud packpack backport repository. > > Met the issue with app-tap/pwd.test.lua which was allready allready -> already > fixed in local commit 7732daded14f86a314d44486e306e17bbc6ab293 > (lua: treat ENOENT as success in getpwall/getgrall), but it > wasn't merged into trunk - decided to block temporary the test > till the correct fix will be created with the new issue created: > https://github.com/tarantool/tarantool/issues/4592 I don't sure this fix was right, so I would not say that 'it was already fixed'. I think than mentioning of an issue that track the problem is enough here. > > Added CentOS 8 into regular testing at gitlab-ci. Let me cite the comment from my previous review: | We should add it to Travis-CI too, because deployments are made only | from Travis-CI. https://lists.tarantool.org/pipermail/tarantool-patches/2019-October/012046.html > > Closes #4543 > --- > > Github: https://github.com/tarantool/tarantool/tree/avtikhon/gh-4543-centos8 It seems you didn't updated your branch. > Issue: https://github.com/tarantool/tarantool/issues/4543 > > .gitlab-ci.yml | 6 ++++++ > rpm/tarantool.spec | 11 ++++++++++- > test/app-tap/pwd.skipcond | 7 +++++++ > 3 files changed, 23 insertions(+), 1 deletion(-) > create mode 100644 test/app-tap/pwd.skipcond > > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml > index 431730b67..655e68cdc 100644 > --- a/.gitlab-ci.yml > +++ b/.gitlab-ci.yml > @@ -152,6 +152,12 @@ centos_7: > OS: 'el' > DIST: '7' > > +centos_8: > + <<: *deploy_test_definition > + variables: > + OS: 'el' > + DIST: '8' > + > fedora_28: > <<: *deploy_test_definition > variables: > diff --git a/rpm/tarantool.spec b/rpm/tarantool.spec > index 10daf1458..5930f79d7 100644 > --- a/rpm/tarantool.spec > +++ b/rpm/tarantool.spec > @@ -15,7 +15,9 @@ BuildRequires: gcc-c++ >= 4.5 > BuildRequires: coreutils > BuildRequires: sed > BuildRequires: readline-devel > +%if (0%{?fedora} > 0 || 0%{?rhel} <= 7) > BuildRequires: libyaml-devel > +%endif Please, rebase: we removed this dependency on master and 2.2. We anyway need libyaml-devel on 1.10 and 2.1, but there is no need to exclude the dependency on those branches. I already said that: https://github.com/packpack/packpack-docker-images/pull/39#discussion_r338324713 > BuildRequires: openssl-devel > BuildRequires: libicu-devel > #BuildRequires: msgpuck-devel > @@ -64,12 +66,19 @@ BuildRequires: libunwind-devel > %endif > > # For tests > -%if (0%{?fedora} >= 22 || 0%{?rhel} >= 7) > +%if (0%{?fedora} >= 22 || 0%{?rhel} == 7) > BuildRequires: python >= 2.7 > BuildRequires: python-six >= 1.9.0 > BuildRequires: python-gevent >= 1.0 > BuildRequires: python-yaml >= 3.0.9 > %endif > +%if (0%{?rhel} >= 8) > +BuildRequires: python2 >= 2.7 > +BuildRequires: python2-six >= 1.9.0 > +BuildRequires: python2-greenlet Is not this the dependency of python2-gevent? I think we should not add it explicitly here if it is so. > +BuildRequires: python2-gevent >= 1.0 > +BuildRequires: python2-yaml >= 3.0.9 > +%endif > > Name: tarantool > # ${major}.${major}.${minor}.${patch}, e.g. 1.6.8.175 > diff --git a/test/app-tap/pwd.skipcond b/test/app-tap/pwd.skipcond > new file mode 100644 > index 000000000..b001fb82c > --- /dev/null > +++ b/test/app-tap/pwd.skipcond > @@ -0,0 +1,7 @@ > +import subprocess > + > +# Disabled at CentOS 8 build due to issue #4592. > +if subprocess.check_output(["rpm --eval '%{centos_ver}'"], shell=True).strip() == '8': PEP8 forbids so long lines. > + self.skip = 1 > + > +# vim: set ft=python: > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Tarantool-patches] [tarantool-patches] Re: [PATCH v1] build: added centos 8 2019-10-31 13:09 ` Alexander Turenko @ 2019-10-31 15:09 ` Alexander Tikhonov 0 siblings, 0 replies; 3+ messages in thread From: Alexander Tikhonov @ 2019-10-31 15:09 UTC (permalink / raw) To: tarantool-patches; +Cc: tarantool-patches [-- Attachment #1: Type: text/plain, Size: 6886 bytes --] >Четверг, 31 октября 2019, 16:10 +03:00 от Alexander Turenko <alexander.turenko@tarantool.org>: > >Alexander, your mistakes are often the same as ones in other patches and >often the same that was already seen on previous review iterations: I >think you need some kind of check-list to perform it before sending a >patch. > >Please, organize your work in the way that will allow you to don't miss >review comments. Created instructions on Confluence. > >I run build and tests and tests segfaults very often. I didn't >investigate the crashes, but Vlad says they can be related to >https://github.com/tarantool/tarantool/issues/4597 > >I'll wait for a fix, because I'm unable to test your patch when master >crashes randomly. Right, some latest patches broke the stability. > > >I tried to install the resulting package inside ususal centos:8 docker >image and got the following error: > > | # yum install /pwd/build/tarantool-2.3.0.191-1.el8.x86_64.rpm > | Error: > | Problem: conflicting requests > | - nothing provides libunwind-x86_64.so.8()(64bit) needed by tarantool-2.3.0.191-1.el8.x86_64 > | - nothing provides libunwind.so.8()(64bit) needed by tarantool-2.3.0.191-1.el8.x86_64 > | (try to add '--skip-broken' to skip uninstallable packages or '--nobest' to use not only best candidate packages) > >It seems libunwind was moved from base repository to epel. I think we >should not depend on libunwind (and disable related features for now), >but maybe build it statically and bundle into tarantool in the future >(if possible). Added to instructions on Confluence not to miss the check again. Discussed and decided to block backtrace to avoid of libunwind use. > >See other comments below. > >WBR, Alexander Turenko. > >> build: added centos 8 > >Use imperative mood. Fixed. > > >> >> Added CentOS 8 image build, ready for Packpack use. > >Let me cite my previous review comment: > > | I don't got the first sentence. Packpack provides build images and a > | tool to use them. It is not something that a user need to use > | tarantool. > >https://lists.tarantool.org/pipermail/tarantool-patches/2019-October/012046.html Changed. > >> >> Found that CentOS 8 completely moved to the use of python3 >> while Tarantool testing still uses python2, also found that >> CentOS 8 changed naming of the python packages to python2. >> To make these changes workable with Tarantool build added >> use of the python2 packages instead of python. > >This can be said simpler: CentOS 8 uses 'python2-' prefix for Python 2 >modules rather the 'python-'. Used the suggested sentence with additions. > >> >> Found that old packages like python-gevent and python-greenlet >> were completely removed from CentOS 8. To fix it the sources >> if these packages were downloaded from https://cbs.centos.org/ >> and rebuilt, to make them available the binaries were saved >> at the PackageCloud packpack backport repository. >> >> Met the issue with app-tap/pwd.test.lua which was allready > >allready -> already Mistake corrected. > > >> fixed in local commit 7732daded14f86a314d44486e306e17bbc6ab293 >> (lua: treat ENOENT as success in getpwall/getgrall), but it >> wasn't merged into trunk - decided to block temporary the test >> till the correct fix will be created with the new issue created: >> https://github.com/tarantool/tarantool/issues/4592 > >I don't sure this fix was right, so I would not say that 'it was already >fixed'. I think than mentioning of an issue that track the problem is >enough here. Removed extra information. > >> >> Added CentOS 8 into regular testing at gitlab-ci. > >Let me cite the comment from my previous review: > > | We should add it to Travis-CI too, because deployments are made only > | from Travis-CI. > >https://lists.tarantool.org/pipermail/tarantool-patches/2019-October/012046.html Corrected. > >> >> Closes #4543 >> --- >> >> Github: https://github.com/tarantool/tarantool/tree/avtikhon/gh-4543-centos8 > >It seems you didn't updated your branch. Right, forgot to push. > > >> Issue: https://github.com/tarantool/tarantool/issues/4543 >> >> .gitlab-ci.yml | 6 ++++++ >> rpm/tarantool.spec | 11 ++++++++++- >> test/app-tap/pwd.skipcond | 7 +++++++ >> 3 files changed, 23 insertions(+), 1 deletion(-) >> create mode 100644 test/app-tap/pwd.skipcond >> >> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml >> index 431730b67..655e68cdc 100644 >> --- a/.gitlab-ci.yml >> +++ b/.gitlab-ci.yml >> @@ -152,6 +152,12 @@ centos_7: >> OS: 'el' >> DIST: '7' >> >> +centos_8: >> + <<: *deploy_test_definition >> + variables: >> + OS: 'el' >> + DIST: '8' >> + >> fedora_28: >> <<: *deploy_test_definition >> variables: >> diff --git a/rpm/tarantool.spec b/rpm/tarantool.spec >> index 10daf1458..5930f79d7 100644 >> --- a/rpm/tarantool.spec >> +++ b/rpm/tarantool.spec >> @@ -15,7 +15,9 @@ BuildRequires: gcc-c++ >= 4.5 >> BuildRequires: coreutils >> BuildRequires: sed >> BuildRequires: readline-devel >> +%if (0%{?fedora} > 0 || 0%{?rhel} <= 7) >> BuildRequires: libyaml-devel >> +%endif > >Please, rebase: we removed this dependency on master and 2.2. Got the changes with re-base. > > >We anyway need libyaml-devel on 1.10 and 2.1, but there is no need to >exclude the dependency on those branches. I already said that: >https://github.com/packpack/packpack-docker-images/pull/39#discussion_r338324713 Ok. > > >> BuildRequires: openssl-devel >> BuildRequires: libicu-devel >> #BuildRequires: msgpuck-devel >> @@ -64,12 +66,19 @@ BuildRequires: libunwind-devel >> %endif >> >> # For tests >> -%if (0%{?fedora} >= 22 || 0%{?rhel} >= 7) >> +%if (0%{?fedora} >= 22 || 0%{?rhel} == 7) >> BuildRequires: python >= 2.7 >> BuildRequires: python-six >= 1.9.0 >> BuildRequires: python-gevent >= 1.0 >> BuildRequires: python-yaml >= 3.0.9 >> %endif >> +%if (0%{?rhel} >= 8) >> +BuildRequires: python2 >= 2.7 >> +BuildRequires: python2-six >= 1.9.0 >> +BuildRequires: python2-greenlet > >Is not this the dependency of python2-gevent? I think we should not add >it explicitly here if it is so. Removed python2-greenlet due to python2-gevent installs it in dependences. > > >> +BuildRequires: python2-gevent >= 1.0 >> +BuildRequires: python2-yaml >= 3.0.9 >> +%endif >> >> Name: tarantool >> # ${major}.${major}.${minor}.${patch}, e.g. 1.6.8.175 >> diff --git a/test/app-tap/pwd.skipcond b/test/app-tap/pwd.skipcond >> new file mode 100644 >> index 000000000..b001fb82c >> --- /dev/null >> +++ b/test/app-tap/pwd.skipcond >> @@ -0,0 +1,7 @@ >> +import subprocess >> + >> +# Disabled at CentOS 8 build due to issue #4592. >> +if subprocess.check_output(["rpm --eval '%{centos_ver}'"], shell=True).strip() == '8': > >PEP8 forbids so long lines. Corrected. > > >> + self.skip = 1 >> + >> +# vim: set ft=python: >> -- >> 2.17.1 >> > -- Alexander Tikhonov [-- Attachment #2: Type: text/html, Size: 11953 bytes --] ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-10-31 15:09 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-10-29 10:23 [Tarantool-patches] [PATCH v1] build: added centos 8 Alexander V. Tikhonov 2019-10-31 13:09 ` Alexander Turenko 2019-10-31 15:09 ` [Tarantool-patches] [tarantool-patches] " Alexander Tikhonov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox