Skip to content

fix: Use PACKAGE_PREFIX_DIR to ensure relocatable CMake install for conan#30

Merged
caguero merged 1 commit intogazebosim:ign-tools1from
Pro:cmake-relocatable
Nov 20, 2020
Merged

fix: Use PACKAGE_PREFIX_DIR to ensure relocatable CMake install for conan#30
caguero merged 1 commit intogazebosim:ign-tools1from
Pro:cmake-relocatable

Conversation

@Pro
Copy link
Contributor

@Pro Pro commented Nov 17, 2020

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-cmake#129
gazebosim/sdformat#419
gazebosim/gazebo-classic#2782

cc @stefanbuettner

…onan

Signed-off-by: Stefan Profanter <stefan.profanter@agile-robots.com>
@Pro Pro force-pushed the cmake-relocatable branch from 26977fd to fd72ad9 Compare November 17, 2020 14:51
Copy link
Contributor

@caguero caguero left a comment

Choose a reason for hiding this comment

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

@Pro, thanks for the contribution! I tried out and it seems to work fine. Approving!

@caguero caguero merged commit 96f7ebd into gazebosim:ign-tools1 Nov 20, 2020
@traversaro
Copy link
Contributor

Note that the change in the .pc file here could break Debian packaging for the reasons described in dartsim/dart#1527 (comment) . fyi @j-rivero

@caguero
Copy link
Contributor

caguero commented Nov 23, 2020

Thanks @traversaro! @j-rivero , do you foresee any issues?

@scpeters
Copy link
Member

Note that the change in the .pc file here could break Debian packaging for the reasons described in dartsim/dart#1527 (comment) . fyi @j-rivero

I made that comment on dartsim, and yes I think it will break our .pc files on Ubuntu, since the relative path is not correct when installed to an architecture-dependent subfolder like /usr/lib/x86_64-linux-gnu/pkgconfig

@jslee02
Copy link

jslee02 commented Dec 28, 2020

@scpeters Consider using the trick used in dartsim/dart#1529. It should work even for the case you described.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏰 citadel Ignition Citadel 🔮 dome Ignition Dome 🏢 edifice Ignition Edifice

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants