Cleanup cmake files #11
Conversation
These are not find_packaged and thus the corresponding variables are not defined.
They are not needed to build this package, but by downstream packages.
but directly create module .so in target folder.
The distinction between install and devel space was brittle and is not required: devel-space includes are made relative to intall space by catkin
Setting an already cached variable again is a no-op. Thus, most of the variable settings were non-sense. pybind11 should use the same python version as ROS. Thus considering PYBIND_PYTHON_EXECUTABLE doesn't make sense to me. To directly use pybind11_add_module(), it is important to correctly set PYBIND11_INCLUDE_DIR. pybind11_catkin_INCLUDE_DIRS should include PYTHON_INCLUDE_DIRS, maybe EIGEN3_INCLUDE_DIR?
|
Looks like Travis is broken on Melodic. Looking into this now... |
|
The Travis issue was unrelated to my changes: |
|
Thank you for the clean-up!
We have users who under ROS1 use Python3 (and manually pip3 install the rospy, rospack dependencies). Is there another way to have ROS1 with Python3 locally that I missed? |
The way to select the python version for ROS is to set the environment variable |
Ah, yes, that is what I had missed and was looking for. |
|
Which project are you testing and building with these changes? I tried |
I'm in the process of migrating https://github.com/ubi-agni/moveit_task_constructor/tree/wip-python-api from |
|
I just built exotica with the new pybind11_catkin in a clean workspace w/o issues. |
|
The include directories weren't set up correctly. The minimal change to this PR that I required was: Then this PR works well for me :-) |
catkin_package() doesn't export the install-space's include path correctly. It's only named include, missing the subfolder pybind11_catkin.
This addresses #10 and several other minor issues in the cmake files. See individual commits and their commit messages for details.
One open question is whether Eigen and numpy should be really package dependencies here?
These are only required if you actually want to use those features in your downstream project.
Hence, I would argue, the downstream project is responsible for including them.