Skip to content

[gdal] Fix dependency hdf5 #25640

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

Closed
JackBoosY wants to merge 2 commits into from
Closed

[gdal] Fix dependency hdf5 #25640

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a77923f
[gdal] Fix dependency hdf5
Jul 8, 2022
6ff7dd6
version
Jul 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
219 changes: 0 additions & 219 deletions ports/gdal/dependency_win.cmake
View file
Open in desktop

This file was deleted.

29 changes: 29 additions & 0 deletions ports/gdal/fix-dependency-hdf5.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
diff --git a/cmake/helpers/CheckDependentLibraries.cmake b/cmake/helpers/CheckDependentLibraries.cmake
index 7fa3b56..ade5494 100644
--- a/cmake/helpers/CheckDependentLibraries.cmake
+++ b/cmake/helpers/CheckDependentLibraries.cmake
@@ -494,7 +494,10 @@ if (GDAL_USE_LIBKML)
endif ()

# CXX is only needed for KEA driver
-gdal_check_package(HDF5 "Enable HDF5" COMPONENTS "C" "CXX" CAN_DISABLE)
+gdal_check_package(HDF5 NAMES hdf5 CONFIG "Enable HDF5" COMPONENTS "C" "CXX" CAN_DISABLE)
+if (HDF5_FOUND)
+ set(HDF5_C_LIBRARIES $<IF:$<TARGET_EXISTS:hdf5::hdf5-shared>,hdf5::hdf5-shared,hdf5::hdf5-static>)
+endif()

gdal_check_package(WebP "WebP compression" CAN_DISABLE)
gdal_check_package(FreeXL "Enable XLS driver" CAN_DISABLE)
diff --git a/frmts/hdf5/CMakeLists.txt b/frmts/hdf5/CMakeLists.txt
index 4b85797..919f061 100644
--- a/frmts/hdf5/CMakeLists.txt
+++ b/frmts/hdf5/CMakeLists.txt
@@ -24,7 +24,7 @@ if (WIN32)
target_compile_definitions(gdal_HDF5 PRIVATE -DWIN32)
endif ()
target_include_directories(gdal_HDF5 SYSTEM PRIVATE ${HDF5_INCLUDE_DIRS})
-gdal_target_link_libraries(gdal_HDF5 PRIVATE ${HDF5_C_LIBRARIES})
+target_link_libraries(gdal_HDF5 PRIVATE ${HDF5_C_LIBRARIES})

@JackBoosY JackBoosY Jul 8, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

function(gdal_target_interfaces _TARGET)
    foreach (_LIB IN ITEMS ${ARGN})
        if (TARGET ${_LIB})
            get_property(_res TARGET ${_LIB} PROPERTY INTERFACE_INCLUDE_DIRECTORIES)
            if (_res)
                target_include_directories(${_TARGET} PRIVATE ${_res})
            endif ()
            get_property(_res TARGET ${_LIB} PROPERTY INTERFACE_COMPILE_DEFINITIONS)
            if (_res)
                target_compile_definitions(${_TARGET} PRIVATE ${_res})
            endif ()
            get_property(_res TARGET ${_LIB} PROPERTY INTERFACE_COMPILE_OPTIONS)
            if (_res)
                target_compile_options(${_TARGET} PRIVATE ${_res})
            endif ()
        endif ()
    endforeach ()
endfunction()
function (gdal_add_private_link_libraries)
  get_property(tmp GLOBAL PROPERTY gdal_private_link_libraries)
  foreach (arg ${ARGV})
    set(tmp ${tmp} ${arg})
  endforeach ()
  set_property(GLOBAL PROPERTY gdal_private_link_libraries ${tmp})
endfunction (gdal_add_private_link_libraries)

So cmake will not add the INTERFACE_INCLUDE_DIRECTORIES.

@dg0yt dg0yt Jul 8, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So cmake will not add the INTERFACE_INCLUDE_DIRECTORIES.

There is target_include_directories(gdal_HDF5 SYSTEM PRIVATE ${HDF5_INCLUDE_DIRS}) (line 24), and this is enough.
GDAL is meant to use CMake's FindHDF5.cmake. And this passed CI in GDAL and in vcpkg. Multiple times.

This doesn't mean there couldn't bea broken configuration. But you have to clearly identify this configuration (for a clean start). And you proabbly have to find and fix it in the vicinity of port hdf5.

And there are configuration options which might remove the need for patching. It is just not clear enough what you try to fix.

@dg0yt dg0yt Jul 8, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No x64-windows here, but there: #25645

@JackBoosY JackBoosY Jul 8, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The problem is that neither vcpkg-cmake-wrapper nor hdf5-config.cmake provide HDF5_INCLUDE_DIRS.

@dg0yt dg0yt Jul 8, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No. The problem is install port hdf5 without cpp breaking CMake's FindHDF5.cmake. Cf. #25645


if (HDF5_BUILD_SHARED_LIBS)
target_compile_definitions(gdal_HDF5 PRIVATE -DH5_BUILT_AS_DYNAMIC_LIB)
1 change: 1 addition & 0 deletions ports/gdal/portfile.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ vcpkg_from_github(
HEAD_REF master
PATCHES
find-link-libraries.patch
fix-dependency-hdf5.patch
)
# `vcpkg clean` stumbles over one subdir
file(REMOVE_RECURSE "${SOURCE_PATH}/autotest")
Expand Down
1 change: 1 addition & 0 deletions ports/gdal/vcpkg.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"name": "gdal",
"version-semver": "3.5.1",
"port-version": 1,
"description": "The Geographic Data Abstraction Library for reading and writing geospatial raster and vector data",
"homepage": "https://gdal.org",
"license": null,
Expand Down
2 changes: 1 addition & 1 deletion versions/baseline.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2466,7 +2466,7 @@
},
"gdal": {
"baseline": "3.5.1",
"port-version": 0
"port-version": 1
},
"gdcm": {
"baseline": "3.0.12",
Expand Down
5 changes: 5 additions & 0 deletions versions/g-/gdal.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
{
"versions": [
{
"git-tree": "08b76f6f64cfce80bc66389e43c62240669862ff",
"version-semver": "3.5.1",
"port-version": 1
},
{
"git-tree": "f623db2a0771ad4df6c248b046cd8f369ecbdcca",
"version-semver": "3.5.1",
Expand Down