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} /configure --prefix= + + # This flag enables creation of libcurses.a as a symlink to libncurses.a + # and disables subdir creation `ncurses` at /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. >