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

[Mavr Huston <huston.mavr@gmail.com>]

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

Hi, thanks for the review!

libicu installs as ExternalProject_Add too, its missed in commit message;

Problem with curses and ncurses was on macOS and linux, because libcurses
is an entire copy of libncurses, and tarantool links with system
libcurses instead of libncurses installed as tarantool dependency, but
module FindCurses.cmkae provides workaround for this problem -
CURSES_NEED_NCURSES flag.- to use ncurses instead of curses. (i will fix
this part at commit message)

About disable-shred flag, used at libcurl building - we want to link only
with
static libraries, so we prevent creating unused .so.

I've renamed static_build_no_deps_* jobs after review to
static_build_cmake_*

Also about such path tarantool-prefix/* - it's a cmake ExternalProject_Add()
default path (i've also added comment at .travis.mk)

Useless comments "Init macOS test env" deleted.

>      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})
Here we simplified code, by deleting useless variable.

I've added commentaries to cmake/compiler.cmake about libunwind on macOS
and about ignoring flag -static-libstdc++ on macOS

I've fixed static-build for using system compiler:  gcc/g++ on linux
and clang/clang++ on macOS

I've refactored IF (NOT APPLE) condition to IF (APPLE) at
static-build/CMakeLists.txt

I've mentioned macOS dependencies at static-build/README.md xcode-tools and
others, also I've added example with CMAKE_TARANTOOL_ARGS.

Added commentaries about _EP_INSTALL_DIR at static-build/CMakeLists.txt

Also deleted unused use_unix_sockets_iproto = True

Also deleted curl-features.test.lua, because after rebase on master it
fails,
due to missing curl_version_info symbol at tarantool binary. This symbol
lost
after #807c7fa584f21ee955b2a14623d70f7510a3650d (build: update curl
submodule
to 7.71.1 version )


After pass the review I'll squash this changes to base commit and update
commit
message.

Here is a diff of changes:
=====================================

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 5ec5dd9b9..c9aef3dc7 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -534,14 +534,14 @@ static_build:
   script:
     - ${GITLAB_MAKE} test_static_build

-static_build_no_deps_linux:
+static_build_cmake_linux:
   <<: *docker_test_definition
   script:
-    - ${GITLAB_MAKE} test_static_build_no_deps_linux
+    - ${GITLAB_MAKE} test_static_build_cmake_linux

-static_build_no_deps_osx_15:
+static_build_cmake_osx_15:
   stage: test
   tags:
     - osx_15
   script:
-    - ${GITLAB_MAKE} test_static_build_no_deps_osx
+    - ${GITLAB_MAKE} test_static_build_cmake_osx
diff --git a/.travis.mk b/.travis.mk
index 64862348f..482672429 100644
--- a/.travis.mk
+++ b/.travis.mk
@@ -149,8 +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
-
-test_static_build_no_deps_linux:
+# 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 test && /usr/bin/python test-run.py --force \
  --builddir ${PWD}/static-build/tarantool-prefix/src/tarantool-build
$(TEST_RUN_EXTRA_PARAMS)
@@ -218,7 +218,6 @@ INIT_TEST_ENV_OSX=\
  rm -rf /tmp/tnt

 test_osx_no_deps: build_osx
- # Init macOS test env
  ${INIT_TEST_ENV_OSX}; \
  cd test && ./test-run.py --vardir /tmp/tnt --force
$(TEST_RUN_EXTRA_PARAMS)

@@ -233,9 +232,9 @@ base_deps_osx:
  brew install --force ${STATIC_OSX_PKGS} || brew upgrade ${STATIC_OSX_PKGS}
  pip install --force-reinstall -r test-run/requirements.txt

-test_static_build_no_deps_osx: 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
- # Init macOS test env
  ${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 8422181d6..afe480679 100644
--- a/cmake/FindReadline.cmake
+++ b/cmake/FindReadline.cmake
@@ -14,6 +14,14 @@ 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()

diff --git a/cmake/compiler.cmake b/cmake/compiler.cmake
index 14f1e1186..db2ae6227 100644
--- a/cmake/compiler.cmake
+++ b/cmake/compiler.cmake
@@ -131,6 +131,8 @@ set(CMAKE_REQUIRED_INCLUDES "")
 if(BUILD_STATIC AND NOT TARGET_OS_DARWIN)
     set(UNWIND_LIB_NAME libunwind.a)
 else()
+    # libunwind can't be compiled on macOS.
+    # But there exists libunwind.dylib as a part of MacOSSDK
     set(UNWIND_LIB_NAME unwind)
 endif()
 find_library(UNWIND_LIBRARY PATH_SUFFIXES system NAMES ${UNWIND_LIB_NAME})
@@ -192,6 +194,9 @@ if (ENABLE_BACKTRACE)
     find_package_message(UNWIND_LIBRARIES "Found unwind"
"${UNWIND_LIBRARIES}")
 endif()

+# On macOS there is no '-static-libstdc++' flag and it's use will
+# raise following error:
+# error: argument unused during compilation: '-static-libstdc++'
 if(BUILD_STATIC AND NOT TARGET_OS_DARWIN)
     # Static linking for c++ routines
     add_compile_flags("C;CXX" "-static-libstdc++")
diff --git a/static-build/CMakeLists.txt b/static-build/CMakeLists.txt
index 86582af0a..53ceb609c 100644
--- a/static-build/CMakeLists.txt
+++ b/static-build/CMakeLists.txt
@@ -9,11 +9,18 @@ set(NCURSES_VERSION 6.2)
 set(READLINE_VERSION 8.0)
 set(UNWIND_VERSION 1.3-rc1)

-find_program(C_COMPILER gcc)
-find_program(CXX_COMPILER g++)
+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})

+# Install all libraries required by tarantool at current build dir
+
 #
 # OpenSSL
 #
@@ -80,7 +87,18 @@ ExternalProject_Add(readline
 #
 # ICONV
 #
-if (NOT APPLE)
+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
+            --prefix=<INSTALL_DIR>
+            --disable-shared
+            --enable-static
+            --with-gnu-ld
+        STEP_TARGETS download
+    )
+else()
     # In linux iconv is embedded into glibc
     # So we find system header and copy it locally
     find_path(ICONV_INCLUDE_DIR iconv.h)
@@ -101,20 +119,11 @@ if (NOT APPLE)
     add_custom_target(iconv
         DEPENDS "${CMAKE_CURRENT_BINARY_DIR}/iconv-prefix/include/iconv.h"
     )
+    # This is a hack for further getting install directory of library
+    # by ExternalProject_Get_Property
     set_target_properties(iconv
         PROPERTIES _EP_INSTALL_DIR ${ICONV_INSTALL_PREFIX}
     )
-else()
-    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
-            --prefix=<INSTALL_DIR>
-            --disable-shared
-            --enable-static
-            --with-gnu-ld
-        STEP_TARGETS download
-    )
 endif()

 #
@@ -162,6 +171,8 @@ if (APPLE)
     endif()

     add_custom_target(unwind DEPENDS ${UNWIND_DEPENDENCIES})
+    # This is a hack for further getting install directory of library
+    # by ExternalProject_Get_Property
     set_target_properties(unwind
         PROPERTIES _EP_INSTALL_DIR ${UNWIND_INSTALL_PREFIX}
     )
@@ -178,6 +189,8 @@ else()
     )
 endif()

+# Get install directories of builded libraries for building
+# tarantool with custon CMAKE_PREFIX_PATH
 foreach(PROJ openssl icu zlib ncurses readline iconv unwind)
     ExternalProject_Get_Property(${PROJ} install_dir)
     set(CMAKE_PREFIX_PATH ${CMAKE_PREFIX_PATH}:${install_dir})
@@ -197,16 +210,14 @@ ExternalProject_Add(tarantool
         -DCMAKE_PREFIX_PATH=${CMAKE_PREFIX_PATH}
         -DCMAKE_FIND_USE_CMAKE_SYSTEM_PATH=FALSE
         -DOPENSSL_USE_STATIC_LIBS=TRUE
-        -DCMAKE_BUILD_TYPE=Debug
         -DBUILD_STATIC=TRUE
         -DENABLE_DIST=TRUE
         -DENABLE_BACKTRACE=TRUE
-        -DPACKAGE:STRING=${PACKAGE_NAME}
         -DCMAKE_C_COMPILER=${CMAKE_C_COMPILER}
         -DCMAKE_CXX_COMPILER=${CMAKE_CXX_COMPILER}
         ${CMAKE_TARANTOOL_ARGS}
-    BUILD_COMMAND ${CMAKE_MAKE_PROGRAM} -j
     STEP_TARGETS build
+    BUILD_COMMAND $(MAKE)
 )

 enable_testing()
diff --git a/static-build/README.md b/static-build/README.md
index 29fe085c3..0019e963f 100644
--- a/static-build/README.md
+++ b/static-build/README.md
@@ -13,6 +13,24 @@ yum install -y \
     python-msgpack python-yaml python-argparse python-six python-gevent
 ```

+MacOS:
+
+Before you start please install default Xcode Tools by Apple:
+
+```
+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
+```

 ### Usage

@@ -21,3 +39,20 @@ 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`:
+* Debug - default
+* Release
+* RelWithDebInfo
+
+And you want to build tarantool with RelWithDebInfo:
+
+```bash
+cmake -DCMAKE_TARANTOOL_ARGS="-DCMAKE_BUILD_TYPE=RelWithDebInfo" .
+```
diff --git a/static-build/test/static-build/curl-features.test.lua
b/static-build/test/static-build/curl-features.test.lua
deleted file mode 100755
index 57b1c4306..000000000
--- a/static-build/test/static-build/curl-features.test.lua
+++ /dev/null
@@ -1,67 +0,0 @@
-#!/usr/bin/env tarantool
-
-local tap = require('tap')
-local ffi = require('ffi')
-ffi.cdef([[
-    struct curl_version_info_data {
-        int age;                  /* see description below */
-        const char *version;      /* human readable string */
-        unsigned int version_num; /* numeric representation */
-        const char *host;         /* human readable string */
-        int features;             /* bitmask, see below */
-        char *ssl_version;        /* human readable string */
-        long ssl_version_num;     /* not used, always zero */
-        const char *libz_version; /* human readable string */
-        const char * const *protocols; /* protocols */
-
-        /* when 'age' is CURLVERSION_SECOND or higher, the members below
exist */
-        const char *ares;         /* human readable string */
-        int ares_num;             /* number */
-
-        /* when 'age' is CURLVERSION_THIRD or higher, the members below
exist */
-        const char *libidn;       /* human readable string */
-
-        /* when 'age' is CURLVERSION_FOURTH or higher (>= 7.16.1), the
members
-           below exist */
-        int iconv_ver_num;       /* '_libiconv_version' if iconv support
enabled */
-
-        const char *libssh_version; /* human readable string */
-
-        /* when 'age' is CURLVERSION_FIFTH or higher (>= 7.57.0), the
members
-           below exist */
-        unsigned int brotli_ver_num; /* Numeric Brotli version
-                                        (MAJOR << 24) | (MINOR << 12) |
PATCH */
-        const char *brotli_version; /* human readable string. */
-
-        /* when 'age' is CURLVERSION_SIXTH or higher (>= 7.66.0), the
members
-           below exist */
-        unsigned int nghttp2_ver_num; /* Numeric nghttp2 version
-                                         (MAJOR << 16) | (MINOR << 8) |
PATCH */
-        const char *nghttp2_version; /* human readable string. */
-
-        const char *quic_version;    /* human readable quic (+ HTTP/3)
library +
-                                        version or NULL */
-
-        /* when 'age' is CURLVERSION_SEVENTH or higher (>= 7.70.0), the
members
-           below exist */
-        const char *cainfo;          /* the built-in default
CURLOPT_CAINFO, might
-                                        be NULL */
-        const char *capath;          /* the built-in default
CURLOPT_CAPATH, might
-                                        be NULL */
-    };
-
-    struct curl_version_info_data *curl_version_info(int age);
-]])
-
-local info = ffi.C.curl_version_info(7)
-local test = tap.test('curl-features')
-test:plan(2)
-
-if test:ok(info.ssl_version ~= nil, 'Curl built with SSL support') then
-    test:diag('ssl_version: ' .. ffi.string(info.ssl_version))
-end
-if test:ok(info.libz_version ~= nil, 'Curl built with LIBZ') then
-    test:diag('libz_version: ' .. ffi.string(info.libz_version))
-end
-
-os.exit(test:check() and 0 or 1)
diff --git a/static-build/test/static-build/suite.ini
b/static-build/test/static-build/suite.ini
index 4da3d5d2f..92e349466 100644
--- a/static-build/test/static-build/suite.ini
+++ b/static-build/test/static-build/suite.ini
@@ -3,4 +3,3 @@ core = app
 description = Static build tests
 script = box.lua
 is_parallel = True
-use_unix_sockets_iproto = True

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