CMake variable HDF5_INSTALL_CMAKE_DIR should be lib/cmake on Windows

[sdebionne]

On Windows, it looks like HDF5_INSTALL_CMAKE_DIR is set to cmake in

hdf5/config/cmake_ext_mod/HDFMacros.cmake

Line 359 in 5b5a1a8

set (${package_prefix}_INSTALL_CMAKE_DIR cmake)

Shouldn't this have the default value ${CMAKE_INSTALL_LIBDIR}/cmake (given that the GNUInstallDirs module is imported) or just lib/cmake?

The issue is that the CMake package files (e.g. hdf5-config.cmake) ends up being installed in %PREFIX%\cmake\hdf5 which is NOT a path considered by the search procedure. Not convenient as downstream projects have to define HDF5_DIR to workaround this issue.

Related to conda-forge/hdf5-feedstock#109

[byrnHDF]

Unfortunately this was implemented many years ago and used in many projects. We will need to come up with a solution that does not adversely affect all users.

[sdebionne]

What kind of problem do you foresee? If HDF5_DIR is set to %PREFIX%\cmake\hdf5, it should no interfere: it's just a hint. If the package is now installed in %PREFIX%\lib\cmake\hdf5, CMake will find it with it's default search procedure, so it should be transparent for the users.

[byrnHDF]

I should explain a little more, I am sure this is the right way to move, we just need time to test the change, make sure we change all the uses of this (includes other projects that use this setting). It is just the logistics of our resources and process.

[byrnHDF]

According to CMake documentation all search paths (Windows or Unix) are searched on all platforms. Therefore, line 359 in the HDFMacros.cmake file could be set to the same value as line 356. Would result in:
set (${package_prefix}_INSTALL_CMAKE_DIR share/cmake)

[sdebionne]

Yes that should work, I mentioned set (${package_prefix}_INSTALL_CMAKE_DIR lib/cmake) as an example. Most of the libraries I use install CMake packages in lib/cmake folder on Windows though -I guess share is a bit more Posix than lib.

[byrnHDF]

It removes another special setting and results in a single setting:
<
set (${package_prefix}_INSTALL_DATA_DIR share)
else ()
set (${package_prefix}_INSTALL_DATA_DIR ".")
endif ()
set (${package_prefix}_INSTALL_CMAKE_DIR share/cmake)
endif ()

[byrnHDF]

Maybe we could even move all the way out;
<
set (${package_prefix}_INSTALL_DATA_DIR share)
else ()
set (${package_prefix}_INSTALL_DATA_DIR ".")
endif ()
endif ()
set (${package_prefix}_INSTALL_CMAKE_DIR share/cmake)

[byrnHDF]

If we want to allow a user setting it would then be in an if block:
<
endif ()
if (NOT ${package_prefix}_INSTALL_CMAKE_DIR)
set (${package_prefix}_INSTALL_CMAKE_DIR share/cmake)
endif ()

[byrnHDF]

The last would be consistent with the other settings.

[byrnHDF]

If we want to allow a user setting it would then be in an if block:
<
endif ()
if (NOT ${package_prefix}_INSTALL_CMAKE_DIR)
set (${package_prefix}_INSTALL_CMAKE_DIR share/cmake)
endif ()

If this is acceptable, I will create a PR for develop and then push changes to the other branches and products.

[sdebionne]

LGTM. I like the simplification and the user setting, thanks!

[byrnHDF]

I will be putting a general comment on #76 that PRs are first created on the develop branch.
I have created #79 to address this issue.

[byrnHDF]

Changes pushed to all branches.