Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmake config: console_bridge_LIBRARIES should contain absolute paths. #52

Closed
saladpanda opened this issue Jan 25, 2018 · 8 comments
Closed
Labels

Comments

@saladpanda
Copy link

set(@PKG_NAME@_LIBRARIES @PKG_NAME@)

I'm trying to cross-compile console_bridge with catkin_make_isolated. Compiling it works fine, but when other packages try to link against console_bridge they can't find the library as console_bridge_LIBRARIES is only set to console_bridge.
e.g. in rosbag_storage:
https://github.com/ros/ros_comm/blob/kinetic-devel/tools/rosbag_storage/CMakeLists.txt#L41

I'm no cmake expert (so correct me if I'm wrong), but according to the documentation this variable should contain absolute paths to the library:

https://cmake.org/cmake/help/v3.10/command/link_directories.html:

Library locations returned by find_package() and find_library() are absolute paths. Pass these absolute library file paths directly to the target_link_libraries() command.

@sloretz
Copy link
Contributor

sloretz commented Jan 25, 2018

Would you mind posting the text of the error you receive?

It looks like you're correct that console_bridge_LIBRARIES is supposed to include full paths (doc on find module standard variables). CMake is moving away from variables in favor of imported targets. The name of the library console_bridge appears to be the imported target name so I would expect target_link_libraries(rosbag_storage ... console_bridge) to work fine.

@saladpanda
Copy link
Author

I don't get an error, but I have multiple versions of console_bridge available and target_link_libraries(rosbag_storage ... console_bridge) is picking the wrong one although find_package(console_bridge 0.4 REQUIRED) finds the right one.

@sloretz
Copy link
Contributor

sloretz commented Jan 25, 2018

rosbag_storage has find_package(console_bridge REQUIRED) at the top. Does find_package(console_bridge REQUIRED) find the wrong version? What version does it find?

@saladpanda
Copy link
Author

Oh yes, sure. I patched that line to include the version number 0.4. Otherwise it finds the system version, which is the one from the debian jessie repositories (0.2.* something).
The version it is supposed to find is my self-compiled 0.4 version.

The setup is rather complicated and not fully working at the moment, thats why I left out most of the background story. I already have this issue fixed locally for me (ugly hack, so nothing worth a pull request).

@sloretz sloretz removed the question label Jan 25, 2018
@sloretz
Copy link
Contributor

sloretz commented Jan 25, 2018

Understood, thanks for reporting the issue. If later you're willing, a pull request to fix the content of console_bridge_LIBRARIES would be appreciated.

@saladpanda
Copy link
Author

My solution to this looks like this:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index d51e954..514c63e 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -81,7 +81,7 @@ set(PKG_LIBRARIES ${PROJECT_NAME})
 set(cmake_conf_file "${PROJECT_NAME}-config.cmake")
 configure_package_config_file("${cmake_conf_file}.in" "${CMAKE_BINARY_DIR}/${cmake_conf_file}"
                               INSTALL_DESTINATION ${CMAKE_CONFIG_INSTALL_DIR}
-                              PATH_VARS CMAKE_INSTALL_FULL_INCLUDEDIR
+                              PATH_VARS CMAKE_INSTALL_FULL_INCLUDEDIR CMAKE_INSTALL_FULL_LIBDIR
                               NO_SET_AND_CHECK_MACRO
                               NO_CHECK_REQUIRED_COMPONENTS_MACRO)
 set(cmake_conf_version_file "${PROJECT_NAME}-config-version.cmake")
diff --git a/console_bridge-config.cmake.in b/console_bridge-config.cmake.in
index dfaa547..e15b9ee 100644
--- a/console_bridge-config.cmake.in
+++ b/console_bridge-config.cmake.in
@@ -6,6 +6,7 @@ set(@PKG_NAME@_CONFIG_INCLUDED TRUE)
 @PACKAGE_INIT@
 
 set(@PKG_NAME@_INCLUDE_DIRS "@PACKAGE_CMAKE_INSTALL_FULL_INCLUDEDIR@")
+set(@PKG_NAME@_LIBRARY_DIRS "@PACKAGE_CMAKE_INSTALL_FULL_LIBDIR@")
 
 include("${CMAKE_CURRENT_LIST_DIR}/@PKG_NAME@-targets.cmake")
-set(@PKG_NAME@_LIBRARIES @PKG_NAME@)
+set(@PKG_NAME@_LIBRARIES "@PACKAGE_CMAKE_INSTALL_FULL_LIBDIR@/lib@PKG_NAME@.so")

I'm pretty sure this is not a nice solution, but I don't know how to do this properly in cmake.

@scpeters
Copy link
Contributor

Even though console_bridge does export a target, urdfdom is not using that yet; it's still using console_bridge_LIBRARIES, so this issue can crop up far downstream, as in dartsim/dart#1200

@scpeters
Copy link
Contributor

I believe this was resolved by #60

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants