Add workflows to test filter libraries installed in various ways by jhendersonHDF · Pull Request #238 · HDFGroup/hdf5_plugins

jhendersonHDF · 2026-03-10T14:46:07Z

Add GitHub workflows to test building inline with HDF5 and with an already-installed HDF5 using filter libraries installed from both package managers and from CMake's FetchContent

Updated the ZSTD build process with newer files to fix a build issue due to an old CMake macro definition

Fixed the Blosc, Blosc2, LZ4 and ZSTD CMake logic so system-installed libraries can be found with find_package()

Updated h5repack testing to make added filters required instead of optional so that testing issues aren't masked over

Fixed the Bitgroom and Bitround testing after making those filters required in h5repack testing

Synced CMake files with HDF5 develop to fix various issues

jhendersonHDF · 2026-03-10T14:46:39Z

Leaving this as a draft currently as this testing won't pass until some changes are merged into HDF5 develop

jhendersonHDF · 2026-03-10T14:47:29Z

.github/workflows/main-fetchcontent-mac.yml

+            echo "zlib_external=OFF" >> "$GITHUB_OUTPUT"
+          fi
+
+      # TODO: Re-enable ZSTD testing after CMake logic issue is fixed


ZSTD currently has a CMake logic issue preventing building with FetchContent

jhendersonHDF · 2026-03-10T14:51:00Z

BITGROOM/CMakeLists.txt

  SET_HDF_BUILD_TYPE()
 endif ()
-BASIC_SETTINGS (${BITGROOM_PACKAGE_NAME})
+BASIC_SETTINGS (H5${BITGROOM_PACKAGE_NAME})


Updated the hdf5_plugins package name to "h5pl" instead of "pl" because a CMake-built installation of hdf5_plugins couldn't be located otherwise. After doing that, the BASIC_SETTINGS macro needed updating to remove the "H5" that gets added to variable names everywhere, so now it has to be placed here in front of each filter's package name. This will be problematic if a non-default package name is ever specified for a filter, but that's already the case as the CMake variables below are already expected to be in the form "H5".

jhendersonHDF · 2026-03-10T14:52:02Z

BITGROOM/config/cmake/HDFPluginMacros.cmake

+          set (UPPER_BUILD_TYPE "")
+        endif ()
+        get_filename_component (_LIBRARY_PATH ${HDF5_INCLUDE_DIR} DIRECTORY)
+        set (HDF5_LIBRARY_PATH "${_LIBRARY_PATH}/lib")


This pattern found in several other files was just syncing with the main HDFPluginMacros.cmake file to fix an issue with HDF5_LIBRARY_PATH not being set and causing problems later on.

jhendersonHDF · 2026-03-10T14:52:53Z

BITGROOM/config/cmake/binex/config/cmake/runTest.cmake

This file and several identical ones were updated to sync with changes in HDF5 develop to fix various issues, including test error output being hidden.

jhendersonHDF · 2026-03-10T14:54:45Z

BITGROOM/config/cmake/binex/example/CMakeLists.txt

  if (NOT DISABLE_H5PL_ENCODER)
    #UD BITGROOM
-    ADD_H5_UD_TEST (ud_convert 0 h5repack_layout.h5 -v -f UD=32022,1,5,3,4,0,0,0 -l CHUNK=4x8)
+    ADD_H5_UD_TEST (ud_convert 0 h5repack_layout.h5 --enable-error-stack -v -f UD=32022,0,5,3,4,0,0,0 -l CHUNK=4x8)


Updated h5repack testing of several filters to make the filters required rather than optional. Otherwise, issues with the filters (especially with Bitgroom, Bitround and LZF) would be hidden during testing.

jhendersonHDF · 2026-03-10T14:57:20Z

BITGROOM/config/cmake/binex/example/testfiles/h5repack_layout.h5-ud_convert.ddl

The Bitgroom and Bitround testing was incorrect here. These filters remove themselves from the filter pipeline if the data isn't float data, which none of the data in the h5repack input file h5repack_layout.h5 is. But since they were previously added as optional filters, and since they were accidentally not in the plugin paths during testing, the testing skipped over the set_local() callbacks for these filters. So, the resulting file had the filters still applied even though they should have actually been removed from the pipeline. Testing of Bitgroom and Bitround should probably be updated to use float data.

jhendersonHDF · 2026-03-10T14:58:54Z

BLOSC/CMakeLists.txt

-        find_package (BLOSC) # Legacy find
-        if (BLOSC_FOUND)
-          set (H5PL_LINK_LIBS ${H5PL_LINK_LIBS} ${BLOSC_LIBRARIES})
+      if (BLOSC_FOUND)


This logic is needed to fix building against a system-installed Blosc.

jhendersonHDF · 2026-03-10T14:59:38Z

BLOSC2/CMakeLists.txt

