Re: [Tarantool-patches] [PATCH v2] Add missed icu symbols

[Alexandr Barulev <huston.mavr@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2024 bytes --]

Hello, thanks for the review of new patch version!

I've fixed commit message (added `build` prefix) and rebased
from master.

I think commit, mentionied at patch message ("cmake: remove
dynamic-list linker option") is related to #5266 issue.
It's because symbols exports from required libs into tarantool binary
earlier were configured with two options:
by `--whole-archive` option;
and `--dynamic-list,${exports_file}` option, where exports_file was created
with use of mkexports script (symbols was grabbed by nm -D from shared
libraries).

Also, I've checkouted "cmake: remove dynamic-list linker option"
commit, built static tarantool and ran icu-date tests. In result
tests failed with `undefined symbol` errors.
After that, I checkouted previous commit, built tarantool again
and icu-date tests succeed

пт, 25 сент. 2020 г. в 00:10, Vladislav Shpilevoy <v.shpilevoy@tarantool.org
>:

> Hi! Thanks for the patch!
>
> I would add 'build: ' prefix to the commit message.
>
> On 23.09.2020 13:02, HustonMmmavr wrote:
> > After patch 03790ac5510648d1d9648bb2281857a7992d0593 ("cmake: remove
> > dynamic-list linker option") symbols exports was changed (now we have
> > to export required symbols manually).
>
> Actually after some thinking I am not sure it is because of that commit.
> Symbols exports were always done manually. That commit changed which
> symbols were hidden. So essentially it extended the exported symbols
> set, not shrunk it.
>
> And that makes me wonder how could it lead to #5266?
>
> > Icu symbols, required by icu-date
> > rock (as ffi calls) are unused at linkage stage of tarantool binary
> > and thrown away from it so icu-date won't work (in case of tarantool
> > static build).
>
> They would have been thrown anyway. It is not related to the dynamic-list
> option removed in the mentioned commit.
>
> > This patch fixes this behaviour by adding symbols
> > required by icu-date rock to symbols export list.
> >
> > Close #5266
>

[-- Attachment #2: Type: text/html, Size: 2487 bytes --]