From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi1-f196.google.com (mail-oi1-f196.google.com [209.85.167.196]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 110CD469719 for ; Mon, 14 Sep 2020 21:31:17 +0300 (MSK) Received: by mail-oi1-f196.google.com with SMTP id x14so881092oic.9 for ; Mon, 14 Sep 2020 11:31:16 -0700 (PDT) MIME-Version: 1.0 References: <20200909154546.1850-1-huston.mavr@gmail.com> <20200914163245.GI18920@tarantool.org> In-Reply-To: <20200914163245.GI18920@tarantool.org> From: Alexandr Barulev Date: Mon, 14 Sep 2020 21:31:03 +0300 Message-ID: Content-Type: multipart/alternative; boundary="000000000000522f2005af4a3d8b" Subject: Re: [Tarantool-patches] [PATCH v3] build: refactor static build process List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Munkin Cc: Alexander Turenko , tarantool-patches@dev.tarantool.org, Yaroslav Dynnikov --000000000000522f2005af4a3d8b Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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) =D0=BF=D0=BD, 14 =D1=81=D0=B5=D0=BD=D1=82. 2020 =D0=B3. =D0=B2 19:43, Igor = Munkin : > 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 > > > > 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=3D${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 fil= e > > 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 > > > > > > > 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). > > > > > -- > > 2.26.2 > > > > -- > Best regards, > IM > --000000000000522f2005af4a3d8b Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
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..9a2= f85052 100644
--- a/static-build/CMakeLists.txt
+++ b/static-build/CM= akeLists.txt
@@ -1,8 +1,8 @@
=C2=A0cmake_minimum_required(VERSION 2.8= )
=C2=A0
-# Detect system compilers for further configuring dependenc= ies to be
-# builded with these compilers. This is used to build taranto= ol and
-# it's dependencies by usign one compiler system (for exampl= e libicu
+# Detect system compilers for further dependencies configuring= to be
+# built with these compilers. This is used to build tarantool an= d
+# it's dependencies by using one compiler system (for example lib= icu
=C2=A0# by default uses clang if it exists when others uses gcc/g++ = on
=C2=A0# linux machine).
=C2=A0project(tarantool-static C CXX)
<= /div>
= =D0=BF=D0=BD, 14 =D1=81=D0=B5=D0=BD=D1=82. 2020 =D0=B3. =D0=B2 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`:
>=C2=A0 =C2=A0- OpenSSL
>=C2=A0 =C2=A0- Zlib
>=C2=A0 =C2=A0- Ncurses
>=C2=A0 =C2=A0- Readline
>=C2=A0 =C2=A0- Unwind
>=C2=A0 =C2=A0- ICU
>
> * Added support static build for macOS
> * Fixed `CONFIGURE_COMMAND` while building bundled libcurl for static<= br> >=C2=A0 =C2=A0build at file cmake/BuildLibCURL.cmake:
>=C2=A0 =C2=A0 =C2=A0- disable building shared libcurl libraries (by set= ting
>=C2=A0 =C2=A0 =C2=A0 =C2=A0`--disable-shared` option)
>=C2=A0 =C2=A0 =C2=A0- disable hiding libcurl symbols (by setting
>=C2=A0 =C2=A0 =C2=A0 =C2=A0`--disable-symbol-hiding` option)
>=C2=A0 =C2=A0 =C2=A0- prevent linking libcurl with system libz (by sett= ing
>=C2=A0 =C2=A0 =C2=A0 =C2=A0`--with-zlib=3D${FOUND_ZLIB_ROOT_DIR}` optio= n)
> * Removed Dockerfile.staticbuild
> * Added new gitlab.ci jobs to test new style static build:
>=C2=A0 =C2=A0- static_build_cmake_linux
>=C2=A0 =C2=A0- static_build_cmake_osx_15
> * Removed static_docker_build gitlab.ci job
>
> Closes #5095
> ---
>
> Branch: https://github.c= om/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=
>=C2=A0 =C2=A0 branch
> 4. Instead of using static-build/.gitignore use the root .gitignore fi= le
> 5. Refactored static-build/CMakeLists.txt:
>=C2=A0 =C2=A0 * Project name is set to tarantool-static
>=C2=A0 =C2=A0 * Instead of explicitly finding of c/c++ compilers curren= t version uses
>=C2=A0 =C2=A0 =C2=A0 cmake `project(tarantool-static C CXX)` command. T= his change requires
>=C2=A0 =C2=A0 =C2=A0 to manually set CFLAGS/CPPFLAGS/LDFLAGS for all st= atic-build
>=C2=A0 =C2=A0 =C2=A0 dependencies (because building dependencies at mac= OS requires path
>=C2=A0 =C2=A0 =C2=A0 to macOS SDK) the idea was taken from cmake/BuildL= ibCurl.cmake
>=C2=A0 =C2=A0 * Added commentaries about problem with libicu
>=C2=A0 =C2=A0 * Removed unused TEST_COMMAND at ExternalProject_Add(zlib= ...) also
>=C2=A0 =C2=A0 =C2=A0 removed unnecessary # STEP_TARGETS at ExternalProj= ect_Add(readline ...)
>=C2=A0 =C2=A0 * Deleted doubled whitespaces
> 6. Fixed static-build/README.md:
>=C2=A0 =C2=A0 * Added list of required tools
>=C2=A0 =C2=A0 * Added example for ubuntu/debian
>=C2=A0 =C2=A0 * Fixed indentation and typos
> 7. Refactored tests:
>=C2=A0 =C2=A0 * Got rid of test-run
>=C2=A0 =C2=A0 * Deleted doubled whitespaces at exports.test.lua
>=C2=A0 =C2=A0 * Fixed defining of RTLD_DEFAULT
>
>
>=C2=A0 .gitignore=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 |= =C2=A0 =C2=A08 +
>=C2=A0 .gitlab-ci.yml=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 |=C2=A0 11 += -
>=C2=A0 .travis.mk=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 |=C2=A0 = 56 +++-
>=C2=A0 Dockerfile.staticbuild=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 |=C2=A0 98 ------
>=C2=A0 cmake/BuildLibCURL.cmake=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 |=C2=A0 13 +-
>=C2=A0 cmake/compiler.cmake=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 |=C2=A0 24 +-
>=C2=A0 cmake/os.cmake=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 |=C2=A0 =C2= =A05 +-
>=C2=A0 static-build/CMakeLists.txt=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0| 311 ++++++++++++++++++
>=C2=A0 static-build/README.md=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 |=C2=A0 90 +++++
>=C2=A0 static-build/test/CheckDependencies.cmake=C2=A0 =C2=A0 =C2=A0|= =C2=A0 43 +++
>=C2=A0 .../test/static-build/exports.test.lua=C2=A0 =C2=A0 =C2=A0 =C2= =A0 | 142 ++++++++
>=C2=A0 .../test/static-build/traceback.test.lua=C2=A0 =C2=A0 =C2=A0 |= =C2=A0 15 +
>=C2=A0 12 files changed, 692 insertions(+), 124 deletions(-)
>=C2=A0 delete mode 100644 Dockerfile.staticbuild
>=C2=A0 create mode 100644 static-build/CMakeLists.txt
>=C2=A0 create mode 100644 static-build/README.md
>=C2=A0 create mode 100644 static-build/test/CheckDependencies.cmake
>=C2=A0 create mode 100755 static-build/test/static-build/exports.test.l= ua
>=C2=A0 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<= br>
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 lib= icu

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