* [tarantool-patches] [PATCH] build: enable bundled libyaml for all systems.
@ 2019-07-21 13:25 Serge Petrenko
2019-07-22 7:09 ` [tarantool-patches] " Alexander Turenko
0 siblings, 1 reply; 6+ messages in thread
From: Serge Petrenko @ 2019-07-21 13:25 UTC (permalink / raw)
To: alexander.turenko; +Cc: tarantool-patches, Serge Petrenko
After we fixed bundled libyaml to correctly print 4-byte Unicode
characters, it is no longer compatible with the upstream version, so
enable building with bundled libyaml for every platform.
This way the tests will pass.
Follow-up #4090
---
https://github.com/tarantool/tarantool/issues/4090
https://github.com/tarantool/tarantool/tree/sp/gh-4090-enable-bundled-libyaml-full-ci
apk/APKBUILD | 2 --
cmake/BuildLibYAML.cmake | 2 +-
debian/rules | 1 -
rpm/tarantool.spec | 1 -
snapcraft.yaml | 1 -
5 files changed, 1 insertion(+), 6 deletions(-)
diff --git a/apk/APKBUILD b/apk/APKBUILD
index e8ea5aa13..7d61aa4a5 100644
--- a/apk/APKBUILD
+++ b/apk/APKBUILD
@@ -23,10 +23,8 @@ build() {
cd "$builddir"
cmake -DCMAKE_BUILD_TYPE=RelWithDebInfo \
- -DENABLE_BUNDLED_LIBYAML:BOOL=OFF \
-DENABLE_BACKTRACE:BOOL=ON \
-DENABLE_DIST:BOOL=ON \
- -DENABLE_BUNDLED_LIBYAML:BOOL=OFF \
-DCMAKE_INSTALL_PREFIX=/usr \
-DCMAKE_INSTALL_SYSCONFDIR=/etc \
-DCMAKE_INSTALL_LOCALSTATEDIR=/var \
diff --git a/cmake/BuildLibYAML.cmake b/cmake/BuildLibYAML.cmake
index 7caa1828c..95068d73a 100644
--- a/cmake/BuildLibYAML.cmake
+++ b/cmake/BuildLibYAML.cmake
@@ -6,7 +6,7 @@ macro(libyaml_build)
add_subdirectory(${PROJECT_SOURCE_DIR}/third_party/libyaml EXCLUDE_FROM_ALL)
# See comments in BuildLibEV.cmake
- set_target_properties(yaml PROPERTIES COMPILE_FLAGS "-w")
+ set_target_properties(yaml PROPERTIES COMPILE_FLAGS "-w -std=c99")
find_package_message(LIBYAML
"Using bundled libyaml"
diff --git a/debian/rules b/debian/rules
index edfecfc33..904eaa719 100755
--- a/debian/rules
+++ b/debian/rules
@@ -15,7 +15,6 @@ DEB_CMAKE_EXTRA_FLAGS := \
-DCMAKE_INSTALL_LIBDIR=lib/$(DEB_HOST_MULTIARCH) \
-DCMAKE_INSTALL_SYSCONFDIR=/etc \
-DCMAKE_INSTALL_LOCALSTATEDIR=/var \
- -DENABLE_BUNDLED_LIBYAML=OFF \
-DENABLE_DIST=ON \
-DWITH_SYSVINIT=ON \
-DWITH_SYSTEMD=$(WITH_SYSTEMD)
diff --git a/rpm/tarantool.spec b/rpm/tarantool.spec
index 8bee71ed0..002df26b1 100644
--- a/rpm/tarantool.spec
+++ b/rpm/tarantool.spec
@@ -124,7 +124,6 @@ C and Lua/C modules.
%cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo \
-DCMAKE_INSTALL_LOCALSTATEDIR:PATH=%{_localstatedir} \
-DCMAKE_INSTALL_SYSCONFDIR:PATH=%{_sysconfdir} \
- -DENABLE_BUNDLED_LIBYAML:BOOL=OFF \
%if %{with backtrace}
-DENABLE_BACKTRACE:BOOL=ON \
%else
diff --git a/snapcraft.yaml b/snapcraft.yaml
index a7ce540cb..48b32a890 100644
--- a/snapcraft.yaml
+++ b/snapcraft.yaml
@@ -25,7 +25,6 @@ parts:
plugin: cmake
configflags:
- -DCMAKE_BUILD_TYPE=RelWithDebInfo
- - -DENABLE_BUNDLED_LIBYAML=OFF
- -DENABLE_DIST=OFF # Disable tarantoolctl, init scripts, etc.
build-packages:
- cmake
--
2.20.1 (Apple Git-117)
^ permalink raw reply [flat|nested] 6+ messages in thread
* [tarantool-patches] Re: [PATCH] build: enable bundled libyaml for all systems.
2019-07-21 13:25 [tarantool-patches] [PATCH] build: enable bundled libyaml for all systems Serge Petrenko
@ 2019-07-22 7:09 ` Alexander Turenko
2019-07-22 21:48 ` [tarantool-patches] Re[2]: " Sergey Petrenko
0 siblings, 1 reply; 6+ messages in thread
From: Alexander Turenko @ 2019-07-22 7:09 UTC (permalink / raw)
To: Serge Petrenko; +Cc: tarantool-patches
We have -DBUILD_SHARED_LIBS=ON cmake option globally enabled on CentOS /
Fedora, so libyaml is linked into src/tarantool dynamically.
$ git checkout origin/sp/gh-4090-enable-bundled-libyaml-full-ci
$ cmake . -DCMAKE_BUILD_TYPE=Debug -DENABLE_BACKTRACE=ON -DENABLE_DIST=ON -DBUILD_SHARED_LIBS=ON && make -j
$ ldd src/tarantool | grep yaml
libyaml.so => /home/alex/p/tarantool-meta/r/t-2/third_party/libyaml/libyaml.so (0x00007ff0c96de000)
$ rm /home/alex/p/tarantool-meta/r/t-2/third_party/libyaml/libyaml.so
$ ldd src/tarantool | grep yaml
libyaml.so => /usr/lib64/libyaml.so (0x00007f6cc379e000)
This means that tarantool executable cannot be relocated to another
system w/o libyaml.so. I see no reason to build a bundled library
dynamically, so maybe ENABLE_BUNDLED_LIBYAML should enable static build
for libyaml and should have precedence over BUILD_SHARED_LIBS. I don't
sure however that this is good practice, so I'll ask 2nd review from
someone else when you'll fix this problem in this or another way.
WBR, Alexander Turenko.
On Sun, Jul 21, 2019 at 04:25:57PM +0300, Serge Petrenko wrote:
> After we fixed bundled libyaml to correctly print 4-byte Unicode
> characters, it is no longer compatible with the upstream version, so
> enable building with bundled libyaml for every platform.
> This way the tests will pass.
>
> Follow-up #4090
> ---
> https://github.com/tarantool/tarantool/issues/4090
> https://github.com/tarantool/tarantool/tree/sp/gh-4090-enable-bundled-libyaml-full-ci
>
> apk/APKBUILD | 2 --
> cmake/BuildLibYAML.cmake | 2 +-
> debian/rules | 1 -
> rpm/tarantool.spec | 1 -
> snapcraft.yaml | 1 -
> 5 files changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/apk/APKBUILD b/apk/APKBUILD
> index e8ea5aa13..7d61aa4a5 100644
> --- a/apk/APKBUILD
> +++ b/apk/APKBUILD
> @@ -23,10 +23,8 @@ build() {
> cd "$builddir"
>
> cmake -DCMAKE_BUILD_TYPE=RelWithDebInfo \
> - -DENABLE_BUNDLED_LIBYAML:BOOL=OFF \
> -DENABLE_BACKTRACE:BOOL=ON \
> -DENABLE_DIST:BOOL=ON \
> - -DENABLE_BUNDLED_LIBYAML:BOOL=OFF \
> -DCMAKE_INSTALL_PREFIX=/usr \
> -DCMAKE_INSTALL_SYSCONFDIR=/etc \
> -DCMAKE_INSTALL_LOCALSTATEDIR=/var \
> diff --git a/cmake/BuildLibYAML.cmake b/cmake/BuildLibYAML.cmake
> index 7caa1828c..95068d73a 100644
> --- a/cmake/BuildLibYAML.cmake
> +++ b/cmake/BuildLibYAML.cmake
> @@ -6,7 +6,7 @@ macro(libyaml_build)
>
> add_subdirectory(${PROJECT_SOURCE_DIR}/third_party/libyaml EXCLUDE_FROM_ALL)
> # See comments in BuildLibEV.cmake
> - set_target_properties(yaml PROPERTIES COMPILE_FLAGS "-w")
> + set_target_properties(yaml PROPERTIES COMPILE_FLAGS "-w -std=c99")
>
> find_package_message(LIBYAML
> "Using bundled libyaml"
> diff --git a/debian/rules b/debian/rules
> index edfecfc33..904eaa719 100755
> --- a/debian/rules
> +++ b/debian/rules
> @@ -15,7 +15,6 @@ DEB_CMAKE_EXTRA_FLAGS := \
> -DCMAKE_INSTALL_LIBDIR=lib/$(DEB_HOST_MULTIARCH) \
> -DCMAKE_INSTALL_SYSCONFDIR=/etc \
> -DCMAKE_INSTALL_LOCALSTATEDIR=/var \
> - -DENABLE_BUNDLED_LIBYAML=OFF \
> -DENABLE_DIST=ON \
> -DWITH_SYSVINIT=ON \
> -DWITH_SYSTEMD=$(WITH_SYSTEMD)
> diff --git a/rpm/tarantool.spec b/rpm/tarantool.spec
> index 8bee71ed0..002df26b1 100644
> --- a/rpm/tarantool.spec
> +++ b/rpm/tarantool.spec
> @@ -124,7 +124,6 @@ C and Lua/C modules.
> %cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo \
> -DCMAKE_INSTALL_LOCALSTATEDIR:PATH=%{_localstatedir} \
> -DCMAKE_INSTALL_SYSCONFDIR:PATH=%{_sysconfdir} \
> - -DENABLE_BUNDLED_LIBYAML:BOOL=OFF \
> %if %{with backtrace}
> -DENABLE_BACKTRACE:BOOL=ON \
> %else
> diff --git a/snapcraft.yaml b/snapcraft.yaml
> index a7ce540cb..48b32a890 100644
> --- a/snapcraft.yaml
> +++ b/snapcraft.yaml
> @@ -25,7 +25,6 @@ parts:
> plugin: cmake
> configflags:
> - -DCMAKE_BUILD_TYPE=RelWithDebInfo
> - - -DENABLE_BUNDLED_LIBYAML=OFF
> - -DENABLE_DIST=OFF # Disable tarantoolctl, init scripts, etc.
> build-packages:
> - cmake
> --
> 2.20.1 (Apple Git-117)
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [tarantool-patches] Re[2]: [PATCH] build: enable bundled libyaml for all systems.
2019-07-22 7:09 ` [tarantool-patches] " Alexander Turenko
@ 2019-07-22 21:48 ` Sergey Petrenko
2019-07-23 9:40 ` [tarantool-patches] " Alexander Turenko
0 siblings, 1 reply; 6+ messages in thread
From: Sergey Petrenko @ 2019-07-22 21:48 UTC (permalink / raw)
To: Alexander Turenko; +Cc: tarantool-patches
[-- Attachment #1: Type: text/plain, Size: 4562 bytes --]
Hi! Thank you for review.
>Понедельник, 22 июля 2019, 10:10 +03:00 от Alexander Turenko <alexander.turenko@tarantool.org>:
>
>We have -DBUILD_SHARED_LIBS=ON cmake option globally enabled on CentOS /
>Fedora, so libyaml is linked into src/tarantool dynamically.
>
>$ git checkout origin/sp/gh-4090-enable-bundled-libyaml-full-ci
>$ cmake . -DCMAKE_BUILD_TYPE=Debug -DENABLE_BACKTRACE=ON -DENABLE_DIST=ON -DBUILD_SHARED_LIBS=ON && make -j
>$ ldd src/tarantool | grep yaml
>libyaml.so => /home/alex/p/tarantool-meta/r/t-2/third_party/libyaml/libyaml.so (0x00007ff0c96de000)
>$ rm /home/alex/p/tarantool-meta/r/t-2/third_party/libyaml/libyaml.so
>$ ldd src/tarantool | grep yaml
>libyaml.so => /usr/lib64/libyaml.so (0x00007f6cc379e000)
>
>This means that tarantool executable cannot be relocated to another
>system w/o libyaml.so. I see no reason to build a bundled library
>dynamically, so maybe ENABLE_BUNDLED_LIBYAML should enable static build
>for libyaml and should have precedence over BUILD_SHARED_LIBS. I don't
>sure however that this is good practice, so I'll ask 2nd review from
>someone else when you'll fix this problem in this or another way.
I addressed this issue in a patch to tarantool/libyaml. Please check it out.
This patch remains intact.
>
>
>WBR, Alexander Turenko.
>
>On Sun, Jul 21, 2019 at 04:25:57PM +0300, Serge Petrenko wrote:
>> After we fixed bundled libyaml to correctly print 4-byte Unicode
>> characters, it is no longer compatible with the upstream version, so
>> enable building with bundled libyaml for every platform.
>> This way the tests will pass.
>>
>> Follow-up #4090
>> ---
>> https://github.com/tarantool/tarantool/issues/4090
>> https://github.com/tarantool/tarantool/tree/sp/gh-4090-enable-bundled-libyaml-full-ci
>>
>> apk/APKBUILD | 2 --
>> cmake/BuildLibYAML.cmake | 2 +-
>> debian/rules | 1 -
>> rpm/tarantool.spec | 1 -
>> snapcraft.yaml | 1 -
>> 5 files changed, 1 insertion(+), 6 deletions(-)
>>
>> diff --git a/apk/APKBUILD b/apk/APKBUILD
>> index e8ea5aa13..7d61aa4a5 100644
>> --- a/apk/APKBUILD
>> +++ b/apk/APKBUILD
>> @@ -23,10 +23,8 @@ build() {
>> cd "$builddir"
>>
>> cmake -DCMAKE_BUILD_TYPE=RelWithDebInfo \
>> - -DENABLE_BUNDLED_LIBYAML:BOOL=OFF \
>> -DENABLE_BACKTRACE:BOOL=ON \
>> -DENABLE_DIST:BOOL=ON \
>> - -DENABLE_BUNDLED_LIBYAML:BOOL=OFF \
>> -DCMAKE_INSTALL_PREFIX=/usr \
>> -DCMAKE_INSTALL_SYSCONFDIR=/etc \
>> -DCMAKE_INSTALL_LOCALSTATEDIR=/var \
>> diff --git a/cmake/BuildLibYAML.cmake b/cmake/BuildLibYAML.cmake
>> index 7caa1828c..95068d73a 100644
>> --- a/cmake/BuildLibYAML.cmake
>> +++ b/cmake/BuildLibYAML.cmake
>> @@ -6,7 +6,7 @@ macro(libyaml_build)
>>
>> add_subdirectory(${PROJECT_SOURCE_DIR}/third_party/libyaml EXCLUDE_FROM_ALL)
>> # See comments in BuildLibEV.cmake
>> - set_target_properties(yaml PROPERTIES COMPILE_FLAGS "-w")
>> + set_target_properties(yaml PROPERTIES COMPILE_FLAGS "-w -std=c99")
>>
>> find_package_message(LIBYAML
>> "Using bundled libyaml"
>> diff --git a/debian/rules b/debian/rules
>> index edfecfc33..904eaa719 100755
>> --- a/debian/rules
>> +++ b/debian/rules
>> @@ -15,7 +15,6 @@ DEB_CMAKE_EXTRA_FLAGS := \
>> -DCMAKE_INSTALL_LIBDIR=lib/$(DEB_HOST_MULTIARCH) \
>> -DCMAKE_INSTALL_SYSCONFDIR=/etc \
>> -DCMAKE_INSTALL_LOCALSTATEDIR=/var \
>> - -DENABLE_BUNDLED_LIBYAML=OFF \
>> -DENABLE_DIST=ON \
>> -DWITH_SYSVINIT=ON \
>> -DWITH_SYSTEMD=$(WITH_SYSTEMD)
>> diff --git a/rpm/tarantool.spec b/rpm/tarantool.spec
>> index 8bee71ed0..002df26b1 100644
>> --- a/rpm/tarantool.spec
>> +++ b/rpm/tarantool.spec
>> @@ -124,7 +124,6 @@ C and Lua/C modules.
>> %cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo \
>> -DCMAKE_INSTALL_LOCALSTATEDIR:PATH=%{_localstatedir} \
>> -DCMAKE_INSTALL_SYSCONFDIR:PATH=%{_sysconfdir} \
>> - -DENABLE_BUNDLED_LIBYAML:BOOL=OFF \
>> %if %{with backtrace}
>> -DENABLE_BACKTRACE:BOOL=ON \
>> %else
>> diff --git a/snapcraft.yaml b/snapcraft.yaml
>> index a7ce540cb..48b32a890 100644
>> --- a/snapcraft.yaml
>> +++ b/snapcraft.yaml
>> @@ -25,7 +25,6 @@ parts:
>> plugin: cmake
>> configflags:
>> - -DCMAKE_BUILD_TYPE=RelWithDebInfo
>> - - -DENABLE_BUNDLED_LIBYAML=OFF
>> - -DENABLE_DIST=OFF # Disable tarantoolctl, init scripts, etc.
>> build-packages:
>> - cmake
>> --
>> 2.20.1 (Apple Git-117)
>>
--
Sergey Petrenko
[-- Attachment #2: Type: text/html, Size: 5884 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* [tarantool-patches] Re: [PATCH] build: enable bundled libyaml for all systems.
2019-07-22 21:48 ` [tarantool-patches] Re[2]: " Sergey Petrenko
@ 2019-07-23 9:40 ` Alexander Turenko
2019-07-23 18:25 ` [tarantool-patches] " Sergey Petrenko
0 siblings, 1 reply; 6+ messages in thread
From: Alexander Turenko @ 2019-07-23 9:40 UTC (permalink / raw)
To: Sergey Petrenko; +Cc: tarantool-patches
> I addressed this issue in a patch to tarantool/libyaml. Please check it out.
> This patch remains intact.
Okay. I don't have objections, but one comment.
> >> - set_target_properties(yaml PROPERTIES COMPILE_FLAGS "-w")
> >> + set_target_properties(yaml PROPERTIES COMPILE_FLAGS "-w -std=c99")
Maybe it worth to declare a loop variable outside a loop initialization
statement in libyaml (and update the upstream PR)? I mean that if all
libyaml code follow C89, then we maybe should do it too in our changes.
What do you think?
^ permalink raw reply [flat|nested] 6+ messages in thread
* [tarantool-patches] Re: [tarantool-patches] Re: [PATCH] build: enable bundled libyaml for all systems.
2019-07-23 9:40 ` [tarantool-patches] " Alexander Turenko
@ 2019-07-23 18:25 ` Sergey Petrenko
2019-07-24 8:03 ` Alexander Turenko
0 siblings, 1 reply; 6+ messages in thread
From: Sergey Petrenko @ 2019-07-23 18:25 UTC (permalink / raw)
To: Alexander Turenko; +Cc: tarantool-patches
[-- Attachment #1: Type: text/plain, Size: 951 bytes --]
Hi!
>Вторник, 23 июля 2019, 12:40 +03:00 от Alexander Turenko <alexander.turenko@tarantool.org>:
>
>> I addressed this issue in a patch to tarantool/libyaml. Please check it out.
>> This patch remains intact.
>
>Okay. I don't have objections, but one comment.
>
>> >> - set_target_properties(yaml PROPERTIES COMPILE_FLAGS "-w")
>> >> + set_target_properties(yaml PROPERTIES COMPILE_FLAGS "-w -std=c99")
>
>Maybe it worth to declare a loop variable outside a loop initialization
>statement in libyaml (and update the upstream PR)? I mean that if all
>libyaml code follow C89, then we maybe should do it too in our changes.
>What do you think?
This seems reasonable.
I updated both branches
( https://github.com/tarantool/libyaml/tree/sp/static-linking and
https://github.com/tarantool/tarantool/tree/sp/gh-4090-enable-bundled-libyaml-full-ci )
I've also updated the pull request to libyaml upstream.
>
>
--
Sergey Petrenko
[-- Attachment #2: Type: text/html, Size: 2268 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* [tarantool-patches] Re: [PATCH] build: enable bundled libyaml for all systems.
2019-07-23 18:25 ` [tarantool-patches] " Sergey Petrenko
@ 2019-07-24 8:03 ` Alexander Turenko
0 siblings, 0 replies; 6+ messages in thread
From: Alexander Turenko @ 2019-07-24 8:03 UTC (permalink / raw)
To: Sergey Petrenko; +Cc: tarantool-patches, Kirill Yukhin
So, I'm okay. Thanks!
Kirill, can you proceed? We need to push sp/static-linking into
libyaml's master, update the submodule in tarantool and push
sp/gh-4090-enable-bundled-libyaml-full-ci into tarantool.
issue: https://github.com/tarantool/tarantool/issues/4090
WBR, Alexander Turenko.
On Tue, Jul 23, 2019 at 09:25:21PM +0300, Sergey Petrenko wrote:
> Hi!
>
>
> >Вторник, 23 июля 2019, 12:40 +03:00 от Alexander Turenko <alexander.turenko@tarantool.org>:
> >
> >> I addressed this issue in a patch to tarantool/libyaml. Please check it out.
> >> This patch remains intact.
> >
> >Okay. I don't have objections, but one comment.
> >
> >> >> - set_target_properties(yaml PROPERTIES COMPILE_FLAGS "-w")
> >> >> + set_target_properties(yaml PROPERTIES COMPILE_FLAGS "-w -std=c99")
> >
> >Maybe it worth to declare a loop variable outside a loop initialization
> >statement in libyaml (and update the upstream PR)? I mean that if all
> >libyaml code follow C89, then we maybe should do it too in our changes.
> >What do you think?
> This seems reasonable.
> I updated both branches
> ( https://github.com/tarantool/libyaml/tree/sp/static-linking and
> https://github.com/tarantool/tarantool/tree/sp/gh-4090-enable-bundled-libyaml-full-ci )
> I've also updated the pull request to libyaml upstream.
>
> >
> >
>
>
> --
> Sergey Petrenko
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-07-24 8:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-21 13:25 [tarantool-patches] [PATCH] build: enable bundled libyaml for all systems Serge Petrenko
2019-07-22 7:09 ` [tarantool-patches] " Alexander Turenko
2019-07-22 21:48 ` [tarantool-patches] Re[2]: " Sergey Petrenko
2019-07-23 9:40 ` [tarantool-patches] " Alexander Turenko
2019-07-23 18:25 ` [tarantool-patches] " Sergey Petrenko
2019-07-24 8:03 ` Alexander Turenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox