-
Notifications
You must be signed in to change notification settings - Fork 357
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Static build support #1864
Static build support #1864
Conversation
c658dde
to
e5d4bd8
Compare
return ddsrt_platform_dlopen (name, translate, handle); | ||
} | ||
|
||
dds_return_t ddsrt_dlclose (ddsrt_dynlib_t handle) |
Check failure
Code scanning / CodeQL
RULE-8-2: Function types shall be in prototype form with named parameters
|
||
#else | ||
|
||
dds_return_t ddsrt_dlopen (const char *name, bool translate, ddsrt_dynlib_t *handle) |
Check failure
Code scanning / CodeQL
RULE-8-2: Function types shall be in prototype form with named parameters
return ddsrt_platform_dlsym (handle, symbol, address); | ||
} | ||
|
||
dds_return_t ddsrt_dlerror (char *buf, size_t buflen) |
Check failure
Code scanning / CodeQL
RULE-8-2: Function types shall be in prototype form with named parameters
return ddsrt_platform_dlclose (handle); | ||
} | ||
|
||
dds_return_t ddsrt_dlsym (ddsrt_dynlib_t handle, const char *symbol, void **address) |
Check failure
Code scanning / CodeQL
RULE-8-2: Function types shall be in prototype form with named parameters
@@ -60,7 +60,7 @@ | |||
return (*address == NULL) ? DDS_RETCODE_ERROR : DDS_RETCODE_OK; | |||
} | |||
|
|||
dds_return_t ddsrt_dlerror (char *buf, size_t buflen) | |||
dds_return_t ddsrt_platform_dlerror (char *buf, size_t buflen) |
Check failure
Code scanning / CodeQL
RULE-8-2: Function types shall be in prototype form with named parameters
{ | ||
assert (handle); | ||
return (dlclose (handle) == 0) ? DDS_RETCODE_OK : DDS_RETCODE_ERROR; | ||
} | ||
|
||
dds_return_t ddsrt_dlsym (ddsrt_dynlib_t handle, const char *symbol, void **address) | ||
dds_return_t ddsrt_platform_dlsym (ddsrt_dynlib_t handle, const char *symbol, void **address) |
Check failure
Code scanning / CodeQL
RULE-8-2: Function types shall be in prototype form with named parameters
@@ -45,13 +45,13 @@ | |||
return *handle != NULL ? DDS_RETCODE_OK : DDS_RETCODE_ERROR; | |||
} | |||
|
|||
dds_return_t ddsrt_dlclose (ddsrt_dynlib_t handle) | |||
dds_return_t ddsrt_platform_dlclose (ddsrt_dynlib_t handle) |
Check failure
Code scanning / CodeQL
RULE-8-2: Function types shall be in prototype form with named parameters
@@ -17,7 +17,7 @@ | |||
#include "dds/ddsrt/io.h" | |||
#include "dds/ddsrt/string.h" | |||
|
|||
dds_return_t ddsrt_dlopen (const char *name, bool translate, ddsrt_dynlib_t *handle) | |||
dds_return_t ddsrt_platform_dlopen (const char *name, bool translate, ddsrt_dynlib_t *handle) |
Check failure
Code scanning / CodeQL
RULE-8-2: Function types shall be in prototype form with named parameters
Signed-off-by: Erik Boasson <[email protected]>
f316659
to
823ac28
Compare
5cbf287
to
99ff1ba
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good and works as expected on my dev machine (nice macro hack for the dynamic lib loader 😄). Below a few minor remarks/questions, but nothing blocking.
CMakeLists.txt
Outdated
# It appears that on macOS, all code is position independent and it'll work regardless. | ||
option(BUILD_SHARED_LIBS "Build using shared libraries" ON) | ||
if(NOT BUILD_SHARED_LIBS AND NOT CMAKE_CROSSCOMPILING) | ||
message(WARNING "It is probably best to build a static library as-if cross compiling (e.g., use -DCMAKE_CROSSCOMPILING=1 -DCMAKE_SYSTEM_NAME=${CMAKE_SYSTEM_NAME})") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe also add a note that in this case CMAKE_PREFIX_PATH
needs to include the install directory of a shared-lib build, in order to find the cycloneddsidlc
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I added that.
init_test_${libname}_remote_reader_not_allowed | ||
init_test_${libname}_remote_reader_relay_only | ||
init_test_${libname}_remote_permissions_not_allowed | ||
finalize_test_${libname}_not_allowed) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updating these symbol lists can easily be missed when adding functions, but I guess that leads to an error in the static build on CI?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, leaving them out means that dlsym
will fail to find the symbols and that causes test failures. It would be nicer to automatically generate this list, but then I need to figure out a way to build the library first, then extract the symbols, add them to the plug-in properties, then continue building the library. I'd have done it if it were normal make
... Maybe one day I can find the courage to try it with CMake!
With apologies for the CMake abuse, this adds support for statically linking the Cyclone core library and the security and Iceoryx plugins. It does not support statically linking IDLC because that would at best only support the C binding, but not other language bindings. The recommendation for a static linked Cyclone is to build it as if it were a cross build to the native platform, that way the IDLC complications are avoided. Almost all the changes are in the build system, only the "dlopen" functions in ddsrt are modified to consult a table of compiled-in plugins first and some security tests need a small change in the specification of the security plugin wrapper libraries. The build system defines a global CMake property "plugin_list" containing a list of all the plugins to include, each plugin P defines a global "P_symbols" property that lists all the symbols to be exported from the library. The modified "dlopen" functionality consults the table of plugins, the modified "dlsym" functionality consults the list of exported symbols. These properties are translated to macros on compiler command-line into comma-separated lists of plugin names (PLUGINS) and symbol names (PLUG_SUMBOLS_P for plugin P). The macros are then expanded into static tables of names and addresses by the C preprocessor. For a static build, the plugins are defined as OBJECT libraries, "installed" because that's the only way I have found so far to keep CMake happy with the references, and pulled into the Cyclone library using the aforementioned properties. For example: if(BUILD_SHARED_LIBS) add_library(dds_security_ac SHARED ${sources} ${private_headers}) else() add_library(dds_security_ac OBJECT ${sources} ${private_headers}) set_property(GLOBAL APPEND PROPERTY cdds_plugin_list dds_security_ac) set_property(GLOBAL PROPERTY dds_security_ac_symbols init_access_control finalize_access_control) endif() The other changes are then to only link the plugin against "ddsc" if building as a shared library and, sometimes, adding a bunch of include paths because it won't work otherwise. In a static build, the tests for PSMX that rely on the Cyclone-based plugin are disabled because of the complications that introduces. Those tests do run if Iceoryx is available. Signed-off-by: Erik Boasson <[email protected]>
Signed-off-by: Erik Boasson <[email protected]>
Installing openssl 3.2.1 with Chocolatey if openssl 1.x is already present gives interesting results (it uses the include files from 3.x but the libraries of 1.x). You'd think that uninstalling openssl 1.x, then installing openssl 3.2.1 would solve all problems, but then CMake says it can't find OpenSSL at all, and indeed it can't find the libraries in the locations where it is looking for them. Signed-off-by: Erik Boasson <[email protected]>
99ff1ba
to
273b06b
Compare
Master currently doesn't support a static build including Iceoryx support.
This PR aims to address that. It works locally for me on macOS but there are some lingering issues. The nice bit is that it now also supports statically linking in the security plugins, so a statically linked build including DDS Security is now possible.
I have been rather careless (reckless?) in the CMake bits and it is horribly ugly now. I don't actually know the detailed behaviour of
OBJECT
libraries, how much pain is caused byinstall
entry on some internal things just to be allowed to reference it elsewhere, how to set properties for include directories so that I don't have to write them out by hand, etc.Maybe someone with a good idea on how to clean this up sees it and is willing to help. That's actually the primary reason for making it a draft PR now.