[Tarantool-patches] [PATCH v1] build: implement SuSE build with testing

[Tarantool-patches] [PATCH v1] build: implement SuSE build with testing

[Alexander V. Tikhonov]

Added makefile to build the SuSE based packages.
Implemented build with testing for opensuse images:
SuSE 15.0, 15.1, 15.2

Tarantool spec file changed with:
Added checks of %{sle_version} according to
https://en.opensuse.org/openSUSE:Packaging_for_Leap#RPM_Distro_Version_Macros

Found that opensuse adding linker flag like '--no-undefined' which
produces the fails on building test modules at app-tap/app/box-tap
suites. Decided to block this flag.

Running tests by make test, because opensuse uses out-of-source
build by default.
Set complete testing except replication suite.
Test box-tap/cfg fails and temporary blocked according to the
issue created for it #4594.

Closes #4562
---

Github: https://github.com/tarantool/tarantool/tree/avtikhon/gh-4562-suse-pack-full-ci
Issue: https://github.com/tarantool/tarantool/issues/4562

 rpm/tarantool.spec          | 26 ++++++++++++++++++++------
 test/app-tap/CMakeLists.txt |  1 +
 test/app/CMakeLists.txt     |  1 +
 test/box-tap/cfg.skipcond   |  6 ++++++
 test/box/CMakeLists.txt     |  1 +
 5 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/rpm/tarantool.spec b/rpm/tarantool.spec
index ff95ed646..09de015e2 100644
--- a/rpm/tarantool.spec
+++ b/rpm/tarantool.spec
@@ -1,5 +1,5 @@
 # Enable systemd for on RHEL >= 7 and Fedora >= 15
-%if (0%{?fedora} >= 15 || 0%{?rhel} >= 7)
+%if (0%{?fedora} >= 15 || 0%{?rhel} >= 7 || 0%{?sle_version} >= 1500)
 %bcond_without systemd
 %else
 %bcond_with systemd
@@ -7,7 +7,7 @@
 
 BuildRequires: cmake >= 2.8
 BuildRequires: make
-%if (0%{?fedora} >= 22 || 0%{?rhel} >= 7)
+%if (0%{?fedora} >= 22 || 0%{?rhel} >= 7 || 0%{?sle_version} >= 1500)
 # RHEL 6 requires devtoolset
 BuildRequires: gcc >= 4.5
 BuildRequires: gcc-c++ >= 4.5
@@ -70,7 +70,7 @@ BuildRequires: libunwind-devel
 %endif
 
 # For tests
-%if (0%{?fedora} >= 22 || 0%{?rhel} == 7)
+%if (0%{?fedora} >= 22 || 0%{?rhel} == 7 || 0%{?sle_version} >= 1500)
 BuildRequires: python >= 2.7
 BuildRequires: python-six >= 1.9.0
 BuildRequires: python-gevent >= 1.0
@@ -102,7 +102,7 @@ Requires: /etc/services
 # Deps for built-in package manager
 # https://github.com/tarantool/tarantool/issues/2612
 Requires: openssl
-%if (0%{?fedora} >= 22 || 0%{?rhel} >= 8)
+%if (0%{?fedora} >= 22 || 0%{?rhel} >= 8 || 0%{?sle_version} >= 1500)
 # RHEL <= 7 doesn't support Recommends:
 Recommends: tarantool-devel
 Recommends: git-core
@@ -138,7 +138,14 @@ C and Lua/C modules.
 
 %build
 # RHBZ #1301720: SYSCONFDIR an LOCALSTATEDIR must be specified explicitly
-%cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo \
+%if (0%{?sle_version} >= 1500)
+# SuSE uses only out-of-source builds in its macros
+%cmake .. \
+         -DCMAKE_INSTALL_PREFIX:PATH=/usr \
+%else
+%cmake . \
+%endif
+	 -DCMAKE_BUILD_TYPE=RelWithDebInfo \
          -DCMAKE_INSTALL_LOCALSTATEDIR:PATH=%{_localstatedir} \
          -DCMAKE_INSTALL_SYSCONFDIR:PATH=%{_sysconfdir} \
 %if %{with backtrace}
@@ -154,20 +161,27 @@ C and Lua/C modules.
          -DENABLE_DIST:BOOL=ON
 make %{?_smp_mflags}
 
+%if (0%{?sle_version} == 0)
 %install
+%endif
 %make_install
 # %%doc and %%license macroses are used instead
 rm -rf %{buildroot}%{_datarootdir}/doc/tarantool/
 
 %check
-%if (0%{?fedora} >= 22 || 0%{?rhel} >= 7)
+%if (0%{?fedora} >= 22 || 0%{?rhel} >= 7 || 0%{?sle_version} >= 1500)
 # https://github.com/tarantool/tarantool/issues/1227
 echo "self.skip = True" > ./test/app/socket.skipcond
 # https://github.com/tarantool/tarantool/issues/1322
 echo "self.skip = True" > ./test/app/digest.skipcond
 # run a safe subset of the test suite
