* [tarantool-patches] [PATCH v2] add FindICONV and iconv wrapper
@ 2018-08-01 15:49 Konstantin Belyavskiy
2018-08-13 4:21 ` [tarantool-patches] " Georgy Kirichenko
2018-08-21 14:56 ` [tarantool-patches] " Vladimir Davydov
0 siblings, 2 replies; 3+ messages in thread
From: Konstantin Belyavskiy @ 2018-08-01 15:49 UTC (permalink / raw)
To: tarantool-patches
Fixing build under FreeBSD: Undefined symbol "iconv_open"
Add compile time build check with FindICONV.cmake
and a wrapper to import relevant symbol names with include file.
Changes in V2:
- update wrong comment
- rename file and functions iconv_wrap* to tnt_iconv*
- remove unnecessary include file
Resend branch from 23/07 since it was a reply with request to
attach a patch to a mail and then further discussion stops.
Closes #3441
---
ticket: https://github.com/tarantool/tarantool/issues/3441
branch: https://github.com/tarantool/tarantool/tree/kbelyavs/gh-3441-fix-linking-with-iconv
CMakeLists.txt | 6 ++++++
cmake/FindICONV.cmake | 44 ++++++++++++++++++++++++++++++++++++++++++++
| 3 +++
src/CMakeLists.txt | 9 +++------
src/lua/iconv.lua | 41 +++++++++--------------------------------
src/lua/tnt_iconv.c | 21 +++++++++++++++++++++
6 files changed, 86 insertions(+), 38 deletions(-)
create mode 100644 cmake/FindICONV.cmake
create mode 100644 src/lua/tnt_iconv.c
diff --git a/CMakeLists.txt b/CMakeLists.txt
index d0cae0f01..cea4a7d3e 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -299,6 +299,12 @@ find_package(CURL)
set(Readline_FIND_REQUIRED ON)
find_package(Readline)
+#
+# ICONV
+#
+set(ICONV_FIND_REQUIRED ON)
+find_package(ICONV)
+
#
# ICU
#
diff --git a/cmake/FindICONV.cmake b/cmake/FindICONV.cmake
new file mode 100644
index 000000000..2ae9c8197
--- /dev/null
+++ b/cmake/FindICONV.cmake
@@ -0,0 +1,44 @@
+# - Find the iconv include file and optional library
+#
+# ICONV_INCLUDE_DIRS
+# ICONV_LIBRARIES
+#
+
+if (TARGET_OS_LINUX)
+ set(ICONV_LIBRARY "")
+else()
+ find_library(ICONV_LIBRARY iconv)
+ if(NOT ICONV_LIBRARY)
+ message(WARNING "iconv library not found")
+ set(ICONV_LIBRARY "")
+ endif()
+endif()
+find_path(ICONV_INCLUDE_DIR iconv.h)
+if(NOT ICONV_INCLUDE_DIR)
+ message(FATAL_ERROR "iconv include header not found")
+endif()
+set(ICONV_LIBRARIES ${ICONV_LIBRARY})
+set(ICONV_INCLUDE_DIRS ${ICONV_INCLUDE_DIR})
+set(CMAKE_REQUIRED_LIBRARIES ${ICONV_LIBRARIES})
+set(CMAKE_REQUIRED_INCLUDES ${ICONV_INCLUDE_DIRS})
+check_c_source_runs("
+ #include <iconv.h>
+ int main()
+ {
+ void* foo = iconv_open(\"UTF-8\", \"ISO-8859-1\");
+ iconv_close(foo);
+ return 0;
+ }
+ " ICONV_RUNS)
+set(CMAKE_REQUIRED_LIBRARIES "")
+set(CMAKE_REQUIRED_INCLUDES "")
+if (NOT DEFINED ICONV_RUNS_EXITCODE OR ICONV_RUNS_EXITCODE)
+ unset(ICONV_LIBRARIES)
+ unset(ICONV_INCLUDE_DIRS)
+ set(ICONV_FOUND false)
+ if (ICONV_FIND_REQUIRED)
+ message(FATAL_ERROR "ICONV does not run")
+ endif()
+endif()
+
+mark_as_advanced(ICONV_INCLUDE_DIRS ICONV_LIBRARIES)
--git a/extra/exports b/extra/exports
index 61a2e0a54..490118e36 100644
--- a/extra/exports
+++ b/extra/exports
@@ -38,6 +38,9 @@ title_set_custom
title_get_custom
title_set_status
title_get_status
+tnt_iconv_open
+tnt_iconv_close
+tnt_iconv
exception_get_string
exception_get_int
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 8ab09e968..b1c4cd711 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -12,6 +12,7 @@ include_directories(${LIBYAML_INCLUDE_DIRS})
include_directories(${MSGPUCK_INCLUDE_DIRS})
include_directories(${CURL_INCLUDE_DIRS})
include_directories(${ICU_INCLUDE_DIRS})
+include_directories(${ICONV_INCLUDE_DIRS})
set(LIBUTIL_FREEBSD_SRC ${CMAKE_SOURCE_DIR}/third_party/libutil_freebsd)
include_directories(${LIBUTIL_FREEBSD_SRC})
@@ -159,6 +160,7 @@ set (server_sources
lua/msgpack.c
lua/utils.c
lua/errno.c
+ lua/tnt_iconv.c
lua/socket.c
lua/pickle.c
lua/fio.c
@@ -221,6 +223,7 @@ set (common_libraries
${READLINE_LIBRARIES}
${OPENSSL_LIBRARIES}
${CURL_LIBRARIES}
+ ${ICONV_LIBRARIES}
)
if (TARGET_OS_LINUX OR TARGET_OS_DEBIAN_FREEBSD)
@@ -234,12 +237,6 @@ if (TARGET_OS_FREEBSD AND NOT TARGET_OS_DEBIAN_FREEBSD)
else()
set (common_libraries ${common_libraries} ${INTL})
endif()
- find_library (ICONV iconv)
- if (NOT ICONV)
- message(FATAL_ERROR "iconv library not found")
- else()
- set (common_libraries ${common_libraries} ${ICONV})
- endif()
endif()
set (common_libraries ${common_libraries} ${LIBUUID_LIBRARIES})
diff --git a/src/lua/iconv.lua b/src/lua/iconv.lua
index 6a6bfeb76..ade66d760 100644
--- a/src/lua/iconv.lua
+++ b/src/lua/iconv.lua
@@ -4,17 +4,10 @@ local buffer = require('buffer')
ffi.cdef[[
typedef struct iconv *iconv_t;
-iconv_t iconv_open(const char *tocode, const char *fromcode);
-void iconv_close(iconv_t cd);
-size_t iconv(iconv_t cd, const char **inbuf, size_t *inbytesleft,
- char **outbuf, size_t *outbytesleft);
-/*
- * add prefix 'lib' under FreeBSD
- */
-iconv_t libiconv_open(const char *tocode, const char *fromcode);
-void libiconv_close(iconv_t cd);
-size_t libiconv(iconv_t cd, const char **inbuf, size_t *inbytesleft,
- char **outbuf, size_t *outbytesleft);
+iconv_t tnt_iconv_open(const char *tocode, const char *fromcode);
+void tnt_iconv_close(iconv_t cd);
+size_t tnt_iconv(iconv_t cd, const char **inbuf, size_t *inbytesleft,
+ char **outbuf, size_t *outbytesleft);
]]
local iconv_t = ffi.typeof('struct iconv')
@@ -23,22 +16,6 @@ local cchar_ptr_arr_t = ffi.typeof('const char *[1]')
local cchar_ptr_t = ffi.typeof('const char *')
local size_t_arr_t = ffi.typeof('size_t [1]')
-local _iconv_open
-local _iconv_close
-local _iconv
-
--- To fix #3073, BSD iconv implementation is not fully
--- compatible with iconv, so use external iconv.so lib
-if jit.os == 'BSD' then
- _iconv_open = ffi.C.libiconv_open
- _iconv_close = ffi.C.libiconv_close
- _iconv = ffi.C.libiconv
-else
- _iconv_open = ffi.C.iconv_open
- _iconv_close = ffi.C.iconv_close
- _iconv = ffi.C.iconv
-end
-
local E2BIG = errno['E2BIG']
local EINVAL = errno['EINVAL']
local EILSEQ = errno['EILSEQ']
@@ -64,10 +41,10 @@ local function iconv_convert(iconv, data)
while data_left[0] > 0 do
buf_ptr[0] = buf:reserve(output_len)
buf_left[0] = buf:unused()
- local res = _iconv(iconv, data_ptr, data_left,
+ local res = ffi.C.tnt_iconv(iconv, data_ptr, data_left,
buf_ptr, buf_left)
if res == ffi.cast('size_t', -1) and errno() ~= E2BIG then
- _iconv(iconv, nil, nil, nil, nil)
+ ffi.C.tnt_iconv(iconv, nil, nil, nil, nil)
if errno() == EINVAL then
error('Invalid multibyte sequence')
end
@@ -80,7 +57,7 @@ local function iconv_convert(iconv, data)
end
-- iconv function sets cd's conversion state to the initial state
- _iconv(iconv, nil, nil, nil, nil)
+ ffi.C.tnt_iconv(iconv, nil, nil, nil, nil)
local result = ffi.string(buf.rpos, buf:size())
buf:reset()
return result
@@ -88,7 +65,7 @@ end
local iconv_mt = {
__call = iconv_convert,
- __gc = _iconv_close,
+ __gc = ffi.C.tnt_iconv_close,
__tostring = function(iconv) return string.format("iconv: %p", iconv) end
}
@@ -98,7 +75,7 @@ local function iconv_new(to, from)
if type(to) ~= 'string' or type(from) ~= 'string' then
error('Usage: iconv.new("CP1251", "KOI8-R")')
end
- local iconv = _iconv_open(to, from)
+ local iconv = ffi.C.tnt_iconv_open(to, from)
if iconv == conv_rv_error then
error('iconv: '..errno.strerror())
end
diff --git a/src/lua/tnt_iconv.c b/src/lua/tnt_iconv.c
new file mode 100644
index 000000000..c91fef140
--- /dev/null
+++ b/src/lua/tnt_iconv.c
@@ -0,0 +1,21 @@
+#include <iconv.h>
+
+iconv_t
+tnt_iconv_open(const char *tocode, const char *fromcode)
+{
+ return iconv_open(tocode, fromcode);
+}
+
+int
+tnt_iconv_close(iconv_t cd)
+{
+ return iconv_close(cd);
+}
+
+size_t
+tnt_iconv(iconv_t cd, char **inbuf, size_t *inbytesleft,
+ char **outbuf, size_t *outbytesleft)
+{
+ return iconv(cd, inbuf, inbytesleft,
+ outbuf, outbytesleft);
+}
--
2.14.3 (Apple Git-98)
^ permalink raw reply [flat|nested] 3+ messages in thread
* [tarantool-patches] Re: [PATCH v2] add FindICONV and iconv wrapper
2018-08-01 15:49 [tarantool-patches] [PATCH v2] add FindICONV and iconv wrapper Konstantin Belyavskiy
@ 2018-08-13 4:21 ` Georgy Kirichenko
2018-08-21 14:56 ` [tarantool-patches] " Vladimir Davydov
1 sibling, 0 replies; 3+ messages in thread
From: Georgy Kirichenko @ 2018-08-13 4:21 UTC (permalink / raw)
To: tarantool-patches; +Cc: Konstantin Belyavskiy
[-- Attachment #1: Type: text/plain, Size: 8732 bytes --]
LGTM, thanks
On Wednesday, August 1, 2018 6:49:31 PM MSK Konstantin Belyavskiy wrote:
> Fixing build under FreeBSD: Undefined symbol "iconv_open"
> Add compile time build check with FindICONV.cmake
> and a wrapper to import relevant symbol names with include file.
>
> Changes in V2:
> - update wrong comment
> - rename file and functions iconv_wrap* to tnt_iconv*
> - remove unnecessary include file
>
> Resend branch from 23/07 since it was a reply with request to
> attach a patch to a mail and then further discussion stops.
>
> Closes #3441
> ---
> ticket: https://github.com/tarantool/tarantool/issues/3441
> branch:
> https://github.com/tarantool/tarantool/tree/kbelyavs/gh-3441-fix-linking-wi
> th-iconv
>
> CMakeLists.txt | 6 ++++++
> cmake/FindICONV.cmake | 44 ++++++++++++++++++++++++++++++++++++++++++++
> extra/exports | 3 +++
> src/CMakeLists.txt | 9 +++------
> src/lua/iconv.lua | 41 +++++++++--------------------------------
> src/lua/tnt_iconv.c | 21 +++++++++++++++++++++
> 6 files changed, 86 insertions(+), 38 deletions(-)
> create mode 100644 cmake/FindICONV.cmake
> create mode 100644 src/lua/tnt_iconv.c
>
> diff --git a/CMakeLists.txt b/CMakeLists.txt
> index d0cae0f01..cea4a7d3e 100644
> --- a/CMakeLists.txt
> +++ b/CMakeLists.txt
> @@ -299,6 +299,12 @@ find_package(CURL)
> set(Readline_FIND_REQUIRED ON)
> find_package(Readline)
>
> +#
> +# ICONV
> +#
> +set(ICONV_FIND_REQUIRED ON)
> +find_package(ICONV)
> +
> #
> # ICU
> #
> diff --git a/cmake/FindICONV.cmake b/cmake/FindICONV.cmake
> new file mode 100644
> index 000000000..2ae9c8197
> --- /dev/null
> +++ b/cmake/FindICONV.cmake
> @@ -0,0 +1,44 @@
> +# - Find the iconv include file and optional library
> +#
> +# ICONV_INCLUDE_DIRS
> +# ICONV_LIBRARIES
> +#
> +
> +if (TARGET_OS_LINUX)
> + set(ICONV_LIBRARY "")
> +else()
> + find_library(ICONV_LIBRARY iconv)
> + if(NOT ICONV_LIBRARY)
> + message(WARNING "iconv library not found")
> + set(ICONV_LIBRARY "")
> + endif()
> +endif()
> +find_path(ICONV_INCLUDE_DIR iconv.h)
> +if(NOT ICONV_INCLUDE_DIR)
> + message(FATAL_ERROR "iconv include header not found")
> +endif()
> +set(ICONV_LIBRARIES ${ICONV_LIBRARY})
> +set(ICONV_INCLUDE_DIRS ${ICONV_INCLUDE_DIR})
> +set(CMAKE_REQUIRED_LIBRARIES ${ICONV_LIBRARIES})
> +set(CMAKE_REQUIRED_INCLUDES ${ICONV_INCLUDE_DIRS})
> +check_c_source_runs("
> + #include <iconv.h>
> + int main()
> + {
> + void* foo = iconv_open(\"UTF-8\", \"ISO-8859-1\");
> + iconv_close(foo);
> + return 0;
> + }
> + " ICONV_RUNS)
> +set(CMAKE_REQUIRED_LIBRARIES "")
> +set(CMAKE_REQUIRED_INCLUDES "")
> +if (NOT DEFINED ICONV_RUNS_EXITCODE OR ICONV_RUNS_EXITCODE)
> + unset(ICONV_LIBRARIES)
> + unset(ICONV_INCLUDE_DIRS)
> + set(ICONV_FOUND false)
> + if (ICONV_FIND_REQUIRED)
> + message(FATAL_ERROR "ICONV does not run")
> + endif()
> +endif()
> +
> +mark_as_advanced(ICONV_INCLUDE_DIRS ICONV_LIBRARIES)
> diff --git a/extra/exports b/extra/exports
> index 61a2e0a54..490118e36 100644
> --- a/extra/exports
> +++ b/extra/exports
> @@ -38,6 +38,9 @@ title_set_custom
> title_get_custom
> title_set_status
> title_get_status
> +tnt_iconv_open
> +tnt_iconv_close
> +tnt_iconv
> exception_get_string
> exception_get_int
>
> diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
> index 8ab09e968..b1c4cd711 100644
> --- a/src/CMakeLists.txt
> +++ b/src/CMakeLists.txt
> @@ -12,6 +12,7 @@ include_directories(${LIBYAML_INCLUDE_DIRS})
> include_directories(${MSGPUCK_INCLUDE_DIRS})
> include_directories(${CURL_INCLUDE_DIRS})
> include_directories(${ICU_INCLUDE_DIRS})
> +include_directories(${ICONV_INCLUDE_DIRS})
>
> set(LIBUTIL_FREEBSD_SRC ${CMAKE_SOURCE_DIR}/third_party/libutil_freebsd)
> include_directories(${LIBUTIL_FREEBSD_SRC})
> @@ -159,6 +160,7 @@ set (server_sources
> lua/msgpack.c
> lua/utils.c
> lua/errno.c
> + lua/tnt_iconv.c
> lua/socket.c
> lua/pickle.c
> lua/fio.c
> @@ -221,6 +223,7 @@ set (common_libraries
> ${READLINE_LIBRARIES}
> ${OPENSSL_LIBRARIES}
> ${CURL_LIBRARIES}
> + ${ICONV_LIBRARIES}
> )
>
> if (TARGET_OS_LINUX OR TARGET_OS_DEBIAN_FREEBSD)
> @@ -234,12 +237,6 @@ if (TARGET_OS_FREEBSD AND NOT TARGET_OS_DEBIAN_FREEBSD)
> else()
> set (common_libraries ${common_libraries} ${INTL})
> endif()
> - find_library (ICONV iconv)
> - if (NOT ICONV)
> - message(FATAL_ERROR "iconv library not found")
> - else()
> - set (common_libraries ${common_libraries} ${ICONV})
> - endif()
> endif()
>
> set (common_libraries ${common_libraries} ${LIBUUID_LIBRARIES})
> diff --git a/src/lua/iconv.lua b/src/lua/iconv.lua
> index 6a6bfeb76..ade66d760 100644
> --- a/src/lua/iconv.lua
> +++ b/src/lua/iconv.lua
> @@ -4,17 +4,10 @@ local buffer = require('buffer')
>
> ffi.cdef[[
> typedef struct iconv *iconv_t;
> -iconv_t iconv_open(const char *tocode, const char *fromcode);
> -void iconv_close(iconv_t cd);
> -size_t iconv(iconv_t cd, const char **inbuf, size_t *inbytesleft,
> - char **outbuf, size_t *outbytesleft);
> -/*
> - * add prefix 'lib' under FreeBSD
> - */
> -iconv_t libiconv_open(const char *tocode, const char *fromcode);
> -void libiconv_close(iconv_t cd);
> -size_t libiconv(iconv_t cd, const char **inbuf, size_t *inbytesleft,
> - char **outbuf, size_t *outbytesleft);
> +iconv_t tnt_iconv_open(const char *tocode, const char *fromcode);
> +void tnt_iconv_close(iconv_t cd);
> +size_t tnt_iconv(iconv_t cd, const char **inbuf, size_t *inbytesleft,
> + char **outbuf, size_t *outbytesleft);
> ]]
>
> local iconv_t = ffi.typeof('struct iconv')
> @@ -23,22 +16,6 @@ local cchar_ptr_arr_t = ffi.typeof('const char *[1]')
> local cchar_ptr_t = ffi.typeof('const char *')
> local size_t_arr_t = ffi.typeof('size_t [1]')
>
> -local _iconv_open
> -local _iconv_close
> -local _iconv
> -
> --- To fix #3073, BSD iconv implementation is not fully
> --- compatible with iconv, so use external iconv.so lib
> -if jit.os == 'BSD' then
> - _iconv_open = ffi.C.libiconv_open
> - _iconv_close = ffi.C.libiconv_close
> - _iconv = ffi.C.libiconv
> -else
> - _iconv_open = ffi.C.iconv_open
> - _iconv_close = ffi.C.iconv_close
> - _iconv = ffi.C.iconv
> -end
> -
> local E2BIG = errno['E2BIG']
> local EINVAL = errno['EINVAL']
> local EILSEQ = errno['EILSEQ']
> @@ -64,10 +41,10 @@ local function iconv_convert(iconv, data)
> while data_left[0] > 0 do
> buf_ptr[0] = buf:reserve(output_len)
> buf_left[0] = buf:unused()
> - local res = _iconv(iconv, data_ptr, data_left,
> + local res = ffi.C.tnt_iconv(iconv, data_ptr, data_left,
> buf_ptr, buf_left)
> if res == ffi.cast('size_t', -1) and errno() ~= E2BIG then
> - _iconv(iconv, nil, nil, nil, nil)
> + ffi.C.tnt_iconv(iconv, nil, nil, nil, nil)
> if errno() == EINVAL then
> error('Invalid multibyte sequence')
> end
> @@ -80,7 +57,7 @@ local function iconv_convert(iconv, data)
> end
>
> -- iconv function sets cd's conversion state to the initial state
> - _iconv(iconv, nil, nil, nil, nil)
> + ffi.C.tnt_iconv(iconv, nil, nil, nil, nil)
> local result = ffi.string(buf.rpos, buf:size())
> buf:reset()
> return result
> @@ -88,7 +65,7 @@ end
>
> local iconv_mt = {
> __call = iconv_convert,
> - __gc = _iconv_close,
> + __gc = ffi.C.tnt_iconv_close,
> __tostring = function(iconv) return string.format("iconv: %p", iconv)
> end }
>
> @@ -98,7 +75,7 @@ local function iconv_new(to, from)
> if type(to) ~= 'string' or type(from) ~= 'string' then
> error('Usage: iconv.new("CP1251", "KOI8-R")')
> end
> - local iconv = _iconv_open(to, from)
> + local iconv = ffi.C.tnt_iconv_open(to, from)
> if iconv == conv_rv_error then
> error('iconv: '..errno.strerror())
> end
> diff --git a/src/lua/tnt_iconv.c b/src/lua/tnt_iconv.c
> new file mode 100644
> index 000000000..c91fef140
> --- /dev/null
> +++ b/src/lua/tnt_iconv.c
> @@ -0,0 +1,21 @@
> +#include <iconv.h>
> +
> +iconv_t
> +tnt_iconv_open(const char *tocode, const char *fromcode)
> +{
> + return iconv_open(tocode, fromcode);
> +}
> +
> +int
> +tnt_iconv_close(iconv_t cd)
> +{
> + return iconv_close(cd);
> +}
> +
> +size_t
> +tnt_iconv(iconv_t cd, char **inbuf, size_t *inbytesleft,
> + char **outbuf, size_t *outbytesleft)
> +{
> + return iconv(cd, inbuf, inbytesleft,
> + outbuf, outbytesleft);
> +}
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [tarantool-patches] [PATCH v2] add FindICONV and iconv wrapper
2018-08-01 15:49 [tarantool-patches] [PATCH v2] add FindICONV and iconv wrapper Konstantin Belyavskiy
2018-08-13 4:21 ` [tarantool-patches] " Georgy Kirichenko
@ 2018-08-21 14:56 ` Vladimir Davydov
1 sibling, 0 replies; 3+ messages in thread
From: Vladimir Davydov @ 2018-08-21 14:56 UTC (permalink / raw)
To: tarantool-patches
Pushed to 1.9
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-08-21 14:56 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-01 15:49 [tarantool-patches] [PATCH v2] add FindICONV and iconv wrapper Konstantin Belyavskiy
2018-08-13 4:21 ` [tarantool-patches] " Georgy Kirichenko
2018-08-21 14:56 ` [tarantool-patches] " Vladimir Davydov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox