Re: [Tarantool-patches] [PATCH v1] Block linker flag '--no-undefined'

["Alexander V. Tikhonov" <avtikhon@tarantool.org>]

Hi Alexander, thanks for the review, I've corrected the messages and
checked the usage of the functions from test/CMakeLists.txt file.

On Wed, Jul 08, 2020 at 10:22:00PM +0300, Alexander Turenko wrote:
> > Minor: I reworded the message a bit:
> > | Found that OpenSUSE toolchain adds '--no-undefined' linked flag leading
> 
> Typo: linked -> linker.
>

Corrected.

> > 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 <build_lualib> to build these extensions can you make
> > this change there?
> 
> I missed this way. It should work, but we should care about not leaving
> a test/foo subdirectory scope. I think it should be verified manually. I
> would even add a comment regarding the macro behaviour: it affects
> current cmake variable scope and so a user should care to don't use it
> in a top level scope.
>

I've added the the message below to the test/CMakeLists.txt file just
before the change:

 | # WARNING: This change affects current cmake variable scope and so
 | #          a user should care to don't use it in a top level scope.

Also I've checked all found fuctions from test/CMakeLists.txt file to be
sure that its in use only in test/* scopes:

$ for f in `grep "^function(" test/CMakeLists.txt | sed "s#^function(##g" | awk '{print($1)}'` ; do echo "========== Checking $f" ; grep -RI $f 2>/dev/null ; done
========== Checking build_module
cmake/module.cmake:function(rebuild_module_api)
src/CMakeLists.txt:rebuild_module_api(${api_headers})
test/app/CMakeLists.txt:build_module(loaderslib loaderslib.c)
test/CMakeLists.txt:function(build_module module files)
test/box/CMakeLists.txt:build_module(function1 function1.c)
test/box/CMakeLists.txt:build_module(reload1 reload1.c)
test/box/CMakeLists.txt:build_module(reload2 reload2.c)
test/box/CMakeLists.txt:build_module(tuple_bench tuple_bench.c)
test/app-tap/CMakeLists.txt:build_module(module_api module_api.c)
========== Checking build_lualib
test/CMakeLists.txt:function(build_lualib lib sources)
test/luajit-tap/lj-flush-on-trace/CMakeLists.txt:build_lualib(libflush libflush.c)
test/luajit-tap/gh-4427-ffi-sandwich/CMakeLists.txt:build_lualib(libsandwich libsandwich.c)
third_party/luajit/test/lj-flush-on-trace/CMakeLists.txt:build_lualib(libflush libflush.c)
third_party/luajit/test/gh-4427-ffi-sandwich/CMakeLists.txt:build_lualib(libsandwich libsandwich.c)

As seen above all usages of "Checking <foo>"are inside the test/ scope
except "rebuild_module_api" which is not from the checking file, but
from the src/CMakeLists.txt file and it not interesting for us.

> Sadly, we cannot solve the problem with properties, because flags from
> the LINK_FLAGS target property are added before
> CMAKE_SHARED_LINKER_FLAGS flags and because there is no --no-undefined
> counterpart linker option (there is `--undefined foo` to allow certain
> symbols to be undefined, but is not what we need here).
> 

Agree.

> WBR, Alexander Turenko.