+%if (0%{?sle_version} >= 1500)
+# SuSE uses only out-of-source builds always at build/ subdir in its macros
+cd build && TEST_RUN_EXCLUDE='replication/' make test-force
+%else
 cd test && ./test-run.py --force -j 1 unit/ app/ app-tap/ box/ box-tap/ engine/ vinyl/
 %endif
+%endif
 
 %pre
 /usr/sbin/groupadd -r tarantool > /dev/null 2>&1 || :
diff --git a/test/app-tap/CMakeLists.txt b/test/app-tap/CMakeLists.txt
index ee67cf533..68aa3690b 100644
--- a/test/app-tap/CMakeLists.txt
+++ b/test/app-tap/CMakeLists.txt
@@ -1 +1,2 @@
+SET(CMAKE_SHARED_LINKER_FLAGS "")
 build_module(module_api module_api.c)
diff --git a/test/app/CMakeLists.txt b/test/app/CMakeLists.txt
index 059ee8f3d..8bafa6516 100644
--- a/test/app/CMakeLists.txt
+++ b/test/app/CMakeLists.txt
@@ -1 +1,2 @@
+SET(CMAKE_SHARED_LINKER_FLAGS "")
 build_module(loaderslib loaderslib.c)
diff --git a/test/box-tap/cfg.skipcond b/test/box-tap/cfg.skipcond
index 7950a5d93..9d5535f87 100644
--- a/test/box-tap/cfg.skipcond
+++ b/test/box-tap/cfg.skipcond
@@ -1,8 +1,14 @@
+import os
 import platform
 
 # Disabled on FreeBSD due to fail #4271:
 #   Data segment size exceeds process limit
 if platform.system() == 'FreeBSD':
     self.skip = 1
+try:
+    if os.popen('lsb_release -i').read().split(':')[1].strip() == 'openSUSE':
+        self.skip = 1
+except IndexError:
+    pass
 
 # vim: set ft=python:
diff --git a/test/box/CMakeLists.txt b/test/box/CMakeLists.txt
index 06bfbbe9d..e65559e81 100644
--- a/test/box/CMakeLists.txt
+++ b/test/box/CMakeLists.txt
@@ -1,3 +1,4 @@
+SET(CMAKE_SHARED_LINKER_FLAGS "")
 include_directories(${MSGPUCK_INCLUDE_DIRS})
 build_module(function1 function1.c)
 build_module(reload1 reload1.c)
-- 
2.17.1

Re: [Tarantool-patches] [PATCH v1] build: implement SuSE build with testing

[Sergey Bronnikov]

LGTM

On 12:03 Thu 23 Apr , Alexander V. Tikhonov wrote:
> Added makefile to build the SuSE based packages.
> Implemented build with testing for opensuse images:
> SuSE 15.0, 15.1, 15.2
> 
> Tarantool spec file changed with:
> Added checks of %{sle_version} according to
> https://en.opensuse.org/openSUSE:Packaging_for_Leap#RPM_Distro_Version_Macros
> 
> Found that opensuse adding linker flag like '--no-undefined' which
> produces the fails on building test modules at app-tap/app/box-tap
> suites. Decided to block this flag.
> 
> Running tests by make test, because opensuse uses out-of-source
> build by default.
> Set complete testing except replication suite.
> Test box-tap/cfg fails and temporary blocked according to the
> issue created for it #4594.
> 
> Closes #4562
> ---
> 
> Github: https://github.com/tarantool/tarantool/tree/avtikhon/gh-4562-suse-pack-full-ci
> Issue: https://github.com/tarantool/tarantool/issues/4562
> 
>  rpm/tarantool.spec          | 26 ++++++++++++++++++++------
>  test/app-tap/CMakeLists.txt |  1 +
>  test/app/CMakeLists.txt     |  1 +
>  test/box-tap/cfg.skipcond   |  6 ++++++
>  test/box/CMakeLists.txt     |  1 +
>  5 files changed, 29 insertions(+), 6 deletions(-)
> 
> diff --git a/rpm/tarantool.spec b/rpm/tarantool.spec
> index ff95ed646..09de015e2 100644
> --- a/rpm/tarantool.spec
> +++ b/rpm/tarantool.spec
> @@ -1,5 +1,5 @@
>  # Enable systemd for on RHEL >= 7 and Fedora >= 15
> -%if (0%{?fedora} >= 15 || 0%{?rhel} >= 7)
> +%if (0%{?fedora} >= 15 || 0%{?rhel} >= 7 || 0%{?sle_version} >= 1500)
>  %bcond_without systemd
>  %else
>  %bcond_with systemd
> @@ -7,7 +7,7 @@
>  
>  BuildRequires: cmake >= 2.8
>  BuildRequires: make
> -%if (0%{?fedora} >= 22 || 0%{?rhel} >= 7)
> +%if (0%{?fedora} >= 22 || 0%{?rhel} >= 7 || 0%{?sle_version} >= 1500)
>  # RHEL 6 requires devtoolset
>  BuildRequires: gcc >= 4.5
>  BuildRequires: gcc-c++ >= 4.5
> @@ -70,7 +70,7 @@ BuildRequires: libunwind-devel
>  %endif
>  
>  # For tests
> -%if (0%{?fedora} >= 22 || 0%{?rhel} == 7)
> +%if (0%{?fedora} >= 22 || 0%{?rhel} == 7 || 0%{?sle_version} >= 1500)
>  BuildRequires: python >= 2.7
>  BuildRequires: python-six >= 1.9.0
>  BuildRequires: python-gevent >= 1.0
> @@ -102,7 +102,7 @@ Requires: /etc/services
>  # Deps for built-in package manager
>  # https://github.com/tarantool/tarantool/issues/2612
>  Requires: openssl
> -%if (0%{?fedora} >= 22 || 0%{?rhel} >= 8)
> +%if (0%{?fedora} >= 22 || 0%{?rhel} >= 8 || 0%{?sle_version} >= 1500)
>  # RHEL <= 7 doesn't support Recommends:
>  Recommends: tarantool-devel
>  Recommends: git-core
> @@ -138,7 +138,14 @@ C and Lua/C modules.
>  
>  %build
>  # RHBZ #1301720: SYSCONFDIR an LOCALSTATEDIR must be specified explicitly
> -%cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo \
> +%if (0%{?sle_version} >= 1500)
> +# SuSE uses only out-of-source builds in its macros
> +%cmake .. \
> +         -DCMAKE_INSTALL_PREFIX:PATH=/usr \
> +%else
> +%cmake . \
> +%endif
> +	 -DCMAKE_BUILD_TYPE=RelWithDebInfo \
>           -DCMAKE_INSTALL_LOCALSTATEDIR:PATH=%{_localstatedir} \
>           -DCMAKE_INSTALL_SYSCONFDIR:PATH=%{_sysconfdir} \
>  %if %{with backtrace}
> @@ -154,20 +161,27 @@ C and Lua/C modules.
>           -DENABLE_DIST:BOOL=ON
>  make %{?_smp_mflags}
>  
> +%if (0%{?sle_version} == 0)
>  %install
> +%endif
>  %make_install
>  # %%doc and %%license macroses are used instead
>  rm -rf %{buildroot}%{_datarootdir}/doc/tarantool/
>  
>  %check
> -%if (0%{?fedora} >= 22 || 0%{?rhel} >= 7)
> +%if (0%{?fedora} >= 22 || 0%{?rhel} >= 7 || 0%{?sle_version} >= 1500)
>  # https://github.com/tarantool/tarantool/issues/1227
>  echo "self.skip = True" > ./test/app/socket.skipcond
>  # https://github.com/tarantool/tarantool/issues/1322
>  echo "self.skip = True" > ./test/app/digest.skipcond
>  # run a safe subset of the test suite
> +%if (0%{?sle_version} >= 1500)
> +# SuSE uses only out-of-source builds always at build/ subdir in its macros
> +cd build && TEST_RUN_EXCLUDE='replication/' make test-force
> +%else
>  cd test && ./test-run.py --force -j 1 unit/ app/ app-tap/ box/ box-tap/ engine/ vinyl/
>  %endif
> +%endif
>  
>  %pre
>  /usr/sbin/groupadd -r tarantool > /dev/null 2>&1 || :
> diff --git a/test/app-tap/CMakeLists.txt b/test/app-tap/CMakeLists.txt
> index ee67cf533..68aa3690b 100644
> --- a/test/app-tap/CMakeLists.txt
> +++ b/test/app-tap/CMakeLists.txt
> @@ -1 +1,2 @@
> +SET(CMAKE_SHARED_LINKER_FLAGS "")
>  build_module(module_api module_api.c)
> diff --git a/test/app/CMakeLists.txt b/test/app/CMakeLists.txt
> index 059ee8f3d..8bafa6516 100644
> --- a/test/app/CMakeLists.txt
> +++ b/test/app/CMakeLists.txt
> @@ -1 +1,2 @@
> +SET(CMAKE_SHARED_LINKER_FLAGS "")
>  build_module(loaderslib loaderslib.c)
> diff --git a/test/box-tap/cfg.skipcond b/test/box-tap/cfg.skipcond
> index 7950a5d93..9d5535f87 100644
> --- a/test/box-tap/cfg.skipcond
> +++ b/test/box-tap/cfg.skipcond
> @@ -1,8 +1,14 @@
> +import os
>  import platform
>  
>  # Disabled on FreeBSD due to fail #4271:
>  #   Data segment size exceeds process limit
>  if platform.system() == 'FreeBSD':
>      self.skip = 1
> +try:
> +    if os.popen('lsb_release -i').read().split(':')[1].strip() == 'openSUSE':
> +        self.skip = 1
> +except IndexError:
> +    pass
>  
>  # vim: set ft=python:
> diff --git a/test/box/CMakeLists.txt b/test/box/CMakeLists.txt
> index 06bfbbe9d..e65559e81 100644
> --- a/test/box/CMakeLists.txt
> +++ b/test/box/CMakeLists.txt
> @@ -1,3 +1,4 @@
> +SET(CMAKE_SHARED_LINKER_FLAGS "")
>  include_directories(${MSGPUCK_INCLUDE_DIRS})
>  build_module(function1 function1.c)
>  build_module(reload1 reload1.c)
> -- 
> 2.17.1
> 

-- 
sergeyb@

Re: [Tarantool-patches] [PATCH v1] build: implement SuSE build with testing

[Oleg Piskunov]

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


LGTM.
  
>Четверг, 23 апреля 2020, 12:03 +03:00 от Alexander V. Tikhonov <avtikhon@tarantool.org>:
> 
>Added makefile to build the SuSE based packages.
>Implemented build with testing for opensuse images:
>SuSE 15.0, 15.1, 15.2
>
>Tarantool spec file changed with:
>Added checks of %{sle_version} according to
>https://en.opensuse.org/openSUSE:Packaging_for_Leap#RPM_Distro_Version_Macros
>
>Found that opensuse adding linker flag like '--no-undefined' which
>produces the fails on building test modules at app-tap/app/box-tap
>suites. Decided to block this flag.
>
>Running tests by make test, because opensuse uses out-of-source
>build by default.
>Set complete testing except replication suite.
>Test box-tap/cfg fails and temporary blocked according to the
>issue created for it #4594.
>
>Closes #4562
>---
>
>Github:  https://github.com/tarantool/tarantool/tree/avtikhon/gh-4562-suse-pack-full-ci
>Issue:  https://github.com/tarantool/tarantool/issues/4562
>
> rpm/tarantool.spec | 26 ++++++++++++++++++++------
> test/app-tap/CMakeLists.txt | 1 +
> test/app/CMakeLists.txt | 1 +
> test/box-tap/cfg.skipcond | 6 ++++++
> test/box/CMakeLists.txt | 1 +
> 5 files changed, 29 insertions(+), 6 deletions(-)
>
>diff --git a/rpm/tarantool.spec b/rpm/tarantool.spec
>index ff95ed646..09de015e2 100644
>--- a/rpm/tarantool.spec
>+++ b/rpm/tarantool.spec
>@@ -1,5 +1,5 @@
> # Enable systemd for on RHEL >= 7 and Fedora >= 15
>-%if (0%{?fedora} >= 15 || 0%{?rhel} >= 7)
>+%if (0%{?fedora} >= 15 || 0%{?rhel} >= 7 || 0%{?sle_version} >= 1500)
> %bcond_without systemd
> %else
> %bcond_with systemd
>@@ -7,7 +7,7 @@
> 
> BuildRequires: cmake >= 2.8
> BuildRequires: make
>-%if (0%{?fedora} >= 22 || 0%{?rhel} >= 7)
>+%if (0%{?fedora} >= 22 || 0%{?rhel} >= 7 || 0%{?sle_version} >= 1500)
> # RHEL 6 requires devtoolset
> BuildRequires: gcc >= 4.5
> BuildRequires: gcc-c++ >= 4.5
>@@ -70,7 +70,7 @@ BuildRequires: libunwind-devel
> %endif
> 
> # For tests
>-%if (0%{?fedora} >= 22 || 0%{?rhel} == 7)
>+%if (0%{?fedora} >= 22 || 0%{?rhel} == 7 || 0%{?sle_version} >= 1500)
> BuildRequires: python >= 2.7
> BuildRequires: python-six >= 1.9.0
> BuildRequires: python-gevent >= 1.0
>@@ -102,7 +102,7 @@ Requires: /etc/services
> # Deps for built-in package manager
> #  https://github.com/tarantool/tarantool/issues/2612
> Requires: openssl
>-%if (0%{?fedora} >= 22 || 0%{?rhel} >= 8)
>+%if (0%{?fedora} >= 22 || 0%{?rhel} >= 8 || 0%{?sle_version} >= 1500)
> # RHEL <= 7 doesn't support Recommends:
> Recommends: tarantool-devel
> Recommends: git-core
>@@ -138,7 +138,14 @@ C and Lua/C modules.
> 
> %build
> # RHBZ #1301720: SYSCONFDIR an LOCALSTATEDIR must be specified explicitly
>-%cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo \
>+%if (0%{?sle_version} >= 1500)
>+# SuSE uses only out-of-source builds in its macros
>+%cmake .. \
>+ -DCMAKE_INSTALL_PREFIX:PATH=/usr \
>+%else
>+%cmake . \
>+%endif
>+ -DCMAKE_BUILD_TYPE=RelWithDebInfo \
>          -DCMAKE_INSTALL_LOCALSTATEDIR:PATH=%{_localstatedir} \
>          -DCMAKE_INSTALL_SYSCONFDIR:PATH=%{_sysconfdir} \
> %if %{with backtrace}
>@@ -154,20 +161,27 @@ C and Lua/C modules.
>          -DENABLE_DIST:BOOL=ON
> make %{?_smp_mflags}
> 
>+%if (0%{?sle_version} == 0)
> %install
>+%endif
> %make_install
> # %%doc and %%license macroses are used instead
> rm -rf %{buildroot}%{_datarootdir}/doc/tarantool/
> 
> %check
>-%if (0%{?fedora} >= 22 || 0%{?rhel} >= 7)
>+%if (0%{?fedora} >= 22 || 0%{?rhel} >= 7 || 0%{?sle_version} >= 1500)
> #  https://github.com/tarantool/tarantool/issues/1227
> echo "self.skip = True" > ./test/app/socket.skipcond
> #  https://github.com/tarantool/tarantool/issues/1322
> echo "self.skip = True" > ./test/app/digest.skipcond
> # run a safe subset of the test suite
>+%if (0%{?sle_version} >= 1500)
>+# SuSE uses only out-of-source builds always at build/ subdir in its macros
>+cd build && TEST_RUN_EXCLUDE='replication/' make test-force
>+%else
> cd test && ./test-run.py --force -j 1 unit/ app/ app-tap/ box/ box-tap/ engine/ vinyl/
> %endif
>+%endif
> 
> %pre
> /usr/sbin/groupadd -r tarantool > /dev/null 2>&1 || :
>diff --git a/test/app-tap/CMakeLists.txt b/test/app-tap/CMakeLists.txt
>index ee67cf533..68aa3690b 100644
>--- a/test/app-tap/CMakeLists.txt
>+++ b/test/app-tap/CMakeLists.txt
>@@ -1 +1,2 @@
>+SET(CMAKE_SHARED_LINKER_FLAGS "")
> build_module(module_api module_api.c)
>diff --git a/test/app/CMakeLists.txt b/test/app/CMakeLists.txt
>index 059ee8f3d..8bafa6516 100644
>--- a/test/app/CMakeLists.txt
>+++ b/test/app/CMakeLists.txt
>@@ -1 +1,2 @@
>+SET(CMAKE_SHARED_LINKER_FLAGS "")
> build_module(loaderslib loaderslib.c)
>diff --git a/test/box-tap/cfg.skipcond b/test/box-tap/cfg.skipcond
>index 7950a5d93..9d5535f87 100644
>--- a/test/box-tap/cfg.skipcond
>+++ b/test/box-tap/cfg.skipcond
>@@ -1,8 +1,14 @@
>+import os
> import platform
> 
> # Disabled on FreeBSD due to fail #4271:
> # Data segment size exceeds process limit
> if platform.system() == 'FreeBSD':
>     self.skip = 1
>+try:
>+ if os.popen('lsb_release -i').read().split(':')[1].strip() == 'openSUSE':
>+ self.skip = 1
>+except IndexError:
>+ pass
> 
> # vim: set ft=python:
>diff --git a/test/box/CMakeLists.txt b/test/box/CMakeLists.txt
>index 06bfbbe9d..e65559e81 100644
>--- a/test/box/CMakeLists.txt
>+++ b/test/box/CMakeLists.txt
>@@ -1,3 +1,4 @@
>+SET(CMAKE_SHARED_LINKER_FLAGS "")
> include_directories(${MSGPUCK_INCLUDE_DIRS})
> build_module(function1 function1.c)
> build_module(reload1 reload1.c)
>--
>2.17.1
>  
 
 
--
Oleg Piskunov
 

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

