Skip to content
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

Merged
merged 4 commits into from Mar 13, 2024
Merged

Conversation

eboasson
Copy link
Contributor

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 by install 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.

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

Function ddsrt_dlclose declares parameter in unsupported declaration list.

#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

Function ddsrt_dlopen declares parameter in unsupported declaration list.
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

Function ddsrt_dlerror declares parameter in unsupported declaration list.
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

Function ddsrt_dlsym declares parameter in unsupported declaration list.
@@ -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

Function ddsrt_platform_dlerror declares parameter in unsupported declaration list.
{
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

Function ddsrt_platform_dlsym declares parameter in unsupported declaration list.
@@ -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

Function ddsrt_platform_dlclose declares parameter in unsupported declaration list.
@@ -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

Function ddsrt_platform_dlopen declares parameter in unsupported declaration list.
Signed-off-by: Erik Boasson <eb@ilities.com>
@eboasson eboasson marked this pull request as ready for review March 7, 2024 13:05
Copy link
Contributor

@dpotman dpotman left a 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})")
Copy link
Contributor

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?

Copy link
Contributor Author

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)
Copy link
Contributor

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?

Copy link
Contributor Author

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 <eb@ilities.com>
Signed-off-by: Erik Boasson <eb@ilities.com>
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 <eb@ilities.com>
@eboasson eboasson merged commit 79680a1 into eclipse-cyclonedds:master Mar 13, 2024
4 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants