Explicitly export public API #458
Conversation
Introduce a new header <themis/themis_api.h> which defines a function attribute marker THEMIS_API. It is used to explicitly mark API that is considered public. These functions will be exported from the library. Themis does not really provide separate public header files so it's not immediately obvious which functions have to be marked public. We go with functions visible when including <themis/themis.h>. Currently we support only UNIX-like systems so we can limit ourselves to just Clang and GCC. MSVC requires slightly different syntax but we'll get back to that when we start supporting Windows again.
Just like with Themis, introduce <soter/soter_api.h> with SOTER_API and mark all public declarations as such. Note that Soter also has a... complicated story with header files. It's not apparent which headers are public and which are private. We consider public those ones included from <soter/soter.h>.
Due to historically sloppy export management, many functions in Themis (as well as in unit-tests) rely on not-so-public API to be available. Normally you cannot get access to these functions, but you can if you import <soter/soter_t.h> or some other internal header files. Introduce a new marker SOTER_PRIVATE_API to distinguish such functions from normal exports, and mark all of them exported. There are quite a few of functions, yeah... Note that while public functions are marked in header files, private export functions are marked at definition site. This is required because some of the source files do not actually include corresponding headers. Plus, this way this definitions are ugly to look at and discourage from adding new 'private' exports.
Finally, compile all source code with default symbol visibility set to "hidden". With this only symbols marked with THEMIS_API, SOTER_API, or SOTER_PRIVATE_API will get exported from libraries. "static" functions will never be exported, and non-static functions are also not exported by default unless explicitly marked as public.
ilammy
requested review from
Lagovas,
ignatk,
shadinua,
storojs72 and
vixentael
as code owners
DEPRECATED macro expands into [[deprecated]] attribute when compiling C++14. Make sure this attribute goes before GCC-like __attribute__ specifiers, or else the compiler will not recognize them properly. Unfortunately, there is no standard equivalent to visibility attributes. (And I doubt there'll ever be, based on how C++ standards committee operates.)
|
is it hard to add windows support and extend macros? found an example when read about visibility attribute - https://gcc.gnu.org/wiki/Visibility |
Add [experimental and untested] support for Windows to the API macros. Note that Windows uses slightly different approach to DLL linkage. Just like on Linux and macOS, exported functions should be marked on Windows. However, _imported_ functions should be explicitly marked as well. Effectively, this means that we should mark exported functions with __declspec(dllexport) when compiling DLL object files, but users of the DLL should see functions marked with __declspec(dllimport). We achieve that by using a special macro like THEMIS_EXPORT which is defined only when compling Themis source code, but not when using it.
|
@Lagovas, here you go, but please note that this is a blind change. Our build system does not support Windows, our CI does not do Windows builds and does not run any test, and I do not have a Windows dev box available at the moment. But something like that should do for Windows export support, assuming that we'll get the Makefile build on Windows. |
|
My initial idea was that developers shouldn't use this often. If ones does add And as for Getting back to your question... Bad news: unit tests (as they are now) do not catch exporting mistakes. The tests are linked against Themis statically, and visibility is ignored for the most part with static linkage. That is, if you add a new public function but do not mark it as exported then the tests will still link and run fine, because of the way static linkage works. However, any user of the dynamic library (e.g., our wrappers) will see weird linkage errors about missing symbols. This is true for any private dependencies as well. So I'd assume that wrapper tests will catch these mistakes, but in a late and not obvious way. I guess we can improve this by linking Soter and Themis Core tests dynamically against the libraries in the build directory. That way |
|
I've experimented with dynamic linkage for tests a bit and recalled why I made tests link statically. On Linux dynamic linkage can be more or less easily hacked with rpath, but on macOS the install name thingy causes troubles: the loader wants the libraries to reside in
@vixentael, let's leave it as is in this PR. I'll test all these things above and send a separate PR to make our tests link against libraries dynamically. That should give us nice compilation errors if we miss something. |
While not strictly required for build, let's add the headers so that they are visible in Xcode when browsing.
The header files are not strictly necessary for the build to be successful (they are not compiled). But they were missing from the project file list in Xcode. I've added them there and verified that the framework can be built, and that our Carthage examples work with that result. @vixentael, thanks for reminding me about this. I think we should work on adding Carthage to CI in order to relieve you from this duty... |
Historically, Themis did not manage public API exports at all. We exported all non-static functions and that's it. WebAssembly builds with Emscripten will require marking exported functions with
EMSCRIPTEN_KEEPALIVEattribute so now is a good moment to explicitly define our public API.Only functions explicitly marked as
THEMIS_APIandSOTER_APIwill be exported from libraries and available to users. Everything else will be hidden and not available (even if the users include some private header and try calling a function).Here are details on functions that are removed from publicly visible exports with these changes:
Soter
Themis
All these functions are not accessible normally, via
<soter/soter.h>or<themis/themis.h>.Note that some functions from private headers are required to be exported due to hysterical raisins. These ones are marked with
SOTER_PRIVATE_APIand are exceptional by all means.Introduce a new header
<themis/themis_api.h>which defines a function attribute markerTHEMIS_API. It is used to explicitly mark API that is considered public. These functions will be exported from the library.Themis does not really provide separate public header files so it's not immediately obvious which functions have to be marked public. We go with functions visible when including
<themis/themis.h>.Currently we support only UNIX-like systems so we can limit ourselves to just Clang and GCC. MSVC requires slightly different syntax but we'll get back to that when we start supporting Windows again.
Just like with Themis, introduce
<soter/soter_api.h>withSOTER_APIand mark all public declarations as such.Note that Soter also has a... complicated story with header files. It's not apparent which headers are public and which are private. We consider public those ones included from
<soter/soter.h>.Due to historically sloppy export management, many functions in Themis (as well as in unit-tests) rely on not-so-public API to be available. Normally you cannot get access to these functions, but you can if you import
<soter/soter_t.h>or some other internal header files.Introduce a new marker
SOTER_PRIVATE_APIto distinguish such functions from normal exports, and mark all of them exported. There are quite a few of functions, yeah...Note that while public functions are marked in header files, private export functions are marked at definition site. This is required because some of the source files do not actually include corresponding headers. Plus, this way this definitions are ugly to look at and discourage from adding new 'private' exports.
Finally, compile all source code with default symbol visibility set to "hidden". With this only symbols marked with
THEMIS_API,SOTER_API, orSOTER_PRIVATE_APIwill get exported from libraries.staticfunctions will never be exported, and non-static functions are also not exported by default unless explicitly marked as public.Before merge:
Make sure API mistakes are caught by automatic tests(will do later)