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

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

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

Hello!

Here is a new fix for building curses/ncurses library. Curses uses
terminfo db (https://linux.die.net/man/5/terminfo). Previous version
of static-build patch didn't link with terminfo database and this led
to the problem that backspace and arrows doesn't work at tarantool
terminal. Now this behaviour is fixed  by setting search paths of
terminfo db.

Diff:

diff --git a/static-build/CMakeLists.txt b/static-build/CMakeLists.txt
index ecfdd0455..d07cae176 100644
--- a/static-build/CMakeLists.txt
+++ b/static-build/CMakeLists.txt
@@ -82,9 +82,18 @@ ExternalProject_Add(ncurses
         # 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
+
+        # enable building libtinfo to prevent linking with libtinfo from
system
+        # directories
+        --with-termlib
+
+        # set search paths for terminfo db
+
 --with-terminfo-dirs=/lib/terminfo:/usr/share/terminfo:/etc/terminfo
+
+        # disable install created terminfo db, use db from system
+        --disable-db-install
+        --without-progs
+        --without-manpages
 )

 #

пт, 28 авг. 2020 г. в 00:43, Alexandr Barulev <huston.mavr@gmail.com>:

> 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: 11679 bytes --]