Best practices for ExternalProject #97

stertingen · 2021-02-22T14:34:15Z

stertingen
Feb 22, 2021

Hello folks.

Currently I'm trying to integrate an external C++ library into my ROS workspace using ExternalProject. I cannot use FetchContent which is not available on Kinetic and Melodic.

While it is possible to perform this task working for both devel and install destinations, catkin_lint has a lot to complain about:

cmake_minimum_required(VERSION 3.5)
project(amqpcpp)

find_package(catkin REQUIRED)
find_package(OpenSSL REQUIRED)

# Fetch version from package.xml
catkin_package_xml()

# Download AMQP-CPP and install into devel space
include(ExternalProject)
externalproject_add(${PROJECT_NAME}_ep
    # Downloading archive is faster and less verbose than using git
    URL "https://github.com/CopernicaMarketingSoftware/AMQP-CPP/archive/v${${PROJECT_NAME}_VERSION}.tar.gz"
    # Forward install prefix to external project
    CMAKE_ARGS
        -DCMAKE_INSTALL_PREFIX=<INSTALL_DIR>
    # Configuration of AMQP-CPP
    CMAKE_CACHE_ARGS
        -DAMQP-CPP_BUILD_EXAMPLES:BOOL=OFF
        -DAMQP-CPP_BUILD_SHARED:BOOL=ON
        -DAMQP-CPP_LINUX_TCP:BOOL=ON
    # Output files of external project
    BUILD_BYPRODUCTS
        <INSTALLL_DIR>/lib/lib${PROJECT_NAME}.so
        <INSTALLL_DIR>/lib/lib${PROJECT_NAME}.so.4.3
    # Set devel space as install prefix
    INSTALL_DIR "${CATKIN_DEVEL_PREFIX}"
)

# Workaround for cmake complaining about missing include directory
file(MAKE_DIRECTORY ${CATKIN_DEVEL_PREFIX}/include)

# Workaround for catkin_lint complaining about external include directory
list(APPEND ${PROJECT_NAME}_INCLUDE_DIRS ${CATKIN_DEVEL_PREFIX}/include)

# Declare libraries and dependencies for catkin
# Include directories are declared above.
catkin_package(
    LIBRARIES ${PROJECT_NAME}
    DEPENDS OPENSSL
)

# Add interface library which can be used for non-install builds
add_library(${PROJECT_NAME} INTERFACE)
# Declare include directories for our interface target (from installed AMQP-CPP)
target_include_directories(${PROJECT_NAME} SYSTEM INTERFACE ${CATKIN_DEVEL_PREFIX}/include)
# Declare libraries for our interface target (from installed AMQP-CPP)
target_link_libraries(${PROJECT_NAME} INTERFACE ${CATKIN_DEVEL_PREFIX}/lib/lib${PROJECT_NAME}.so OpenSSL::SSL)
# Make our interface target depend on the external project
add_dependencies(${PROJECT_NAME} ${PROJECT_NAME}_ep)

# Workaround for catkin_lint complaining about amqpcpp not being installed
install(TARGETS ${PROJECT_NAME}
    DESTINATION ${CATKIN_PACKAGE_LIB_DESTINATION}
)

# Install library binaries (from AMQP-CPP which is currently installed to devel space)
install(FILES ${CATKIN_DEVEL_PREFIX}/lib/lib${PROJECT_NAME}.so ${CATKIN_DEVEL_PREFIX}/lib/lib${PROJECT_NAME}.so.4.3
    DESTINATION ${CATKIN_PACKAGE_LIB_DESTINATION}
)
# Install header files (from AMQP-CPP which is currently installed to devel space)
install(FILES ${CATKIN_DEVEL_PREFIX}/include/${PROJECT_NAME}.h
    DESTINATION ${CATKIN_GLOBAL_INCLUDE_DESTINATION}
)
install(DIRECTORY ${CATKIN_DEVEL_PREFIX}/include/${PROJECT_NAME}/
    DESTINATION ${CATKIN_PACKAGE_INCLUDE_DESTINATION}
    FILES_MATCHING PATTERN "*.h"
)

