ubarsc / kealib

KEALib provides an implementation of the GDAL data model. The format supports raster attribute tables, image pyramids, meta-data and in-built statistics while also handling very large files and compression throughout.
http://kealib.org/
MIT License
12 stars 7 forks source link

Fix Kealib interface target #45

Closed dg0yt closed 11 months ago

dg0yt commented 11 months ago

This PR fixes using libkea::Kealib with static library linkage. This change comes from trying to add kealib support to the vcpkg port of GDAL: https://github.com/microsoft/vcpkg/pull/35437

If I understand the intention correctly, Kealib is just a constant interface name for the variable actual library target name. However, ATM it is hard-coded to use the shared link library which isn't created in static builds, and there is a TODO comment attached to this CMake code. This PR suggests to simply let CMake take care of forwarding transitive usage requirements from Kealib's interface link libraries by refering to the actual library as a cmake target.

It wouldn't be necessary to add any interface include directories for pure CMake target usage. But I left the installed interface include directory in case some consumer wants to read the property.

gillins commented 11 months ago

Thanks @dg0yt - cmake is a mystery to me... Any idea why the Python bindings now fail to link on Windows? Is there a problem with that CMakeLists.txt?

dg0yt commented 11 months ago

Thanks @dg0yt - cmake is a mystery to me... Any idea why the Python bindings now fail to link on Windows? Is there a problem with that CMakeLists.txt?

I have no exact idea about the python issue now. But the vcpkg reviewers just notified me about hdf5 targets appearing in the exported link libraries at least for static linkage. I know how to fix that, but ATM I'm surprised that it pops up at all, with module-mode find_package(HDF5) and using HDF5_LIBRARIES. This might be related, but I don't want to explain details before verifying.

dg0yt commented 11 months ago

Hm, sometimes vcpkg doesn't seem to have a good effect on the CMake ecosystem: https://gitlab.kitware.com/cmake/cmake/-/merge_requests/7416

dg0yt commented 11 months ago

This passed GHA in my fork.

The problem was caused by CMake's HDF5 find module behaving unusual (returning imported targets in HDF5_LIBRARIES). This is resolved by find_dependency(HDF5) in Kealib CMake config. It doesn't add any further obligations for downstream users of Kealib.

Given that the installed Kealib headers don't expose HDF5 headers, I also marked the link relationship as PRIVATE. If downstream users want to use HDF5, they have to do it explicitly. This also makes it possible to skip find_dependency(HDF5) for shared linkage of Kealib.

Last not least I dropped the explicit include directory property on Kealib. Transitive dependencies and nothing else.

dg0yt commented 11 months ago

CC @gillins. Does this PR accomplishes what you wanted to originally achieve with #31, or does it conflict?

gillins commented 11 months ago

Thanks for your help @dg0yt I really struggle with this stuff.