Skip to content

CMake: fix handling of geotiff-config.cmake #5341

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 18, 2022
Merged

CMake: fix handling of geotiff-config.cmake #5341

merged 1 commit into from
Feb 18, 2022

Conversation

rouault
Copy link
Member

@rouault rouault commented Feb 17, 2022

Windows Conda builds with the MrSID drivers failed because it couldn't
find the geotiff headers. Other drivers need it but must include the
Conda include directories because of other packages, hence it was
unnoticed.

geotiff-config.cmake doesn't define an explicit CMake target but only
CMake variables.

CC @dg0yt

@rouault
CMake: fix handling of geotiff-config.cmake
8d2bbac 
Windows Conda builds with the MrSID drivers failed because it couldn't
find the geotiff headers. Other drivers need it but must include the
Conda include directories because of other packages, hence it was
unnoticed.

geotiff-config.cmake doesn't define an explicit CMake target but only
CMake variables.
@rouault rouault force-pushed the cmake_fix_geotiff_config branch from 3bce63b to 8d2bbac Compare February 17, 2022 21:40
@dg0yt
Copy link
Contributor

dg0yt commented Feb 18, 2022

Okay.

geotiff-config.cmake seems to focus on providing the variables generally used with Find Modules. Apart from the include dirs, the target geotiff_library also lacks a proper name suffix, and doesn't use targets for its interface link libraries. This needs to be improved in libgeotiff.

I hope that there are not too many more surprises left, because we use only a few configs now. Another observation: In CI on Linux, cmake now finds zstd::zst_shared from linuxbrew intead of the system's zstd. This has an effect on the include dirs and on the rpath.

@rouault rouault mentioned this pull request Feb 18, 2022
Closed
@rouault rouault merged commit 1b04aec into OSGeo:master Feb 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rouault @dg0yt