Re: [Tarantool-patches] [PATCH 0/4] RFC: Isolate serializer helpers

[Alexander Turenko via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

On Wed, Jul 07, 2021 at 01:08:10PM +0300, Alexander Turenko wrote:
> On Wed, Jun 23, 2021 at 10:12:38PM +0300, Alexander Turenko wrote:
> > Moved the serializer helpers into its own compilation unit, add some
> > comments and a basic test: everything is just to simplify diving into
> > this code.
> > 
> > Guys, please, look, whether it seems useful enough to include into
> > tarantool's mainline? Should we name it serializer.[ch] or
> > somehow like serializer_helpers.[ch]?
> > 
> > Part of https://github.com/tarantool/tarantool/issues/3228
> > Branch: Totktonada/gh-3228-extract-serializer-helpers
> > 
> > Alexander Turenko (4):
> >   lua: move serializer helpers into its own file
> >   lua: move luaL_newserializer() comment into header
> >   lua: split serializer functions into sections
> >   test: add a basic unit test for serializer helpers
> 
> Thanks for reviews!
> 
> Pushed to master, 2.8, 2.7.
> 
> I'll backport it to 1.10 and show you first.
> 
> WBR, Alexander Turenko.

Given 4 commits are backported almost trivially to 1.10, I'll not resend
them. I added two separate oneliners for 1.10. I'll just place them at
bottom of the email.

All 6 commits could be found on the
Totktonada/gh-3228-extract-serializer-helpers-1.10 branch.

I think that it would be convenient for further development to backport
this patchset to 1.10. It looks safe enough, because it is mainly just
code rearrangement: unlikely it may lead to new subtle bugs (aside of
building, which should be verified quite good by CI).

Please, share thoughts if you have objections against pushing it to
1.10.

CCed Kirill Yu.

----

commit 4a22b90866dfda4252577aac4dd7252e518490ae
Author: Alexander Turenko <alexander.turenko@tarantool.org>
Date:   Wed Jul 7 18:28:37 2021 +0300

    build/curl: disable ZSTD content encoding support
    
    I'm not against ZSTD content encoding.
    
    The problem I want to solve is the following. I'll add a unit test in a
    future commit:
    
     | add_executable(serializer.test serializer.c)
     | target_link_libraries(serializer.test unit box ${LUAJIT_LIBRARIES})
    
    Linking of the test fails if curl's configure script detects ZSTD
    presence and enables it. It is irrelevant for building of libcurl using
    CMake (since 2.6.0-196-g2b0760192).
    
    ZSTD content encoding support was added in curl-7.72.0, we pull it in
    1.10.9-124-ged90a8c22 ('security: update libcurl from 7.71.1 to
    7.76.0'). The key problem is that curl's configure script (unlike CMake)
    performs autodetection of present libraries by default.
    
    If we'll want to enable this content encoding, it is better to do so
    deliberately: with explicit linking against our built-in ZSTD (now the
    choice depends on presence of the library in the system) and correct
    dependencies in our CMake scripts (correct CURL_LIBRARIES).
    
    Part of #3228

diff --git a/cmake/BuildLibCURL.cmake b/cmake/BuildLibCURL.cmake
index 313adfb97..8fe8ff25a 100644
--- a/cmake/BuildLibCURL.cmake
+++ b/cmake/BuildLibCURL.cmake
@@ -95,6 +95,7 @@ macro(curl_build)
                 --with-ca-fallback
 
                 --without-brotli
+                --without-zstd
                 --without-gnutls
                 --without-mbedtls
                 --without-cyassl

commit 378a18a0850ad63ceb0bb5ef141eaa34988b01ed
Author: Alexander Turenko <alexander.turenko@tarantool.org>
Date:   Wed Jul 7 19:27:38 2021 +0300

    build: add uri library as dependency for core
    
    evio.cc from libcore.a uses uri_parse() from liburi.a. This dependency
    is already there on master.
    
    In a future commit I'll add a test:
    
     | add_executable(serializer.test serializer.c)
     | target_link_libraries(serializer.test unit box ${LUAJIT_LIBRARIES})
    
    Linking of the test fails if the dependency is not set in CMake scripts.
    
    Part of #3228

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index d8706b9a8..d5a7c6d22 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -120,7 +120,7 @@ endif ()
 add_library(core STATIC ${core_sources})
 add_dependencies(core curl)
 target_link_libraries(core
-    salad small
+    salad small uri
     ${LIBEV_LIBRARIES}
     ${LIBEIO_LIBRARIES}
     ${LIBCORO_LIBRARIES}