catkin icon indicating copy to clipboard operation
catkin copied to clipboard

Mark include directories as system directories

Open kunaltyagi opened this issue 2 years ago • 4 comments

Allows use of warning flags without the compiler complaining about gtest/gmock related issues

kunaltyagi avatar Apr 13 '22 01:04 kunaltyagi

Can you please provide some sample output of before and after this change that demonstrates what effect it has?

gbiggs avatar Jun 22 '22 00:06 gbiggs

Haven't seen this issue with catkin build, but with colcon build.

I've seen two difference between their behaviors:

  • catkin created an imported target for gtest/gmock etc, while colcon creates a new target per package with the tests
  • warnings (via compiler flags) are propagated across include <header > for colcon, but not catkin

I don't know if these 2 are related, but adding SYSTEM seems to have stopped the warnings for me

kunaltyagi avatar Jun 23 '22 12:06 kunaltyagi

We have avoided using SYSTEM in the past as it has some unexpected side effects, see for example https://github.com/ament/ament_cmake/pull/321#issuecomment-830817601.

ivanpauno avatar Jun 23 '22 20:06 ivanpauno

Reading that chain, towards the end, after investigation, they have accepted that this makes sense for ament.

I don't feel there would be any unexpected side-effects (as pointed out later):

  • We can't control what flags are used by users
  • Errors/Warnings in gtest shouldn't affect user code unless someone has that in the same overlay
  • Later inclusions on both -I and -isystem override the earlier ones (left to right). This allows proper usage with overlays

kunaltyagi avatar Jun 25 '22 06:06 kunaltyagi