[Tarantool-patches] [PATCH v2] build: refactor static build process
Igor Munkin
imun at tarantool.org
Fri Sep 4 20:41:43 MSK 2020
Hello,
Thanks for the patch! It's strange to me why this patch is not reviewed
by Sasha Ti., though there are many changes in the build and testing
infrastructure. Nevertheless, please consider my comments below.
On 25.08.20, HustonMmmavr wrote:
> From: Yaroslav Dynnikov <yaroslav.dynnikov at gmail.com>
>
> 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
> * Prevented linking tarantool binary with system libcurses (which is
> entire copy of libncurses) by setting flag `CURSES_NEED_NCURSES`
> to TRUE at file cmake/FindReadline.cmake. (This flag defined at
> FindCurses.cmake module and it's a workaround for linking with
> correct library)
> * Fixed `CONFIGURE_COMMAND` while building bundled libcurl for staic
Typo: s/staic/static/.
> 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
Typo: s/settign/setting/.
> `--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
> ---
> .gitlab-ci.yml | 11 +-
> .travis.mk | 52 +++-
> Dockerfile.staticbuild | 98 -------
> cmake/BuildLibCURL.cmake | 18 +-
> cmake/FindReadline.cmake | 10 +
> cmake/compiler.cmake | 24 +-
> cmake/os.cmake | 5 +-
> static-build/.gitignore | 4 +
> static-build/CMakeLists.txt | 240 ++++++++++++++++++
> static-build/README.md | 58 +++++
> static-build/test/CheckDependencies.cmake | 43 ++++
> static-build/test/static-build/box.lua | 3 +
> .../test/static-build/exports.test.lua | 148 +++++++++++
> static-build/test/static-build/suite.ini | 5 +
> .../test/static-build/traceback.test.lua | 15 ++
> static-build/test/test-run.py | 1 +
> 16 files changed, 608 insertions(+), 127 deletions(-)
> delete mode 100644 Dockerfile.staticbuild
> create mode 100644 static-build/.gitignore
> create mode 100644 static-build/CMakeLists.txt
> create mode 100644 static-build/README.md
> create mode 100644 static-build/test/CheckDependencies.cmake
> create mode 100755 static-build/test/static-build/box.lua
> create mode 100755 static-build/test/static-build/exports.test.lua
> create mode 100644 static-build/test/static-build/suite.ini
> create mode 100755 static-build/test/static-build/traceback.test.lua
> create mode 120000 static-build/test/test-run.py
>
<snipped>
> diff --git a/.travis.mk b/.travis.mk
> index efc05cf05..482672429 100644
> --- a/.travis.mk
> +++ b/.travis.mk
> @@ -148,8 +148,12 @@ deps_debian_static:
> test_static_build: deps_debian_static
> CMAKE_EXTRA_PARAMS=-DBUILD_STATIC=ON make -f .travis.mk test_debian_no_deps
>
> -test_static_docker_build:
> - docker build --no-cache --network=host --build-arg RUN_TESTS=ON -f Dockerfile.staticbuild .
> +# New static build
> +# builddir used in this target - is a default build path from cmake ExternalProject_Add()
The comment exceeds 80 symbols.
> +test_static_build_cmake_linux:
<snipped>
> diff --git a/cmake/BuildLibCURL.cmake b/cmake/BuildLibCURL.cmake
> index 5f8b15a63..365c14284 100644
> --- a/cmake/BuildLibCURL.cmake
> +++ b/cmake/BuildLibCURL.cmake
> @@ -4,15 +4,20 @@ macro(curl_build)
<snipped>
> if (BUILD_STATIC)
> - set(LIBZ_LIB_NAME libz.a)
> + find_library(LIBZ_LIBRARY NAMES libz.a)
> else()
> - set(LIBZ_LIB_NAME z)
> + find_library(LIBZ_LIBRARY NAMES z)
> endif()
> - find_library(LIBZ_LIBRARY NAMES ${LIBZ_LIB_NAME})
Well... What is the difference from the previous approach?
> - if ("${LIBZ_LIBRARY}" STREQUAL "LIBZ_LIBRARY-NOTFOUND")
> + message(STATUS "Looking for libz - ${LIBZ_LIBRARY}")
> +
<snipped>
> diff --git a/static-build/.gitignore b/static-build/.gitignore
> new file mode 100644
> index 000000000..c8028a870
> --- /dev/null
> +++ b/static-build/.gitignore
Minor: Why did you introduce a separate .gitignore instead of adjusting
the root one?
> @@ -0,0 +1,4 @@
<snipped>
> diff --git a/static-build/CMakeLists.txt b/static-build/CMakeLists.txt
> new file mode 100644
> index 000000000..d90a642e6
> --- /dev/null
> +++ b/static-build/CMakeLists.txt
> @@ -0,0 +1,240 @@
> +cmake_minimum_required(VERSION 2.8)
> +
> +project(tarantool-env NONE)
Minor: Why is the name tarantool-env and not e.g. tarantool-static?
> +
<snipped>
> +
> +if (APPLE)
> + find_program(C_COMPILER clang)
> + find_program(CXX_COMPILER clang++)
> +else()
> + find_program(C_COMPILER gcc)
> + find_program(CXX_COMPILER g++)
> +endif()
> +set(CMAKE_C_COMPILER ${C_COMPILER})
> +set(CMAKE_CXX_COMPILER ${CXX_COMPILER})
I see you've added a comment for this passage, *but* at least for me
Sasha's comment is still left unaddressed. AFAICS Sasha asked whether we
can solve the linkage issue by explicitly setting CMAKE_C{,XX}_COMPILER
variables. If the problem isn't fixed this way, then I guess your
solution is fine.
> +
> +# Install all libraries required by tarantool at current build dir
> +
<snipped>
> +#
> +# ICU
> +#
> +ExternalProject_Add(icu
> + URL https://github.com/unicode-org/icu/releases/download/release-62-1/icu4c-62_1-src.tgz
> + CONFIGURE_COMMAND CC=${CMAKE_C_COMPILER}
> + CXX=${CMAKE_CXX_COMPILER}
OK, I see, you've already set the variables I mentioned above. Anyway,
please mention this fact if it doesn't fix the linkage issue (maybe
refer the corresponding place in libicu configure / the issue / etc).
> + <SOURCE_DIR>/source/configure
> + --with-data-packaging=static
> + --prefix=<INSTALL_DIR>
> + --disable-shared
> + --enable-static
> +)
> +
> +#
> +# ZLIB
> +#
> +ExternalProject_Add(zlib
> + URL https://zlib.net/zlib-${ZLIB_VERSION}.tar.gz
> + CONFIGURE_COMMAND CC=${CMAKE_C_COMPILER}
> + <SOURCE_DIR>/configure
> + --prefix=<INSTALL_DIR>
> + --static
> + TEST_COMMAND ${CMAKE_MAKE_PROGRAM} check
This TEST_COMMAND line looks odd and I guess it need some commenting
with the rationale, since other commands don't have it.
> +)
> +
<snipped>
> +
> +#
> +# ReadLine
> +#
> +ExternalProject_Add(readline
> + URL https://ftp.gnu.org/gnu/readline/readline-${READLINE_VERSION}.tar.gz
> + CONFIGURE_COMMAND CC=${CMAKE_C_COMPILER}
> + <SOURCE_DIR>/configure
> + --prefix=<INSTALL_DIR>
> + --disable-shared
> + # STEP_TARGETS download
Looks like this line can be dropped.
> +)
> +
<snipped>
> diff --git a/static-build/README.md b/static-build/README.md
> new file mode 100644
> index 000000000..0019e963f
> --- /dev/null
> +++ b/static-build/README.md
> @@ -0,0 +1,58 @@
> +# Tarantool static build tooling
> +
> +These files help to prepare environment for building Tarantool
> +statically. And builds it.
> +
> +## Prerequisites
> +
> +CentOS:
Minor: Does this mean that other distros are not supported?
> +
> +```bash
> +yum install -y \
> + git perl gcc cmake make gcc-c++ libstdc++-static autoconf automake libtool \
> + python-msgpack python-yaml python-argparse python-six python-gevent
> +```
> +
> +MacOS:
> +
> +Before you start please install default Xcode Tools by Apple:
> +
> +```
Typo: bash specifier is missing (considering verbatim blocks below).
> +sudo xcode-select --install
> +sudo xcode-select -switch /Applications/Xcode.app/Contents/Developer
> +```
> +
> +Install brew using command from
> +[Homebrew repository instructions](https://github.com/Homebrew/inst)
> +
> +After that run next script:
> +
> +```bash
> + brew install autoconf automake libtool cmake file://$${PWD}/tools/brew_taps/tntpython2.rbs
> + pip install --force-reinstall -r test-run/requirements.txt
Typo: Looks like misindentation.
> +```
> +
> +### Usage
Typo: I guess it should be h2, not h3.
> +
> +```bash
> +cmake .
> +make -j
> +ctest -V
> +```
> +
> +## Customize your build
> +
> +If you want to customise build, you need to set `CMAKE_TARANTOOL_ARGS` variable
> +
> +### Usage
> +
> +There is three types of `CMAKE_BUILD_TYPE`:
Typo: s/is/are/.
> +* Debug - default
> +* Release
> +* RelWithDebInfo
> +
> +And you want to build tarantool with RelWithDebInfo:
> +
> +```bash
> +cmake -DCMAKE_TARANTOOL_ARGS="-DCMAKE_BUILD_TYPE=RelWithDebInfo" .
> +```
<snipped>
> diff --git a/static-build/test/static-build/exports.test.lua b/static-build/test/static-build/exports.test.lua
> new file mode 100755
> index 000000000..63dc163a9
> --- /dev/null
> +++ b/static-build/test/static-build/exports.test.lua
> @@ -0,0 +1,148 @@
<snipped>
> +
> +local RTLD_DEFAULT
> +-- See `man 3 dlsym`:
> +-- RTLD_DEFAULT
> +-- Find the first occurrence of the desired symbol using the default
> +-- shared object search order. The search will include global symbols
Typo: There are several places above with doubled whitespace.
> +-- in the executable and its dependencies, as well as symbols in shared
> +-- objects that were dynamically loaded with the RTLD_GLOBAL flag.
> +if jit.os == "OSX" then
> + RTLD_DEFAULT = ffi.cast("void *", -2LL)
> +else
> + RTLD_DEFAULT = ffi.cast("void *", 0LL)
> +end
Minor: OK, since the difference is only in the constant value, the
following way looks more convenient to me:
| local RTLD_DEFAULT = ffi.cast("void *", jit.os == "OSX" and -2LL or 0LL)
> +
<snipped>
> --
> 2.26.2
>
--
Best regards,
IM
More information about the Tarantool-patches
mailing list