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.

Sign up for GitHub

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

Jump to bottom

Ensure relocatable config files #419

Merged
merged 7 commits into from
Feb 4, 2021
Merged

Ensure relocatable config files #419

merged 7 commits into from
Feb 4, 2021

Conversation

Pro
Copy link
Contributor

@Pro Pro commented Nov 17, 2020 •
edited
Loading

Using CMAKE_INSTALL_PREFIX causes build issues when you relocate the installed CMake package, i.e., as it is done in conan.

CMAKE_INSTALL_PREFIX should not be used in the cmake.in files directly. The @PACKAGE_INIT@ macro initializes PACKAGE_PREFIX_DIR correspondingly.

See also:
gazebosim/gz-tools#30
gazebosim/gz-cmake#129
gazebosim/gazebo-classic#2782

cc @stefanbuettner

This was referenced Nov 17, 2020
Merged
Merged
@codecov-io
Copy link

Codecov Report

Merging #419 (51f143a) into master (0093747) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #419   +/-   ##
=======================================
  Coverage   87.40%   87.40%           
=======================================
  Files          60       60           
  Lines        9351     9351           
=======================================
  Hits         8173     8173           
  Misses       1178     1178

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0093747...51f143a. Read the comment docs.

@traversaro
Copy link
Contributor

The pc.in files still use CMAKE_INSTALL_PREFIX but quickly googling, I did not find a good solution for that.

If you are interested in fully relocatable .pc files, you can always get the (relocatable) location of the .pc file with the ${pcfiledir} variable, and then compute the relocatable install prefix knowing the relative installation path of the .pc file. See a few links in ros2/ros2#606 (comment) .

@Pro
fix: Use PACKAGE_PREFIX_DIR to ensure relocatable CMake install for c…
44962b5 
…onan

Signed-off-by: Stefan Profanter <stefan.profanter@agile-robots.com>
@Pro
Copy link
Contributor Author

Pro commented Nov 17, 2020

Thanks @traversaro, I changed it now to ${pcfiledir}

@Pro Pro mentioned this pull request Nov 17, 2020
Closed
scpeters
scpeters requested changes Nov 23, 2020
View reviewed changes
cmake/sdformat_pc.in Outdated
@@ -1,4 +1,4 @@
prefix="@CMAKE_INSTALL_PREFIX@"
prefix=${pcfiledir}/../..
Copy link
Member

Choose a reason for hiding this comment

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

as discussed in dartsim/dart#1527 (comment), this relative path is incorrect on Ubuntu

Copy link
Member

Choose a reason for hiding this comment

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

based on a tip from @jslee02 in dartsim/dart#1529, I've updated this in 41ac64a to compute the relative path to the pkgconfig folder in cmake

@scpeters
Compute relative path for pkgconfig folder
41ac64a 
This accounts for arch-dependent lib folders.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@scpeters
Merge branch 'master' into cmake-relocatable
c8423c5
scpeters
scpeters approved these changes Jan 8, 2021
View reviewed changes
Copy link
Member

@scpeters scpeters left a comment

Choose a reason for hiding this comment

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

I think it looks good now, but maybe someone else wants to take a look since I've pushed to this branch? @j-rivero ?

@j-rivero j-rivero self-requested a review January 19, 2021 15:41
@scpeters scpeters changed the title fix: Use PACKAGE_PREFIX_DIR to ensure relocatable CMake install for conan Ensure relocatable config files Jan 20, 2021
@scpeters scpeters merged commit 82d7b96 into gazebosim:master Feb 4, 2021
scpeters added a commit that referenced this pull request Jul 27, 2022
@Pro @scpeters @j-rivero
Ensure relocatable config files (#419)
d10bbda 
* fix: Use PACKAGE_PREFIX_DIR to ensure relocatable CMake install for conan

Signed-off-by: Stefan Profanter <stefan.profanter@agile-robots.com>

* Compute relative path for pkgconfig folder.
  This accounts for arch-dependent lib folders.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Co-authored-by: Steve Peters <scpeters@openrobotics.org>
Co-authored-by: Jose Luis Rivero <jrivero@osrfoundation.org>
@scpeters scpeters mentioned this pull request Jul 27, 2022
Merged
scpeters added a commit that referenced this pull request Jul 27, 2022
@Pro @scpeters @j-rivero
Ensure relocatable config files (#419)
3e76d92 
* fix: Use PACKAGE_PREFIX_DIR to ensure relocatable CMake install for conan

Signed-off-by: Stefan Profanter <stefan.profanter@agile-robots.com>

* Compute relative path for pkgconfig folder.
  This accounts for arch-dependent lib folders.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Co-authored-by: Steve Peters <scpeters@openrobotics.org>
Co-authored-by: Jose Luis Rivero <jrivero@osrfoundation.org>
@scpeters scpeters mentioned this pull request Aug 11, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@j-rivero j-rivero j-rivero approved these changes

@scpeters scpeters scpeters approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Pro @codecov-io @traversaro @j-rivero @scpeters