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

Normalize header install path (backport #467) #474

Merged
merged 2 commits into from
Feb 14, 2025

Conversation

mergify[bot]
Copy link

@mergify mergify bot commented Feb 13, 2025

🦟 Bug fix

Fixes #461

Summary

The "meta" header file that contains includes of all other core / component header files is installed to a relative path, one folder above the other include files. This was causing a cmake warning that the path wasn't normalized. This PR normalizes that path variable to fix the warning. It also renames some variables from master_* to meta_* (6bd2d3d) for consistency and improved language and defines distinct variables for the config_header_install_dir and meta_header_install_dir (part of d32068d) to clarify what each variable represents, since the "meta" header previously wasn't actually installed to meta_header_install_dir.

This should be tested with downstream packages, including gz-plugin (to confirm the cmake warning is fixed) and sdformat (which uses the REPLACE_INCLUDE_PATH option).

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.


This is an automatic backport of pull request #467 done by [Mergify](https://mergify.com).

The "meta" header containing a list of all includes in
the package is currently installed one folder above the
other includes. This cleans up the definition of that
install path by normalizing it to fix a cmake warning
and also renaming some variables for clarity.

Signed-off-by: Steve Peters <[email protected]>
(cherry picked from commit 5b556fb)
@mergify mergify bot requested review from j-rivero and scpeters as code owners February 13, 2025 19:36
@mergify mergify bot mentioned this pull request Feb 13, 2025
8 tasks
@github-actions github-actions bot added the 🎵 harmonic Gazebo Harmonic label Feb 13, 2025
# Generate the "master" header that includes all of the headers
configure_file(${master_header_in} ${master_header_out})
# Generate the install directory for the "meta" header one folder above the "config" header
cmake_path(SET meta_header_install_dir NORMALIZE ${config_header_install_dir}/..)
Copy link
Member

Choose a reason for hiding this comment

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

this command was added in cmake 3.20, so wrap this in a ${CMAKE_VERSION} check

@scpeters
Copy link
Member

scpeters commented Feb 13, 2025

before merging:

@scpeters
Copy link
Member

before merging:

this test has only the same 5 compilation warnings as the previous run:

@scpeters scpeters requested a review from azeey February 14, 2025 00:45
@scpeters
Copy link
Member

I'll merge this and make a prerelease for further testing

@scpeters scpeters merged commit 796ad62 into gz-cmake3 Feb 14, 2025
10 checks passed
@scpeters scpeters deleted the mergify/bp/gz-cmake3/pr-467 branch February 14, 2025 03:28
@scpeters
Copy link
Member

I'll merge this and make a prerelease for further testing

#475

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎵 harmonic Gazebo Harmonic
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

1 participant