Re: [Tarantool-patches] [PATCH v2] build: refactor static build process

[Alexandr Barulev <huston.mavr@gmail.com>]

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

Hello, thanks for the new iteration of review!

I deleted unused box.lua script and reference to it in suite.ini
at static-build tests. I added comments about explicitly setting
compilers at static-build/CMakeLists.txt.

Also I fixed building libncurses/libcurses for correct work of
FindCurses.cmake module. After this change there is no need to set
CURSES_NEED_NCURSES at cmake/FindReadline.cmake, so I checkouted
cmake/FindReadline.cmake from master and update patch commit massage.

And fixed .travis.mk static_build_cmake_* jobs - build tarantool
with -DCMAKE_BUILD_TYPE=RelWithDebInfo cmake option.


Here is a new commit message (deleted part about CURSES_NEED_NCURSES):

build: refactor static build process

Refactored static build process to use static-build/CMakeLists.txt
instead of Dockerfile.staticbuild (this allows to support static
build on macOS). Following third-party dependencies for static build
are installed via cmake `ExternalProject_Add`:
    - OpenSSL
    - Zlib
    - Ncurses
    - Readline
    - Unwind
    - ICU

* Added support static build for macOS
* Fixed `CONFIGURE_COMMAND` while building bundled libcurl for staic
    build at file cmake/BuildLibCURL.cmake:
    - disable building shared libcurl libraries (by setting
        `--disable-shared` option)
    - disable hiding libcurl symbols (by setting
        `--disable-symbol-hiding` option)
    - prevent linking libcurl with system libz by settign
        `--with-zlib=${FOUND_ZLIB_ROOT_DIR}` option)
* Removed Dockerfile.staticbuild
* Added new gitlab.ci jobs to test new style static build:
    - static_build_cmake_linux
    - static_build_cmake_osx_15
* Removed static_docker_build gitlab.ci job

Closes #5095


Here is a link to branch:
https://github.com/tarantool/tarantool/tree/rosik/refactor-static-build


And here is a diff:

 diff --git a/.travis.mk b/.travis.mk
index 482672429..ccd9d6db1 100644
--- a/.travis.mk
+++ b/.travis.mk
@@ -151,7 +151,8 @@ test_static_build: deps_debian_static
 # New static build
 # builddir used in this target - is a default build path from cmake
ExternalProject_Add()
 test_static_build_cmake_linux:
- cd static-build && cmake . && make -j && ctest -V
+ cd static-build && cmake
-DCMAKE_TARANTOOL_ARGS="-DCMAKE_BUILD_TYPE=RelWithDebInfo;-DENABLE_WERROR=ON"
. && \
+ make -j && ctest -V
  cd test && /usr/bin/python test-run.py --force \
  --builddir ${PWD}/static-build/tarantool-prefix/src/tarantool-build
$(TEST_RUN_EXTRA_PARAMS)

@@ -234,7 +235,8 @@ base_deps_osx:

 # builddir used in this target - is a default build path from cmake
ExternalProject_Add()
 test_static_build_cmake_osx: base_deps_osx
- cd static-build && cmake . && make -j && ctest -V
+ cd static-build && cmake
-DCMAKE_TARANTOOL_ARGS="-DCMAKE_BUILD_TYPE=RelWithDebInfo;-DENABLE_WERROR=ON"
. && \
+ make -j && ctest -V
  ${INIT_TEST_ENV_OSX}; \
  cd test && ./test-run.py --vardir /tmp/tnt \
  --builddir ${PWD}/static-build/tarantool-prefix/src/tarantool-build \
diff --git a/cmake/FindReadline.cmake b/cmake/FindReadline.cmake
index afe480679..c48bdcb3e 100644
--- a/cmake/FindReadline.cmake
+++ b/cmake/FindReadline.cmake
@@ -14,17 +14,7 @@ if(BUILD_STATIC)
     if (NOT CURSES_INFO_LIBRARY)
         set(CURSES_INFO_LIBRARY "")
     endif()
-
-    # From Modules/FindCurses.cmake:
-    # Set ``CURSES_NEED_NCURSES`` to ``TRUE`` before the
-    # ``find_package(Curses)`` call if NCurses functionality is required.
-    # This flag is set for linking with required library (installed
-    # via static-build/CMakeLists.txt). If this variable won't be set
-    # then tarantool binary links with system library curses which is an
-    # entire copy of ncurses
-    set(CURSES_NEED_NCURSES TRUE)
 endif()
-
 find_package(Curses)
 if(NOT CURSES_FOUND)
     find_package(Termcap)
