From: "Alexander Tikhonov" <avtikhon@tarantool.org> To: tarantool-patches@freelists.org Cc: tarantool-patches@dev.tarantool.org Subject: Re: [Tarantool-patches] [tarantool-patches] Re: [PATCH v1] build: added centos 8 Date: Thu, 31 Oct 2019 18:09:27 +0300 [thread overview] Message-ID: <1572534567.590887939@f455.i.mail.ru> (raw) In-Reply-To: <20191031130953.nbdns2r3xjrz5jwb@tkn_work_nb> [-- 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 --]
prev parent reply other threads:[~2019-10-31 15:09 UTC|newest] Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-10-29 10:23 [Tarantool-patches] " Alexander V. Tikhonov 2019-10-31 13:09 ` Alexander Turenko 2019-10-31 15:09 ` Alexander Tikhonov [this message]
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=1572534567.590887939@f455.i.mail.ru \ --to=avtikhon@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='Re: [Tarantool-patches] [tarantool-patches] Re: [PATCH v1] build: added centos 8' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox