orocos / orocos_kinematics_dynamics

Orocos Kinematics and Dynamics C++ library
679 stars 407 forks source link

(PyKDL) let catkin export CMake and PkgConfig files #412

Open MatthijsBurgh opened 1 year ago

MatthijsBurgh commented 1 year ago

@meyerj You added this in 7e1f40d4d043811ea12ac1f620dae431d1338df1. But this prevents to find the package in a CMakeLists.

Is there any reason why you did add these flags?

meyerj commented 1 year ago

I do not remember the details, but it was a part of https://github.com/orocos/orocos_kinematics_dynamics/pull/285. In its description I wrote:

Macro catkin_package() takes care of installing package.xml and setup scripts. Usually it also installs cmake and pkg-config configuration files, but we skip this because orocos_kdl already provides its own configuration and the Python bindings do not require it.

If the Python module is built in the correct path inside the devel-space, pointed to by PYTHON_INSTALL_DIR and CATKIN_GLOBAL_PYTHON_DESTINATION, and also installed to this directory, no env-hooks are required to update the PYTHONPATH and python_orocos_kdl even works in devel-space.

So on the one hand I still think that it is not required, because you would typically not try to find python_orocos_kdl in a CMake project, but on the other hand there must have been an issue with that with at least one of the tested build tools. Before https://github.com/orocos/orocos_kinematics_dynamics/pull/285, python_orocos_kdl was built as a pure CMake package and finding it in CMake was not possible either.

The package does not provide any linkable C++ library nor installed headers. So what is your use case of writing something like

find_package(python_orocos_kdl REQUIRED)

or

find_package(catkin REQUIRED COMPONENTS python_orocos_kdl)

? But if there is one, and if it does not break anything, please feel free to merge this pull request. For Python packages that use PyKDL it is sufficient to list python_orocos_kdl as a dependency in the package.xml manifest without mentioning it in CMakeLists.txt.

For package orocos_kdl (the C++ library) the two arguments SKIP_CMAKE_CONFIG_GENERATION and SKIP_PKG_CONFIG_GENERATION are passed to catkin_package() because otherwise the generated configuration files would clash with the ones generated by orocos_kdl/CMakeLists.txt itself (from orocos_kdl-config.cmake.in and orocos_kdl.pc.in). For that reason, and if there is indeed a valid use case, it probably would be better to also add a custom python_orocos_kdl-config.cmake.in template such that it behaves the same, independent of whether it has been built with catkin (for ROS users) or CMake directly (for non-ROS users).