<div dir="ltr">Hello!<br><br>Here is a new fix for building curses/ncurses library. Curses uses<br>terminfo db (<a href="https://linux.die.net/man/5/terminfo">https://linux.die.net/man/5/terminfo</a>). Previous version<br>of static-build patch didn't link with terminfo database and this led<br>to the problem that backspace and arrows doesn't work at tarantool<br>terminal. Now this behaviour is fixed  by setting search paths of<br>terminfo db.<br><br>Diff:<br><br>diff --git a/static-build/CMakeLists.txt b/static-build/CMakeLists.txt<br>index ecfdd0455..d07cae176 100644<br>--- a/static-build/CMakeLists.txt<br>+++ b/static-build/CMakeLists.txt<br>@@ -82,9 +82,18 @@ ExternalProject_Add(ncurses<br>         # necessary for correct work of FindCurses.cmake module (this module is<br>         # builtin at cmake package) which used in cmake/FindReadline.cmake<br>         --enable-overwrite<br>-        --with-termlib          # enable building libtinfo to prevent linking<br>-                                # with libtinfo from system directories<br>-    INSTALL_COMMAND ${CMAKE_MAKE_PROGRAM} install.libs<br>+<br>+        # enable building libtinfo to prevent linking with libtinfo from system<br>+        # directories<br>+        --with-termlib<br>+<br>+        # set search paths for terminfo db<br>+        --with-terminfo-dirs=/lib/terminfo:/usr/share/terminfo:/etc/terminfo<br>+<br>+        # disable install created terminfo db, use db from system<br>+        --disable-db-install<br>+        --without-progs<br>+        --without-manpages<br> )<br> <br> #<br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">пт, 28 авг. 2020 г. в 00:43, Alexandr Barulev <<a href="mailto:huston.mavr@gmail.com">huston.mavr@gmail.com</a>>:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr">Hello, thanks for the new iteration of review!<br><br>I deleted unused box.lua script and reference to it in suite.ini<br>at static-build tests. I added comments about explicitly setting <br>compilers at static-build/CMakeLists.txt.<br><br>Also I fixed building libncurses/libcurses for correct work of <br>FindCurses.cmake module. After this change there is no need to set <br>CURSES_NEED_NCURSES at cmake/FindReadline.cmake, so I checkouted <br>cmake/FindReadline.cmake from master and update patch commit massage.<br><br>And fixed .<a href="http://travis.mk" target="_blank">travis.mk</a> static_build_cmake_* jobs - build tarantool<br>with -DCMAKE_BUILD_TYPE=RelWithDebInfo cmake option.<br><br><br>Here is a new commit message (deleted part about CURSES_NEED_NCURSES):<br><br>build: refactor static build process<br><br>Refactored static build process to use static-build/CMakeLists.txt<br>instead of Dockerfile.staticbuild (this allows to support static<br>build on macOS). Following third-party dependencies for static build<br>are installed via cmake `ExternalProject_Add`:<br>    - OpenSSL<br>    - Zlib<br>    - Ncurses<br>    - Readline<br>    - Unwind<br>    - ICU<br><br>* Added support static build for macOS<br>* Fixed `CONFIGURE_COMMAND` while building bundled libcurl for staic<br>    build at file cmake/BuildLibCURL.cmake:<br>    - disable building shared libcurl libraries (by setting<br>        `--disable-shared` option)<br>    - disable hiding libcurl symbols (by setting<br>        `--disable-symbol-hiding` option)<br>    - prevent linking libcurl with system libz by settign<br>        `--with-zlib=${FOUND_ZLIB_ROOT_DIR}` option)<br>* Removed Dockerfile.staticbuild<br>* Added new <a href="http://gitlab.ci" target="_blank">gitlab.ci</a> jobs to test new style static build:<br>    - static_build_cmake_linux<br>    - static_build_cmake_osx_15<br>* Removed static_docker_build <a href="http://gitlab.ci" target="_blank">gitlab.ci</a> job<br><br>Closes #5095<br><br><div><br></div><div>Here is a link to branch: <a href="https://github.com/tarantool/tarantool/tree/rosik/refactor-static-build" target="_blank">https://github.com/tarantool/tarantool/tree/rosik/refactor-static-build</a></div><div><br></div><br>And here is a diff:<br><br> diff --git a/.<a href="http://travis.mk" target="_blank">travis.mk</a> b/.<a href="http://travis.mk" target="_blank">travis.mk</a><br>index 482672429..ccd9d6db1 100644<br>--- a/.<a href="http://travis.mk" target="_blank">travis.mk</a><br>+++ b/.<a href="http://travis.mk" target="_blank">travis.mk</a><br>@@ -151,7 +151,8 @@ test_static_build: deps_debian_static<br> # New static build<br> # builddir used in this target - is a default build path from cmake ExternalProject_Add()<br> test_static_build_cmake_linux:<br>-     cd static-build && cmake . && make -j && ctest -V<br>+    cd static-build && cmake -DCMAKE_TARANTOOL_ARGS="-DCMAKE_BUILD_TYPE=RelWithDebInfo;-DENABLE_WERROR=ON" . && \<br>+      make -j && ctest -V<br>  cd test && /usr/bin/python test-run.py --force \<br>             --builddir ${PWD}/static-build/tarantool-prefix/src/tarantool-build $(TEST_RUN_EXTRA_PARAMS)<br> <br>@@ -234,7 +235,8 @@ base_deps_osx:<br> <br> # builddir used in this target - is a default build path from cmake ExternalProject_Add()<br> test_static_build_cmake_osx: base_deps_osx<br>-  cd static-build && cmake . && make -j && ctest -V<br>+    cd static-build && cmake -DCMAKE_TARANTOOL_ARGS="-DCMAKE_BUILD_TYPE=RelWithDebInfo;-DENABLE_WERROR=ON" . && \<br>+      make -j && ctest -V<br>  ${INIT_TEST_ENV_OSX}; \<br>      cd test && ./test-run.py --vardir /tmp/tnt \<br>                 --builddir ${PWD}/static-build/tarantool-prefix/src/tarantool-build \<br>diff --git a/cmake/FindReadline.cmake b/cmake/FindReadline.cmake<br>index afe480679..c48bdcb3e 100644<br>--- a/cmake/FindReadline.cmake<br>+++ b/cmake/FindReadline.cmake<br>@@ -14,17 +14,7 @@ if(BUILD_STATIC)<br>     if (NOT CURSES_INFO_LIBRARY)<br>         set(CURSES_INFO_LIBRARY "")<br>     endif()<br>-<br>-    # From Modules/FindCurses.cmake:<br>-    # Set ``CURSES_NEED_NCURSES`` to ``TRUE`` before the<br>-    # ``find_package(Curses)`` call if NCurses functionality is required.<br>-    # This flag is set for linking with required library (installed<br>-    # via static-build/CMakeLists.txt). If this variable won't be set<br>-    # then tarantool binary links with system library curses which is an<br>-    # entire copy of ncurses<br>-    set(CURSES_NEED_NCURSES TRUE)<br> endif()<br>-<br> find_package(Curses)<br> if(NOT CURSES_FOUND)<br>     find_package(Termcap)<br>diff --git a/static-build/CMakeLists.txt b/static-build/CMakeLists.txt<br>index d90a642e6..ecfdd0455 100644<br>--- a/static-build/CMakeLists.txt<br>+++ b/static-build/CMakeLists.txt<br>@@ -9,6 +9,12 @@ set(NCURSES_VERSION 6.2)<br> set(READLINE_VERSION 8.0)<br> set(UNWIND_VERSION 1.3-rc1)<br> <br>+# Set compilers explicitly for further configuring dependencies with<br>+# these compilers. This gonna solve libicu building problem in case when<br>+# at dependency configure stage no compiler specified and clang compiler<br>+# exists on linux machine, libicu sources would be compiled with clang<br>+# while for other dependencies (including tarantool) gcc would be used.<br>+# This behaviour causes problem at tarantool linkage stage.<br> if (APPLE)<br>     find_program(C_COMPILER clang)<br>     find_program(CXX_COMPILER clang++)<br>@@ -70,6 +76,15 @@ ExternalProject_Add(ncurses<br>         CXX=${CMAKE_CXX_COMPILER}<br>         <SOURCE_DIR>/configure<br>         --prefix=<INSTALL_DIR><br>+<br>+        # This flag enables creation of libcurses.a as a symlink to libncurses.a<br>+        # and disables subdir creation `ncurses` at  <install_dir>/include. It is<br>+        # necessary for correct work of FindCurses.cmake module (this module is<br>+        # builtin at cmake package) which used in cmake/FindReadline.cmake<br>+        --enable-overwrite<br>+        --with-termlib          # enable building libtinfo to prevent linking<br>+                                # with libtinfo from system directories<br>+    INSTALL_COMMAND ${CMAKE_MAKE_PROGRAM} install.libs<br> )<br> <br> #<br>diff --git a/static-build/test/static-build/box.lua b/static-build/test/static-build/box.lua<br>deleted file mode 100755<br>index bad8a9055..000000000<br>--- a/static-build/test/static-build/box.lua<br>+++ /dev/null<br>@@ -1,3 +0,0 @@<br>-#!/usr/bin/env tarantool<br>-<br>-require('console').listen(os.getenv('ADMIN'))<br>diff --git a/static-build/test/static-build/suite.ini b/static-build/test/static-build/suite.ini<br>index 92e349466..5aabadd92 100644<br>--- a/static-build/test/static-build/suite.ini<br>+++ b/static-build/test/static-build/suite.ini<br>@@ -1,5 +1,4 @@<br> [default]<br> core = app<br> description = Static build tests<br>-script = box.lua<br> is_parallel = True<br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">вт, 25 авг. 2020 г. в 17:51, Alexander Turenko <<a href="mailto:alexander.turenko@tarantool.org" target="_blank">alexander.turenko@tarantool.org</a>>:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">I looked very briefly (not thoroughly at all) on this iteration.<br>
<br>
There is nothing that confuses me (except few tiny comments below).<br>
<br>
I hope Igor will do thorough review.<br>
<br>
WBR, Alexander Turenko.<br>
<br>
> +if (APPLE)<br>
> +    find_program(C_COMPILER clang)<br>
> +    find_program(CXX_COMPILER clang++)<br>
> +else()<br>
> +    find_program(C_COMPILER gcc)<br>
> +    find_program(CXX_COMPILER g++)<br>
> +endif()<br>
<br>
Can we just leave it default?<br>
<br>
In offline discussion Alexandr B. said that tarantool builds with gcc,<br>
but icu with clang that gives some problem.<br>
<br>
Possible solution is to pass ${CMAKE_C_COMPILER} (and CXX too where<br>
necessary) to a subproject as we do for c-ares and curl. It seems it is<br>
already done, so maybe it worth to re-check whether it solves the<br>
problem.<br>
<br>
Anyway, if we really need to set a compiler here explicitly, I don't<br>
mind. Just noted that this way is somewhat unusual as I see.<br>
<br>
> diff --git a/static-build/test/static-build/box.lua b/static-build/test/static-build/box.lua<br>
> new file mode 100755<br>
> index 000000000..bad8a9055<br>
> --- /dev/null<br>
> +++ b/static-build/test/static-build/box.lua<br>
> @@ -0,0 +1,3 @@<br>
> +#!/usr/bin/env tarantool<br>
> +<br>
> +require('console').listen(os.getenv('ADMIN'))<br>
<br>
Is looks redundant, see the comment below.<br>
<br>
> diff --git a/static-build/test/static-build/suite.ini b/static-build/test/static-build/suite.ini<br>
> new file mode 100644<br>
> index 000000000..92e349466<br>
> --- /dev/null<br>
> +++ b/static-build/test/static-build/suite.ini<br>
> @@ -0,0 +1,5 @@<br>
> +[default]<br>
> +core = app<br>
> +description = Static build tests<br>
> +script = box.lua<br>
> +is_parallel = True<br>
<br>
'script' does not have sense for 'core = app' test, it is for 'core =<br>
tarantool' tests.<br>
</blockquote></div></div>
</blockquote></div>