add new macro to export ts libraries through different CMake variable

[dirk-thomas]

The vendor specific typesupport libraries must not be exported through the message packages <pkgname>_LIBRARIES variable in order to address ros2/ros2#437.

This patch provides a macro to export the typesupport libraries through a separate variable which can be used by the vendor specific packages.

[dirk-thomas]

https://ci.ros2.org/job/ci_packaging_osx/37/ provides a fat archive built against Connect and FastRTPS. otool -L rcl/lib/librclp.dylib should not contain the Connext specific typesupport libraries anymore and as a consequence should continue to work when Connext is not available.

This is this in progress since I didn't have the chance to check this in a macOS machine yet... If nobody bets me to it I will check it on Monday. If it "works" the same patch as in ros2/rmw_connext#291 needs to be applied to other vendors.

[dhood]

works for me:

$ otool -L ~/Downloads/ros2-osx-avoid-overlinking/lib/librcl.dylib
/Users/deanna/Downloads/ros2-osx-avoid-overlinking/lib/librcl.dylib:
	@rpath/librcl.dylib (compatibility version 0.0.0, current version 0.0.0)
	@rpath/librcl_interfaces__rosidl_typesupport_c.dylib (compatibility version 0.0.0, current version 0.0.0)
	@rpath/librcl_interfaces__rosidl_typesupport_cpp.dylib (compatibility version 0.0.0, current version 0.0.0)
	@rpath/librcl_interfaces__rosidl_generator_c.dylib (compatibility version 0.0.0, current version 0.0.0)
	@rpath/librcl_interfaces__rosidl_typesupport_introspection_c.dylib (compatibility version 0.0.0, current version 0.0.0)
	@rpath/librcl_interfaces__rosidl_typesupport_introspection_cpp.dylib (compatibility version 0.0.0, current version 0.0.0)
	@rpath/librmw_implementation.dylib (compatibility version 0.0.0, current version 0.0.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1238.60.2)
	@rpath/librmw.dylib (compatibility version 0.0.0, current version 0.0.0)
	@rpath/librcutils.dylib (compatibility version 0.0.0, current version 0.0.0)
	@rpath/librosidl_generator_c.dylib (compatibility version 0.0.0, current version 0.0.0)

I can build an overlay without connext and running fastrtps nodes works, also. If I then update share/connext_cmake_module/environment/connext.sh so that it finds connext, I can run the overlay demos with connext 👍

[dirk-thomas]

I also created a PR for OpenSplice and put the PRs in review.

CI builds with all rmw to check for regressions:

• Linux
• Linux-aarch64
• macOS
• Windows

The "positive" effect of the changes won't be visible in the CI builds though. Both both @dhood and my checking indicates that it addresses the referenced issue.

[dhood]

Were you going to make one for fastrtps? We always ship introspection typesupport so it doesn't matter much, but in theory introspection typesupport shouldn't be linked against by default either, yeah?

[dirk-thomas]

For now I didn't plan to do the same for introspection since I think all places have an explicit dependency on it. Assuming you could remove FastRTPS and "everything" still works of course (same as for the other rmw impl.).

[dhood]

can you clarify what sort of places are you referring to? The only thing I only found was https://github.com/ros2/rosidl_typesupport/blob/aa730a885be300c8026a435b6270ee62b910091a/rosidl_default_runtime/package.xml#L17, which is the same for static typesupports.

[dirk-thomas]

Yes, that was the location I had in mind. The static typesupports are only mentioned as a workaround there (as described in the comment).

To turn the question around why should we add the same to introspection ts? I don't see any immediate use case why that would be beneficial. We can always consider it later / after the release but I would keep the changes until the release as small as possible.

[wjwwood]

lgtm overall, a few questions/suggestions you might want to take before merging though.

[wjwwood]

Copyright?

[dirk-thomas]

Neither of the other .cmake.in templates have copyright notes. I think that was mostly so that generated code embedded into other packages doesn't have implicitly the same copyright (e.g. if that package wants to choose a different license).

[wjwwood]

Should this be STATUS or WARNING or something else? Otherwise you're not going to "ignore it" as the comment says. It will stop here. I'm not sure what the behavior should be in this context.

[dirk-thomas]

Hm, this is the same as in ament_export_libraries: https://github.com/ament/ament_cmake/blob/409ee0403d8d1eba0dc5a58cf52dcec5eadc91d2/ament_cmake_export_libraries/cmake/ament_cmake_export_libraries-extras.cmake.in#L47-L48

I would rather not change the behavior. So I just updated the comment to match the actual behavior: 5047c44#diff-d8035aae44630fe14ea78f925e0f4effR25

[wjwwood]

Should this maco be here? Rather than its own file or in the rosidl_export_typesupport_libraries.cmake file?

[dirk-thomas]

This file is the CMake extra file of the rosidl_cmake package. Therefore I just added it there. It could be placed into a second -extras.cmake file but it would be rather uncommon that a single CMake package has multiple extra files.

[dirk-thomas]

I also addressed the CMake linter warnings.

[dirk-thomas]

• Linux good, only unrelated test failure and Jenkins heap error
• Linux-aarch64 unrelated test failures
• macOS good, only unrelated test failure and Jenkins heap error
• Windows unrelated test failures

[dirk-thomas]

CI builds with Connext only:

• Linux
• Linux-aarch64 using only OpenSplice instead (since Connext isn't available), unrelated test failures
• macOS
• Windows

[t0ny-peng]

@dirk-thomas I'm sorry to ask a question on an old pull request.

I noticed that this PR adds a CMake function rosidl_export_typesupport_libraries which will exports the libraries to a variable named <pkg>_LIBRARIES_<implementation_xxx>. This usually happens in the msg packages, e.g., rcl_interface.

However, in the down stream packages like rcl, when they call ament_target_dependencies on rcl_interface, they have no knowledge about the existence of RMW specific _LIBRARIES variable. In that case, what's the correct way to add dependencies on those generated msg libraries specific to one RMW implementation?

Thanks.

[dirk-thomas]

@left4taco Why do you need to depend on an RMW specific library in the first place? Commonly you will only need to depend on the <pkg>_LIBRARIES and that will either be the right library already (if there is only one typesupport) or it will give you a library which dynamically loads the right library at runtime.

[t0ny-peng]

@dirk-thomas The reason is that cross compiling with only RTI Connext Pro/Micro failed for not being able to find symbols defined in those DDS msg libraries.

More specifically, since there's only one RMW implementation(FastRTPS is disabled), libbuiltin_interfaces__rosidl_typesupport_cpp.so will directly and implicitly depend on librcl_interfaces__rosidl_typesupport_apex_dds_cpp.so, which is for sure in the NEEDED field of the generic xxx_typesupport_cpp.so. Poco is not needed.

$ readelf -a /home/hao.peng/gc/apex_ws/install/rcl_interfaces/lib/librcl_interfaces__rosidl_typesupport_cpp.so | grep NEEDED
 0x0000000000000001 (NEEDED)             Shared library: [librcl_interfaces__rosidl_typesupport_apex_dds_cpp.so]
...

With the changes in this PR, only the generic xxx_typesupport_cpp.so is in the <MSG_PKG>_LIBRARIES field, which will be explicitly linked against in the compiling command.

This works fine for native compile because for native compile, ld will search missing libraries(in this case the DDS specific one) in rpath. However, according to the cross compiler documentation:

Searching -rpath in this way is only supported by native linkers and cross linkers which have been configured with the --with-sysroot option.

..., which means that the linker will not search for the missing libraries in rpath. Also --with-sysroot doesn't exist anymore. So here comes with two solutions:

1. Use ament_export_dependencies to export the DDS specific libraries explicitly to <MSG_PKG>_LIBRARIES. But that defeat the whole purpose of this PR
2. Add the directory install/rcl_interface/lib/ to --rpath-link, which needs some tweak to the CMake settings. Tricky and not perfect.

We are using the first one since we won't ship with FastRTPS and RTI Connext Micro is a prerequisite. My whole point is that cross compiling with RMW implementation other than FastRTPS might not work.

Thanks.

[dirk-thomas]

@left4taco I suggest you move this topic to a new ticket since it is not obvious what the right solution for your use case would be and in this already closed ticket it will certainly get lost.