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 assumptions that CMAKE_INSTALL_*DIR paths are relative. #2778

Merged
merged 1 commit into from
Aug 31, 2020
Merged

Fix assumptions that CMAKE_INSTALL_*DIR paths are relative. #2778

merged 1 commit into from
Aug 31, 2020

Conversation

lopsided98
Copy link
Contributor

When an absolute path is desired, the CMAKE_INSTALL_FULL_*DIR variables should be used instead of concatenating with CMAKE_INSTALL_PREFIX. Special handling is needed for pkg-config files to ensure they use interpolations when possible.

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

When an absolute path is desired, the CMAKE_INSTALL_FULL_*DIR variables should
be used instead of concatenating with CMAKE_INSTALL_PREFIX. Special handling is
also needed for pkg-config files.
@j-rivero
Copy link
Contributor

I think that the change proposed is correct, it uses the right variables from GNUInstallDirs. We need to verify that values in pkgconfig and cmake files looks correct when they are generated.

@mjcarroll
Copy link
Contributor

@osrf-jenkins run tests

Copy link
Contributor

@mjcarroll mjcarroll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes LGTM, thanks for the contribution.

I'm going to let this run through CI again before merging, it looks like it may have been infrastructure issues before.

@mjcarroll
Copy link
Contributor

@osrf-jenkins run tests

1 similar comment
@mjcarroll
Copy link
Contributor

@osrf-jenkins run tests

@mjcarroll
Copy link
Contributor

Really struggling to get CI to turn over here, I checked build.osrfoundation.org before kicking this one off, so hopefully we'll be in better shape this time.

@mjcarroll mjcarroll merged commit adc8931 into gazebosim:gazebo11 Aug 31, 2020
@lopsided98 lopsided98 deleted the gazebo11-pkgconfig-paths branch August 31, 2020 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
11 Gazebo 11
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants