Change header install path

[AugusteBourgois]

Hi everyone,

Thanks for the great work behind generate_parameter_library ! It definitely spares us many lines of code.

A problem met by myself and some users according to #180 and #204 is that the generated header cannot be easily used outside the package that generated them. So I've made a small change to fix this. I'm open to discuss this more in depth if necessary, and add tests if required.

This PR solves #180 and #204 by installed the generated header in the same include folder as the current package, so that it can be used in other packages, and not only in the current one.

[AugusteBourgois]

Hi, thanks for the feedback. I pushed a little patch as I noticed this first version did not work in all cases.

Now, say you have package1 generating a header param_file.hpp.

You need to link you libs and executables against the generated target to access the include file, and export it so that it can be accessed from other packages. Finally, you need to add generate_package_library to your ament_export_dependencies so that other packages have access to GPL's dependencies (rsl, package_traits...). I guess a find_package and ament_export_dependencies` could work, but I'm not sure.

find_package(ament_cmake REQUIRED)
find_package(generate_package_library REQUIRED)

generate_package_library(param_lib path_to_param_file.hpp)

# compiled library
add_library(lib1 PUBLIC lib1.cpp)
target_include_directories(lib1 PUBLIC
    $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
    $<INSTALL_INTERFACE:include>)
target_link_libraries(lib1 PUBLIC param_lib)

# header only library
add_library(lib2 INTERFACE)
target_include_directories(lib2 INTERFACE
    $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
    $<INSTALL_INTERFACE:include>)
target_link_libraries(lib2 PUBLIC param_lib)

# export targets
install(TARGETS lib1 lib2 param_lib EXPORT ${PROJECT_NAME}Targets)

ament_export_targets(${PROJECT_NAME}Targets HAS_LIBRARY_TARGET)
ament_export_dependencies(generate_parameter_library ...)
ament_package()

The generated header can be included in lib1.cpp and lib2.hpp using

#include <package1/param_file.hpp>

The include line will be the same for other packages.

The current solution breaks existing packages. I can improve it so that it does not, at the cost of duplicating the generated header in the build folder (seems fine to me). I can also try to avoid exporting generate_parameter_library as a dependency, which, I agree, would be ideal.

Any thoughts ?

[christophfroehlich]

You need to link you libs and executables against the generated target to access the include file, and export it so that it can be accessed from other packages. Finally, you need to add generate_package_library to your ament_export_dependencies so that other packages have access to GPL's dependencies (rsl, package_traits...). I guess a find_package and ament_export_dependencies` could work, but I'm not sure.

find_package(ament_cmake REQUIRED)
find_package(generate_package_library REQUIRED)

generate_package_library(param_lib path_to_param_file.hpp)

# compiled library
add_library(lib1 PUBLIC lib1.cpp)
target_include_directories(lib1 PUBLIC
    $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
    $<INSTALL_INTERFACE:include>)
target_link_libraries(lib1 PUBLIC param_lib)

# header only library
add_library(lib2 INTERFACE)
target_include_directories(lib2 INTERFACE
    $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
    $<INSTALL_INTERFACE:include>)
target_link_libraries(lib2 PUBLIC param_lib)

# export targets
install(TARGETS lib1 lib2 param_lib EXPORT ${PROJECT_NAME}Targets)

ament_export_targets(${PROJECT_NAME}Targets HAS_LIBRARY_TARGET)
ament_export_dependencies(generate_parameter_library ...)
ament_package()

The easier it is to use generate_parameter_library, the better. If there is a way to avoid additional cmake stuff in the library, which generates the code, I would vote for it. But maybe there is an argument to not exporting it, if it is not reused in a different package -> then I'm fine for a manual export step together with clear instructions in the docs (which is missing now anyways).

The generated header can be included in lib1.cpp and lib2.hpp using

#include <package1/param_file.hpp>

The include line will be the same for other packages.

This is great.

The current solution breaks existing packages. I can improve it so that it does not, at the cost of duplicating the generated header in the build folder (seems fine to me). I can also try to avoid exporting generate_parameter_library as a dependency, which, I agree, would be ideal.

Could we deprecate the files in the "old" path with some compile-time warning? and then remove that after a couple of releases?

[AugusteBourgois]

My last push :

• installs the generated header (and validation header) so that they are accessible via #include <package_name/parameter_file.hpp> from inside and outside the package.
• copies the generated header (and validation header) so they they remain accessible via #include "parameter_file.hpp" as they used to from inside the same package.
• deprecates the local generated header, so that a warning is printed at build time.
• exports the generated header dependencies (fmt...) so that there is no need for ament_export_dependencies(generate_parameter_library) anymore.

In the next push, I will propose an updated documentation and an extra example package using the generated header from generate_parameter_library_example.

The only thing I haven't tried is the python version, since we don't use python at the moment.

[AugusteBourgois]

Hi @christophfroehlich, I have pushed something to correct the cmake linter error. colcon test now runs without errors, at least locally.