Add NAMESPACE support to ament_export_targets

[Ryanf55]

Purpose

I want to export targets with ament but under a custom namespace. Currently, it's hard coded to be the project name. The intent is to make this backwards compatible with humble, and obvious to people who have used CMake install(EXPORT ... NAMESPACE foo) that it does the same thing.

Motivation

#292 (comment)
BehaviorTree/BehaviorTree.CPP#751 (comment)

Example usage in BehaviorTreeCPP

Top level CMakeLists.txt

INSTALL(TARGETS ${BTCPP_LIBRARY}
    EXPORT ${PROJECT_NAME}Targets
    ARCHIVE DESTINATION ${BTCPP_LIB_DESTINATION}
    LIBRARY DESTINATION ${BTCPP_LIB_DESTINATION}
    RUNTIME DESTINATION ${BTCPP_BIN_DESTINATION}
    INCLUDES DESTINATION ${BTCPP_INCLUDE_DESTINATION}
    )

INSTALL( DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/include/
    DESTINATION ${BTCPP_INCLUDE_DESTINATION}
    FILES_MATCHING PATTERN "*.h*")

export_btcpp_package()

ament_build.cmake

macro(export_btcpp_package)
    ament_export_targets(
        ${PROJECT_NAME}Targets
        NAMESPACE BT::
        HAS_LIBRARY_TARGET
    )
    ament_export_dependencies(ament_index_cpp)
    ament_package()
endmacro()

Then, to use the library, whether you consume it build with ament or conan enabled, you can always call either of the two.

using ament_target_dependencies - sadly this exposes the BTCPP to the public interface even if it's an implementation detail. This only matters if you are building a re-usable library and don't want your consumers know you use BTCPP.

cmake_minimum_required(VERSION 3.22.1) # version on Ubuntu Jammy
project(btcpp_ament_tgt_dep LANGUAGES CXX)


set(CMAKE_CXX_STANDARD 17)
set(CMAKE_CXX_STANDARD_REQUIRED ON)

find_package(ament_cmake 1 CONFIG REQUIRED)
find_package(behaviortree_cpp 4 REQUIRED)

add_executable(btcpp_sample main.cpp)


# This function doesn't yet support PRIVATE as an argument.
# See https://github.com/ament/ament_cmake/issues/158#issuecomment-475384147
# Instead, test preserving legacy usage with no linkage specifier.
ament_target_dependencies(btcpp_sample behaviortree_cpp)

OR , the preferred approach:

cmake_minimum_required(VERSION 3.22.1) # version on Ubuntu Jammy
project(btcpp_tgt_link_libs LANGUAGES CXX)


set(CMAKE_CXX_STANDARD 17)
set(CMAKE_CXX_STANDARD_REQUIRED ON)

find_package(ament_cmake 1 CONFIG REQUIRED)
find_package(behaviortree_cpp 4 CONFIG REQUIRED)

add_executable(btcpp_sample main.cpp)

# Check it works with the same target that conan users use
target_link_libraries(btcpp_sample PRIVATE BT::behaviortree_cpp)

Feel free to take a look at the full branch here: https://github.com/Ryanf55/BehaviorTree.CPP/tree/751-use-ament-export-targets

Assumptions

• All targets and executables installed will have the same namespace
• Must preserve existing behavior by default

Risks

There's this assumption the target name would never have colons, however it doesn't seem to have any ill effect

list(APPEND _IMPORT_CHECK_FILES_FOR_BT::behaviortree_cpp "${_IMPORT_PREFIX}/lib/libbehaviortree_cpp.so" )

https://cmake.org/cmake/help/latest/manual/cmake-language.7.html#variables

Variable names are case-sensitive and may consist of almost any text, but we recommend sticking to names consisting only of alphanumeric characters plus _ and -.

[Ryanf55]

Ready for review! I tested this with BTCPP, and it works great! The only thing this needs is a proper docstring for the new NAMESPACE argument. I need help because I can't figure out the schema for the docstrings.

[mjcarroll]

Upon discussion, there is no reason to prevent users from doing this, but it definitely not be the default as the expected behavior from calling find_package(foo) should give you a foo::foo target. Any divergence from this would be unexpected.

Do you mind adding some documentation to this effect? Something along the lines of "this should be used carefully and documented in any packages that make use of this."

[Ryanf55]

Upon discussion, there is no reason to prevent users from doing this, but it definitely not be the default as the expected behavior from calling find_package(foo) should give you a foo::foo target. Any divergence from this would be unexpected.

Do you mind adding some documentation to this effect? Something along the lines of "this should be used carefully and documented in any packages that make use of this."

Yep! Can do. It's an "advanced" option for sure.

[Ryanf55]

Upon discussion, there is no reason to prevent users from doing this, but it definitely not be the default as the expected behavior from calling find_package(foo) should give you a foo::foo target. Any divergence from this would be unexpected.

Do you mind adding some documentation to this effect? Something along the lines of "this should be used carefully and documented in any packages that make use of this."

Done! Ready for another pass.

[mjcarroll]

• Linux
• Linux-aarch64
• Windows

[Ryanf55]

Can we backport to iron and humble?

[mjcarroll]

@Mergifyio backport humble
@Mergifyio backport iron

[mergify]

backport humble

✅ Backports have been created

• #505 Add NAMESPACE support to ament_export_targets (backport #498) has been created for branch humble