From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 1637B445320 for ; Wed, 8 Jul 2020 17:25:04 +0300 (MSK) Date: Wed, 8 Jul 2020 17:25:02 +0300 From: "Alexander V. Tikhonov" Message-ID: <20200708142502.GA4879@hpalx> References: <81870339991bd3f54fc532b631f48d8bf4aa2b57.1594039762.git.avtikhon@tarantool.org> <20200708113726.GH5559@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200708113726.GH5559@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v1] Block linker flag '--no-undefined' List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Munkin Cc: tarantool-patches@dev.tarantool.org Hi Igor, thanks for the review, I've tried your suggestion and it successfully worked. So I've updated the patch with it and commit message as you suggested. On Wed, Jul 08, 2020 at 02:37:26PM +0300, Igor Munkin wrote: > Sasha, > > Thanks for the patch! > > On 06.07.20, Alexander V. Tikhonov wrote: > > Found that opensuse adding linker flag '--no-undefined' which produces > > the fails on building tests. Decided to block this flag due to dynamic > > libraries will be loaded from tarantool executable and will use symbols > > from it. So it is completely okay to have unresolved symbols at build > > time. > > Minor: I reworded the message a bit: > | Found that OpenSUSE toolchain adds '--no-undefined' linked flag leading > | to fails while building tests. The changes suppress this flag since > | dynamic libraries are loaded via Tarantool executable and use its > | symbols. So it is completely OK to have undefined symbols at build time. > Feel free to adjust it on your own. > > Also, it would be nice to add "test: " prefix to explicitly mention the > affected parts. > Done, thank you. > > > > Relates to tarantool/tarantool#4562 > > --- > > > > Github: https://github.com/tarantool/luajit/tree/avtikhon/gh-4562-suse-block-linker-flag > > Issue: https://github.com/tarantool/tarantool/issues/4562 > > > > test/gh-4427-ffi-sandwich/CMakeLists.txt | 1 + > > test/lj-flush-on-trace/CMakeLists.txt | 1 + > > 2 files changed, 2 insertions(+) > > > > Strictly saying, I see no reason to fix the problem here. These changes > are similar and one ought to add this "woodoo magic" line to every > CMakeLists.txt used for building dynamic libraries using Lua C API. > Since we are using to build these extensions can you make > this change there? > Right, checked and moved there. > > diff --git a/test/gh-4427-ffi-sandwich/CMakeLists.txt b/test/gh-4427-ffi-sandwich/CMakeLists.txt > > index 995c6bb..6028381 100644 > > --- a/test/gh-4427-ffi-sandwich/CMakeLists.txt > > +++ b/test/gh-4427-ffi-sandwich/CMakeLists.txt > > @@ -1 +1,2 @@ > > +string(REPLACE "-Wl,--no-undefined" "" CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS}") > > build_lualib(libsandwich libsandwich.c) > > diff --git a/test/lj-flush-on-trace/CMakeLists.txt b/test/lj-flush-on-trace/CMakeLists.txt > > index a90452d..4f2f956 100644 > > --- a/test/lj-flush-on-trace/CMakeLists.txt > > +++ b/test/lj-flush-on-trace/CMakeLists.txt > > @@ -1 +1,2 @@ > > +string(REPLACE "-Wl,--no-undefined" "" CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS}") > > build_lualib(libflush libflush.c) > > -- > > 2.17.1 > > > > -- > Best regards, > IM