include Ogre headers via system path

[v4hn]

... to silence warnings about upstream files.

According to the pkgconfig and the cmake files of Ogre, /usr/include
is an include folder for the project and thus <OGRE/Ogre...> is a valid include path.

The following illustrates the behavior with one of various warnings:
<<<
❯ cat bar.cpp
❯ clang++ -Werror -Wall -Wextra -pedantic -std=c++14 -c bar.cpp -I/usr/include/OGRE
In file included from bar.cpp:1:
In file included from /usr/include/OGRE/OgrePixelFormat.h:31:
In file included from /usr/include/OGRE/OgrePrerequisites.h:326:
In file included from /usr/include/OGRE/OgreMemoryAllocatorConfig.h:188:
/usr/include/OGRE/OgreMemorySTLAllocator.h:130:4: error: 'register' storage class specifier is deprecated and incompatible with C++17 [-Werror,-Wdeprecated-register]
register size_type sz = count*sizeof( T );
^~~~~~~~~
1 error generated.
❯ cat foo.cpp
❯ clang++ -Werror -Wall -Wextra -pedantic -std=c++14 -c foo.cpp
❯
<<<

RViz surpresses these warnings with include_directories(SYSTEM ${OGRE_INCLUDE_DIRS})
but passes the plain OGRE_INCLUDE_DIRS variable on in catkin_package(INCLUDE_DIRS ...).
There is no way to specify a SYSTEM include there.
Consequently all downstream projects including rviz headers will see OGRE's warnings
and have no easy way to avoid them because the correct OGRE include path is only one entry in
${rviz_INCLUDE_DIRS} (or more likely ${catkin_INCLUDE_DIRS}).

Came up in moveit/moveit_task_constructor#273 (comment)

[rhaschke]

I hesitate to adapt all the include directives. I'd prefer the solution suggested in moveit/moveit_task_constructor#273 (comment).

[v4hn]

Actually this is the way gazebo includes ogre for at least 10 years, so I believe this would be a viable option for RViz as well.

As I stated elsewhere, the alternative to fixing it using the include path would be to overlay the used Ogre headers in a folder in RViz and use something like this

#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wdeprecated-register"
#include_next <OgrePixelFormat.h>
#pragma GCC diagnostic pop

in the respective files.

As you stated yourself, this is a limitation in catkin, but it should be fixed in RViz.

[rhaschke]

The OGRE tutorials recommend the old way of including OGRE headers.
However, I'm fine merging this as long as it builds 😉

[rhaschke]

I fixed the build issues and merged these changes as 41d62f5.

[v4hn]

I guess you really don't like such issues to remain open PRs for long.
Sorry I couldn't find the time to clean it up and thank you a lot!

[rhaschke]

I just have some time today to handle open rviz issues and this was an easy one ;-)

[rhaschke]

Hi @v4hn, @simonschmeisser, @jspricke
9 months after merging this change, I ran into a problem because we changed OGRE include paths: Trying to build rviz against OGRE 1.13.x installed in a non-standard location, ${OGRE_INCLUDE_DIRS} points to <install folder>/include/OGRE, not <install folder>/include. Thus, all rviz includes of the (new) form (#include <OGRE/...>) fail and fallback to /usr/include/OGRE instead! I'm tempted to revert this change. Any other suggestions?

[jspricke]

${OGRE_INCLUDE_DIRS} points to /usr/include/OGRE for me (Debian unstable, ogre 1.12.10) as well and also OGRE.pc has Cflags: -I${includedir} -I${includedir}/OGRE -pthread so reverting this seems correct to me.