Extra include folder pybind11_catkin causes issues

[rhaschke]

I'm not sure why this package uses an extra layer pybind11_catkin in its include folder hierarchy, which is essentially skipped then by the macro pybind_add_module:
https://github.com/ipab-slmc/pybind11_catkin/blob/2990ceed6522b0c61c700210714a120ca4abe959/cmake/pybind11_catkin.cmake.in#L19

This causes issues with catkin's normal mechanism to transitively pass upstream dependencies to downstream packages: While catkin_INCLUDE_DIRS will only contain the base include path, what is actually needed to find headers is include/pybind11_catkin. However, this path is only provided in the pybind_add_module macro, such that downstream packages will fail on corresponding includes.

[wxmerkt]

This is due to the need for all headers to be encapsulated in pybind11_catkin due to pybind11-dev existing natively on bionic. This was a requirement for release. @VladimirIvan can comment some more.

Is there a possible workaround you can think of?

[VladimirIvan]

Modules created using the pybind_add_module macro are not intended to be further included in c++. As such, the include directories don't have to be passed down stream. If this not the case for a special case please let us know. Otherwise what @wxmerkt described is more critical to maintain compatibility on bionic.

[rhaschke]

I agree that for distinguishing Bionic's system package and this ROS package, there should be a separate folder, otherwise /opt/ros/*/include would automatically override those system headers - even if the pybind11_catkin package was not requested.
I was using a common helper library (not a pybind11 module) that needed to include pybind11.h. However, due to the additional include folder, the header was not found by downstream libs via the default catkin_INCLUDE_DIRS mechanism.
My suggestion is to correctly export pybind11_catkin_INCLUDE_DIRS - including the pybind11_catkin folder. This way, as soon as somebody includes this package, the include dirs are correctly set.

[wxmerkt]

I suppose this would have to be done by a manually created pybind11_catkinConfig.cmake and not the CFG_EXTRAS as currently, correct? Would you perhaps have a minimally working example?

[rhaschke]

If we agree on the approach, I can propose a PR.

[wxmerkt]

That would be wonderful, thank you very much for your time and effort. I agree that the hidden include in the macro is confusing. I wonder if moving the line using include_directories from the macro up to re-define pybind11_catkin_INCLUDE_DIRS would do the trick?

[rhaschke]

My idea was to adapt the CFG_EXTRAS cmake file to correctly set the include dir. Needs to be tested though.