Migrate ZFP to use H5Z-ZFP as git subtree #211
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Remove local copy of H5Z-ZFP code - Add https://github.com/LLNL/H5Z-ZFP as git submodule - Create wrapper CMakeLists.txt for integration - Add ZFP/README.md documenting submodule usage - Implements upstream-first development policy This ensures HDF Group does not maintain third-party code. Changes to the ZFP filter should go to LLNL/H5Z-ZFP upstream.
brtnfld
requested review from
ajelenak,
bmribler,
byrnHDF,
fortnern,
glennsong09,
jhendersonHDF,
lrknox and
mattjala
as code owners
git-subtree-dir: ZFP/H5Z-ZFP git-subtree-split: 30258a99ccced1e94ad6c12f93db2a9ca535b78a
|
Generated with ❤️ by ellipsis.dev |
- Update ZFP/README.md with subtree instructions - Update CLAUDE.md to reference subtree instead of submodule - Update migration docs to reflect subtree choice - Add SUBMODULE_VS_SUBTREE.md comparison document Git subtree provides better user experience: - No submodule initialization required - Tarball releases work automatically - Simpler CI/CD workflows
ZFP/H5Z-ZFP is upstream code from LLNL maintained via git subtree. We should not apply HDF Group formatting to upstream code.
brtnfld
changed the title
Restore the HDF Group's testing infrastructure (example/) and build configuration (config/) directories that work with the hdf5_plugins build system. These are kept separate from the H5Z-ZFP/ subtree which contains the upstream filter code.
Replace simple wrapper with full integration that: - Uses HDF Group's build infrastructure (config/, macros) - Reads version from upstream H5Z-ZFP source - Handles ZFP compression library (find or build externally) - Delegates filter building to H5Z-ZFP/CMakeLists.txt - Integrates HDF Group examples and testing - Exports targets for parent build system
Add Makefile.am, configure.ac, config.h.in, and config.make to support building ZFP filter with autotools in addition to CMake.
Remove Makefile.am, configure.ac, config.h.in, and config.make as they are incompatible with the H5Z-ZFP subtree structure. CMake is the primary build system for ZFP filter plugin.
- Include FetchContent module required by EXTERNAL_ZFP_LIBRARY macro - Set ZFP_DIR and ZFP_ROOT after building external ZFP library - Add debug messages to help troubleshoot ZFP library detection
H5Z-ZFP requires find_package(ZFP CONFIG) which expects a ZFPConfig.cmake file. When ZFP is built via FetchContent, generate a minimal config file that provides the ZFP target and required variables for H5Z-ZFP to use.
H5Z-ZFP's CMakeLists.txt uses CMAKE_SOURCE_DIR which points to the wrong location when used as a subdirectory. Build H5Z-ZFP/src/ and H5Z-ZFP/test/ directly instead of including the top-level H5Z-ZFP CMakeLists.txt to avoid path issues.
Create an alias from the old target name 'h5zfp' to the new H5Z-ZFP target 'h5z_zfp_shared' to maintain compatibility with the parent hdf5_plugins build system which expects the old naming convention.
The HDF Group tests require example programs to exist. Automatically enable H5PL_BUILD_EXAMPLES when H5PL_BUILD_TESTING is ON to ensure tests can run successfully.
H5Z-ZFP's src/CMakeLists.txt expects HDF5_INCLUDE_DIRS and HDF5_LIBRARIES to be set. Map them from the HDF Group's H5PL_HDF5_INCLUDE_DIRS and H5PL_HDF5_LINK_LIBS variables before building H5Z-ZFP source.
The upstream H5Z-ZFP tests expect ZFP_LIB_VERSION to be a hex constant but the EXTERNAL_ZFP_LIBRARY macro sets it incorrectly. Disable these tests and use H5PL_BUILD_TESTING for HDF Group tests which work correctly.
Add POST_BUILD command to copy h5z_zfp_shared to build/plugins/ so tests can find it via HDF5_PLUGIN_PATH. Maintains compatibility with the old build system's behavior.
The custom command must come after add_subdirectory(H5Z-ZFP/src) so the h5z_zfp_shared target exists. Remove the if(TARGET) check since it runs at configure time before the target is created.
add_custom_command(TARGET) only works for targets in the same directory. Since h5z_zfp_shared is created in H5Z-ZFP/src/, use add_custom_target with DEPENDS instead to copy the plugin to build/plugins/.
The upstream H5Z-ZFP version string doesn't include the github URL at the end. Update the reference file to match the actual output.
If find_package(ZFP) finds a legacy installation that provides ZFP_LIBRARIES but not the modern zfp::zfp imported target, create the target manually. H5Z-ZFP requires zfp::zfp for linking.
brtnfld
changed the title
|
Generated with ❤️ by ellipsis.dev |
Contributor
Author
|
Since the Actions don't run tests on PRs, I've verified that, for at least Linux, all the tests pass. |
brtnfld
changed the title
The ZFP filter now uses git subtree instead of submodules. The .gitmodules file is no longer needed. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
There was a problem hiding this comment.
If there's a way to do so, most dotfiles should probably be skipped for subtrees in the future, as it doesn't seem like they'll really be useful.
jhendersonHDF
approved these changes
Jan 5, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This ensures HDF Group does not maintain third-party code.
Required changes to the H5Z-ZFP should be made upstream at LLNL/H5Z-ZFP.
Fixes #194