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

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

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

Hello, thank you for the review!

Here are required fixes:

1. Fixed typos at commit message, here is a new:

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 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 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

2. Fixed commentaries at .travis.mk

3. Fixed finding libz library at cmake/BuildLibCurl.cmake as at master
   branch

4. Instead of using static-build/.gitignore use the root .gitignore file

5. Refactored static-build/CMakeLists.txt:
* Project name is set to tarantool-static
* Instead of explicitly finding of c/c++ compilers current version uses
cmake `project(tarantool-static C CXX)` command. This change requires
to manually set CFLAGS/CPPFLAGS/LDFLAGS for all static-build
dependencies (because building dependencies at macOS requires path
to macOS SDK) the idea was taken from cmake/BuildLibCurl.cmake
* Added commentaries about problem with libicu
* Removed unused TEST_COMMAND at ExternalProject_Add(zlib ...) also
removed unnecessary # STEP_TARGETS at ExternalProject_Add(readline ...)
* Deleted doubled whitespaces

6. Fixed static-build/README.md:
* Added list of required tools
* Added example for ubuntu/debian
* Fixed indentation and typos

7. Refactored tests:
* Got rid of test-run
* Deleted whitespaces at exports.test.lua
* Fixed defining of RTLD_DEFAULT


Here is a diff:

diff --git a/.gitignore b/.gitignore
index a42c7db35..c6e261e18 100644
--- a/.gitignore
+++ b/.gitignore
@@ -151,3 +151,11 @@ src/box/sql/parse.h
 src/box/sql/parse.c
 src/box/sql/opcodes.h
 src/box/sql/opcodes.c
