[Tarantool-patches] [PATCH v2] build: enable CentOS 8 packaging
Alexander Tikhonov
avtikhon at tarantool.org
Fri Nov 8 18:28:23 MSK 2019
Alexander,
I'm completely LGTM with the patch, please push it.
>Пятница, 8 ноября 2019, 18:22 +03:00 от Alexander Turenko <alexander.turenko at tarantool.org>:
>
>Please, look into the changes I proposed below. They are about style,
>nothing intruisive.
>
>I also pushed them to avtikhon/gh-4543-centos8-more-full-ci branch to
>look into CI and ease reading of the resulting changeset.
>
>CI is green:
>
>https://gitlab.com/tarantool/tarantool/pipelines/94633088
>https://travis-ci.org/tarantool/tarantool/builds/609234538
>
>If you're okay, I'll push the patch.
>
>WBR, Alexander Turenko.
>
>> build: enable CentOS 8 packaging
>>
>> Enabled packages build rules for CentOS 8.
>
>Aren't this duplicate of the header? (See the proposal below.)
>
>> For now CentOS 8 uses 'python2-' prefix for Python 2 modules
>> rather the 'python-', changed CentOS 8 packages installation
>> rules to use 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 test from the commit
>> 7732daded14f86a314d44486e306e17bbc6ab293 (lua: treat ENOENT as
>> success in getpwall/getgrall), temporary blocked the test (till
>> the fix will be pushed) with the new issue:
>> https://github.com/tarantool/tarantool/issues/4592
>
>$ git show 7732daded14f86a314d44486e306e17bbc6ab293
>fatal: bad object 7732daded14f86a314d44486e306e17bbc6ab293
>
>It is part of a branch that was already deleted. Please, link only those
>commits that are part of of long-term branches.
>
>I would link only the issue here.
>
>(See the proposal below.)
>
>>
>> Found that libunwind library couldn't be installed from dependecies
>> which was needed by backtrace feature. Decided to block it temporary
>> till it will be enabled by issue:
>> https://github.com/tarantool/tarantool/issues/4611
>
>I would make it clear that it is available via EPEL, but does not via
>the base repositories. (See the proposal below.)
>
>> Added CentOS 8 into regular testing at gitlab-ci and travis-ci.
>>
>> Closes #4543
>> ---
>
>My variant of the commit message is below. Please, let me know whether
>you are okay with it. I left all points you highlighted and tried to
>make its description more easy to understand w/o much context.
>
> | travis-ci/gitlab-ci: add CentOS 8
> |
> | Added build + test jobs in GitLab-CI and build + test + deploy jobs on
> | Travis-CI for CentOS 8.
> |
> | Updated testing dependencies in the RPM spec to follow the new Python 2
> | package naming scheme that was introduced in CentOS 8: it uses
> | 'python2-' prefix rather then 'python-'.
> |
> | CentOS 8 does not provide python2-gevent and python2-greenlet packages,
> | so they were pushed to https://packagecloud.io/packpack/backports
> | repository. This repository is enabled in our build image
> | (packpack/packpack:el-8) by default. Those dependencies are build-time,
> | so nothing was changed for a user. The source RPM packages were gathered
> | from https://cbs.centos.org .
> |
> | Disabled app-tap/pwd.test.lua on CentOS 8 due to systemd-nss issue,
> | which was not worked around properly. Filed #4592 to resolved it in the
> | future.
> |
> | Eliminated libunwind runtime dependency (and libunwind-devel build
> | dependency) on CentOS 8, because the base system does not provide it.
> | fiber.info() backtraces and printing of a backtrace after a crash will
> | not be available on this system. Hopefully we'll fix it in the future,
> | filed #4611 on this.
> |
> | Closes #4543
>
>Also added a docbot comment to the issue.
>
>>
>> Github: https://github.com/tarantool/tarantool/tree/avtikhon/gh-4543-centos8-full-ci
>> Issue: https://github.com/tarantool/tarantool/issues/4543
>>
>> .gitlab-ci.yml | 6 ++++++
>> .travis.yml | 3 +++
>> rpm/tarantool.spec | 13 ++++++++++++-
>> test/app-tap/pwd.skipcond | 11 +++++++++++
>> 4 files changed, 32 insertions(+), 1 deletion(-)
>> create mode 100644 test/app-tap/pwd.skipcond
>>
>> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
>> index cd5f2806f..cf13c382e 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/.travis.yml b/.travis.yml
>> index db7d80bd2..d48ef39e0 100644
>> --- a/.travis.yml
>> +++ b/.travis.yml
>> @@ -43,6 +43,9 @@ jobs:
>> - name: "CentOS 7 build + test + deploy RPM"
>> env: OS=el DIST=7
>> if: branch = "master"
>> + - name: "CentOS 8 build + test + deploy RPM"
>> + env: OS=el DIST=8
>> + if: branch = "master"
>> - name: "Fedora 28 build + test + deploy RPM"
>> env: OS=fedora DIST=28
>> if: branch = "master"
>> diff --git a/rpm/tarantool.spec b/rpm/tarantool.spec
>> index b837214a6..416b4ebb4 100644
>> --- a/rpm/tarantool.spec
>> +++ b/rpm/tarantool.spec
>> @@ -63,12 +63,18 @@ 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)
>
>I removed parentheses around the condition.
>
>> +BuildRequires: python2 >= 2.7
>> +BuildRequires: python2-six >= 1.9.0
>> +BuildRequires: python2-gevent >= 1.0
>> +BuildRequires: python2-yaml >= 3.0.9
>> +%endif
>>
>> Name: tarantool
>> # ${major}.${major}.${minor}.${patch}, e.g. 1.6.8.175
>> @@ -125,14 +131,19 @@ C and Lua/C modules.
>>
>> %build
>> # RHBZ #1301720: SYSCONFDIR an LOCALSTATEDIR must be specified explicitly
>> +# gh-4611: temporary blocked the backtrace on CentOS 8 till completely enabled
>> %cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo \
>> -DCMAKE_INSTALL_LOCALSTATEDIR:PATH=%{_localstatedir} \
>> -DCMAKE_INSTALL_SYSCONFDIR:PATH=%{_sysconfdir} \
>> +%if (0%{?rhel} >= 8)
>> + -DENABLE_BACKTRACE:BOOL=OFF \
>> +%else
>> %if %{with backtrace}
>> -DENABLE_BACKTRACE:BOOL=ON \
>> %else
>> -DENABLE_BACKTRACE:BOOL=OFF \
>> %endif
>> +%endif
>
>Look at the code above:
>
> | %bcond_without backtrace # enabled by default
> |
> | %if %{with backtrace}
> | BuildRequires: libunwind-devel
>
>It would be better to set %{with backtrace} to appropriate value here
>and avoid the unnecessary build dependency.
>
>I changed it in the following way:
>
> | diff --git a/rpm/tarantool.spec b/rpm/tarantool.spec
> | index 416b4ebb4..41dbaf726 100644
> | --- a/rpm/tarantool.spec
> | +++ b/rpm/tarantool.spec
> | @@ -44,7 +44,14 @@ Requires(preun): chkconfig
> | Requires(preun): initscripts
> | %endif
> |
> | -%bcond_without backtrace # enabled by default
> | +%if 0%{?rhel} >= 8
> | +# gh-4611: Disable backtraces on CentOS 8 by default due to lack
> | +# of libunwind package in the base system.
> | +%bcond_with backtrace
> | +%else
> | +# Enable backtraces by default.
> | +%bcond_without backtrace
> | +%endif
> |
> | %if %{with backtrace}
> | BuildRequires: libunwind-devel
> | @@ -131,19 +138,14 @@ C and Lua/C modules.
> |
> | %build
> | # RHBZ #1301720: SYSCONFDIR an LOCALSTATEDIR must be specified explicitly
> | -# gh-4611: temporary blocked the backtrace on CentOS 8 till completely enabled
> | %cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo \
> | -DCMAKE_INSTALL_LOCALSTATEDIR:PATH=%{_localstatedir} \
> | -DCMAKE_INSTALL_SYSCONFDIR:PATH=%{_sysconfdir} \
> | -%if (0%{?rhel} >= 8)
> | - -DENABLE_BACKTRACE:BOOL=OFF \
> | -%else
> | %if %{with backtrace}
> | -DENABLE_BACKTRACE:BOOL=ON \
> | %else
> | -DENABLE_BACKTRACE:BOOL=OFF \
> | %endif
> | -%endif
> | %if %{with systemd}
> | -DWITH_SYSTEMD:BOOL=ON \
> | -DSYSTEMD_UNIT_DIR:PATH=%{_unitdir} \
>
>BTW, I was wonder how it works w/o libunwind runtime dependency. The
>answer is here if you are interesting:
>https://rpm.org/user_doc/more_dependencies.html#automatic-dependencies
>
>After this change I found that packpack passes `--with backtrace` into
>rpmbuild and fixed it in https://github.com/packpack/packpack/pull/114
>
>After this I found another problem and fixed it in a separate commit:
>
> | commit 41359c8b6e4e1dac20fbd211ea64f2fc3b9abf16
> | Author: Alexander Turenko < alexander.turenko at tarantool.org >
> | Date: Fri Nov 8 14:12:24 2019 +0300
> |
> | build: don't pass LDFLAGS from environment to curl
> |
> | After ea5929db4d400f5013a1c521870b9e14a8edcf85 ('build: fix OpenSSL
> | linking problems on FreeBSD') we set CFLAGS explicitly (possibly to an
> | empty value) when invoking a configure script for curl. When this
> | parameter is set the script does not use a value of environment variable
> | CFLAGS.
> |
> | Before this commit LDFLAGS environment variable can affect build of curl
> | submodule. This can lead to a problem when a user or a tool set CFLAGS
> | and LDFLAGS both and some linker flag assumes that some compilation flag
> | is present. Here we set empty LDFLAGS explicitly to avoid using of the
> | environment variable.
> |
> | A distributive build tool such as rpmbuild or emerge usually sets CFLAGS
> | and LDFLAGS. The problem with incompatible compiler / linker options has
> | been reveal under rpmbuild on CentOS 8 with hardened build enabled
> | (which is so when backtraces are disabled).
> |
> | It is not clear whether we should follow environment variables or values
> | determined by CMake for CFLAGS, CPPFLAGS and LDFLAGS when building a
> | submodule (such as luajit and curl). Let's decide about this later.
> |
> | Part of #4543.
> |
> | diff --git a/cmake/BuildLibCURL.cmake b/cmake/BuildLibCURL.cmake
> | index 12062ec8b..6d40ab045 100644
> | --- a/cmake/BuildLibCURL.cmake
> | +++ b/cmake/BuildLibCURL.cmake
> | @@ -57,9 +57,20 @@ macro(curl_build)
> |
> | # Pass -isysroot=<SDK_PATH> option on Mac OS, see
> | # above.
> | + # Note: Passing of CPPFLAGS / CFLAGS explicitly
> | + # discards using of corresponsing environment
> | + # variables.
> | CPPFLAGS=${LIBCURL_CPPFLAGS}
> | CFLAGS=${LIBCURL_CFLAGS}
> |
> | + # Pass empty LDFLAGS to discard using of
> | + # corresponding environment variable.
> | + # It is possible that a linker flag assumes that
> | + # some compilation flag is set. We don't pass
> | + # CFLAGS from environment, so we should not do it
> | + # for LDFLAGS too.
> | + LDFLAGS=
> | +
> | --prefix <INSTALL_DIR>
> | --enable-static
> | --enable-shared
>
>> %if %{with systemd}
>> -DWITH_SYSTEMD:BOOL=ON \
>> -DSYSTEMD_UNIT_DIR:PATH=%{_unitdir} \
>> diff --git a/test/app-tap/pwd.skipcond b/test/app-tap/pwd.skipcond
>> new file mode 100644
>> index 000000000..cebc6706c
>> --- /dev/null
>> +++ b/test/app-tap/pwd.skipcond
>> @@ -0,0 +1,11 @@
>> +import subprocess
>> +
>> +# Disabled at CentOS 8 build due to issue #4592.
>> +try:
>> + if subprocess.check_output(["rpm --eval '%{centos_ver}'"], \
>> + shell=True).strip() == '8':
>> + self.skip = 1
>
>Made fixes suggested by flake8:
>
>test/app-tap/pwd.skipcond:5:64: E502 the backslash is redundant between brackets
>test/app-tap/pwd.skipcond:6:13: E128 continuation line under-indented for visual indent
>
>Eliminated using of shell=True: we can split arguments ourself, there is
>no problem with it.
>
> | diff --git a/test/app-tap/pwd.skipcond b/test/app-tap/pwd.skipcond
> | index cebc6706c..cf97461bc 100644
> | --- a/test/app-tap/pwd.skipcond
> | +++ b/test/app-tap/pwd.skipcond
> | @@ -1,9 +1,9 @@
> | import subprocess
> |
> | -# Disabled at CentOS 8 build due to issue #4592.
> | +# Disable the test on CentOS 8 until gh-4592 will be resolved.
> | try:
> | - if subprocess.check_output(["rpm --eval '%{centos_ver}'"], \
> | - shell=True).strip() == '8':
> | + cmd = ['rpm', '--eval', '%{centos_ver}']
> | + if subprocess.check_output(cmd).strip() == '8':
> | self.skip = 1
> | except:
> | pass
>
>> +except:
>> + pass
>> +
>> +# vim: set ft=python:
>> --
>> 2.17.1
>>
--
Alexander Tikhonov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20191108/d788dce9/attachment.html>
More information about the Tarantool-patches
mailing list