Convert to pure CMake package

[gavanderhoorn]

Adapt CMakeLists.txt so we can accomodate building the library without Catkin / in a non-ROS (build) environment.

[traversaro]

Hi @gavanderhoorn , if you are still interested in this I would be happy to migrate the build system to use plain CMake .

[gavanderhoorn]

A migration is not something we can do, as we still need to be able to build this project as a ROS package for now.

Alternative CMakeLists.txt or perhaps even a mixed CMakeLists.txt could be approaches we might consider.

[traversaro]

As far as I understand from [1] and [2], as long as the package.xml manifest is installed, there is no difference between a pure-CMake package and a CMake package that uses catkin from the point of view of releasing a ROS package, but to be honest I never tried it. I will experiment a bit to verify that this is true, and report the results here. Thanks for the quick reply.

[1] http://wiki.ros.org/bloom/Tutorials/ReleaseThirdParty
[2] https://github.com/gerkey/ros1_external_use

[gavanderhoorn]

This is only partially true: packages that are pure-cmake will not be directly buildable by catkin_make, but will require using catkin_make_isolated or catkin_tools.

In order to avoid having to create an underlay (in which to build abb_lib*) and then an overlay with the rest of the Catkin packages, or require the use of catkin_tools, it would be good to keep the default build Catkin compatible.

[traversaro]

Thanks, got it. I will try to prepare a mixed CMakeLists.txt approach as you suggested.

[gavanderhoorn]

Perhaps even creating a completely stand-alone, new, CMakeLists.txt would be an option to consider. If we give it a distinctive name (CMakeLists.txt.non-ros), users wanting to build the library without ROS would replace the default CMakeLists.txt with that file and then build.

A hybrid could also work, but tends to become a bit cluttered in my experience.

But if you find a nice way, please submit a PR.

[traversaro]

If anyone is interested in this, a prototype of this is available at traversaro@b2e1559 . I did not open a PR because this need for #23 and #24 merged before, but any comment on the approach (for example if it can considered "too cluttered" or not) is already welcome.

Related CMake issue on simplifying the boilerplate code for exporting pure CMake packages: https://gitlab.kitware.com/cmake/cmake/issues/18634 .

[gavanderhoorn]

@traversaro nice work.

I do feel it's a bit too convoluted, yes. The ROS version of the CMakeLists.txt is relatively straightforward, so what about completely separating the two variants (ie: use different files)?

[traversaro]

Just two clarify: I was trying to avoid the two separate CMakeLists.txt files solution because it complicates the workflow when integrating the project using classical pure CMake approaches such as ExternalProject or FetchContent. However, I get your point.

I would nevertheless wait for #23 and #24 to be merged before opening a PR with the two separate CMakeLists.txt, to minimize the differences between the two CMakeLists.txt for the sake of maintainability.

A related topic is if there is any plan for making this library available in ROS2. As far as I can understand (but please correct me if I am wrong) the catkin_make issue is no longer a problem in ROS2, and so a pure CMake package that installs its package.xml should be equivalent to an ament_cmake package.

[gavanderhoorn]

Similar rationale as given in ros-industrial/abb_librws#3 (comment):

• ROS 2 use
• Colcon
• (easier/better) Windows compatibility
• catkin_tools for ROS 1

[traversaro]

Update version of a pure CMake build system available at traversaro@a8b79d6 .
The only feature missing w.r.t. to the catkin build is the pkgconfiig configuration file generation, is that something you are interested in?

[jontje]

I have finally had some time to work on this issue, and I have created a branch for converting the library into a plain cmake package.

I looked at your work @traversaro, but I opted for basing my initial work on a plain cmake package template from the ros2 pkg create command. Mainly because I wanted to learn and then also to be more inline with other packages I am working on.

However, I am more than happy to incorporate some of your additions. E.g. use write_basic_package_version_file instead of using an input file. I don’t know the pros/cons, but I guess it is recommended to use the CMakePackageConfigHelpers when possible. Also, what are the benefits of using a namespace when exporting targets? Is it mainly useful if the package has multiple targets to export?

Anyway, I have currently tested to build the package on:

• Ubuntu 18.04:

  • ROS Melodic (catkin_make_isolated and catkin_tools)
  • ROS2 Dashing (colcon)

• Windows 10:

  • ROS Melodic (catkin_make_isolated)

I am going to run some RobotStudio simulations next, as well as looking into using ROS2 on Windows.

@gavanderhoorn is it alot of work to updated the travis job for building a plain cmake package? And for building on ROS2 as well?

[traversaro]

I have finally had some time to work on this issue, and I have created a branch for converting the library into a plain cmake package.

I looked at your work @traversaro, but I opted for basing my initial work on a plain cmake package template from the ros2 pkg create command. Mainly because I wanted to learn and then also to be more inline with other packages I am working on.

Great, thanks a lot.

However, I am more than happy to incorporate some of your additions. E.g. use write_basic_package_version_file instead of using an input file. I don’t know the pros/cons, but I guess it is recommended to use the CMakePackageConfigHelpers when possible.

Basically the main advantage of using write_basic_package_version_file is to have less hardcoded logic, and be able to easily switch between the version compatibility policies (for example between same major versior or same minor version). However, if your version checking logic is custom, it is more convenient to use a custom file.

Also, what are the benefits of using a namespace when exporting targets? Is it mainly useful if the package has multiple targets to export?

One of the major advantages is that if a user links a target whose name contains :: but is not defined, CMake raises a fatal error at configuration time, while using a "simple" name, the target name is passed to the linker, moving the eventual error to the linking time.
See open62541/open62541#2415 and https://github.com/robotology/how-to-export-cpp-library/blob/1261ae57bc7c0422f0ab8b456df6f206f4935368/src/LibTemplateCMake/CMakeLists.txt#L55 for more details.

I have a few more comments on your branch, if you want I can make them here, otherwise I can wait for a PR, so that we can track the comments on a line-by-line basis.

[jontje]

I have a few more comments on your branch, if you want I can make them here, otherwise I can wait for a PR, so that we can track the comments on a line-by-line basis.

Thanks for the quick response and the clarifications! I think it's reasonable to wait for the PR, and I will probably do that tomorrow (I have a wall of meetings ahead of me today 😄)

[jontje]

I have created the draft PR #63, and I have tested the following situations against RobotStudio simulations:

• Ubuntu (ROS and ROS2)
• Windows (ROS)

@traversaro You are more than welcome to add your additional comments when you have time for it.

[gavanderhoorn]

@gavanderhoorn is it alot of work to updated the travis job for building a plain cmake package? And for building on ROS2 as well?

The new master branch of industrial_ci does support all of this, but I'm not sure how to configure things for builds of both ROS 1 and ROS 2 from the same .travis.yml.

@ipa-mdl: would you be able to provide some guidance here?

[mathias-luedtke]

would you be able to provide some guidance here?

Just use the master branch and add a new matrix entry for ROS2, e.g.

ROS_DISTRO="dashing" ROS_REPO=ros

[gavanderhoorn]

Nice, thanks. I'd expected something like that, but wanted to make sure.

[gavanderhoorn]

Closing as #63 was merged.

Thanks for the support @ipa-mdl and thanks for contributing to the discussion @traversaro.