Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH v2] build: enable CentOS 8 packaging
@ 2019-11-05  4:10 Alexander V. Tikhonov
  2019-11-08 15:22 ` Alexander Turenko
  0 siblings, 1 reply; 6+ messages in thread
From: Alexander V. Tikhonov @ 2019-11-05  4:10 UTC (permalink / raw)
  To: Alexander Turenko; +Cc: tarantool-patches, tarantool-patches

Enabled packages build rules for CentOS 8.

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

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

Added CentOS 8 into regular testing at gitlab-ci and travis-ci.

Closes #4543
---

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)
+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
 %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
+except:
+    pass
+
+# vim: set ft=python:
-- 
2.17.1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Tarantool-patches] [PATCH v2] build: enable CentOS 8 packaging
  2019-11-05  4:10 [Tarantool-patches] [PATCH v2] build: enable CentOS 8 packaging Alexander V. Tikhonov
@ 2019-11-08 15:22 ` Alexander Turenko
  2019-11-08 15:28   ` Alexander Tikhonov
  2019-11-08 16:20   ` Igor Munkin
  0 siblings, 2 replies; 6+ messages in thread
From: Alexander Turenko @ 2019-11-08 15:22 UTC (permalink / raw)
  To: Alexander V. Tikhonov; +Cc: tarantool-patches, tarantool-patches

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@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
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Tarantool-patches] [PATCH v2] build: enable CentOS 8 packaging
  2019-11-08 15:22 ` Alexander Turenko
@ 2019-11-08 15:28   ` Alexander Tikhonov
  2019-11-08 16:20   ` Igor Munkin
  1 sibling, 0 replies; 6+ messages in thread
From: Alexander Tikhonov @ 2019-11-08 15:28 UTC (permalink / raw)
  To: Alexander Turenko; +Cc: tarantool-patches, tarantool-patches

[-- Attachment #1: Type: text/plain, Size: 13003 bytes --]

Alexander,

I'm completely LGTM with the patch, please push it.


>Пятница,  8 ноября 2019, 18:22 +03:00 от Alexander Turenko <alexander.turenko@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@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

[-- Attachment #2: Type: text/html, Size: 16474 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Tarantool-patches] [PATCH v2] build: enable CentOS 8 packaging
  2019-11-08 15:22 ` Alexander Turenko
  2019-11-08 15:28   ` Alexander Tikhonov
@ 2019-11-08 16:20   ` Igor Munkin
  2019-11-08 17:34     ` Alexander Turenko
  1 sibling, 1 reply; 6+ messages in thread
From: Igor Munkin @ 2019-11-08 16:20 UTC (permalink / raw)
  To: Alexander Turenko; +Cc: tarantool-patches, tarantool-patches

Sasha Ti. and Sasha Tu.,

Thanks, though I left some minor comments below the polished patch is
totally LGTM.

Sasha Tu., feel free to proceed.

On 08.11.19, Alexander Turenko wrote:
> 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

Minor: build label seems to be more suitable for this changeset.

>  |
>  | 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.
>  |

Minor: feel free to mention a co-author here cause you both made a huge
work on this issue.

>  | 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@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
> > 

-- 
Best regards,
IM

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Tarantool-patches] [PATCH v2] build: enable CentOS 8 packaging
  2019-11-08 16:20   ` Igor Munkin
@ 2019-11-08 17:34     ` Alexander Turenko
  0 siblings, 0 replies; 6+ messages in thread
From: Alexander Turenko @ 2019-11-08 17:34 UTC (permalink / raw)
  To: Igor Munkin; +Cc: tarantool-patches, tarantool-patches

On Fri, Nov 08, 2019 at 07:20:09PM +0300, Igor Munkin wrote:
> Sasha Ti. and Sasha Tu.,
> 
> Thanks, though I left some minor comments below the polished patch is
> totally LGTM.
> 
> Sasha Tu., feel free to proceed.

Thanks for review!

Pushed to master, 2.2, 2.1 and 1.10.

CCed Kirill.

> > 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
> 
> Minor: build label seems to be more suitable for this changeset.

Okay. Changed to 'build: add CentOS 8 into CI / CD'.

> >  | 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.

> Minor: feel free to mention a co-author here cause you both made a huge
> work on this issue.

Don't sure how to differentiate situations when I should add
'Reviewed-by' and when 'Co-developed-by'. Added the former, because my
changes were made in response to Alexander's request to review his
patch.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Tarantool-patches] [PATCH v2] build: enable CentOS 8 packaging
@ 2019-11-04 18:41 Alexander V. Tikhonov
  0 siblings, 0 replies; 6+ messages in thread
From: Alexander V. Tikhonov @ 2019-11-04 18:41 UTC (permalink / raw)
  To: Alexander Turenko; +Cc: tarantool-patches, tarantool-patches

Enabled packages build rules for CentOS 8.

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

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

Added CentOS 8 into regular testing at gitlab-ci and travis-ci.

Closes #4543
---

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        | 15 +++++++++++++--
 test/app-tap/pwd.skipcond | 11 +++++++++++
 4 files changed, 33 insertions(+), 2 deletions(-)
 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..196e10c60 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)
+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 %{with backtrace}
+%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} \
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
+except:
+    pass
+
+# vim: set ft=python:
-- 
2.17.1

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-11-08 17:34 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-05  4:10 [Tarantool-patches] [PATCH v2] build: enable CentOS 8 packaging Alexander V. Tikhonov
2019-11-08 15:22 ` Alexander Turenko
2019-11-08 15:28   ` Alexander Tikhonov
2019-11-08 16:20   ` Igor Munkin
2019-11-08 17:34     ` Alexander Turenko
  -- strict thread matches above, loose matches on Subject: below --
2019-11-04 18:41 Alexander V. Tikhonov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox