search

moveit / moveit_planners

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

OMPL/catkin include order #66

Closed dornhege closed 8 years ago

dornhege commented 8 years ago

The include order for OMPL and catkin in ompl/CMakeLists.txt seems wrong to me. However, there are already two commits switching this around: 329fb6300c6cb8540d551f1e002fa9c57c9590b9 and b888117ebb56e71e10b3d4da110d8ac9d95190a0, so making another pull request without discussion does not make sense.

The question is basically if catkin_INCLUDE_DIRS or OMPL_INCLUDE_DIRS should be preferred. I believe OMPL_INCLUDE_DIRS should be preferred. Otherwise, when working with a source build OMPL on a standard ROS install, which includes the released OMPL, this will shadow the source build OMPL.

In contrast 329fb6300c6cb8540d551f1e002fa9c57c9590b9 states that this prevents ROS package shadowing. Shouldn't that still work as either OMPL_INCLUDE_DIRS is not set or should be set to the lowest overlay?

In either case, the order of link_directories should probably match the include order.

mikeferguson commented 8 years ago

ported to new consolidated repo