Skip to content

[gdal] Fix dependency hdf5#25640

Closed
JackBoosY wants to merge 2 commits intomicrosoft:masterfrom
JackBoosY:dev/jack/fix-gdal-hdf5
Closed

[gdal] Fix dependency hdf5#25640
JackBoosY wants to merge 2 commits intomicrosoft:masterfrom
JackBoosY:dev/jack/fix-gdal-hdf5

Conversation

@JackBoosY
Copy link
Contributor

Related: #22392 (comment)

@JackBoosY JackBoosY added category:port-bug The issue is with a library, which is something the port should already support info:internal labels Jul 8, 2022
@JackBoosY
Copy link
Contributor Author

cc @dg0yt

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})
Copy link
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.

Copy link
Contributor

@dg0yt dg0yt Jul 8, 2022

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.

Copy link
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

Copy link
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.

Copy link
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

@JackBoosY JackBoosY linked an issue Jul 8, 2022 that may be closed by this pull request
@JackBoosY JackBoosY mentioned this pull request Jul 8, 2022
@JackBoosY
Copy link
Contributor Author

In favor of #25646

@JackBoosY JackBoosY closed this Jul 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:port-bug The issue is with a library, which is something the port should already support

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[gdal] build failure

3 participants