-
Notifications
You must be signed in to change notification settings - Fork 37
Description
Bug report
Required Info:
- Operating System:
- Windows 10/11
- Installation type:
- Binaries
- Version or commit hash:
- DDS implementation:
- fastrtps
- Client library (if applicable):
- N/A
Steps to reproduce issue
See the CI.yml GitHub of the repo https://github.com/traversaro/test_static_lib_ros2_srv. In particular, this repo generate the library for a .srv file, and the important thing is that the CMake project is compiled with BUILD_SHARED_LIBS set to OFF .
Expected behavior
I would expect the compilation to end successfully, as it was happening in Galactic.
Actual behavior
The compilation fails with error:
Additional information
I think this is a regression of #87, where:
if(WIN32)
target_compile_definitions(${rosidl_generate_interfaces_TARGET}${_target_suffix}
PRIVATE "ROSIDL_TYPESUPPORT_FASTRTPS_CPP_BUILDING_DLL_${PROJECT_NAME}")
endif()
has been substituted with
set_target_properties(${rosidl_generate_interfaces_TARGET}${_target_suffix}
PROPERTIES
DEFINE_SYMBOL "ROSIDL_TYPESUPPORT_FASTRTPS_CPP_BUILDING_DLL_${PROJECT_NAME}"
CXX_STANDARD 14)
However, this is not the fully story. The use of DEFINE_SYMBOL is actually correct in this context. The reason why everything is now failing is that the visibility header generated by this repo does not cover the case of a static libraries, for which the ROSIDL_TYPESUPPORT_FASTRTPS_CPP_PUBLIC_@PROJECT_NAME@ macro should evaluate to an empty string. Everything was working fine just because apparently in this case static libraries do not have problems if functions inside them are defined as __attribute__ ((dllexport)). The most complete fix would be to consider the static case in the visibility header macro (as done for example in the example at the end of https://gcc.gnu.org/wiki/Visibility), and pass to the library some kind of macro to indicate if the library is static or shared.