gazebosim / gz-cmake

A set of CMake modules that are used by the C++-based Gazebo projects.
https://gazebosim.org/libs/cmake
Apache License 2.0
27 stars 31 forks source link

Don't assume `CMAKE_INSTALL_*DIR` variables are relative #305

Closed lopsided98 closed 2 years ago

lopsided98 commented 2 years ago

🦟 Bug fix

The CMAKE_INSTALL_*DIR variables are allowed to be absolute paths, so they cannot be simply appended to CMAKE_INSTALL_PREFIX. When an absolute path is needed, CMAKE_INSTALL_FULL_*DIR must be used. See here for more information: https://github.com/jtojnar/cmake-snips#assuming-cmake_install_dir-is-relative-path

Additionally, pkgconfig files should use ${prefix} relative paths if CMAKE_INSTALL_*DIR are relative, but otherwise use absolute paths. This is handled by the custom join_paths() function.

Note that CMake 3.20 provides cmake_path(APPEND) which implements the same functionality as join_paths(), but this CMake version is not available in all supported distros yet (notably Ubuntu older than 22.04).

Checklist

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

j-rivero commented 2 years ago

Thanks Ben for the pointers and the patch. Changes in code looks good to me, my testing a local wokrspace for packages up to gz-transport is fine and I run a testing build of the whole collection in Build Status for Windows (warnings and test are unrelated as far as I can tell).

Note that CMake 3.20 provides cmake_path(APPEND) which implements the same functionality as join_paths(), but this CMake version is not available in all supported distros yet (notably Ubuntu older than 22.04).

I'm going to add a TODO not to forget about this.