-        if (BLOSC2_FOUND)
-          set (H5PL_LINK_LIBS ${H5PL_LINK_LIBS} ${BLOSC2_LIBRARIES})
+      find_package (BLOSC2 NAMES ${BLOSC2_PACKAGE_NAME}${HDF_PACKAGE_EXT})
+      if (BLOSC2_FOUND)


This logic is needed to fix building against a system-installed Blosc2.

jhendersonHDF · 2026-03-10T15:02:53Z

.github/workflows/main-package-managers-ubuntu.yml

+  # Currently installs and tests against the following filters:
+  #   - Blosc2
+  #   - JPEG
+  #   - ZFP (Temporarily disabled until H5Z-ZFP CMake is fixed)


ZFP currently has a CMake logic issue preventing building in this way

jhendersonHDF · 2026-03-10T15:03:43Z

BLOSC2/config/cmake/cacheinit.cmake

 set (HDF5_NAMESPACE "hdf5::" CACHE STRING "Name space of HDF5 library" FORCE)

-set (BLOSC2_PACKAGE_NAME "blosc2" CACHE STRING "Name of BLOSC2 package" FORCE)
+set (BLOSC2_PACKAGE_NAME "Blosc2" CACHE STRING "Name of BLOSC2 package" FORCE)


Fixed the Blosc2 package name so system-installed Blosc2 can be located

jhendersonHDF · 2026-03-10T15:04:39Z

CMakeLists.txt

  #if (CMAKE_C_COMPILER_ID MATCHES "Intel[Ll][Ll][Vv][Mm]")
  #  FILTER_OPTION (BLOSC2)
  #else
-  if (NOT CMAKE_C_COMPILER_ID MATCHES "Intel" AND NOT CMAKE_C_COMPILER_ID MATCHES "Apple[Cc]lang")


This no longer appears to be needed

jhendersonHDF · 2026-03-10T15:05:46Z

LZ4/CMakeLists.txt

-      find_package (LZ4) # Legacy find
-      if (LZ4_FOUND)
-        set (H5PL_LINK_LIBS ${H5PL_LINK_LIBS} ${LZ4_LIBRARIES})
+    if (LZ4_FOUND)


This logic is needed so system-installed LZ4 can be built against

jhendersonHDF · 2026-03-10T15:07:03Z

LZF/example/CMakeLists.txt

      set_tests_properties (H5LZF_UD-${testname} PROPERTIES DEPENDS H5LZF_UD-${testname}-clearall-objects)
+      if ("H5LZF_UD-${testname}" STREQUAL "H5LZF_UD-ud_convert")
+        # LZF filter currently returns a failure during testing
+        set_tests_properties (H5LZF_UD-${testname} PROPERTIES DISABLED TRUE)


LZF has had some issue with h5repack testing, but it was previously masked when LZF was added as an optional filter.

jhendersonHDF · 2026-03-10T15:07:33Z

ZSTD/CMakeLists.txt

-  message (STATUS "ZSTD C libs: static:${ZSTD_static_FOUND}")
-  if (NOT ZSTD_FOUND)
+  find_package (ZSTD NAMES ${ZSTD_PACKAGE_NAME})
+  if (ZSTD_FOUND)


This logic is needed so system-installed ZSTD can be built against

jhendersonHDF · 2026-03-10T15:08:35Z

ZSTD/config/cmake/H5ZSTDMacros.cmake

The ZSTD build process needed updating since it was out of date and failing. The build process should eventually be updated not to patch the source, but it's necessary for now.

.github/workflows/main-fetchcontent-mac.yml

brtnfld · 2026-03-11T15:09:38Z

ZSTD/config/cmake/H5ZSTDMacros.cmake

  # Make subproject to use 'BUILD_TESTING=OFF' setting.
  set (BUILD_TESTING OFF CACHE INTERNAL "Build Unit Testing" FORCE)

+  set (ZSTD_BUILD_PROGRAMS OFF)


Shouldn't you do the same as you did in H5BLOSCMacros.cmake:

set (ZSTD_BUILD_PROGRAMS OFF CACHE BOOL "Disable ZSTD programs" FORCE)
set (ZSTD_BUILD_CONTRIB OFF CACHE BOOL "Disable ZSTD contrib" FORCE)
set (ZSTD_BUILD_TESTS OFF CACHE BOOL "Disable ZSTD tests" FORCE)

As far as I can tell, the way that Blosc(2) vendors zstd makes these not useful to try and pass down (the vendored versions don't appear to use CMake), but we're also somewhat limited in passing them down with FetchContent anyway since the vendored libraries are "one level down".

brtnfld · 2026-03-11T15:15:10Z

.github/workflows/main-fetchcontent-win.yml

+    runs-on: windows-latest
+    steps:
+      - name: Get sources for HDF5
+        uses: actions/checkout@1af3b93b6815bc44a9784bd300feb67ff0d1eeb3 # v6.0.0


As mentioned in HDF5, we should remove # v. since it becomes stale once Dependabot updates it.

Dependabot put the #v there and it updates it, so it never goes stale. I don't see why we should remove it. Sure maybe if there will be no future updates, but it doesn't hurt anything.

brtnfld · 2026-03-11T15:16:36Z

.github/workflows/main-fetchcontent-ubuntu.yml

+            -DENABLE_BSHUF=ON \
+            ..
+          # NOTE: Currently a serial build has to be forced, as Blosc and Blosc2
+          # appear to have a parallel building dependency issue


Why this happens: When FetchContent brings in both c-blosc and c-blosc2 simultaneously into the same global CMake scope, they both attempt to build their own vendored copies of underlying compression libraries (like zlib, lz4, zstd, snappy). They end up defining identical target names (e.g., zlib_static) or trying to write intermediate files to the exact same paths in CMAKE_BINARY_DIR/internal-complibs. This creates severe race conditions during a parallel build and clobbers the dependency graph.
The Fix: FetchContent is dangerous when multiple dependencies vendor the same sub-dependencies without namespacing them. To fix the parallel build:
Turn on the flags in c-blosc and c-blosc2 that force them to use externally provided libraries (e.g., -DPREFER_EXTERNAL_LZ4=ON, -DPREFER_EXTERNAL_ZSTD=ON), and let HDF5 provide those dependencies.
Alternatively: Use ExternalProject_Add instead of FetchContent for these plugins so their configure and build steps are completely isolated in their own sandboxes.

This has happened in the same way when only when building one of Blosc or Blosc2 rather than both, they'll have their own separate build directories, PREFER_EXTERNAL_ZLIB and others can't be passed down normally due to the way the options are declared in Blosc/Blosc2 and ExternalProject_Add isn't really a great fix because it will involve much more complicated logic and patching. The vendored zlib is the only one that appears to be causing problems; more investigation is needed.

brtnfld · 2026-03-11T15:23:51Z

BITROUND/config/cmake/runTest.cmake

 if (EXISTS "${TEST_FOLDER}/${TEST_OUTPUT}.err")
-  file (READ ${TEST_FOLDER}/${TEST_OUTPUT}.err TEST_STREAM)
-  list (LENGTH TEST_STREAM test_len)
+  file (READ ${TEST_FOLDER}/${TEST_OUTPUT}.err TEST_ERROR)


TEST_ERROR is just standard text. The list() command evaluates standard strings by treating semicolons (;) as delimiters. If the error file outputs Missing file; aborting, CMake sees a list of 2 items. If there are no semicolons, it sees 1 item. It functionally works to check if the file is empty (test_len GREATER 0), but it is an abuse of the list API.

The Fix: Use string length or just a standard string comparison:
string(LENGTH "${TEST_ERROR}" test_len)

Sure maybe it's a little odd to use a list function here but it's all strings at the end of the day so I don't really see it as an abuse of the API since all we care about is whether the file was empty. There are many more places this would need fixed, including several in HDF5.

BLOSC/config/CacheURLs.cmake

BITGROOM/config/cmake/HDFPluginMacros.cmake

Add GitHub workflows to test building inline with HDF5 and with an already-installed HDF5 using filter libraries installed from both package managers and from CMake's FetchContent Updated the ZSTD build process with newer files to fix a build issue due to an old CMake macro definition Fixed the Blosc, Blosc2, LZ4 and ZSTD CMake logic so system-installed libraries can be found with find_package() Updated h5repack testing to make added filters required instead of optional so that testing issues aren't masked over Fixed the Bitgroom and Bitround testing after making those filters required in h5repack testing Synced CMake files with HDF5 develop to fix various issues

jhendersonHDF commented Mar 10, 2026

View reviewed changes

jhendersonHDF assigned ajelenak, brtnfld, lrknox and mattjala Mar 10, 2026

jhendersonHDF mentioned this pull request Mar 10, 2026

Update Bitgroom and Bitround h5repack testing #240

Open

brtnfld reviewed Mar 11, 2026

View reviewed changes

.github/workflows/main-fetchcontent-mac.yml Outdated Show resolved Hide resolved

brtnfld reviewed Mar 11, 2026

View reviewed changes

BLOSC/config/CacheURLs.cmake Show resolved Hide resolved

brtnfld reviewed Mar 11, 2026

View reviewed changes

BITGROOM/config/cmake/HDFPluginMacros.cmake Outdated Show resolved Hide resolved

jhendersonHDF force-pushed the add_testing_workflows branch from afe9f30 to c2cacd7 Compare March 11, 2026 16:59

Uh oh!

Conversation

jhendersonHDF commented Mar 10, 2026

Uh oh!

jhendersonHDF commented Mar 10, 2026

Uh oh!

jhendersonHDF Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

brtnfld Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jhendersonHDF Mar 10, 2026 •

edited

Loading

brtnfld Mar 11, 2026 •

edited

Loading