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 installation of Ign*.cmake modules on newer versions of CMake #425

Merged
merged 1 commit into from
May 2, 2024

Conversation

azeey
Copy link
Contributor

@azeey azeey commented May 2, 2024

🦟 Bug fix

Summary

Previously, we expected install(CODE ...), to run before install(FILES ...) since that is the order they appear in the CMakeLists.txt file. install(CODE ...) was responsible for creating a copy or symlink of Gz*.cmake files to Ign*.cmake
and putting them in a temporary location. install(FILES ...) would then copy those files to the final install destination. However, it appears that on CMake 3.29.2 (tested on windows), we can no longer rely on the order they appear in the code. From my local testing, install(CODE ...) ran after install(FILES ...).

This PR uses add_custom_command to generate the Ign*.cmake files which avoids any reliance on the order of the install calls.

This was caught during the ROS 2 tutorial party (see osrf/ros2_test_cases#1526)

cc @clalancette

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.

Copy link

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

This looks like a good fix to me. Thanks @azeey !

@scpeters scpeters merged commit 60c79ae into gz-cmake3 May 2, 2024
10 checks passed
@scpeters scpeters deleted the azeey/fix_ign_module_install branch May 2, 2024 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌱 garden Ignition Garden 🎵 harmonic Gazebo Harmonic
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants