Hide symbols back #5932

LeonidVas · 2021-03-23T11:26:49Z

After #2971 we experience a bunch of problems (1, 2). The second one still have no good solution. You can find more on the topic here.

Another side of the problem is that I heard from time to time: hey, someone can check that a symbol is present and use it in a module despite that there is no statement that we'll keep it. Yet another side of the problem is that C API description on the website significantly behind the actual state (at least I see no actions around new APIs), but it is out of scope here.

My proposal:

Set default visibility to 'hidden' back.
Either expose the entire public API of a library or hide it and create wrappers / aliases for calls using LuaJIT's FFI or from a Lua/C module.
- The former (expose) will allow to use the bundled library from a module. It'll disallow to bundle your own library version to the module (tarantool's one will anyway in use on Linux) — say, when you want to use a feature from a newer version of the library. It'll lead to more problems with bundling if the library does not provide stable ABI and intended to be statically linked into a project (like small).
- The latter (hidding) will disallow to use the bundled library from a module, but will allow to bundle the library into a module or use a system one. We can expose wrappers for necessary APIs if there is a demand (like we did for the region related APIs).

The only advantage of the unhiding symbols: sometimes you can hack something for certain tarantool executable using LuaJIT's FFI, but you'll anyway unable to use this hack in a module, because the symbol may disappear in any build: even from the same tarantool version range, but with other OS or toolchain.

The situation for tarantool executable is the same as for any other library. A library should expose only its public API and should not increase probability of hard to debug problems due to clash of a user's code with an internal name from the library.

LeonidVas · 2021-03-23T11:27:12Z

See discussion #5733

After unhiding all internal symbols([1]) we experience a bunch of problems ([2], [3]). The second one (clash of symbols from different version of the "small" library) still have no good solution. You can find more on the topic [4]. The situation for tarantool executable is the same as for any other library. A library should expose only its public API and should not increase probability of hard to debug problems due to clash of a user's code with an internal name from the library. Let's hide all symbols by default and create a list of exported symbols. (In fact, this patch is a revert of the patch 03790ac ([5]) taking into account the changes made to the code) 1. #2971 2. #5001 3. tarantool/memcached#59 4. https://lists.tarantool.org/pipermail/tarantool-discussions/2020-September/000095.html 5. 03790ac Part of #5932

After unhiding all internal symbols([1]) we experience a bunch of problems ([2], [3]). The second one (clash of symbols from different version of the "small" library) still have no good solution. You can find more on the topic [4]. The situation for tarantool executable is the same as for any other library. A library should expose only its public API and should not increase probability of hard to debug problems due to clash of a user's code with an internal name from the library. Let's hide all symbols by default and create a list of exported symbols. (In fact, this patch is a revert of the patch 03790ac ([5]) taking into account the changes made to the code) Explanation of adding some controversial symbols to the export list: * The following symbols are used in shared libraries used in tests ("cfunc*.so", "sql_uuid.so", "gh-6024-funcs-return-bin.so", "function1.so", "gh-5938-wrong-string-length.so", "module_api.so") mp_check mp_encode_array mp_encode_bin mp_encode_bool mp_encode_int mp_encode_map mp_encode_nil mp_encode_str mp_encode_uint mp_decode_array_slowpath mp_decode_str mp_next_slowpath mp_load_u8 mp_next mp_sizeof_array mp_sizeof_str mp_type_hint decimal_from_string * These symbols are used in "crypto.lua" and, if absent, will lead to the failure of the "static_build_cmake_linux" on CI (the crypto prefix was used to avoid the clashes of names) crypto_ERR_error_string crypto_ERR_get_error crypto_EVP_DigestInit_ex crypto_EVP_DigestUpdate crypto_EVP_DigestFinal_ex crypto_EVP_get_digestbyname crypto_HMAC_Init_ex crypto_HMAC_Update crypto_HMAC_Final * For correct work of "schema.lua" in the "static_build_cmake_linux" rl_get_screen_size * The following symbols are used in "ssl-cert-paths-discover.test.lua" (I think these symbols will have to be wrapped in the to avoid clashes problems) X509_get_default_cert_dir_env X509_get_default_cert_file_env ssl_cert_paths_discover From "exports.test.lua" have been removed ZSTD symbols checking (see [6]) and "tt_uuid_str" (see [7]). The "check_merge_source.c" test has been changed, now working with "fiber" and "diag" is done through API (changes were caused by hiding "diag_get"). To get a fiber Lua state, the "fiber_lua_state_internal" function has been added to the internal API. 1. #2971 2. #5001 3. tarantool/memcached#59 4. https://lists.tarantool.org/pipermail/tarantool-discussions/2020-September/000095.html 5. 03790ac 6. #4225 7. acf8745 Part of #5932

