[Tarantool-patches] [PATCH] Add missed icu symbols
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Mon Sep 21 23:29:58 MSK 2020
Thanks for the patch!
On 21.09.2020 21:51, HustonMmmavr wrote:
> After patch 03790ac55 symbols exports was changed (now we have to
When we mention a commit, we use a full hash (usually), and we include
the commit's title in ("...").
... your text 03790ac5510648d1d9648bb2281857a7992d0593 ("cmake:
remove dynamic-list linker option") your next text ...
See 5 comments below.
> export required symbols manually). 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). This patch fixes this behaviour by adding symbols
> required by icu-date rock to symbols export list.
>
> Close #5266
> ---
>
> Branch: https://github.com/tarantool/tarantool/tree/HustonMmmavr/gh-5266-icu-symbols
> Issue: https://github.com/tarantool/tarantool/issues/5266
>
> src/exports.c | 2 +
> src/exports_icu.h | 58 ++++++++++++++
> test/box-tap/gh-5266-icu-exports.test.lua | 97 +++++++++++++++++++++++
> 3 files changed, 157 insertions(+)
> create mode 100644 src/exports_icu.h
> create mode 100755 test/box-tap/gh-5266-icu-exports.test.lua
>
> diff --git a/src/exports.c b/src/exports.c
> index a3c27143e..3dfef62f0 100644
> --- a/src/exports.c
> +++ b/src/exports.c
> @@ -95,6 +95,7 @@
> */
> #define EXPORT(symbol) extern void symbol(void);
> #include "exports.h"
> +#include "exports_icu.h"
> #undef EXPORT
>
> void **
> @@ -108,6 +109,7 @@ export_syms(void)
> #define EXPORT(symbol) ((void *)symbol),
> static void *symbols[] = {
> #include "exports.h"
> + #include "exports_icu.h"
1. Lets better not touch exports.c. Better leave it subsystem independent if
possible, and not duplicate the "include".
> };
> #undef EXPORT
> return symbols;
> diff --git a/src/exports_icu.h b/src/exports_icu.h
> new file mode 100644
> index 000000000..1899a6749
> --- /dev/null
> +++ b/src/exports_icu.h
> @@ -0,0 +1,58 @@
> +#include <unicode/urename.h>
> +
> +/**
> +* ICU libraries contains symbols with appended lib version suffix.
> +* (This behaviour conifgures by --enable-renaming/disable flag when
2. conifgures -> is configured.
> +* building libicu; by default --enable-renaming is used)
> +*
> +* To solve symbols suffix problem <unicode/urename.h> file was included.
> +* At this file used #define macros to rename symbols.
> +* For example: symobl `u_strlen` converts to `ustrlen_VER`
> +*
> +* Next symbols used at icu-date rock (also here added u_getVersion symbol)
> +* Also we assume that icu version is >= 55, because two symbols used by
> +* icu-date rock appeared at following library versions:
> +* - ucal_getTimeZoneID at icu 51
> +* - udat_formatCalendar at icu 55
> +*
> +* To add this symbols to export list properly (to keep backward compatibility
> +* with an older versions of icu) was used define from icu library:
> +* U_ICU_VERSION_MAJOR_NUM.
> +*
> +* Older versions of libicu, for example 4.8 (or 48) has following define:
> +* #define U_ICU_VERSION_MAJOR_NUM 4
> +* From version 49 of libicu numbering changed, for example libicu 50 has
> +* following define:
> +* #define U_ICU_VERSION_MAJOR_NUM 50
> +* Here are links for version numbering of libicu:
> +* - Changes at icu 49: http://site.icu-project.org/download/49
> +* - Version numbering: http://userguide.icu-project.org/design#TOC-Version-Numbers-in-ICU
> +*/
> +EXPORT(u_strlen)
> +EXPORT(u_uastrcpy)
> +EXPORT(u_austrcpy)
> +EXPORT(u_errorName)
> +EXPORT(u_getVersion)
> +EXPORT(udat_open)
> +EXPORT(udat_setLenient)
> +EXPORT(udat_close)
> +EXPORT(udat_parseCalendar)
> +#if U_ICU_VERSION_MAJOR_NUM >= 55
> + EXPORT(udat_formatCalendar)
> +#endif
> +EXPORT(ucal_open)
> +EXPORT(ucal_close)
> +EXPORT(ucal_get)
> +EXPORT(ucal_set)
> +EXPORT(ucal_add)
> +EXPORT(ucal_clear)
> +EXPORT(ucal_clearField)
> +EXPORT(ucal_getMillis)
> +EXPORT(ucal_setMillis)
> +EXPORT(ucal_getAttribute)
> +EXPORT(ucal_setAttribute)
> +#if U_ICU_VERSION_MAJOR_NUM >= 51
> + EXPORT(ucal_getTimeZoneID)
> +#endif
> +EXPORT(ucal_setTimeZone)
> +EXPORT(ucal_getNow)
3. Lets better store everything in exports.h. You can add a new
section in this file separated by a comment, that these symbols are
from ICU, to not scatter them among other exports. Also please sort
the exports in the alphabetical order, as it is asked in exports.h
comment.
> diff --git a/test/box-tap/gh-5266-icu-exports.test.lua b/test/box-tap/gh-5266-icu-exports.test.lua
> new file mode 100755
> index 000000000..49e012fac
> --- /dev/null
> +++ b/test/box-tap/gh-5266-icu-exports.test.lua
4. Since nothing depends on box here, I would move it to app-tap.
> @@ -0,0 +1,97 @@
> +#!/usr/bin/env tarantool
> +
> +local tap = require('tap')
> +local ffi = require('ffi')
> +
> +local test = tap.test('icu_exports')
> +test:plan(2)
> +
> +-- Find `u_getVersion` symbol (by nm) from tarantool binary
> +-- for further detection of icu version. There are two approaches:
> +-- * some of icu packages appends icu version to it's symbols as suffix,
> +-- so we need to simple parse it.
> +-- * other packages don't append version as suffix, so in this case
> +-- we have to call u_getVersion through ffi to grab icu version.
> +-- On Freebsd nm requires -D option, while on Linux it's not necessary
> +-- and macOS nm doesn't have this option.
> +local tnt_path = arg[-1]
> +local pipe = io.popen(string.format(
> + 'nm %s %s | grep u_getVersion',
> + jit.os == "BSD" and '-D' or '',
> + tnt_path
> +))
> +local u_strlen_info = pipe:read('*all')
> +pipe:close()
> +
> +test:ok(u_strlen_info ~= '', "Finded u_getVersion symbol (to get icu version)")
5. Finded -> Found.
> +local icu_version_suffix = u_strlen_info:gsub('.*u_getVersion', ''):gsub('\n', '')
More information about the Tarantool-patches
mailing list