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 package config files #2879

Merged
merged 5 commits into from
Mar 15, 2021
Merged

Ensure relocatable package config files #2879

merged 5 commits into from
Mar 15, 2021

Conversation

Pro
Copy link
Contributor

@Pro Pro commented Nov 17, 2020

The CMake config used absolute paths which doesn't allow to move the installed cmake files. This is the case, e.g., if you create a conan package and distribute the binaries to other users.

Fixes: #2755
Fixes: #2782

@traversaro
Copy link
Collaborator

FYI I think that some generated files may still not be relocatable (for example the setup.sh and setup.bat files), but depending on your use-case you may be not interested in those. If you are interested in those, I have some relocatable variants that I never pushed upstream until now:

@Pro
fix: Use proper cmake package config for relocatable CMake config
605be53 
The CMake config used absolute paths which doesn't allow to move the installed cmake files. This is the case, e.g., if you create a conan package and distribute the binaries to other users.

Fixes: gazebosim#2755
Fixes: gazebosim#2782
@Pro Pro force-pushed the gazebo11_11.1.0_cmake branch from adb54b5 to 605be53 Compare November 17, 2020 15:12
@chapulina chapulina added the 11 Gazebo 11 label Nov 23, 2020
@chapulina chapulina mentioned this pull request Nov 25, 2020
Merged
@chapulina chapulina requested a review from j-rivero December 14, 2020 19:38
@chapulina chapulina requested a review from scpeters January 25, 2021 19:40
@scpeters
fix spelling
bf81f02 
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@scpeters
Fix relative path in pkg-config files
31db497 
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@scpeters
Merge branch 'gazebo11' into gazebo11_11.1.0_cmake
709f428
@scpeters scpeters changed the title fix: Use proper cmake package config for relocatable CMake config Ensure relocatable package config files Jan 28, 2021
@scpeters
Copy link
Member

I think I broke something because the tests that compile example code are failing

@scpeters
Copy link
Member

scpeters commented Feb 4, 2021

I think I broke something because the tests that compile example code are failing

I had trouble reproducing this because I was running the tests directly instead of calling ctest -R test_name, which triggers the use of environment variables specified in GazeboTestUtils.cmake, including PKG_CONFIG_PATH and CMAKE_PREFIX_PATH. The intention of setting those variables there is to help tests pass without running make install (7283291), but the -I paths are now broken. A different approach is taken to test compilation of example code without running make install by using a FAKE_INSTALL with ExternalProject_Add, but that may be prohibitive for a project as large as gazebo:

@scpeters
Don't set some environment variables in tests
e7fcb46 
The CMAKE_PREFIX_PATH and PKG_CONFIG_PATH values used
in the tests aren't working with the relocatable
config files.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@scpeters
Copy link
Member

reverting some environment variables in GazeboTestUtils in e7fcb46 fixes the compilation tests, though they now require make install. I think that's a reasonable trade-off unless someone wants to implement a FAKE_INSTALL target for testing

@scpeters
Copy link
Member

this looks good to me now, but I'd like @j-rivero to take a look since I've modified it from the original version

@stefanbuettner stefanbuettner mentioned this pull request Feb 12, 2021
Open
j-rivero
j-rivero approved these changes Mar 15, 2021
View reviewed changes
Copy link
Contributor

@j-rivero j-rivero left a comment

Choose a reason for hiding this comment

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

The generated .pc files looks good to me. The failing tests in Windows are not related to these changes.

@j-rivero j-rivero merged commit 2df3bfb into gazebosim:gazebo11 Mar 15, 2021
@traversaro traversaro mentioned this pull request May 5, 2021
Closed
lopsided98 added a commit to lopsided98/gazebo that referenced this pull request Aug 30, 2021
@lopsided98
Fix pkgconfig prefix with absolute CMAKE_INSTALL_LIBDIR
a7e6e1c 
gazebosim#2879 introduced an assumption that CMAKE_INSTALL_LIBDIR is relative, by concatenating it with CMAKE_INSTALL_PREFIX. Instead, this path should be formed using CMAKE_INSTALL_FULL_LIBDIR.
@lopsided98 lopsided98 mentioned this pull request Aug 30, 2021
Merged
scpeters pushed a commit that referenced this pull request Sep 10, 2021
@lopsided98
Fix pkgconfig prefix with absolute CMAKE_INSTALL_LIBDIR (#3076)
cca6ddd 
#2879 introduced an assumption that CMAKE_INSTALL_LIBDIR is relative, by concatenating it with CMAKE_INSTALL_PREFIX. Instead, this path should be formed using CMAKE_INSTALL_FULL_LIBDIR.
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 Awaiting requested review from scpeters scpeters is a code owner

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

Successfully merging this pull request may close these issues.

User dependent gazebo installation Gazebo does not have relocatable CMake configuration files
5 participants
@Pro @traversaro @scpeters @j-rivero @chapulina