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

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

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

Hello, thank you very much for new iteration of the review!

Typos are fixed.

Here is a diff:

diff --git a/static-build/CMakeLists.txt b/static-build/CMakeLists.txt
index 90029fdd8..9a2f85052 100644
--- a/static-build/CMakeLists.txt
+++ b/static-build/CMakeLists.txt
@@ -1,8 +1,8 @@
 cmake_minimum_required(VERSION 2.8)

-# 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
+# Detect system compilers for further dependencies configuring to be
+# built with these compilers. This is used to build tarantool and
+# it's dependencies by using 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)

пн, 14 сент. 2020 г. в 19:43, 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
>

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