search

ros / dynamic_reconfigure

BSD 3-Clause "New" or "Revised" License
47 stars 112 forks source link

Fix order of cmake keywords in include_directories() for generated headers #161

Open meyerj opened 4 years ago

meyerj commented 4 years ago

Fixes a bug introduced in https://github.com/ros/dynamic_reconfigure/pull/140:

For cmake command include_directories() the order of keywords AFTER|BEFORE and SYSTEM matters and cannot be swapped:

Add include directories to the build.

include_directories([AFTER|BEFORE] [SYSTEM] dir1 [dir2 ...])

With

include_directories(SYSTEM BEFORE ${CATKIN_DEVEL_PREFIX}/${CATKIN_GLOBAL_INCLUDE_DESTINATION})

the BEFORE keyword is considered as another include directory relative to the CMAKE_CURRENT_SOURCE_DIR. The generated compiler command line therefore contained flags like -isystem /path/to/my/package/BEFORE and also -isystem /path/to/devel/include as intended, just not in the expected order relative to other include directories.

Tested on Ubuntu Bionic with ROS melodic and CMake 3.10.2.

meyerj commented 4 years ago

Just noticed that this is a duplicate of https://github.com/ros/dynamic_reconfigure/pull/151. I was still working on an older version of melodic-devel and built dynamic_reconfigure from source.

But apparently the patch has not been applied to branch noetic-devel yet. Shouldn't patches be applied to the newest development branch exclusively and then eventually backported to older versions?

ros-discourse commented 3 years ago

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/dynamic-reconfigure-maintenance/21950/1