Re: [Tarantool-patches] [PATCH v1] build: implement SuSE build with testing

[Alexander Turenko]

On Thu, Apr 23, 2020 at 12:03:44PM +0300, Alexander V. Tikhonov wrote:
> Added makefile to build the SuSE based packages.

Makefile? Is is about packpack update? It is not actual anymore (since
we decided to update existing makefile). But I would rather just remove
the comment, because it is about packpack patch, not this one.

> Implemented build with testing for opensuse images:
> SuSE 15.0, 15.1, 15.2

Nit: SuSE **Leap** 15.0, 15.1, 15.2.

Are we plan to deploy them to our repositories? I guess a user who asked
for this expects that we'll offer those packages like other ones.

Ouch, I see, the branch already contains deployment rules. So this
comment is not actual anymore.

> Tarantool spec file changed with:
> Added checks of %{sle_version} according to
> https://en.opensuse.org/openSUSE:Packaging_for_Leap#RPM_Distro_Version_Macros

Nit: I think that using of %{sle_version} macro is quite obvious and
does not require a comment here. But up to you.

> 
> Found that opensuse adding linker flag like '--no-undefined' which
> produces the fails on building test modules at app-tap/app/box-tap
> suites. Decided to block this flag.
> 
> Running tests by make test, because opensuse uses out-of-source
> build by default.
> Set complete testing except replication suite.
> Test box-tap/cfg fails and temporary blocked according to the
> issue created for it #4594.

Don't forget to update those explanations according to everything said
below.

> Github: https://github.com/tarantool/tarantool/tree/avtikhon/gh-4562-suse-pack-full-ci
> Issue: https://github.com/tarantool/tarantool/issues/4562

> diff --git a/rpm/tarantool.spec b/rpm/tarantool.spec
> index ff95ed646..09de015e2 100644
> --- a/rpm/tarantool.spec
> +++ b/rpm/tarantool.spec

I give some recommendations how to better handle out-of-source build in
different places, but now another idea comes into my mind. We can just
define `%global %__builddir .`, that's all. (Don't forget to leave a
comment for it in the spec, like 'Force in-source build on openSUSE to
simplify the RPM spec'.)

>  %build
>  # RHBZ #1301720: SYSCONFDIR an LOCALSTATEDIR must be specified explicitly
> -%cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo \
> +%if (0%{?sle_version} >= 1500)
> +# SuSE uses only out-of-source builds in its macros
> +%cmake .. \
> +         -DCMAKE_INSTALL_PREFIX:PATH=/usr \
> +%else
> +%cmake . \
> +%endif
> +	 -DCMAKE_BUILD_TYPE=RelWithDebInfo \
>           -DCMAKE_INSTALL_LOCALSTATEDIR:PATH=%{_localstatedir} \
>           -DCMAKE_INSTALL_SYSCONFDIR:PATH=%{_sysconfdir} \

It would look better I think if we'll just use '%cmake
%{_sourcedir} <...>'. It seems it should work with packpack.

It seems -DCMAKE_INSTALL_PREFIX:PATH=/usr is not needed, %cmake macro
already has it on opensuse-leap-15.2.

But I hope that %__builddir redefinition will work, it would allow to
kepp the RPM spec almost same for different distros.

> +%if (0%{?sle_version} == 0)
>  %install
> +%endif
>  %make_install

Do you want to do `make install` in the %build phase rather than
%install? It feels as not right way to overcome a problem.

I don't see anything that explains this change in the commit message or
in a comment here. How should I review it? Build myself, see a result
and try to guess the reason? It would be the waste of time: you already
did this and could explain.

I have a guess! Is it to keep CWD after 'cd build' in %cmake expansion?
If so, you can just use %cmake_install when it is defined and
%make_install otherwise.

But I hope that redefinition of %__builddir will work.

>  %check
> -%if (0%{?fedora} >= 22 || 0%{?rhel} >= 7)
> +%if (0%{?fedora} >= 22 || 0%{?rhel} >= 7 || 0%{?sle_version} >= 1500)
>  # https://github.com/tarantool/tarantool/issues/1227
>  echo "self.skip = True" > ./test/app/socket.skipcond
>  # https://github.com/tarantool/tarantool/issues/1322
>  echo "self.skip = True" > ./test/app/digest.skipcond
>  # run a safe subset of the test suite
> +%if (0%{?sle_version} >= 1500)
> +# SuSE uses only out-of-source builds always at build/ subdir in its macros
> +cd build && TEST_RUN_EXCLUDE='replication/' make test-force
> +%else
>  cd test && ./test-run.py --force -j 1 unit/ app/ app-tap/ box/ box-tap/ engine/ vinyl/
>  %endif
> +%endif

First, this hunk is different now and the branch should be rebased on
top of current master.

Second, it is better to avoid duplication and use something like:

 | %if 0%{?__builddir:1}
 | cd %__builddir
 | %endif

But I hope %__builddir redefinition will work.

> diff --git a/test/app-tap/CMakeLists.txt b/test/app-tap/CMakeLists.txt
> index ee67cf533..68aa3690b 100644
> --- a/test/app-tap/CMakeLists.txt
> +++ b/test/app-tap/CMakeLists.txt
> @@ -1 +1,2 @@
> +SET(CMAKE_SHARED_LINKER_FLAGS "")

Nit: We usually type 'set()' lowercased in our sources.

After the first glance I was sceptic about redefinition of a cmake
variable that may affect build rules for the whole project. But then I
read about CMake scopes and found that the change affects only this
directory, because add_subdirectory() creates a new variable scope.
Then I asked myself, whether it applicable for cached variables
(CMAKE_SHARED_LINKER_FLAGS is cached). And found that set() will create
a new non-cached variable, which will be used directory-wide instead of
global one and will not affect the global cached value (see [1]). So it
is okay.

But we can do it less intruisive:

 | string(REPLACE "-Wl,--no-undefined" "" CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS}")

Also I think it should be properly commented, like 'The dynamic
libraries will be loaded from tarantool executable and will use symbols
from it. So it is completely okay to have unresolved symbols at build
time'.

BTW, you should also update luajit submodule in the same way; new tests
are added here:

- test/gh-4427-ffi-sandwich/CMakeLists.txt
- test/lj-flush-on-trace/CMakeLists.txt

[1]: https://cmake.org/cmake/help/v3.0/command/set.html

