gazebosim / gz-physics

Abstract physics interface designed to support simulation and rapid development of robot applications.

https://gazebosim.org

Apache License 2.0

62 stars 38 forks source link

Use relative install paths for plugin shared libraries #616

Closed azeey closed 2 months ago

azeey commented 3 months ago

🦟 Bug fix

Summary

Fixes an error when building https://github.com/gazebo-release/gz_physics_vendor/ in the ROS buildfarm.

Checklist

[ ] Signed all commits for DCO
[ ] Added tests
[ ] Updated documentation (as needed)
[ ] Updated migration guide (as needed)
[ ] Consider updating Python bindings (if the library has them)
[ ] codecheck passed (See contributing)
[ ] All tests passed (See test coverage)
[ ] While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 78.35%. Comparing base (2bc19ad) to head (ede7dd5). Report is 1 commits behind head on gz-physics7.

:exclamation: Current head ede7dd5 differs from pull request most recent head a141a08. Consider uploading reports for the commit a141a08 to get more accurate results

Additional details and impacted files

```diff @@ Coverage Diff @@ ## gz-physics7 #616 +/- ## ============================================ Coverage 78.35% 78.35% ============================================ Files 140 140 Lines 8079 8079 ============================================ Hits 6330 6330 Misses 1749 1749 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

traversaro commented 2 months ago

Note that GZ_PHYSICS_ENGINE_INSTALL_DIR is used in many places (see https://github.com/search?q=repo%3Agazebosim%2Fgz-physics%20GZ_PHYSICS_ENGINE_INSTALL_DIR%20&type=code), and I am not sure if all those uses work fine with GZ_PHYSICS_ENGINE_INSTALL_DIR being a non-absolute path (in particular see https://github.com/gazebosim/gz-physics/blob/4e400e0e12c3444c432d7c03f6e66f9fa4c82d4f/include/gz/physics/config.hh.in#L31, that even if deprecated is a public macro). Perhaps it could make sense to change only the places where DESTINATION argument of install to use GZ_PHYSICS_ENGINE_RELATIVE_INSTALL_DIR instead of GZ_PHYSICS_ENGINE_INSTALL_DIR ?

azeey commented 2 months ago

Note that GZ_PHYSICS_ENGINE_INSTALL_DIR is used in many places (see https://github.com/search?q=repo%3Agazebosim%2Fgz-physics%20GZ_PHYSICS_ENGINE_INSTALL_DIR%20&type=code), and I am not sure if all those uses work fine with GZ_PHYSICS_ENGINE_INSTALL_DIR being a non-absolute path (in particular see

https://github.com/gazebosim/gz-physics/blob/4e400e0e12c3444c432d7c03f6e66f9fa4c82d4f/include/gz/physics/config.hh.in#L31

, that even if deprecated is a public macro). Perhaps it could make sense to change only the places where DESTINATION argument of install to use GZ_PHYSICS_ENGINE_RELATIVE_INSTALL_DIR instead of GZ_PHYSICS_ENGINE_INSTALL_DIR ?

Good catch. I've used GZ_PHYSICS_ENGINE_RELATIVE_INSTALL_DIR in https://github.com/gazebosim/gz-physics/pull/616/commits/a141a083f5eac6b2b9d575d2bfd4e780ddf2c75a.