ament / ament_cmake

Supporting CMake packages for working with ament
Apache License 2.0
92 stars 118 forks source link

find_package() returns duplicated LIBRARIES #222

Open alsora opened 4 years ago

alsora commented 4 years ago

Doing the following in a CMakeFile:

find_package(rcl CONFIG)
message("${rcl_LIBRARIES}")

Results in printing:

ros2/lib/librcl.so;ros2/lib/libbuiltin_interfaces__rosidl_typesupport_c.so;
ros2/lib/libbuiltin_interfaces__rosidl_typesupport_cpp.so;
ros2/lib/libbuiltin_interfaces__rosidl_typesupport_introspection_c.so;
ros2/lib/libbuiltin_interfaces__rosidl_generator_c.so;
ros2/lib/libbuiltin_interfaces__rosidl_typesupport_introspection_cpp.so;
ros2/lib/librosidl_typesupport_c.so;
ros2/lib/librosidl_typesupport_cpp.so;
ros2/lib/librosidl_generator_c.so;
ros2/lib/librosidl_typesupport_introspection_c.so;
ros2/lib/librosidl_typesupport_introspection_cpp.so;
ros2/lib/librcl_interfaces__rosidl_typesupport_c.so;
ros2/lib/librcl_interfaces__rosidl_typesupport_cpp.so;
ros2/lib/librcl_interfaces__rosidl_typesupport_introspection_c.so;
ros2/lib/librcl_interfaces__rosidl_generator_c.so;
ros2/lib/librcl_interfaces__rosidl_typesupport_introspection_cpp.so;
ros2/lib/librcl_yaml_param_parser.so;
ros2/lib/librcutils.so;
ros2/lib/librmw.so;
ros2/lib/librosidl_generator_c.so;
ros2/lib/librosidl_typesupport_introspection_c.so;
ros2/lib/librosidl_typesupport_introspection_cpp.so;
ros2/lib/librmw_cyclonedds_cpp.so;
-pthread;
ros2/lib/librosidl_typesupport_introspection_c.so;
ros2/lib/librosidl_typesupport_introspection_cpp.so;
ros2/lib/librmw_cyclonedds_cpp.so;
-pthread;
ros2/lib/librcutils.so;
ros2/lib/librosidl_generator_c.so;
ros2/lib/librmw.so;
ros2/lib/librosidl_typesupport_introspection_c.so;
ros2/lib/librosidl_typesupport_introspection_cpp.so;
ros2/lib/librmw_cyclonedds_cpp.so;
-pthread;
ros2/lib/librosidl_generator_c.so;
ros2/lib/librmw.so;
ros2/lib/librcutils.so;
ros2/lib/librosidl_typesupport_introspection_c.so;
ros2/lib/librosidl_typesupport_introspection_cpp.so;
ros2/lib/librmw_cyclonedds_cpp.so;
-pthread;
ros2/lib/librmw.so;
ros2/lib/librcutils.so;
ros2/lib/librosidl_generator_c.so;
ros2/lib/librcutils.so;
ros2/lib/librcl_logging_spdlog.so;
ros2/lib/librcutils.so;
ros2/lib/librcl_logging_spdlog.so;
ros2/lib/libtracetools.so

It would be nice to don't have so many duplicated or triplicated entities here.

Maybe adding something like list (REMOVE_DUPLICATES rcl_LIBRARIES) somewhere in this package is enough to fix the issue

dirk-thomas commented 4 years ago

Yeah, deduplication has been a long standing todo: https://github.com/ament/ament_cmake/blob/37ed0dcde921f4f0dd29e0e6cf64c55f85e193f0/ament_cmake_export_libraries/cmake/ament_cmake_export_libraries-extras.cmake.in#L137-L138

I am not sure it makes sense to still implement it. It would be far better to move forward to use more modern CMake and export targets instead which will avoid these.

Maybe adding something like list (REMOVE_DUPLICATES rcl_LIBRARIES) somewhere in this package is enough to fix the issue

That won't be sufficient since the order is critical and the proposed function isn't maintaining stability (at least when I tested it last a few years ago).

alsora commented 4 years ago

I am not sure it makes sense to still implement it. It would be far better to move forward to use more modern CMake and export targets instead which will avoid these.

Oh, this would be great! Is there already some roadmap for that?

dirk-thomas commented 4 years ago

Is is mentioned on the roadmap but there is no timeline / concrete plan when to work on this.