Add option to enable GNU Install Dirs #19
Conversation
This commit adds an option to enable the GNUInstallDirs.cmake script and use it to define console_bridge's installation directories. When enabled, libraries, headers, and data are installed to paths defined by the GNUInstallDirs.cmake script. Otherwise, the hard-coded "lib", "share", and "include" strings are used, resulting in no change in functionality after this commit is applied. The option is diabled by default. Signed-off-by: Rich Mattes <richmattes@gmail.com>
|
I think this was resolved by #23 |
|
The same change has been reverted a while ago since it did not work on the ROS build farm: b863035 I am not sure if something is different now though. |
|
As I said in the original description, this is a better way to solve #15 (the buildfarm breakage) by adding an off-by-default option to enable paths from GNUInstallDirs. The motivation behind this PR is the fact that when libraries are installed system-wide, distros don't always put all of their libraries in #22 and #23 don't fix this issue, as they don't address the fact that the library installation directory is hard-coded to |
|
Can you resolve conflicts on this branch? |
|
Should be resolved now. |
|
AFAIK, the support for So two things:
|
| @@ -8,7 +8,7 @@ set(@PKG_NAME@_INCLUDE_DIRS "@CMAKE_INSTALL_PREFIX@/include") | |||
| foreach(lib @PKG_LIBRARIES@) | |||
There was a problem hiding this comment.
in the line 6 of this file, we can change:
-set(@PKG_NAME@_INCLUDE_DIRS "@CMAKE_INSTALL_PREFIX@/include")
+set(@PKG_NAME@_INCLUDE_DIRS @CMAKE_INSTALL_FULL_INCLUDEDIR@)
There was a problem hiding this comment.
True, I will fix this.
Remove option to use GNUInstallDirs, so it is always enabled. Change console-bridge-config.cmake.in to use CMAKE_INSTALL_FULL* cmake path variables
| @@ -82,7 +82,7 @@ if (NOT MSVC) | |||
| set(pkg_conf_file "console_bridge.pc") | |||
| configure_file("${pkg_conf_file}.in" "${CMAKE_BINARY_DIR}/${pkg_conf_file}" @ONLY) | |||
| install(FILES "${CMAKE_BINARY_DIR}/${pkg_conf_file}" | |||
| DESTINATION lib/pkgconfig/ COMPONENT pkgconfig) | |||
| DESTINATION ${CMAKE_INSTALL_LIBDIR}/pkgconfig/ COMPONENT pkgconfig) | |||
There was a problem hiding this comment.
We probably need to use CMAKE_INSTALL_LIBDIR with the cmake module, line 72
There was a problem hiding this comment.
Yeah, I would agree with that. Pushing now.
The console_bridge configs contain paths to find the libraries which are architecture-dependent. Those architecture-dependent scripts should be installed to an arch-dependent path, mimicing the pkg-config installation.
|
Thanks Rich for all the fixes. It is fine for me to merge. +1 |
| DESTINATION include | ||
| FILES_MATCHING PATTERN "*.h") | ||
| DESTINATION ${CMAKE_INSTALL_INCLUDEDIR} | ||
| FILES_MATCHING PATTERN "*.h") |
There was a problem hiding this comment.
Could you remove the extra whitespace at the end of these lines?
|
+1, merging, thanks @jpgr87 ! |
Add option to enable GNU Install Dirs
The way issue 15 was solved made it very difficult to install console_bridge system-wide on a distribution that does not use /usr/lib as the library installation path. This patch is a better way to solve the issue 15: rather than just nuking the GNUInstallDirs logic from orbit, it adds a configuration switch to optionally enable the functionality, leaving it disabled by default so ROS doesn't break.