> diff --git a/test/box-tap/cfg.skipcond b/test/box-tap/cfg.skipcond
> index 7950a5d93..9d5535f87 100644
> --- a/test/box-tap/cfg.skipcond
> +++ b/test/box-tap/cfg.skipcond
> @@ -1,8 +1,14 @@
> +import os
>  import platform
>  
>  # Disabled on FreeBSD due to fail #4271:
>  #   Data segment size exceeds process limit
>  if platform.system() == 'FreeBSD':
>      self.skip = 1
> +try:
> +    if os.popen('lsb_release -i').read().split(':')[1].strip() == 'openSUSE':
> +        self.skip = 1
> +except IndexError:
> +    pass
>  
>  # vim: set ft=python:

You should place the issue number (#4594) here.

Neither this file, nor the issue give any idea what is wrong with the
test. We cannot disable a test just because it fails: why we writing
them so? We should at least ensure that it is the testing problem and we
can provide openSUSE packages and say that tarantool works on openSUSE
without critical problems.

The test is disabled on FreeBSD and here there is the certain reason. It
looks like related to os.execute() and so we can state something about
FreeBSD support in tarantool: it works, but there are problems around
forking processes from tarantool.

Re: [Tarantool-patches] [PATCH v1] build: implement SuSE build with testing

[Alexander V. Tikhonov]

Hi Alexander, thanks a lot for the deep review, I've made all
your suggestions.

On Sun, Jul 05, 2020 at 03:37:36AM +0300, Alexander Turenko wrote:
> On Thu, Apr 23, 2020 at 12:03:44PM +0300, Alexander V. Tikhonov wrote:
> > Added makefile to build the SuSE based packages.
> 
> Makefile? Is is about packpack update? It is not actual anymore (since
> we decided to update existing makefile). But I would rather just remove
> the comment, because it is about packpack patch, not this one.
>

Right, removed.

> > Implemented build with testing for opensuse images:
> > SuSE 15.0, 15.1, 15.2
> 
> Nit: SuSE **Leap** 15.0, 15.1, 15.2.
>

Corrected.

> Are we plan to deploy them to our repositories? I guess a user who asked
> for this expects that we'll offer those packages like other ones.
> 
> Ouch, I see, the branch already contains deployment rules. So this
> comment is not actual anymore.
>

Ok.

> > Tarantool spec file changed with:
> > Added checks of %{sle_version} according to
> > https://en.opensuse.org/openSUSE:Packaging_for_Leap#RPM_Distro_Version_Macros
> 
> Nit: I think that using of %{sle_version} macro is quite obvious and
> does not require a comment here. But up to you.
> 

Right, I'd like to save the link on %{sle_version} for fast search.

> > 
> > Found that opensuse adding linker flag like '--no-undefined' which
> > produces the fails on building test modules at app-tap/app/box-tap
> > suites. Decided to block this flag.
> > 
> > Running tests by make test, because opensuse uses out-of-source
> > build by default.
> > Set complete testing except replication suite.
> > Test box-tap/cfg fails and temporary blocked according to the
> > issue created for it #4594.
> 
> Don't forget to update those explanations according to everything said
> below.
> 

Sure, I've updated it as suggested.

> > Github: https://github.com/tarantool/tarantool/tree/avtikhon/gh-4562-suse-pack-full-ci
> > Issue: https://github.com/tarantool/tarantool/issues/4562
> 
> > diff --git a/rpm/tarantool.spec b/rpm/tarantool.spec
> > index ff95ed646..09de015e2 100644
> > --- a/rpm/tarantool.spec
> > +++ b/rpm/tarantool.spec
> 
> I give some recommendations how to better handle out-of-source build in
> different places, but now another idea comes into my mind. We can just
> define `%global %__builddir .`, that's all. (Don't forget to leave a
> comment for it in the spec, like 'Force in-source build on openSUSE to
> simplify the RPM spec'.)
> 

Ok, I've used your suggested macro definition and all my changes below
became unneeded.

> >  %build
> >  # RHBZ #1301720: SYSCONFDIR an LOCALSTATEDIR must be specified explicitly
> > -%cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo \
> > +%if (0%{?sle_version} >= 1500)
> > +# SuSE uses only out-of-source builds in its macros
> > +%cmake .. \
> > +         -DCMAKE_INSTALL_PREFIX:PATH=/usr \
> > +%else
> > +%cmake . \
> > +%endif
> > +	 -DCMAKE_BUILD_TYPE=RelWithDebInfo \
> >           -DCMAKE_INSTALL_LOCALSTATEDIR:PATH=%{_localstatedir} \
> >           -DCMAKE_INSTALL_SYSCONFDIR:PATH=%{_sysconfdir} \
> 
> It would look better I think if we'll just use '%cmake
> %{_sourcedir} <...>'. It seems it should work with packpack.
> 
> It seems -DCMAKE_INSTALL_PREFIX:PATH=/usr is not needed, %cmake macro
> already has it on opensuse-leap-15.2.
> 
> But I hope that %__builddir redefinition will work, it would allow to
> kepp the RPM spec almost same for different distros.
> 

Right not need any more - removed.

> > +%if (0%{?sle_version} == 0)
> >  %install
> > +%endif
> >  %make_install
> 
> Do you want to do `make install` in the %build phase rather than
> %install? It feels as not right way to overcome a problem.
> 
> I don't see anything that explains this change in the commit message or
> in a comment here. How should I review it? Build myself, see a result
> and try to guess the reason? It would be the waste of time: you already
> did this and could explain.
> 
> I have a guess! Is it to keep CWD after 'cd build' in %cmake expansion?
> If so, you can just use %cmake_install when it is defined and
> %make_install otherwise.
> 
> But I hope that redefinition of %__builddir will work.
> 

Right not need any more - removed.

> >  %check
> > -%if (0%{?fedora} >= 22 || 0%{?rhel} >= 7)
> > +%if (0%{?fedora} >= 22 || 0%{?rhel} >= 7 || 0%{?sle_version} >= 1500)
> >  # https://github.com/tarantool/tarantool/issues/1227
> >  echo "self.skip = True" > ./test/app/socket.skipcond
> >  # https://github.com/tarantool/tarantool/issues/1322
> >  echo "self.skip = True" > ./test/app/digest.skipcond
> >  # run a safe subset of the test suite
> > +%if (0%{?sle_version} >= 1500)
> > +# SuSE uses only out-of-source builds always at build/ subdir in its macros
> > +cd build && TEST_RUN_EXCLUDE='replication/' make test-force
> > +%else
> >  cd test && ./test-run.py --force -j 1 unit/ app/ app-tap/ box/ box-tap/ engine/ vinyl/
> >  %endif
> > +%endif
> 
> First, this hunk is different now and the branch should be rebased on
> top of current master.
> 
> Second, it is better to avoid duplication and use something like:
> 
>  | %if 0%{?__builddir:1}
>  | cd %__builddir
>  | %endif
> 
> But I hope %__builddir redefinition will work.
> 

Right not need any more - removed.

> > diff --git a/test/app-tap/CMakeLists.txt b/test/app-tap/CMakeLists.txt
> > index ee67cf533..68aa3690b 100644
> > --- a/test/app-tap/CMakeLists.txt
> > +++ b/test/app-tap/CMakeLists.txt
> > @@ -1 +1,2 @@
> > +SET(CMAKE_SHARED_LINKER_FLAGS "")
> 
> Nit: We usually type 'set()' lowercased in our sources.
> 
> After the first glance I was sceptic about redefinition of a cmake
> variable that may affect build rules for the whole project. But then I
> read about CMake scopes and found that the change affects only this
> directory, because add_subdirectory() creates a new variable scope.
> Then I asked myself, whether it applicable for cached variables
> (CMAKE_SHARED_LINKER_FLAGS is cached). And found that set() will create
> a new non-cached variable, which will be used directory-wide instead of
> global one and will not affect the global cached value (see [1]). So it
> is okay.
> 
> But we can do it less intruisive:
> 
>  | string(REPLACE "-Wl,--no-undefined" "" CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS}")
> 

Sure, I've changed it as you suggested.

> Also I think it should be properly commented, like 'The dynamic
> libraries will be loaded from tarantool executable and will use symbols
> from it. So it is completely okay to have unresolved symbols at build
> time'.
> 
> BTW, you should also update luajit submodule in the same way; new tests
> are added here:
> 
> - test/gh-4427-ffi-sandwich/CMakeLists.txt
> - test/lj-flush-on-trace/CMakeLists.txt
>

Sure, updated too.

> [1]: https://cmake.org/cmake/help/v3.0/command/set.html
> 
> > diff --git a/test/box-tap/cfg.skipcond b/test/box-tap/cfg.skipcond
> > index 7950a5d93..9d5535f87 100644
> > --- a/test/box-tap/cfg.skipcond
> > +++ b/test/box-tap/cfg.skipcond
> > @@ -1,8 +1,14 @@
> > +import os
> >  import platform
> >  
> >  # Disabled on FreeBSD due to fail #4271:
> >  #   Data segment size exceeds process limit
> >  if platform.system() == 'FreeBSD':
> >      self.skip = 1
> > +try:
> > +    if os.popen('lsb_release -i').read().split(':')[1].strip() == 'openSUSE':
> > +        self.skip = 1
> > +except IndexError:
> > +    pass
> >  
> >  # vim: set ft=python:
> 
> You should place the issue number (#4594) here.
> 

Right, added.

> Neither this file, nor the issue give any idea what is wrong with the
> test. We cannot disable a test just because it fails: why we writing
> them so? We should at least ensure that it is the testing problem and we
> can provide openSUSE packages and say that tarantool works on openSUSE
> without critical problems.
> 
> The test is disabled on FreeBSD and here there is the certain reason. It
> looks like related to os.execute() and so we can state something about
> FreeBSD support in tarantool: it works, but there are problems around
> forking processes from tarantool.

Updated issue, added comment to commit message.