Wrap the exported readline function to avoid clash of symbols. Part of #5932

Wrap the symbols used in the "ssl-cert-paths-discover" test to avoid clashes. Symbols from openssl have been wraped to: crypto_X509_get_default_cert_dir_env crypto_X509_get_default_cert_file_env Tarantool symbols have been prefixed by "tnt_": tnt_ssl_cert_paths_discover tnt_default_cert_dir_paths tnt_default_cert_file_paths Part of #5932

Exporting symbols of a third party library is not a best practice, as we know from [1]. Let's wrap the msgpack symbols that need to be exported with the "tnt_" prefix. While working on the patch, it was decided to export the msgpack symbols that are used in "msgpuckffi.lua". In test shared libraries where the symbols "mp_***_{decimal,uuid}" are used, they are replaced to exported "tnt_mp_***_{decimal,uuid}", because in the case of linking with "libcore.a" the "libcore.a" needs to be rebuild with the "-fPIC" flag, that seems as overkill for tests. 1. tarantool/memcached#59 Closes #5932

After unhiding all internal symbols([1]) we experience a bunch of problems ([2], [3]). The second one (clash of symbols from different version of the "small" library) still have no good solution. You can find more on the topic [4]. The situation for tarantool executable is the same as for any other library. A library should expose only its public API and should not increase probability of hard to debug problems due to clash of a user's code with an internal name from the library. Let's hide all symbols by default and create a list of exported symbols. (In fact, this patch is a revert of the patch 03790ac ([5]) taking into account the changes made to the code) Explanation of adding some controversial symbols to the export list: * The following symbols are used in shared libraries used in tests ("cfunc*.so", "sql_uuid.so", "gh-6024-funcs-return-bin.so", "function1.so", "gh-5938-wrong-string-length.so", "module_api.so") mp_check mp_encode_array mp_encode_bin mp_encode_bool mp_encode_int mp_encode_map mp_encode_nil mp_encode_str mp_encode_uint mp_decode_array_slowpath mp_decode_str mp_next_slowpath mp_load_u8 mp_next mp_sizeof_array mp_sizeof_str mp_type_hint decimal_from_string * These symbols are used in "crypto.lua" and, if absent, will lead to the failure of the "static_build_cmake_linux" on CI (the crypto prefix was used to avoid the clashes of names) crypto_ERR_error_string crypto_ERR_get_error crypto_EVP_DigestInit_ex crypto_EVP_DigestUpdate crypto_EVP_DigestFinal_ex crypto_EVP_get_digestbyname crypto_HMAC_Init_ex crypto_HMAC_Update crypto_HMAC_Final * For correct work of "schema.lua" in the "static_build_cmake_linux" rl_get_screen_size * The following symbols are used in "ssl-cert-paths-discover.test.lua" (I think these symbols will have to be wrapped in the to avoid clashes problems) X509_get_default_cert_dir_env X509_get_default_cert_file_env ssl_cert_paths_discover From "exports.test.lua" have been removed ZSTD symbols checking (see [6]) and "tt_uuid_str" (see [7]). 1. #2971 2. #5001 3. tarantool/memcached#59 4. https://lists.tarantool.org/pipermail/tarantool-discussions/2020-September/000095.html 5. 03790ac 6. #4225 7. acf8745 Part of #5932

Wrap the exported readline function to avoid clash of symbols. Part of #5932

Wrap the symbols used in the "ssl-cert-paths-discover" test to avoid clashes. Symbols from openssl have been wraped to: crypto_X509_get_default_cert_dir_env crypto_X509_get_default_cert_file_env Tarantool symbols have been prefixed by "tnt_": tnt_ssl_cert_paths_discover tnt_default_cert_dir_paths tnt_default_cert_file_paths Part of #5932

After changing the way symbols are exported, handling several cases in the "ssl-cert-paths-discover" test is no longer necessary. Let's remove it. Part of #5932

After unhiding all internal symbols([1]) we experience a bunch of problems ([2], [3]). The second one (clash of symbols from different version of the "small" library) still have no good solution. You can find more on the topic [4]. The situation for tarantool executable is the same as for any other library. A library should expose only its public API and should not increase probability of hard to debug problems due to clash of a user's code with an internal name from the library. Let's hide all symbols by default and create a list of exported symbols. (In fact, this patch is a revert of the patch 03790ac ([5]) taking into account the changes made to the code) Explanation of adding some controversial symbols to the export list: * The following symbols are used in shared libraries used in tests ("cfunc*.so", "sql_uuid.so", "gh-6024-funcs-return-bin.so", "function1.so", "gh-5938-wrong-string-length.so", "module_api.so") mp_check mp_encode_array mp_encode_bin mp_encode_bool mp_encode_int mp_encode_map mp_encode_nil mp_encode_str mp_encode_uint mp_decode_array_slowpath mp_decode_str mp_next_slowpath mp_load_u8 mp_next mp_sizeof_array mp_sizeof_str mp_type_hint decimal_from_string * These symbols are used in "crypto.lua" and, if absent, will lead to the failure of the "static_build_cmake_linux" on CI (the crypto prefix was used to avoid the clashes of names) crypto_ERR_error_string crypto_ERR_get_error crypto_EVP_DigestInit_ex crypto_EVP_DigestUpdate crypto_EVP_DigestFinal_ex crypto_EVP_get_digestbyname crypto_HMAC_Init_ex crypto_HMAC_Update crypto_HMAC_Final * For correct work of "schema.lua" in the "static_build_cmake_linux" rl_get_screen_size * The following symbols are used in "ssl-cert-paths-discover.test.lua" (I think these symbols will have to be wrapped in the to avoid clashes problems) X509_get_default_cert_dir_env X509_get_default_cert_file_env ssl_cert_paths_discover From "exports.test.lua" have been removed ZSTD symbols checking (see [6]) and "tt_uuid_str" (see [7]). 1. #2971 2. #5001 3. tarantool/memcached#59 4. https://lists.tarantool.org/pipermail/tarantool-discussions/2020-September/000095.html 5. 03790ac 6. #4225 7. acf8745 Part of #5932

Wrap the exported readline function to avoid clash of symbols. Part of #5932

Wrap the symbols used in the "ssl-cert-paths-discover" test to avoid clashes. Symbols from openssl have been wraped to: crypto_X509_get_default_cert_dir_env crypto_X509_get_default_cert_file_env Tarantool symbols have been prefixed by "tnt_": tnt_ssl_cert_paths_discover tnt_default_cert_dir_paths tnt_default_cert_file_paths Part of #5932

After changing the way symbols are exported, handling several cases in the "ssl-cert-paths-discover" test is no longer necessary. Let's remove it. Part of #5932

The Rust module [1] leans on several internal symbols. They were open in Tarantool 2.8 (see tarantool#2971 and tarantool#5932), but never were in the public API. Tarantool 2.10.0 hides the symbols and we need a way to get them back to use in the module. We have the following options: 1. Design and expose a module API for fiber channels. 2. Export the symbols with a prefix like `tnt_internal_` (to don't spoil the global namespace). 3. Provide a `dlsym()` alike function to get an address of an internal symbol for users who knows what they're doing. I think that the third way offers the best compromise between amount of effort, quality of the result and opportunities to extend. In this commit I hardcoded the list of functions to make the change as safe as possible. Later I'll return here to autogenerate the list. Exported the following function from the tarantool executable: ```c void * tnt_internal_symbol(const char *name); ``` I don't add it into the module API headers, because the function is to perform a dark magic and we don't suggest it for users. While I'm here, added `static` to a couple of fiber channel functions, which are only used within the compilation unit. [1]: https://github.com/picodata/tarantool-module Part of tarantool#7228 Related to tarantool#6372 NO_DOC=don't advertize the dangerous API NO_CHANGELOG=don't advertize the dangerous API

The Rust module [1] leans on several internal symbols. They were open in Tarantool 2.8 (see #2971 and #5932), but never were in the public API. Tarantool 2.10.0 hides the symbols and we need a way to get them back to use in the module. We have the following options: 1. Design and expose a module API for fiber channels. 2. Export the symbols with a prefix like `tnt_internal_` (to don't spoil the global namespace). 3. Provide a `dlsym()` alike function to get an address of an internal symbol for users who knows what they're doing. I think that the third way offers the best compromise between amount of effort, quality of the result and opportunities to extend. In this commit I hardcoded the list of functions to make the change as safe as possible. Later I'll return here to autogenerate the list. Exported the following function from the tarantool executable: ```c void * tnt_internal_symbol(const char *name); ``` I don't add it into the module API headers, because the function is to perform a dark magic and we don't suggest it for users. While I'm here, added `static` to a couple of fiber channel functions, which are only used within the compilation unit. [1]: https://github.com/picodata/tarantool-module Part of #7228 Related to #6372 NO_DOC=don't advertize the dangerous API NO_CHANGELOG=don't advertize the dangerous API

The Rust module [1] leans on several internal symbols. They were open in Tarantool 2.8 (see #2971 and #5932), but never were in the public API. Tarantool 2.10.0 hides the symbols and we need a way to get them back to use in the module. We have the following options: 1. Design and expose a module API for fiber channels. 2. Export the symbols with a prefix like `tnt_internal_` (to don't spoil the global namespace). 3. Provide a `dlsym()` alike function to get an address of an internal symbol for users who knows what they're doing. I think that the third way offers the best compromise between amount of effort, quality of the result and opportunities to extend. In this commit I hardcoded the list of functions to make the change as safe as possible. Later I'll return here to autogenerate the list. Exported the following function from the tarantool executable: ```c void * tnt_internal_symbol(const char *name); ``` I don't add it into the module API headers, because the function is to perform a dark magic and we don't suggest it for users. While I'm here, added `static` to a couple of fiber channel functions, which are only used within the compilation unit. [1]: https://github.com/picodata/tarantool-module Part of #7228 Related to #6372 NO_DOC=don't advertize the dangerous API NO_CHANGELOG=don't advertize the dangerous API (cherry picked from commit 395c30e)

The Rust module [1] leans on several internal symbols. They were open in Tarantool 2.8 (see tarantool#2971 and tarantool#5932), but never were in the public API. Tarantool 2.10.0 hides the symbols and we need a way to get them back to use in the module. We have the following options: 1. Design and expose a module API for fiber channels. 2. Export the symbols with a prefix like `tnt_internal_` (to don't spoil the global namespace). 3. Provide a `dlsym()` alike function to get an address of an internal symbol for users who knows what they're doing. I think that the third way offers the best compromise between amount of effort, quality of the result and opportunities to extend. In this commit I hardcoded the list of functions to make the change as safe as possible. Later I'll return here to autogenerate the list. Exported the following function from the tarantool executable: ```c void * tnt_internal_symbol(const char *name); ``` I don't add it into the module API headers, because the function is to perform a dark magic and we don't suggest it for users. While I'm here, added `static` to a couple of fiber channel functions, which are only used within the compilation unit. [1]: https://github.com/picodata/tarantool-module Part of tarantool#7228 Related to tarantool#6372 NO_DOC=don't advertize the dangerous API NO_CHANGELOG=don't advertize the dangerous API

kyukhin added feature A new functionality teamE labels Mar 23, 2021

kyukhin added this to the 2.8.1 milestone Mar 23, 2021

Totktonada added the backlog label Mar 23, 2021

Totktonada added the tmp label Apr 7, 2021

kyukhin modified the milestones: 2.8.1, 2.9.1 Apr 12, 2021

Totktonada added the 5sp label Apr 22, 2021

kyukhin removed the tmp label Apr 30, 2021

Totktonada mentioned this issue Jun 9, 2021

build: linking failure when building with external ZSTD #6135

Closed

LeonidVas self-assigned this Jun 24, 2021

LeonidVas mentioned this issue Jul 29, 2021

cmake: hide tarantool symbols back #6265

Merged

LeonidVas linked a pull request Jul 29, 2021 that will close this issue

cmake: hide tarantool symbols back #6265

Merged

LeonidVas mentioned this issue Jul 29, 2021

Static build: ZSTD symbols are not exported in tarantool binary #4225

Open

LeonidVas removed a link to a pull request Jul 30, 2021

cmake: hide tarantool symbols back #6265

Merged

Totktonada mentioned this issue Aug 2, 2021

Export libsmall symbols to prevent version mismatch #6286

Closed

LeonidVas added a commit that referenced this issue Aug 3, 2021

cmake: wrap the exported readline function

88ac7ff

Wrap the exported readline function to avoid clash of symbols. Part of #5932

kyukhin removed the backlog label Aug 19, 2021

kyukhin modified the milestones: 2.9.1, 2.10.1 Aug 19, 2021

LeonidVas mentioned this issue Aug 20, 2021

Unhide symbols from the Mons lists #6372

Open

Totktonada assigned Totktonada and unassigned Totktonada Sep 9, 2021

kyukhin closed this as completed in #6307 Sep 27, 2021

yanshtunder pushed a commit that referenced this issue Oct 4, 2021

cmake: wrap the exported readline function

acfdd09

Wrap the exported readline function to avoid clash of symbols. Part of #5932

yanshtunder pushed a commit that referenced this issue Oct 4, 2021

test: remove unused cases

1a3f3fc

After changing the way symbols are exported, handling several cases in the "ssl-cert-paths-discover" test is no longer necessary. Let's remove it. Part of #5932

yanshtunder pushed a commit that referenced this issue Oct 4, 2021

cmake: wrap the exported readline function

f774db8

Wrap the exported readline function to avoid clash of symbols. Part of #5932

yanshtunder pushed a commit that referenced this issue Oct 4, 2021

test: remove unused cases

2c796a3

After changing the way symbols are exported, handling several cases in the "ssl-cert-paths-discover" test is no longer necessary. Let's remove it. Part of #5932

Totktonada mentioned this issue Dec 21, 2021

All memcached tests fail with tarantool 2.10.0-beta1-376-gd2a012455 (master branch) tarantool/memcached#96

Closed

Totktonada mentioned this issue Feb 24, 2022

Hide all small library symbols #6873

Open

Totktonada mentioned this issue Jul 16, 2022

fiber_channel: add accessor to internal functions #7423

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hide symbols back #5932

Hide symbols back #5932

LeonidVas commented Mar 23, 2021 •

edited

Loading

LeonidVas commented Mar 23, 2021

Hide symbols back #5932

Hide symbols back #5932

Comments

LeonidVas commented Mar 23, 2021 • edited Loading

LeonidVas commented Mar 23, 2021

LeonidVas commented Mar 23, 2021 •

edited

Loading