Especially the lines

list(APPEND ${PROJECT_NAME}_INCLUDE_DIRS ${CATKIN_DEVEL_PREFIX}/include)

and

install(TARGETS ${PROJECT_NAME}
    DESTINATION ${CATKIN_PACKAGE_LIB_DESTINATION}
)

bother me because they clearly circumvent checks performed by catkin_lint.

Even with these workarounds, I get warnings about installing files from the devel space (because that's where the external project is installed to). I think most issues boil down to doing things in the devel space and exporting references to the devel space.

So can I somehow use ExternalProject with catkin and still satisfy catkin_lint?

gavanderhoorn · 2021-02-22T14:40:05Z

gavanderhoorn
Feb 22, 2021

I'm not the maintainer here, nor is this an answer, but do you really need to wrap your library in a Catkin package?

If you're just trying to use amqpcpp as a system dependency, it would not be needed (as it would just be another system dependency, found using find_package(..)).

If you're looking to be able to build it as part of a Catkin workspace, it would not be needed (as you could just place a minimal package.xml in your projects directory, and set the build_type to cmake).

If you're attempting to release amqpcpp as a package on the ROS buildfarm -- or a private buildfarm -- you could use a thirdparty release.

Edit: and this is highly opinionated, but:

I cannot use FetchContent which is not available on Kinetic and Melodic.

Both FetchContent(..) and ExternalProject(..) are really not very nice: they mix the responsibility of building a piece of software with making sure the dependencies are present. There is a very clear separation of concerns in a typical ROS workflow, and FetchContent(..) and ExternalProject(..) break it.

0 replies

stertingen · 2021-02-22T15:35:10Z

stertingen
Feb 22, 2021
Author

I'm not the maintainer here, nor is this an answer, but do you really need to wrap your library in a Catkin package?

If you're just trying to use amqpcpp as a system dependency, it would not be needed (as it would just be another system dependency, found using find_package(..)).

If you're looking to be able to build it as part of a Catkin workspace, it would not be needed (as you could just place a minimal package.xml in your projects directory, and set the build_type to cmake).

If you're attempting to release amqpcpp as a package on the ROS buildfarm -- or a private buildfarm -- you could use a thirdparty release.

Edit: and this is highly opinionated, but:

I cannot use FetchContent which is not available on Kinetic and Melodic.

Both FetchContent(..) and ExternalProject(..) are really not very nice: they mix the responsibility of building a piece of software with making sure the dependencies are present. There is a very clear separation of concerns in a typical ROS workflow, and FetchContent(..) and ExternalProject(..) break it.

I totally aggree with you that including amqpcpp as system package would be the better solution. However, creating and maintaining a release repository seems to require some amount of extra maintenance effort. Just throwing a package.xml into the project does not work for catkin_make.

Also, integrating own modifications (using a forked version for development purposes) does not seem to be possible while I could always change the URL in the CMakeLists.txt.

0 replies

gavanderhoorn · 2021-02-22T15:49:37Z

gavanderhoorn
Feb 22, 2021

However, creating and maintaining a release repository seems to require some amount of extra maintenance effort.

more than what you're doing now? It's a trade-off, but a third-party release is essentially a one-time configuration job, followed by periodic rebasing of your manifest + any potential patches you want to include + running bloom again (that is: if you want to release updates).

That's not really that much work, and seems comparable to what you'd be doing with the wrapper you're creating now.

Just throwing a package.xml into the project does not work for catkin_make.

No, as it does not support mixed-build-tool builds.

You'd use catkin_tools or catkin_make_isolated for those cases.

Also, integrating own modifications (using a forked version for development purposes) does not seem to be possible while I could always change the URL in the CMakeLists.txt.

I'm not sure I understand this.

I've not checked whether it the resulting library actually works, but the following manifest:

boilerplate cmake buildtool manifest

<?xml version="1.0"?>
<package format="2">
  <name>amqpcpp</name>
  <version>0.1.0</version>
  <description>Something</description>

  <maintainer email="[email protected]">Someone</maintainer>
  <author>Someone</author>

  <license>Apache 2.0</license>

  <buildtool_depend>cmake</buildtool_depend>

  <export>
    <build_type>cmake</build_type>
  </export>
</package>

builds amqpcpp in a ROS Melodic workspace using catkin_tools without any warnings or errors:

ls -al devel/ output

root@machine:/catkin_ws# ll devel/lib/
total 2288
drwxr-xr-x 3 root root    4096 Feb 22 16:55 ./
drwxr-xr-x 8 root root    4096 Feb 22 16:55 ../
lrwxrwxrwx 1 root root      17 Feb 22 16:55 libamqpcpp.so -> libamqpcpp.so.4.3
-rw-r--r-- 1 root root 2328792 Feb 22 16:55 libamqpcpp.so.4.3
drwxr-xr-x 2 root root    4096 Feb 22 16:55 pkgconfig/
root@machine:/catkin_ws# ll devel/include/
total 16
drwxr-xr-x 3 root root 4096 Feb 22 16:55 ./
drwxr-xr-x 8 root root 4096 Feb 22 16:55 ../
drwxr-xr-x 3 root root 4096 Feb 22 16:55 amqpcpp/
-rw-r--r-- 1 root root 2201 Feb 22 16:51 amqpcpp.h

Could be that the location of amqpcppConfig.cmake and the headers is not entirely where it should be, that would be something to check.

I haven't checked the vanilla CMakeLists.txt, perhaps something does need some patching (that's typically done in a release repository with a 3rd party build).

Edit 2: anyway, I'll mark this as off-topic, as that's what it is. I just wanted to make you aware of options other than wrapping your library in a Catkin package.

0 replies

roehling · 2021-02-22T16:19:14Z

roehling
Feb 22, 2021
Maintainer

I don't think there's much to add on my part. @gavanderhoorn pretty much laid out the most sensible options and I don't think there's enough of a use case for me to add support for ExternalProject to catkin_lint. I can turn this into a discussion if you like, as this might be off-topic, but it still provides valuable information which others might find useful.

0 replies

stertingen · 2021-02-22T17:20:44Z

stertingen
Feb 22, 2021
Author

However, creating and maintaining a release repository seems to require some amount of extra maintenance effort.

more than what you're doing now? It's a trade-off, but a third-party release is essentially a one-time configuration job, followed by periodic rebasing of your manifest + any potential patches you want to include + running bloom again (that is: if you want to release updates).

That's not really that much work, and seems comparable to what you'd be doing with the wrapper you're creating now.

Once the package is released into the ROS ecosystem, people might depend on it even when my current project does not use this library any more. So releasing something with bloom comes with a responsibility to maintain the package, I think.

Just throwing a package.xml into the project does not work for catkin_make.

No, as it does not support mixed-build-tool builds.

You'd use catkin_tools or catkin_make_isolated for those cases.

Yes. I do see the rationale for using these, but for deveopment I prefer catkin_make due to the faster build times.

Also, integrating own modifications (using a forked version for development purposes) does not seem to be possible while I could always change the URL in the CMakeLists.txt.

I'm not sure I understand this.

There are situations in which I need a modified version of a package (for example when working on a PR) while testing my ROS workspace. In such situations, I just put the source package into the workspace and it gets built by catkin automatically.

I don't think there's much to add on my part. @gavanderhoorn pretty much laid out the most sensible options and I don't think there's enough of a use case for me to add support for ExternalProject to catkin_lint. I can turn this into a discussion if you like, as this might be off-topic, but it still provides valuable information which others might find useful.

Yes, this can be turned into a discussion. Sorry for opening an issue for this!

0 replies

gavanderhoorn · 2021-02-22T20:55:19Z

gavanderhoorn
Feb 22, 2021

final comment:

There are situations in which I need a modified version of a package (for example when working on a PR) while testing my ROS workspace. In such situations, I just put the source package into the workspace and it gets built by catkin automatically.

I don't really understand how having a package.xml in an otherwise plain CMake project hinders this sort of workflow.

Google Cartographer for instance is setup and distributed in exactly the same way, and has almost infinite forks which can all be built from source in a Catkin workspace.

