Tarantool development patches archive
 help / color / mirror / Atom feed
From: Alexander Turenko via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>,
	Cyrill Gorcunov <gorcunov@tarantool.org>,
	Roman Khabibov <roman.habibov@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH 0/4] RFC: Isolate serializer helpers
Date: Wed, 7 Jul 2021 22:09:59 +0300
Message-ID: <20210707190959.jv67vdoxhhsr4nqm@tkn_work_nb> (raw)
In-Reply-To: <20210707100809.nflftsb47hq66efn@tkn_work_nb>

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}

  reply	other threads:[~2021-07-07 19:10 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-23 19:12 Alexander Turenko via Tarantool-patches
2021-06-23 19:12 ` [Tarantool-patches] [PATCH 1/4] lua: move serializer helpers into its own file Alexander Turenko via Tarantool-patches
2021-07-04 13:10   ` Vladislav Shpilevoy via Tarantool-patches
2021-07-05  6:30     ` Alexander Turenko via Tarantool-patches
2021-07-05 20:59       ` Vladislav Shpilevoy via Tarantool-patches
2021-06-23 19:12 ` [Tarantool-patches] [PATCH 2/4] lua: move luaL_newserializer() comment into header Alexander Turenko via Tarantool-patches
2021-06-23 19:12 ` [Tarantool-patches] [PATCH 3/4] lua: split serializer functions into sections Alexander Turenko via Tarantool-patches
2021-06-23 19:12 ` [Tarantool-patches] [PATCH 4/4] test: add a basic unit test for serializer helpers Alexander Turenko via Tarantool-patches
2021-06-24  6:17 ` [Tarantool-patches] [PATCH 0/4] RFC: Isolate " Cyrill Gorcunov via Tarantool-patches
2021-06-28  6:31 ` Cyrill Gorcunov via Tarantool-patches
2021-07-04 13:09 ` Vladislav Shpilevoy via Tarantool-patches
2021-07-05  6:30   ` Alexander Turenko via Tarantool-patches
2021-07-07 10:08 ` Alexander Turenko via Tarantool-patches
2021-07-07 19:09   ` Alexander Turenko via Tarantool-patches [this message]
2021-07-07 22:16     ` Vladislav Shpilevoy via Tarantool-patches
2021-07-12  7:51       ` Alexander Turenko via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210707190959.jv67vdoxhhsr4nqm@tkn_work_nb \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=alexander.turenko@tarantool.org \
    --cc=gorcunov@tarantool.org \
    --cc=roman.habibov@tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Tarantool development patches archive

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://lists.tarantool.org/tarantool-patches/0 tarantool-patches/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 tarantool-patches tarantool-patches/ https://lists.tarantool.org/tarantool-patches \
		tarantool-patches@dev.tarantool.org.
	public-inbox-index tarantool-patches

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git