search

moveit / moveit_ros

THIS REPO HAS MOVED TO https://github.com/ros-planning/moveit
69 stars 118 forks source link

explicitly link rviz' default_plugin library - indigo request #658

Closed v4hn closed 8 years ago

v4hn commented 8 years ago

This is the indigo version of #657. Feel free to cherry-pick the commit to indigo instead of merging this request.

The library is not exported anymore and now is provided separately from rviz_LIBRARIES. See https://github.com/ros-visualization/rviz/pull/979 for details.

k-okada commented 8 years ago

This fixes #654, merge please!

dornhege commented 8 years ago

This does not seem to build with catkin_make (but with catkin build), because the generated target cmake file is build at compile time, but required at cmake configuration time.

I will merge this now as it is an improvement and should remove the regression in the build farm.

v4hn commented 8 years ago

@dornhege: you're right. This only resolves the problem with catkin_make_isolated and for cases where rviz is already installed before compiling moveit_ros.

The CFG_EXTRAS mechanism @wjwwood pulled out of his magic hat seems to be broken w.r.t. that. Maybe one could also export the real library target as rviz_DEFAULT_PLUGIN_LIBRARIES in rviz' CMakeLists.txt? I'm not sure there's a clean solution for the general issue though.

Anyways, this solves most of the use-cases. rviz is usually only in the core system that must be built with catkin_build_isolated afair.

Thanks for merging.

wjwwood commented 8 years ago

I can put the default plugins in the exported targets maybe. I'd have to think about that some more.

The problem with getting the library name at configure time is that CMake doesn't actually know it, or in the future it won't, until generation is done, which is why they introduced generator expressions.

wjwwood commented 8 years ago

Actually I think I can just put the target in the rviz_DEFAULT_PLUGIN_LIBRARIES cmake variable within the rviz CMakeLists.txt. That way with catkin_make the rviz_DEFAULT_PLUGIN_LIBRARIES will be defined and point to the right target.

wjwwood commented 8 years ago

I opened https://github.com/ros-visualization/rviz/pull/983 as a potential solution.