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

CMake Install: Capitalization fixes data and CMake directories; fixed include settings #2693

Merged
merged 1 commit into from
Feb 5, 2025

Conversation

emminizer
Copy link
Contributor

Minor fixes for CMake installation support on Linux

  • Minimum version fixed from 3.10 to 3.11 to avoid constant warnings about GL on configure
  • Added missing Threads dependency for UNIX systems
  • Capitalization fix: osgEarth-config.cmake to osgearth-config.cmake, so that find_package(osgEarth) can work on Linux
  • Filename rename from osgEarth-configVersion.cmake, to osgearth-config-version.cmake, to match style of other file
  • Install config files to share/osgEarth instead of share/osgearth, matching install location for shaders

Tested with a small Linux app that uses find_package(osgEarth). It did still require explicit links to GDAL and GEOS in the final application (Linux only) but it found the package and I was able to link successfully. Before this change, CMake could not be coaxed to find the package because of the capitalization in both share/osgEarth and osgEarth-config.cmake being inconsistent with what CMake expected.

@plevy
Copy link
Collaborator

plevy commented Jan 30, 2025

Dan, sorry about this, there were two cmake related PRs and the other was a bigger change, switching over to using the GNUInstallDirs. I merged that change, tried to make the changes from your PR and still might need a few more changes. Can you take a look at master and see what else needs to be done to get you back on track with the lower case changes.

Again, sorry for making you go over this again

@emminizer emminizer force-pushed the fix-capitalization-install branch from 781cd6f to 89148ed Compare February 3, 2025 14:45
@emminizer emminizer changed the title CMake Install: Capitalization fixes for config.cmake file CMake Install: Capitalization fixes data and CMake directories; fixed include settings Feb 3, 2025
@emminizer
Copy link
Contributor Author

I have reworked the patch with the required changes to get this working on Windows and Linux:

  • OSGEARTH_INSTALL_CMAKEDIR and OSGEARTH_INSTALL_DATADIR had improper capitalization based on what Glenn was suggesting in email separately
  • INCLUDE_INSTALL_DIR was set incorrectly. Thought this was a capitalization error, but it is not just that; it shouldn't include a suffix because the user needs to specify the osgEarth portion, e.g. #include "osgEarth/Map". Without this, #include <optional> can pick up osgEarth/optional instead of the system header, leading to compile bugs.
  • Removed an extra include_directores() that was a noop
  • Added required INCLUDES DESTINATION which fixes the osgEarth Targets.cmake output file to have the right INTERFACE_INCLUDE_DIRECTORIES value. Without this, a make install to a common location (e.g. /usr or /usr/local) might work in some cases, but a custom location would have the wrong include path associated with the target. This was removed around the time of the recent GNUInstallDirs change.

This should be good to go now. I tested with a make install on Linux gcc-13 and on Windows MSVC 2022.

@plevy plevy merged commit 1bc68de into gwaldron:master Feb 5, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants