Conversation
At present, the installed CMake package exports linkage-qualified targets (`hdf5-shared`, `hdf5-static`, `hdf5_hl-shared`). That pushes the implementation details of internal target naming and provided linkage onto consumers in a way we would ideally avoid. This PR adds a new `config/cmake/HDF5PublicTargets.cmake` which defines stable names as ALIASes onto the concrete targets: | Target | Library | | --- | --- | | `hdf5::hdf5` | C | | `hdf5::hdf5_hl` | High-level C | | `hdf5::hdf5_cpp` / `hdf5::hdf5_hl_cpp` | C++ | | `hdf5::hdf5_fortran` / `hdf5::hdf5_hl_fortran` | Fortran | | `hdf5::<tool>` | each installed tool, e.g. `hdf5::h5diff` | | `HDF5::HDF5` | aggregate of the available libraries | The find process then simplifies to: ```cmake find_package (HDF5 NAMES hdf5 CONFIG REQUIRED COMPONENTS C) target_link_libraries (app PRIVATE hdf5::hdf5) ``` The same module now has identical names across the HDF5 build tree, `add_subdirectory` embeddings, and `find_package()` installations. The linkage type is decided while resolving the package, by (in order of precedence): 1. explicit `static`/`shared` component 2. `HDF5_USE_STATIC_LIBRARIES` (matching `FindHDF5`) 3. shared if available, static otherwise Requesting both components will give a warning and fall back to the default. This changeset is purely additive. The old shared/static targets, `HDF5_<lang>_<LINKAGE>_LIBRARY` variables, and the exported target set are unchanged, and the public names are ALIASes onto the same targets, so a project with both on its link line still gets only one library. Nothing is deprecated yet, though we may want to do so in the future. Partially addresses HDFGroup#6580
There was a problem hiding this comment.
Pull request overview
This PR modernizes HDF5’s installed (and build-tree) CMake package interface by introducing stable, linkage-agnostic namespaced targets (e.g., hdf5::hdf5) that alias onto the existing linkage-qualified targets, so downstream consumers no longer need to encode static/shared in their link logic.
Changes:
- Add
config/cmake/HDF5PublicTargets.cmakeimplementing stable public targets (hdf5::…), tool targets (hdf5::<tool>), and an aggregateHDF5::HDF5. - Update
hdf5-config.cmake.inand build-tree CMake to define these public targets consistently across install, build tree, andadd_subdirectory()use. - Update docs/examples/changelog to recommend linking via
hdf5::…targets and describe linkage selection rules.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| release_docs/CHANGELOG.md | Documents the new stable public CMake targets and linkage resolution behavior. |
| HDF5Examples/config/cmake/HDFExampleMacros.cmake | Prefer hdf5::hdf5 when available when configuring examples against external HDF5. |
| docs/USING_HDF5_CMake.md | Updates guidance to link via hdf5::hdf5 and explains public targets + linkage selection. |
| config/install/hdf5-config.cmake.in | Includes the new public-target module and selects linkage to bind public aliases. |
| config/cmake/HDF5PublicTargets.cmake | New module defining the stable public aliases and aggregate target. |
| CMakeLists.txt | Defines public aliases in the build tree (and for add_subdirectory() consumers). |
| CMakeInstallation.cmake | Ships the new module in build/install trees alongside the generated config package. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
This is unfortunately something I also happened to be working on at the time, but let me at least list a few things that I've had to think about while moving in this direction:
|
|
I'm not quite sure if we really need to introduce additional workflow to handle this. Ideally the implementation should be clean, and I don't think the targets like |
Personally I would say let's use the For an installed HDF5 package consumed through On the other hand, for HDF5 embedded through |
|
I've looked into it, and I'm leaning towards deprecating or removing HDF_PACKAGE_NAMESPACE. From what I can tell, it's not common convention to expose project-level CMake namespacing decisions like this to consumers at all. Other C libraries that use CMake (SDL3, zstd, json-c, netCDF-C, curl) all hardcode their namespace. I went looking for anyone actually using HDF_PACKAGE_NAMESPACE, and most of the results are forks or vendored copies of HDF5. Excluding those, there are two external consumers, and both set it to hdf5:: anyway:
VTK doesn't use the variable at all despite being the kind of consumer it seems to have been designed for. When VTK builds against an external HDF5 it just consumes hdf5::hdf5 and hdf5::hdf5_hl from FindHDF5. When it vendors HDF5, it sets HDF5_EXTERNALLY_CONFIGURED and renames the whole thing into its own namespace as VTK::hdf5 / vtkhdf5 via vtk.module. The namespace prefix wouldn't have even helped them, because their goal in doing the renaming seems to be keeping their embedded copy from colliding with any other HDF5 in the same build or process. HDF_PACKAGE_NAMESPACE also leaves the installed library as (Notably, the renaming contract that VTK needs is already handled by other variables. CMakeLists.txt:153-162 documents am embedding case with Paraview using HDF5_EXTERNALLY_CONFIGURED and HDF5_EXTERNAL_LIB_PREFIX, with no use of HDF_PACKAGE_NAMESPACE.) I think we should keep the exported linkage-qualified target names for now, deprecate and eventually remove HDF_PACKAGE_NAMESPACE, and have the hdf5:: namespace be the standard going forward. |
… slated for removal
|
Attempting to collect thoughts together into a single reply (apologies for the length and intermixing of text).
This would generally be ideal yes, but would also involve changes that I wouldn't necessarily be comfortable with in a minor release of HDF5. Using aliases for now is a relatively small change that slowly moves toward removing the old names. Interestingly, the targets used to be
To be clear on this point, I do believe that a stable namespace should be used on the installed targets going forward. It's likely that
So unless a package specifically requests version 2.3.0 or newer when calling
This is what I see as being the intent behind
Notably, this is not true for libcurl (though for reasons unrelated to this discussion); I didn't look too much into other projects.
I'm not sure if VTK is a great example here, as it uses a fairly heavily modified fork of HDF5 (https://gitlab.kitware.com/third-party/hdf5/-/tree/for/vtk-20260701-2.1.1) when building, likely because the current build process of the library doesn't give them enough flexibility to build the way they want. This includes substantially reworking the build of HDF5 and namespacing the resulting targets into their own
This is mostly irrelevant, as the implicit assumption is that a project would use |
6496118 to
dca2be7
Compare
|
This pull request has had no activity for 30 days and has been marked stale. Push a commit or comment to keep it open, or it will be flagged for maintainer review. |
|
|
||
| ### Added stable, linkage-agnostic CMake target names to the installed package | ||
|
|
||
| Downstream projects can now link `hdf5::hdf5`, `hdf5::hdf5_hl`, `hdf5::hdf5_cpp`, `hdf5::hdf5_hl_cpp`, `hdf5::hdf5_fortran` and `hdf5::hdf5_hl_fortran` instead of the linkage-qualified `hdf5-shared` and `hdf5-static` targets, along with an `HDF5::HDF5` aggregate and `hdf5::<tool>` for each installed tool. The names do not encode whether the library is static or shared, so the same project file works against a static installation, a shared one, or one providing both. These are the names CMake's own `FindHDF5` module provides, so a project using them no longer needs different code depending on whether HDF5 was located through `FindHDF5` or through HDF5's `hdf5-config.cmake`, and they are defined identically when HDF5 is built as a subproject with `add_subdirectory()`. Which linkage the targets refer to is chosen while resolving the package: by a `static` or `shared` component, by `HDF5_USE_STATIC_LIBRARIES`, or by default shared-if-available and static otherwise, which is the same order the rest of the package resolution now follows (see Breaking Changes). The existing `hdf5-shared` / `hdf5-static` targets and the `HDF5_<lang>_<LINKAGE>_LIBRARY` variables are otherwise unchanged, and the public names are aliases onto those same targets, so a project naming both on its link line still gets a single library. |
There was a problem hiding this comment.
"both" refers to old/existing targets and new new hdf5:: targets?
At present, the installed CMake package exports linkage-qualified targets (
hdf5-shared,hdf5-static,hdf5_hl-shared). That pushes the implementation details of internal target naming and provided linkage onto consumers in a way we would ideally avoid.This PR adds a new
config/cmake/HDF5PublicTargets.cmakewhich defines stable names as ALIASes onto the concrete targets:hdf5::hdf5hdf5::hdf5_hlhdf5::hdf5_cpp/hdf5::hdf5_hl_cpphdf5::hdf5_fortran/hdf5::hdf5_hl_fortranThe find process in CMake then simplifies to:
The same module now has identical names across the HDF5 build tree,
add_subdirectoryembeddings, andfind_package()installations.The linkage type is decided while resolving the package, by (in order of precedence):
static/sharedcomponentHDF5_USE_STATIC_LIBRARIES(matchingFindHDF5)Requesting both components will give a warning and fall back to the default.
This changeset is purely additive. The old shared/static targets,
HDF5_<lang>_<LINKAGE>_LIBRARYvariables, and the exported target set are unchanged, and the public names are ALIASes onto the same targets, so a project with both on its link line still gets only one library.Nothing is deprecated yet, though we may want to do so in the future.
Partially addresses #6580