Perhaps unclear, but adding a manifest does not equal 3rd party release. It's perfectly possible to have a manifest in an unreleased package.

0 replies

roehling · 2021-02-23T10:25:29Z

roehling
Feb 23, 2021
Maintainer

Yes, this can be turned into a discussion. Sorry for opening an issue for this!

No need to be sorry. It's a legitimate question to ask, and I hope the answers have been helpful for you.

0 replies

stertingen · 2021-02-23T15:15:13Z

stertingen
Feb 23, 2021
Author

Okay, I played around with forking the upstream repository and adding a package.xml. Building with colcon kind of works (catkin_make_isolated and catkin_tools do not generate a single compile_commands.json for my IDE, colcon does) up to a certain point. Contrary to my previous assumptions, this is something I can live with. (Thank you for your help so far.)

Now I need to set some CMake options (enable TCP support and shared libraries). Besides adding --cmake-args -D... to the command line, is there another way to specify extra CMake arguments, which is more persistent? (like, when others clone my ROSified version, they do not have to provide these extra arguments) I guess I'll have to patch the CMakeLists.

EDIT: Made myself a small script to automatically add package.xml and patch CMakeLists.txt:

#!/bin/sh
set -e

TYPE_TO_UPDATE=$1
TARGET_TO_UPDATE=$2

usage() {
  echo "Usage: $0 [branch|tag] <target>" >&2
  echo "Examples:" >&2
  echo "  $0 branch master" >&2
  echo "  $0 tag v4.3.11" >&2
  exit 1
}

# Basic validation
if [ -z "${TYPE_TO_UPDATE}" ] || [ -z "${TARGET_TO_UPDATE}" ]; then
  usage
fi

# Configure upstream remote if not exists
if ! git config remote.upstream.url > /dev/null; then
  git remote add --no-tags upstream https://github.com/CopernicaMarketingSoftware/AMQP-CPP.git
fi

# Configure how to fetch branches or tags
if [ "${TYPE_TO_UPDATE}" = "branch" ]; then
  git config --replace-all remote.upstream.fetch "+refs/heads/*:refs/remotes/upstream/*"
elif [ "${TYPE_TO_UPDATE}" = "tag" ]; then
  git config --replace-all remote.upstream.fetch "+refs/tags/*:refs/tags/upstream/*"
else
  usage
fi

# Fetch the thing to update
git fetch upstream ${TARGET_TO_UPDATE}

# Checkout thing to update
if [ "${TYPE_TO_UPDATE}" = "branch" ]; then
  # And create new branch (overwriting existing)
  git checkout -B ros-patched/${TARGET_TO_UPDATE} upstream/${TARGET_TO_UPDATE}
elif [ "${TYPE_TO_UPDATE}" = "tag" ]; then
  git checkout upstream/${TARGET_TO_UPDATE}
fi

# Create package.xml with git data
cat > package.xml << PACKAGE_XML_END
<?xml version="1.0"?>
<package format="2">
  <name>amqpcpp</name>
  <version>$(git describe --tags | cut -d/ -f2)</version>
  <description>
    AMQP-CPP is a C++ library for communicating with a RabbitMQ message broker. The library can be used to parse
    incoming data from a RabbitMQ server, and to generate frames that can be sent to a RabbitMQ server.
  </description>

  <maintainer email="$(git config user.email)">$(git config user.name)</maintainer>

  <license>Apache-2.0</license>

  <buildtool_depend>cmake</buildtool_depend>
  <depend>libssl-dev</depend>
  <exec_depend>catkin</exec_depend>

  <export>
    <build_type>cmake</build_type>
  </export>
</package>
PACKAGE_XML_END

# Enable shared libraries and TCP support
sed -i 's/^option(\(AMQP-CPP_BUILD_SHARED ".*"\) OFF)$/option(\1 ON)/g' CMakeLists.txt
sed -i 's/^option(\(AMQP-CPP_LINUX_TCP ".*"\) OFF)$/option(\1 ON)/g' CMakeLists.txt

# Append install for package.xml
cat >> CMakeLists.txt << CMAKELISTS_TXT_END

# Install ROS package.xml
install(FILES package.xml DESTINATION share)
CMAKELISTS_TXT_END

# Create commit with our new package.xml
git add package.xml CMakeLists.txt
git commit -m "Add package.xml and enable shared libraries and TCP"

# Create tag if we are updating a tag
if [ "${TYPE_TO_UPDATE}" = "tag" ]; then
  git tag -f ros-patched/${TARGET_TO_UPDATE}
fi

# Inform user how to continue
if [ "${TYPE_TO_UPDATE}" = "branch" ]; then
  echo "Finished. Please review commit/branch and push using:"
  echo "git push --set-upstream origin ros-patched/${TARGET_TO_UPDATE}"
elif [ "${TYPE_TO_UPDATE}" = "tag" ]; then
  echo "Finished. Please review commit/tag and push using:"
  echo "git push origin ros-patched/${TARGET_TO_UPDATE}"
fi

0 replies

berndpfrommer · 2022-12-06T17:30:01Z

berndpfrommer
Dec 6, 2022

Although this is a fairly old discussion it is still very relevant. What if several different ROS packages in the workspace depend on the same external package via the described just-add-package.xml-method? Wouldn't the packages collide?

3 replies

Timple Dec 13, 2022

No, this "package" only needs to exist once in your workspace and multiple packages can depend on it.

Lot of ROS packages depend on std_msgs. Luckily it only needs to exist once in our workspace.

berndpfrommer Dec 13, 2022

Your direct answer exposes how poorly my question was phrased. This is what I was trying to get at:
The original post uses cmake's ExternalProject mechanism to bring the sources of a non-catkin package into the workspace. The alternative approach (no wrapper package) means there needs to be an additional mechanism to pull the sources into the workspace, which could potentially lead to the same source package being in the workspace multiple times.
But come to think of it, this problem will even more so exist with the ExternalProject method. And probably if the required source dependencies are brought in with wstool, it will detect if the same source package is already in the source tree.

roehling Dec 13, 2022
Maintainer

ExternalProject() will usually download into the build folder. This is a CMake feature which completely sidesteps the catkin dependency management, so you are most likely to end up with multiple independent copies in various ~/catkin_ws/build/<PACKAGE> folders. You can probably get away with this if it is a private dependency, i.e. a library that is used internally only and not exposed in any way to users of your package. You'll end up with conflicts (and probably need to reimplement a sizeable chunk of catkin code) if you try to install the external project into your catkin develspace or installspace, though.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Best practices for ExternalProject #97

{{title}}

Replies: 9 comments 3 replies

{{title}}

{{editor}}'s edit

{{editor}}'s edit

{{title}}

{{title}}

{{editor}}'s edit

{{editor}}'s edit

{{title}}

{{title}}

{{title}}

{{title}}

{{title}}

{{editor}}'s edit

{{editor}}'s edit

{{title}}

{{title}}

{{title}}

{{title}}

Select a reply

Best practices for ExternalProject #97

stertingen Feb 22, 2021

Replies: 9 comments · 3 replies

gavanderhoorn Feb 22, 2021

stertingen Feb 22, 2021 Author

gavanderhoorn Feb 22, 2021

roehling Feb 22, 2021 Maintainer

stertingen Feb 22, 2021 Author

gavanderhoorn Feb 22, 2021

roehling Feb 23, 2021 Maintainer

stertingen Feb 23, 2021 Author

berndpfrommer Dec 6, 2022

Timple Dec 13, 2022

berndpfrommer Dec 13, 2022

roehling Dec 13, 2022 Maintainer

stertingen
Feb 22, 2021

Replies: 9 comments 3 replies

gavanderhoorn
Feb 22, 2021

stertingen
Feb 22, 2021
Author

gavanderhoorn
Feb 22, 2021

roehling
Feb 22, 2021
Maintainer

stertingen
Feb 22, 2021
Author

gavanderhoorn
Feb 22, 2021

roehling
Feb 23, 2021
Maintainer

stertingen
Feb 23, 2021
Author

berndpfrommer
Dec 6, 2022

roehling Dec 13, 2022
Maintainer