diff --git a/static-build/CMakeLists.txt b/static-build/CMakeLists.txt
index d90a642e6..ecfdd0455 100644
--- a/static-build/CMakeLists.txt
+++ b/static-build/CMakeLists.txt
@@ -9,6 +9,12 @@ set(NCURSES_VERSION 6.2)
 set(READLINE_VERSION 8.0)
 set(UNWIND_VERSION 1.3-rc1)

+# Set compilers explicitly for further configuring dependencies with
+# these compilers. This gonna solve libicu building problem in case when
+# at dependency configure stage no compiler specified and clang compiler
+# exists on linux machine, libicu sources would be compiled with clang
+# while for other dependencies (including tarantool) gcc would be used.
+# This behaviour causes problem at tarantool linkage stage.
 if (APPLE)
     find_program(C_COMPILER clang)
     find_program(CXX_COMPILER clang++)
@@ -70,6 +76,15 @@ ExternalProject_Add(ncurses
         CXX=${CMAKE_CXX_COMPILER}
         <SOURCE_DIR>/configure
         --prefix=<INSTALL_DIR>
+
+        # This flag enables creation of libcurses.a as a symlink to
libncurses.a
+        # and disables subdir creation `ncurses` at
 <install_dir>/include. It is
+        # necessary for correct work of FindCurses.cmake module (this
module is
+        # builtin at cmake package) which used in cmake/FindReadline.cmake
+        --enable-overwrite
+        --with-termlib          # enable building libtinfo to prevent
linking
+                                # with libtinfo from system directories
+    INSTALL_COMMAND ${CMAKE_MAKE_PROGRAM} install.libs
 )

 #
diff --git a/static-build/test/static-build/box.lua
b/static-build/test/static-build/box.lua
deleted file mode 100755
index bad8a9055..000000000
--- a/static-build/test/static-build/box.lua
+++ /dev/null
@@ -1,3 +0,0 @@
-#!/usr/bin/env tarantool
-
-require('console').listen(os.getenv('ADMIN'))
diff --git a/static-build/test/static-build/suite.ini
b/static-build/test/static-build/suite.ini
index 92e349466..5aabadd92 100644
--- a/static-build/test/static-build/suite.ini
+++ b/static-build/test/static-build/suite.ini
@@ -1,5 +1,4 @@
 [default]
 core = app
 description = Static build tests
-script = box.lua
 is_parallel = True

вт, 25 авг. 2020 г. в 17:51, Alexander Turenko <
alexander.turenko@tarantool.org>:

> I looked very briefly (not thoroughly at all) on this iteration.
>
> There is nothing that confuses me (except few tiny comments below).
>
> I hope Igor will do thorough review.
>
> WBR, Alexander Turenko.
>
> > +if (APPLE)
> > +    find_program(C_COMPILER clang)
> > +    find_program(CXX_COMPILER clang++)
> > +else()
> > +    find_program(C_COMPILER gcc)
> > +    find_program(CXX_COMPILER g++)
> > +endif()
>
> Can we just leave it default?
>
> In offline discussion Alexandr B. said that tarantool builds with gcc,
> but icu with clang that gives some problem.
>
> Possible solution is to pass ${CMAKE_C_COMPILER} (and CXX too where
> necessary) to a subproject as we do for c-ares and curl. It seems it is
> already done, so maybe it worth to re-check whether it solves the
> problem.
>
> Anyway, if we really need to set a compiler here explicitly, I don't
> mind. Just noted that this way is somewhat unusual as I see.
>
> > diff --git a/static-build/test/static-build/box.lua
> b/static-build/test/static-build/box.lua
> > new file mode 100755
> > index 000000000..bad8a9055
> > --- /dev/null
> > +++ b/static-build/test/static-build/box.lua
> > @@ -0,0 +1,3 @@
> > +#!/usr/bin/env tarantool
> > +
> > +require('console').listen(os.getenv('ADMIN'))
>
> Is looks redundant, see the comment below.
>
> > diff --git a/static-build/test/static-build/suite.ini
> b/static-build/test/static-build/suite.ini
> > new file mode 100644
> > index 000000000..92e349466
> > --- /dev/null
> > +++ b/static-build/test/static-build/suite.ini
> > @@ -0,0 +1,5 @@
> > +[default]
> > +core = app
> > +description = Static build tests
> > +script = box.lua
> > +is_parallel = True
>
> 'script' does not have sense for 'core = app' test, it is for 'core =
> tarantool' tests.
>

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