<HTML><BODY>Hi! Thank you for review.<br><br><br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;">
Понедельник, 22 июля 2019, 10:10 +03:00 от Alexander Turenko <alexander.turenko@tarantool.org>:<br><br><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15637794141973928369_BODY">We have -DBUILD_SHARED_LIBS=ON cmake option globally enabled on CentOS /<br>
Fedora, so libyaml is linked into src/tarantool dynamically.<br><br>
$ git checkout origin/sp/gh-4090-enable-bundled-libyaml-full-ci<br>
$ cmake . -DCMAKE_BUILD_TYPE=Debug -DENABLE_BACKTRACE=ON -DENABLE_DIST=ON -DBUILD_SHARED_LIBS=ON && make -j<br>
$ ldd src/tarantool | grep yaml<br>
libyaml.so => /home/alex/p/tarantool-meta/r/t-2/third_party/libyaml/libyaml.so (0x00007ff0c96de000)<br>
$ rm /home/alex/p/tarantool-meta/r/t-2/third_party/libyaml/libyaml.so<br>
$ ldd src/tarantool | grep yaml<br>
libyaml.so => /usr/lib64/libyaml.so (0x00007f6cc379e000)<br><br>
This means that tarantool executable cannot be relocated to another<br>
system w/o libyaml.so. I see no reason to build a bundled library<br>
dynamically, so maybe ENABLE_BUNDLED_LIBYAML should enable static build<br>
for libyaml and should have precedence over BUILD_SHARED_LIBS. I don't<br>
sure however that this is good practice, so I'll ask 2nd review from<br>
someone else when you'll fix this problem in this or another way.</div></div></div></div></blockquote><br>I addressed this issue in a patch to tarantool/libyaml. Please check it out.<br>This patch remains intact.<br><br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_15637794141973928369_BODY"><br><br>
WBR, Alexander Turenko.<br><br>
On Sun, Jul 21, 2019 at 04:25:57PM +0300, Serge Petrenko wrote:<br><div class="mail-quote-collapse">> After we fixed bundled libyaml to correctly print 4-byte Unicode<br>
> characters, it is no longer compatible with the upstream version, so<br>
> enable building with bundled libyaml for every platform.<br>
> This way the tests will pass.<br>
> <br>
> Follow-up #4090<br>
> ---<br>
> <a href="https://github.com/tarantool/tarantool/issues/4090" target="_blank">https://github.com/tarantool/tarantool/issues/4090</a><br>
> <a href="https://github.com/tarantool/tarantool/tree/sp/gh-4090-enable-bundled-libyaml-full-ci" target="_blank">https://github.com/tarantool/tarantool/tree/sp/gh-4090-enable-bundled-libyaml-full-ci</a><br>
> <br>
> apk/APKBUILD | 2 --<br>
> cmake/BuildLibYAML.cmake | 2 +-<br>
> debian/rules | 1 -<br>
> rpm/tarantool.spec | 1 -<br>
> snapcraft.yaml | 1 -<br>
> 5 files changed, 1 insertion(+), 6 deletions(-)<br>
> <br>
> diff --git a/apk/APKBUILD b/apk/APKBUILD<br>
> index e8ea5aa13..7d61aa4a5 100644<br>
> --- a/apk/APKBUILD<br>
> +++ b/apk/APKBUILD<br>
> @@ -23,10 +23,8 @@ build() {<br>
> cd "$builddir"<br>
> <br>
> cmake -DCMAKE_BUILD_TYPE=RelWithDebInfo \<br>
> - -DENABLE_BUNDLED_LIBYAML:BOOL=OFF \<br>
> -DENABLE_BACKTRACE:BOOL=ON \<br>
> -DENABLE_DIST:BOOL=ON \<br>
> - -DENABLE_BUNDLED_LIBYAML:BOOL=OFF \<br>
> -DCMAKE_INSTALL_PREFIX=/usr \<br>
> -DCMAKE_INSTALL_SYSCONFDIR=/etc \<br>
> -DCMAKE_INSTALL_LOCALSTATEDIR=/var \<br>
> diff --git a/cmake/BuildLibYAML.cmake b/cmake/BuildLibYAML.cmake<br>
> index 7caa1828c..95068d73a 100644<br>
> --- a/cmake/BuildLibYAML.cmake<br>
> +++ b/cmake/BuildLibYAML.cmake<br>
> @@ -6,7 +6,7 @@ macro(libyaml_build)<br>
> <br>
> add_subdirectory(${PROJECT_SOURCE_DIR}/third_party/libyaml EXCLUDE_FROM_ALL)<br>
> # See comments in BuildLibEV.cmake<br>
> - set_target_properties(yaml PROPERTIES COMPILE_FLAGS "-w")<br>
> + set_target_properties(yaml PROPERTIES COMPILE_FLAGS "-w -std=c99")<br>
> <br>
> find_package_message(LIBYAML<br>
> "Using bundled libyaml"<br>
> diff --git a/debian/rules b/debian/rules<br>
> index edfecfc33..904eaa719 100755<br>
> --- a/debian/rules<br>
> +++ b/debian/rules<br>
> @@ -15,7 +15,6 @@ DEB_CMAKE_EXTRA_FLAGS := \<br>
> -DCMAKE_INSTALL_LIBDIR=lib/$(DEB_HOST_MULTIARCH) \<br>
> -DCMAKE_INSTALL_SYSCONFDIR=/etc \<br>
> -DCMAKE_INSTALL_LOCALSTATEDIR=/var \<br>
> - -DENABLE_BUNDLED_LIBYAML=OFF \<br>
> -DENABLE_DIST=ON \<br>
> -DWITH_SYSVINIT=ON \<br>
> -DWITH_SYSTEMD=$(WITH_SYSTEMD)<br>
> diff --git a/rpm/tarantool.spec b/rpm/tarantool.spec<br>
> index 8bee71ed0..002df26b1 100644<br>
> --- a/rpm/tarantool.spec<br>
> +++ b/rpm/tarantool.spec<br>
> @@ -124,7 +124,6 @@ C and Lua/C modules.<br>
> %cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo \<br>
> -DCMAKE_INSTALL_LOCALSTATEDIR:PATH=%{_localstatedir} \<br>
> -DCMAKE_INSTALL_SYSCONFDIR:PATH=%{_sysconfdir} \<br>
> - -DENABLE_BUNDLED_LIBYAML:BOOL=OFF \<br>
> %if %{with backtrace}<br>
> -DENABLE_BACKTRACE:BOOL=ON \<br>
> %else<br>
> diff --git a/snapcraft.yaml b/snapcraft.yaml<br>
> index a7ce540cb..48b32a890 100644<br>
> --- a/snapcraft.yaml<br>
> +++ b/snapcraft.yaml<br>
> @@ -25,7 +25,6 @@ parts:<br>
> plugin: cmake<br>
> configflags:<br>
> - -DCMAKE_BUILD_TYPE=RelWithDebInfo<br>
> - - -DENABLE_BUNDLED_LIBYAML=OFF<br>
> - -DENABLE_DIST=OFF # Disable tarantoolctl, init scripts, etc.<br>
> build-packages:<br>
> - cmake<br>
> -- <br>
> 2.20.1 (Apple Git-117)<br>
> <br></div></div></div></div></div></blockquote>
<br>
<br>-- <br>Sergey Petrenko<br></BODY></HTML>