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

Fix assumption that CMAKE_INSTALL_*DIR paths are relative. #66

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lopsided98
Copy link

The CMAKE_INSTALL_*DIR variables are allowed to contain absolute paths according to the CMake documentation. This PR fixes the handling of these absolute paths while retaining the current behavior if they are relative. I also fixed the hardcoded includedir in the pkg-config file.

See https://github.com/jtojnar/cmake-snips#assuming-cmake_install_dir-is-relative-path for more information about this problem.

@lopsided98 lopsided98 force-pushed the cmake-absolute-paths branch 2 times, most recently from 185ca35 to c9c9931 Compare October 19, 2022 23:00
@lopsided98
Copy link
Author

I fixed the conflicts and tested the updated patch. I would appreciate getting this reviewed, since we depend on this patch to build this package in nixpkgs.

@lopsided98 lopsided98 force-pushed the cmake-absolute-paths branch 4 times, most recently from 4936c97 to 6e0cea1 Compare January 21, 2024 19:01
muellerbernd added a commit to muellerbernd/urdfdom_headers that referenced this pull request Jan 29, 2024
muellerbernd added a commit to muellerbernd/urdfdom_headers that referenced this pull request Jan 29, 2024
Signed-off-by: Bernd Müller <[email protected]>
This solution uses relative paths if possible, allowing the package to be
relocatable, but still works correctly if CMAKE_INSTALL_*DIR paths are absolute.

Signed-off-by: Ben Wolsieffer <[email protected]>
@lopsided98 lopsided98 force-pushed the cmake-absolute-paths branch from 6e0cea1 to fa89f2d Compare October 11, 2024 17:01
@lopsided98
Copy link
Author

I fixed the conflicts again.

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

Successfully merging this pull request may close these issues.

1 participant