[Tarantool-patches] [PATCH v2] build: enable CentOS 8 packaging
Alexander Turenko
alexander.turenko at tarantool.org
Fri Nov 8 18:22:05 MSK 2019
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
>
More information about the Tarantool-patches
mailing list