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

Remove example_INSTALL_DIR from PREFIX_PATH on examples #421

Merged
merged 2 commits into from
Apr 16, 2024

Conversation

j-rivero
Copy link
Contributor

@j-rivero j-rivero commented Apr 4, 2024

🦟 Bug fix

Summary

Remove the example_INSTALL_DIR to be passed to CMAKE_PREFIX_PATH since it is probably unused since the separator is wrong (semicolon instead of a colon). This triggered my attention since Noble has a warning with 3.28 CMake version.

CMake Warning:
  Ignoring extra path from command line:

   "/home/jenkins/workspace/gz_cmake-ci-gz-cmake3-noble-amd64/build/install/RelWithDebInfo"


CMake Warning:
  Ignoring extra path from command line:

   "/home/jenkins/workspace/gz_cmake-ci-gz-cmake3-noble-amd64/build/install/RelWithDebInfo"


CMake Warning:
  Ignoring extra path from command line:

   "/home/jenkins/workspace/gz_cmake-ci-gz-cmake3-noble-amd64/build/install/Release"


CMake Warning:
  Ignoring extra path from command line:

   "/home/jenkins/workspace/gz_cmake-ci-gz-cmake3-noble-amd64/build/install/Release"

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
Contributor

@azeey azeey left a comment

Choose a reason for hiding this comment

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

The documentation for CMAKE_PREFIX_PATH says semicolon is the correct separator, so I'm not sure if the warning is coming from that. But I think it's fine to remove since CMAKE_INSTALL_PREFIX is set to ${example_INSTALL_DIR}, which should allow find_package to find things in there (see https://cmake.org/cmake/help/latest/variable/CMAKE_INSTALL_PREFIX.html)

@@ -88,7 +88,7 @@ foreach(example ${example_directories})
# See alternate approach in a2113e0997c9 if this becomes too slow
BUILD_ALWAYS 1
CMAKE_ARGS
"-DCMAKE_PREFIX_PATH=${FAKE_INSTALL_PREFIX};${example_INSTALL_DIR}"
Copy link
Member

@scpeters scpeters Apr 4, 2024

Choose a reason for hiding this comment

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

this seems fine

the history of using two paths here goes back a long time (3ea5909), so if CI is happy with just FAKE_INSTALL_PREFIX, then that should be fine

@j-rivero j-rivero merged commit b93b52c into gz-cmake3 Apr 16, 2024
8 checks passed
@j-rivero j-rivero deleted the jrivero/fix_cmake_warn_noble branch April 16, 2024 14: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