In general, this seems like a good idea to me. I have a few concerns:

1. Should we consider pinning to a particular tag of orocos? This may not be necessary as the API seems really stable, but we should consider it.
2. Is this going to work for Windows and macOS (I guess CI will let us know for sure)?
3. What does releasing a package like this look like? I'm slightly worried because some of the other places we package "pure-CMake" packages, they end up being special snowflakes and hard to release. I just want to make sure that we don't end up in a similar situation here. Also pinging @nuclearsandwich for thoughts.

I want to discuss a few different options. I think for options 1 or 2 below, we need the connected PRs. I opened this PR to run CI to see where we stand.

1. Depend on system package and vendor orocos_kdl for Windows

   • 1.4.0 is available in Debian Bullseye and Ubuntu Focal
     • https://packages.debian.org/source/stable/orocos-kdl
     • https://packages.ubuntu.com/source/focal/orocos-kdl
   • 1.5.1 is available with homebrew (if we care to support macOS).
     • https://formulae.brew.sh/formula/orocos-kdl
   • In this option, I think we'd need to create a vendor package for Windows.

2. Track upstream source repository

   • We can update ros2.repos to point upstream (this PR). I agree that it probably is better to pin to a particular tag.
   • Since orocos_kdl contains a git submodule for pybind11, infrastructure and docs should be updated to use
     vcs import --recurse.
   • We should help maintainers bloom-release (by providing guidance or just doing it ourselves)
     • I don't know how much of a snowflake this will be.

3. Status Quo

   • Sync select changes from upstream. E.g. Get up-to-date with upstream orocos_kinematics_dynamics#24

For both options (1) and (2):

• We need to change places that assume orocos_kdl is an ament package (see connected PRs)
  • Possibly a breaking change for downstream ROS 2 packages depending directly on orocos_kdl
• Results in version downgrade. ROS 2 fork is at 3.3.3 vs upstream at 1.5.1.

There is a 4th option, which is to create a orocos_kdl_vendor package. On Linux and macOS, it would get those from the system while on Windows it would build it from source.

That option is nice in that it uses the system package wherever possible, only building it as required. There, I think we'd also need the connected PRs as well. The version downgrade shouldn't be too much of a problem in this case; since it is a "different" package, it can have a different version number.

The downside to doing orocos_kdl_vendor is that it generally relies on CMake's ExternalProject_add, which we know is problematic. But that's no different from a bunch of other vendored packages, so it is not a new problem.

1. Depend on system package and vendor orocos_kdl for Windows

If orocos-kdl is suitably available on most of our host platforms then I would absolutely advocate for an explicit vendor package as the way forward. It doesn't seem like RHEL 8 or EPEL8 currently have an orocos-kdl-devel package or similar so we're at 50% availability or 60% if you count macOS, which I have not been anymore. We're working with the RoboStack folks to make conda the primarily supported installation method on Windows and orocos-kdl is packaged on conda-forge so depending on whether we can get the package in EPEL there's a long term possibility for removing the vendor package after a certain point.

I'm very in favor of the vendor package approach. If orocos-kdl is one of the packages particularly sensitive to the downsides of ExternalProject_Add then it may be that we allocate some time to the ament_cmake wrapper/alternative which was discussed offline previously.

• Since orocos_kdl contains a git submodule for pybind11, infrastructure and docs should be updated to use
  vcs import --recurse.

This sends shivers down my spine. Is there no way to point it at a system or already-vendored pybind11 installation?

Is there no way to point it at a system or already-vendored pybind11 installation?

I'll look into this; I suspect it may require some changes to oroco_kdl upstream.

I've created a vendor package for orocos_kdl (ros2/orocos_kdl_vendor#1).
It does not currently handle python_orocos_kdl, but it doesn't look like we're depending on the Python bindings yet so maybe that's not a deal breaker.

Correct me if I'm wrong, but I think for this to work as desired we should add rosdep keys for orocos_kdl and python_orocos_kdl pointing to system packages.

I've also proposed a patch upstream to avoid building pybind11 if it is already available on the system (orocos/orocos_kinematics_dynamics#375).

I've also proposed a patch upstream to avoid building pybind11 if it is already available on the system (orocos/orocos_kinematics_dynamics#375).

You have my exquisite gratitude for this. 🙇 🙇 🙇

Correct me if I'm wrong, but I think for this to work as desired we should add rosdep keys for orocos_kdl and python_orocos_kdl pointing to system packages.

Yes, the vendor package should declare build and runtime dependencies on the system packages it provides so it can, essentially, re-export those dependencies when they are present and when they're not it should also declare build dependencies on the vendored package's build dependencies so it can be built when the system packages aren't available.

I've updated this PR to switch to the vendor packages, updated other ROS 2 packages depending on orocos_kdl, and triggered CI (see updated ticket description).

Seems like the last roadblock is Windows CI, failing to build PyKDL with:

21:34:28   CMake Error at C:/ci/ws/install/share/cmake/pybind11/pybind11Tools.cmake:119 (set_property):
21:34:28     Property INTERFACE_LINK_LIBRARIES may not contain link-type keyword
21:34:28     "debug".  The INTERFACE_LINK_LIBRARIES property may contain
21:34:28     configuration-sensitive generator-expressions which may be used to specify
21:34:28     per-configuration rules.

@MatthijsBurgh Any insight into this error?

I've opened a PR upstream to fix the CMake warnings: orocos/orocos_kinematics_dynamics#385

@jacobperron I am not developing any software on Windows. So I can't help you with this one.

Alright, thanks to @sloretz we are now successfully building on Windows 🎉

There are now 22 MSBuild warnings to resolve upstream and a few cmake lint test failures in the vendor package. I should be able to resolve the remaining warnings and test failures. I think we just need reviews on this and the connected PRs (see PR description).

@clalancette @nuclearsandwich Is there anything else that we should do and/or do you have any reservations about actually vendoring orocos_kdl (versus just releasing upstream as a ROS 2 package)?

I've opened a PR upstream to try and resolve MSBuild warnings: orocos/orocos_kinematics_dynamics#390

orocos/orocos_kinematics_dynamics#391 addresses another CMake warning.

CI is green except for one Cmake warning coming from pybind11. I'll see if we can quiet the warning in python_orocos_kdl_vendor.

CI is green except for one Cmake warning coming from pybind11. I'll see if we can quiet the warning in python_orocos_kdl_vendor.

Resolve in orocos/orocos_kinematics_dynamics#392