Tarantool development patches archive
 help / color / mirror / Atom feed
From: Igor Munkin <imun@tarantool.org>
To: "Alexander V. Tikhonov" <avtikhon@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org,
	Alexander Turenko <alexander.turenko@tarantool.org>
Subject: Re: [Tarantool-patches] [PATCH v1] Block linker flag '--no-undefined'
Date: Wed, 8 Jul 2020 14:37:26 +0300	[thread overview]
Message-ID: <20200708113726.GH5559@tarantool.org> (raw)
In-Reply-To: <81870339991bd3f54fc532b631f48d8bf4aa2b57.1594039762.git.avtikhon@tarantool.org>

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.

> 
> 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 <build_lualib> to build these extensions can you make
this change 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

  parent reply	other threads:[~2020-07-08 11:47 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-06 12:51 Alexander V. Tikhonov
2020-07-06 20:41 ` Alexander Turenko
2020-07-08 11:37 ` Igor Munkin [this message]
2020-07-08 14:25   ` Alexander V. Tikhonov
2020-07-08 19:22   ` Alexander Turenko
2020-07-09  5:41     ` Alexander V. Tikhonov
2020-07-09 19:14       ` Igor Munkin
2020-07-14 11:39 ` Kirill Yukhin

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=20200708113726.GH5559@tarantool.org \
    --to=imun@tarantool.org \
    --cc=alexander.turenko@tarantool.org \
    --cc=avtikhon@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v1] Block linker flag '\''--no-undefined'\''' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox