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

[Igor Munkin <imun@tarantool.org>]

Hello,

Thanks, the patch is LGTM, except the nits below.

Minor: Do we need a ChangeLog entry for it? I don't know.

On 09.09.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
> * 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
> ---
> 
> Branch: https://github.com/tarantool/tarantool/tree/rosik/refactor-static-build
> Issue: https://github.com/tarantool/tarantool/issues/5095
> 
> Changes in v3:
> 1. Fixed typos at commit message
> 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 doubled whitespaces at exports.test.lua
>    * Fixed defining of RTLD_DEFAULT
> 
> 
>  .gitignore                                    |   8 +
>  .gitlab-ci.yml                                |  11 +-
>  .travis.mk                                    |  56 +++-
>  Dockerfile.staticbuild                        |  98 ------
>  cmake/BuildLibCURL.cmake                      |  13 +-
>  cmake/compiler.cmake                          |  24 +-
>  cmake/os.cmake                                |   5 +-
>  static-build/CMakeLists.txt                   | 311 ++++++++++++++++++
>  static-build/README.md                        |  90 +++++
>  static-build/test/CheckDependencies.cmake     |  43 +++
>  .../test/static-build/exports.test.lua        | 142 ++++++++
>  .../test/static-build/traceback.test.lua      |  15 +
>  12 files changed, 692 insertions(+), 124 deletions(-)
>  delete mode 100644 Dockerfile.staticbuild
>  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/exports.test.lua
>  create mode 100755 static-build/test/static-build/traceback.test.lua
> 

<snipped>

> diff --git a/static-build/CMakeLists.txt b/static-build/CMakeLists.txt
> new file mode 100644
> index 000000000..90029fdd8
> --- /dev/null
> +++ b/static-build/CMakeLists.txt
> @@ -0,0 +1,311 @@
> +cmake_minimum_required(VERSION 2.8)
> +
> +# Detect system compilers for further configuring dependencies to be

Typo: s/configuring dependencies/dependencies configuring/.

> +# builded with these compilers. This is used to build tarantool and

Typo: s/builded/built/.

> +# it's dependencies by usign one compiler system (for example libicu

Typo: s/usign/using/.

> +# by default uses clang if it exists when others uses gcc/g++ on
> +# linux machine).

<snipped>

> -- 
> 2.26.2
> 

-- 
Best regards,
IM