RoboStack / ros-noetic

vinca configuration files for ros-noetic
https://robostack.github.io
448 stars 70 forks source link

Add lanelet2 #468

Closed Tobias-Fischer closed 2 months ago

mantkiew commented 2 months ago

I'd like to comment about the error:

Could NOT find GeographicLib (missing: GeographicLib_LIBRARIES)

The conda-forge libgeographic package does not install Modules, only Config cmake files. Here's the explanation:

https://github.com/conda-forge/geographiclib-cpp-feedstock/issues/12#issuecomment-2050562797

The problem with lanelet2 is that it gets libgeographic via crt_cmake_modules package, which is convenient but non-standard.

I've looked into it but my CMake skills are not sufficient. The idea is to change libgeographic lookup from as a module to as a config.

Tobias-Fischer commented 2 months ago

Do you think this simple trick is ok @traversaro ?

traversaro commented 2 months ago

Do you think this simple trick is ok @traversaro ?

I am a bit afraid of this change, as it changes GeographicLib_LIBRARIES from pointing to the absolute path of the libraries, to use imported targets. The main problem of imported targets is that if code was developed assuming that GeographicLib_LIBRARIES are absolute paths, it may be possible that it calls target_link_libraries(libname PUBLIC ${GeographicLib_LIBRARIES}), and then install an exported target for libname, switching GeographicLib_LIBRARIES to use imported target would require find_package(GeographicLib CONFIG) to be called in libname CMake config files, introducing subtle bugs.

I think it may be safer to just fix the FindGeographicLib to use correct library names, see https://github.com/KIT-MRT/mrt_cmake_modules/pull/34 .

Tobias-Fischer commented 2 months ago

Great, thanks Silvio!

mantkiew commented 2 months ago

Thanks a lot! I confirm I am able to install ros-noetic-lanelet2 in a new environment. There's still the other problem that it cannot be installed together with ros-noetic-dekstop due to ros-distro-mutex 0.5.* vs ros-distro-mutex 0.4.* required by noetic desktop. See https://github.com/RoboStack/ros-noetic/issues/466#issuecomment-2063458733

But I can install both lanelet2 and velodyne-pointcloud together.