+
+# static-build
+static-build/*-prefix
+static-build/Makefile
+static-build/build
+
+# macOS files
+.DS_Store
diff --git a/.travis.mk b/.travis.mk
index ccd9d6db1..58d0c1596 100644
--- a/.travis.mk
+++ b/.travis.mk
@@ -149,7 +149,8 @@ test_static_build: deps_debian_static
  CMAKE_EXTRA_PARAMS=-DBUILD_STATIC=ON make -f .travis.mk
test_debian_no_deps

 # New static build
-# builddir used in this target - is a default build path from cmake
ExternalProject_Add()
+# builddir used in this target - is a default build path from cmake
+# ExternalProject_Add()
 test_static_build_cmake_linux:
  cd static-build && cmake
-DCMAKE_TARANTOOL_ARGS="-DCMAKE_BUILD_TYPE=RelWithDebInfo;-DENABLE_WERROR=ON"
. && \
  make -j && ctest -V
@@ -233,7 +234,8 @@ base_deps_osx:
  brew install --force ${STATIC_OSX_PKGS} || brew upgrade ${STATIC_OSX_PKGS}
  pip install --force-reinstall -r test-run/requirements.txt

-# builddir used in this target - is a default build path from cmake
ExternalProject_Add()
+# 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
-DCMAKE_TARANTOOL_ARGS="-DCMAKE_BUILD_TYPE=RelWithDebInfo;-DENABLE_WERROR=ON"
. && \
  make -j && ctest -V
diff --git a/cmake/BuildLibCURL.cmake b/cmake/BuildLibCURL.cmake
index 365c14284..86fec39e9 100644
--- a/cmake/BuildLibCURL.cmake
+++ b/cmake/BuildLibCURL.cmake
@@ -8,10 +8,11 @@ macro(curl_build)
     find_path(ZLIB_INCLUDE_DIR zlib.h)
     message(STATUS "Looking for zlib.h - ${ZLIB_INCLUDE_DIR}")
     if (BUILD_STATIC)
-        find_library(LIBZ_LIBRARY NAMES libz.a)
+        set(LIBZ_LIB_NAME libz.a)
     else()
-        find_library(LIBZ_LIBRARY NAMES z)
+        set(LIBZ_LIB_NAME z)
     endif()
+    find_library(LIBZ_LIBRARY NAMES ${LIBZ_LIB_NAME})
     message(STATUS "Looking for libz - ${LIBZ_LIBRARY}")

     if (NOT ZLIB_INCLUDE_DIR OR NOT LIBZ_LIBRARY)
diff --git a/static-build/.gitignore b/static-build/.gitignore
deleted file mode 100644
index c8028a870..000000000
--- a/static-build/.gitignore
+++ /dev/null
@@ -1,4 +0,0 @@
-*-prefix
-/Makefile
-/test/var
-/build
diff --git a/static-build/CMakeLists.txt b/static-build/CMakeLists.txt
index d07cae176..90029fdd8 100644
--- a/static-build/CMakeLists.txt
+++ b/static-build/CMakeLists.txt
@@ -1,6 +1,11 @@
 cmake_minimum_required(VERSION 2.8)

-project(tarantool-env NONE)
+# Detect system compilers for further configuring dependencies to be
+# builded with these compilers. This is used to build tarantool and
+# it's dependencies by usign one compiler system (for example libicu
+# by default uses clang if it exists when others uses gcc/g++ on
+# linux machine).
+project(tarantool-static C CXX)

 include(ExternalProject)
 set(OPENSSL_VERSION 1.1.1f)
@@ -9,21 +14,24 @@ 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.
+# Pass -isysroot=<SDK_PATH> option on Mac OS to a preprocessor and a C
+# compiler to find header files installed with an SDK.
+#
+# The idea is to set these (DEPENDENCY_*) variables to corresponding
+# environment variables at each depenency configure script.
+#
+# Note: Passing of CPPFLAGS / CFLAGS explicitly discards using of
+# corresponsing environment variables. So pass empty LDFLAGS to discard
+# using of corresponding environment variable. It is possible that a
+# linker flag assumes that some compilation flag is set. We don't pass
+# CFLAGS from environment, so we should not do it for LDFLAGS too.
+set(DEPENDENCY_CFLAGS "")
+set(DEPENDENCY_CPPFLAGS "")
+set(DEPENDENCY_LDFLAGS)
 if (APPLE)
-    find_program(C_COMPILER clang)
-    find_program(CXX_COMPILER clang++)
-else()
-    find_program(C_COMPILER gcc)
-    find_program(CXX_COMPILER g++)
+    set(DEPENDENCY_CFLAGS   "${CMAKE_C_SYSROOT_FLAG} ${CMAKE_OSX_SYSROOT}")
+    set(DEPENDENCY_CPPFLAGS "${CMAKE_C_SYSROOT_FLAG} ${CMAKE_OSX_SYSROOT}")
 endif()
-set(CMAKE_C_COMPILER ${C_COMPILER})
-set(CMAKE_CXX_COMPILER ${CXX_COMPILER})

 # Install all libraries required by tarantool at current build dir

@@ -32,9 +40,13 @@ set(CMAKE_CXX_COMPILER ${CXX_COMPILER})
 #
 ExternalProject_Add(openssl
     URL https://www.openssl.org/source/openssl-${OPENSSL_VERSION}.tar.gz
-    CONFIGURE_COMMAND CC=${CMAKE_C_COMPILER}
+    CONFIGURE_COMMAND <SOURCE_DIR>/config
+        CC=${CMAKE_C_COMPILER}
         CXX=${CMAKE_CXX_COMPILER}
-        <SOURCE_DIR>/config
+        CFLAGS=${DEPENDENCY_CFLAGS}
+        CPPFLAGS=${DEPENDENCY_CPPFLAGS}
+        LDFLAGS=${DEPENDENCY_LDFLAGS}
+
         --prefix=<INSTALL_DIR>
         --libdir=lib
         no-shared
@@ -46,9 +58,21 @@ ExternalProject_Add(openssl
 #
 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}
+    # By default libicu is built by using clang/clang++ compiler (if it
+    # exists). Here is a link for detecting compilers at libicu configure
+    # script:
https://github.com/unicode-org/icu/blob/7c7b8bd5702310b972f888299169bc3cc88bf0a6/icu4c/source/configure.ac#L135
+    # This will cause the problem on linux machine: tarantool is built
+    # with gcc/g++ and libicu is built with clang/clang++ (if it exists)
+    # so at linking stage `rellocation` errors will occur. To solve this,
+    # we can set CC/CXX to CMAKE_C_COMPILER/CMAKE_CXX_COMPILER variables
+    # manually which are detected above (by cmake `project()` command)
+    CONFIGURE_COMMAND <SOURCE_DIR>/source/configure
+        CC=${CMAKE_C_COMPILER}
         CXX=${CMAKE_CXX_COMPILER}
-        <SOURCE_DIR>/source/configure
+        CFLAGS=${DEPENDENCY_CFLAGS}
+        CPPFLAGS=${DEPENDENCY_CPPFLAGS}
+        LDFLAGS=${DEPENDENCY_LDFLAGS}
+
         --with-data-packaging=static
         --prefix=<INSTALL_DIR>
         --disable-shared
@@ -60,11 +84,14 @@ ExternalProject_Add(icu
 #
 ExternalProject_Add(zlib
     URL https://zlib.net/zlib-${ZLIB_VERSION}.tar.gz
-    CONFIGURE_COMMAND CC=${CMAKE_C_COMPILER}
+    CONFIGURE_COMMAND env
+        CC=${CMAKE_C_COMPILER}
+        CFLAGS=${DEPENDENCY_CFLAGS}
+        CPPFLAGS=${DEPENDENCY_CPPFLAGS}
+        LDFLAGS=${DEPENDENCY_LDFLAGS}
         <SOURCE_DIR>/configure
         --prefix=<INSTALL_DIR>
         --static
-    TEST_COMMAND ${CMAKE_MAKE_PROGRAM} check
 )

 #
@@ -72,13 +99,17 @@ ExternalProject_Add(zlib
 #
 ExternalProject_Add(ncurses
     URL https://ftp.gnu.org/gnu/ncurses/ncurses-${NCURSES_VERSION}.tar.gz
-    CONFIGURE_COMMAND CC=${CMAKE_C_COMPILER}
+    CONFIGURE_COMMAND <SOURCE_DIR>/configure
+        CC=${CMAKE_C_COMPILER}
         CXX=${CMAKE_CXX_COMPILER}
-        <SOURCE_DIR>/configure
+        CFLAGS=${DEPENDENCY_CFLAGS}
+        CPPFLAGS=${DEPENDENCY_CPPFLAGS}
+        LDFLAGS=${DEPENDENCY_LDFLAGS}
+
         --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
+        # 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
@@ -101,11 +132,14 @@ ExternalProject_Add(ncurses
 #
 ExternalProject_Add(readline
     URL
https://ftp.gnu.org/gnu/readline/readline-${READLINE_VERSION}.tar.gz
-    CONFIGURE_COMMAND CC=${CMAKE_C_COMPILER}
-        <SOURCE_DIR>/configure
+    CONFIGURE_COMMAND <SOURCE_DIR>/configure
+        CC=${CMAKE_C_COMPILER}
+        CFLAGS=${DEPENDENCY_CFLAGS}
+        CPPFLAGS=${DEPENDENCY_CPPFLAGS}
+        LDFLAGS=${DEPENDENCY_LDFLAGS}
+
         --prefix=<INSTALL_DIR>
         --disable-shared
-    # STEP_TARGETS download
 )

 #
@@ -114,8 +148,12 @@ ExternalProject_Add(readline
 if (APPLE)
     ExternalProject_Add(iconv
         URL https://ftp.gnu.org/pub/gnu/libiconv/libiconv-1.16.tar.gz
-        CONFIGURE_COMMAND CC=${CMAKE_C_COMPILER}
-            <SOURCE_DIR>/configure
+        CONFIGURE_COMMAND <SOURCE_DIR>/configure
+            CC=${CMAKE_C_COMPILER}
+            CFLAGS=${DEPENDENCY_CFLAGS}
+            CPPFLAGS=${DEPENDENCY_CPPFLAGS}
+            LDFLAGS=${DEPENDENCY_LDFLAGS}
+
             --prefix=<INSTALL_DIR>
             --disable-shared
             --enable-static
@@ -203,9 +241,12 @@ if (APPLE)
 else()
     ExternalProject_Add(unwind
         URL
https://download.savannah.nongnu.org/releases/libunwind/libunwind-${UNWIND_VERSION}.tar.gz
-        CONFIGURE_COMMAND CC=${CMAKE_C_COMPILER}
+        CONFIGURE_COMMAND <SOURCE_DIR>/configure
+            CC=${CMAKE_C_COMPILER}
             CXX=${CMAKE_CXX_COMPILER}
-            <SOURCE_DIR>/configure
+            CFLAGS=${DEPENDENCY_CFLAGS}
+            CPPFLAGS=${DEPENDENCY_CPPFLAGS}
+            LDFLAGS=${DEPENDENCY_LDFLAGS}
             --prefix=<INSTALL_DIR>
             --disable-shared
             --enable-static
@@ -258,7 +299,13 @@ add_test(
 )

 add_test(
-    NAME test-run-static
-    COMMAND ./test-run.py --builddir ${TARANTOOL_BINARY_DIR}
-    WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/test
+    NAME check-exports
+    COMMAND ${TARANTOOL_BINARY_DIR}/src/tarantool
+            ${CMAKE_CURRENT_SOURCE_DIR}/test/static-build/exports.test.lua
+)
+
+add_test(
+    NAME check-traceback
+    COMMAND ${TARANTOOL_BINARY_DIR}/src/tarantool
+
 ${CMAKE_CURRENT_SOURCE_DIR}/test/static-build/traceback.test.lua
 )
diff --git a/static-build/README.md b/static-build/README.md
index 0019e963f..2219b5026 100644
--- a/static-build/README.md
+++ b/static-build/README.md
@@ -5,6 +5,29 @@ statically. And builds it.

 ## Prerequisites

+Please install following tools and libraries that will
+be necessary for building and testing:
+* git
+* A C/C++ compiler.
+
+  Ordinarily, this is gcc and g++ version 4.6 or later. On Mac OS X, this
+  is Clang version 3.2+.
+* cmake
+* autoconf automake libtool
+* make
+* Python and modules.
+
+  Python interpreter is not necessary for building Tarantool itself,
unless you
+  intend to use the “Run the test suite". For all platforms, this is python
+  version 2.7+ (but not 3.x). You need the following Python modules:
+  * pyyaml
+  * argparse
+  * msgpack-python
+  * gevent
+  * six
+
+### Here is an examples for your OS:
+
 CentOS:

 ```bash
@@ -13,11 +36,20 @@ yum install -y \
     python-msgpack python-yaml python-argparse python-six python-gevent
 ```

+Ubuntu/Debian:
+
+```bash
+apt-get install -y \
+    build-essential cmake make coreutils autoconf automake libtool sed \
+    python python-pip python-setuptools python-dev \
+    python-msgpack python-yaml python-argparse python-six python-gevent
+```
+
 MacOS:

 Before you start please install default Xcode Tools by Apple:

-```
+```bash
 sudo xcode-select --install
 sudo xcode-select -switch /Applications/Xcode.app/Contents/Developer
 ```
@@ -28,11 +60,11 @@ Install brew using command from
 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
+brew install autoconf automake libtool cmake
file://$${PWD}/tools/brew_taps/tntpython2.rbs
+pip install --force-reinstall -r test-run/requirements.txt
 ```

-### Usage
+## Usage

 ```bash
 cmake .
@@ -46,7 +78,7 @@ If you want to customise build, you need to set
`CMAKE_TARANTOOL_ARGS` variable

 ### Usage

-There is three types of `CMAKE_BUILD_TYPE`:
+There are three types of `CMAKE_BUILD_TYPE`:
 * Debug - default
 * Release
 * RelWithDebInfo
diff --git a/static-build/test/static-build/exports.test.lua
b/static-build/test/static-build/exports.test.lua
index 63dc163a9..de54973d8 100755
--- a/static-build/test/static-build/exports.test.lua
+++ b/static-build/test/static-build/exports.test.lua
@@ -8,19 +8,13 @@ ffi.cdef([[

 local test = tap.test('exports')

-
-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
+--   Find the first occurrence of the desired symbol using the default
+--   shared object search order. The search will include global symbols
 --   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
+local RTLD_DEFAULT = ffi.cast("void *", jit.os == "OSX" and -2LL or 0LL)

 local function check_symbol(sym)
     test:ok(ffi.C.dlsym(RTLD_DEFAULT, sym) ~= nil, ('Symbol %q
found'):format(sym))
diff --git a/static-build/test/static-build/suite.ini
b/static-build/test/static-build/suite.ini
deleted file mode 100644
index 5aabadd92..000000000
--- a/static-build/test/static-build/suite.ini
+++ /dev/null
@@ -1,4 +0,0 @@
-[default]
-core = app
-description = Static build tests
-is_parallel = True
diff --git a/static-build/test/test-run.py b/static-build/test/test-run.py
deleted file mode 120000
index 02187cdc5..000000000
--- a/static-build/test/test-run.py
+++ /dev/null
@@ -1 +0,0 @@
-../../test-run/test-run.py
\ No newline at end of file


пт, 4 сент. 2020 г. в 20:52, Igor Munkin <imun@tarantool.org>:

> 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@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
>

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