Skip to content

Install ZFP filter shared library to lib/plugin instead of plugin#270

Open
lrknox wants to merge 2 commits into
HDFGroup:masterfrom
lrknox:fix-libh5zzfp-install-dest
Open

Install ZFP filter shared library to lib/plugin instead of plugin#270
lrknox wants to merge 2 commits into
HDFGroup:masterfrom
lrknox:fix-libh5zzfp-install-dest

Conversation

@lrknox

@lrknox lrknox commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator

H5Z-ZFP/src/CMakeLists.txt (vendored via subtree) hardcodes the shared plugin's install destination to plugin, instead of following this project's lib/plugin convention. Relocate the installed file at install time so libh5zzfp.so lands in /lib/plugin like every other filter, without patching the vendored subtree.

H5Z-ZFP/src/CMakeLists.txt (vendored via subtree) hardcodes the shared
plugin's install destination to plugin, instead of following this
project's lib/plugin convention. Relocate the installed file at
install time so libh5zzfp.so lands in /lib/plugin like
every other filter, without patching the vendored subtree.
@lrknox lrknox marked this pull request as draft July 11, 2026 14:05
@lrknox lrknox self-assigned this Jul 11, 2026
@lrknox

lrknox commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator Author

Additional fix needed for a packaging test failure on ubuntu.

CPack (e.g. the DEB generator used by the Ubuntu CI job) stages
installs into  while leaving CMAKE_INSTALL_PREFIX at its
literal configured value. install(TARGETS)/install(FILES) handle this
automatically via the DESTDIR-aware file(INSTALL), but the plain
file(MAKE_DIRECTORY)/file(RENAME) calls added to relocate libh5zzfp.so
did not, so packaging tried to create/rename outside the staging tree
and failed with 'No such file or directory'. Prepend {DESTDIR} by
hand to match.
@lrknox

lrknox commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator Author

All tests now pass.

@lrknox lrknox marked this pull request as ready for review July 11, 2026 14:52
Comment thread ZFP/CMakeLists.txt
file(RENAME
\"\${_h5zfp_destdir}/plugin/$<TARGET_FILE_NAME:h5z_zfp_shared>\"
\"\${_h5zfp_destdir}/${H5ZFP_INSTALL_LIB_DIR}/$<TARGET_FILE_NAME:h5z_zfp_shared>\")
")

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.

With the file renaming logic, this seems quite a bit like hacking around the ZFP code and also doesn't seem particularly maintainable; changing the installation prefix should have been as easy as setting a CMake variable before the add_subdirectory call. If that's not the case, we should be able to make a PR to the ZFP code so that's possible.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

claude says that because "plugin" is a literal string in these lines 62 - 64 of ZFP/H5Z-ZFP/src/CMakeLists.txt:
install(TARGETS h5z_zfp_shared
RUNTIME DESTINATION plugin
ARCHIVE DESTINATION plugin
LIBRARY DESTINATION plugin)
setting a variable beforehand won't have any effect. We'll need a PR to change "plugin" to a variable in those lines in src/CMakeLists.txt in the ZFP code?

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.

Yes I think the ZFP code should allow overriding the install destination. If GNUInstallDirs is followed, then it should probably be ${CMAKE_INSTALL_LIBDIR}/plugin by default anyway, unless the project wants to keep $prefix/plugin as the default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants