Skip to content

Move h5repacktst file generation to h5repackgentest #5564

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 9 commits into
base: develop
Choose a base branch
from
Draft

Move h5repacktst file generation to h5repackgentest #5564

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ab7ad5d
Move h5repacktst file generation to h5repackgentest
mattjala Jun 3, 2025
9a56491
Add h5repacktst files to h5repack gen func
mattjala Jun 3, 2025
e48eab5
Committing clang-format changes
github-actions[bot] Jun 3, 2025
d5dc41b
Move USERBLOCK_SIZE definition to header
mattjala Jun 3, 2025
c2460d0
Remove h5repacktst's file cleanup
mattjala Jun 6, 2025
61b1a35
Fix inconsistency in h5repacktst build
mattjala Jun 6, 2025
6b4f5d8
Count errors during make_h5repack_testfiles
mattjala Jun 11, 2025
83a53b7
Move filename to gentest header
mattjala Jun 17, 2025
566e70b
Remove build generator reqs
mattjala Jun 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions tools/test/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,15 @@ if (HDF5_BUILD_GENERATORS)
${HDF5_LIB_TARGET}
${HDF5_TOOLS_LIB_TARGET}
${HDF5_REQUIRED_LIBRARIES} # For math lib
${HDF5_TEST_LIB_TARGET}
)
else ()
TARGET_C_PROPERTIES(${tool}gentest SHARED)
target_link_libraries (${tool}gentest PRIVATE
${HDF5_LIBSH_TARGET}
${HDF5_TOOLS_LIBSH_TARGET}
${HDF5_REQUIRED_LIBRARIES}
${HDF5_TEST_LIBSH_TARGET}
)
endif ()

Expand Down
1 change: 1 addition & 0 deletions tools/test/h5gentest.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,7 @@ gen_h5repack_files(void)
nerrors += (generate_f32le(external) < 0 ? 1 : 0);
} /* end for external data storage or not */

nerrors += (make_h5repack_testfiles() < 0 ? 1 : 0);
return nerrors;
}

Expand Down
8 changes: 4 additions & 4 deletions tools/test/h5repack/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,19 +27,19 @@ set (REPACK_COMMON_SOURCES
${HDF5_TOOLS_SRC_H5REPACK_SOURCE_DIR}/h5repack_verify.c
${HDF5_TOOLS_SRC_H5REPACK_SOURCE_DIR}/h5repack.c
)
# Add h5repack test executable
add_executable (h5repacktest ${REPACK_COMMON_SOURCES} ${HDF5_TOOLS_TEST_H5REPACK_SOURCE_DIR}/h5repacktst.c)
target_include_directories (h5repacktest
PRIVATE "${HDF5_TOOLS_SRC_H5REPACK_SOURCE_DIR};${HDF5_TOOLS_ROOT_DIR}/lib;${HDF5_SRC_INCLUDE_DIRS};${HDF5_TEST_SRC_DIR};${HDF5_SRC_BINARY_DIR};$<$<BOOL:${HDF5_ENABLE_PARALLEL}>:${MPI_C_INCLUDE_DIRS}>"
)
if (BUILD_STATIC_LIBS)
Copy link
Collaborator

@jhendersonHDF jhendersonHDF Jun 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think this is an issue where we need to determine the full purpose of HDF5_BUILD_STATIC_TOOLS and align between it and BUILD_STATIC_LIBS. Since this is a regular test program, it should probably be using BUILD_STATIC_LIBS as before rather than HDF5_BUILD_STATIC_TOOLS. But, when it comes time to link against the tools library we need to check HDF5_BUILD_STATIC_TOOLS to link against the correct one (though we should probably solve this higher up and just give lower level CMake code a single target to link against unconditionally).

if (HDF5_BUILD_STATIC_TOOLS)
TARGET_C_PROPERTIES (h5repacktest STATIC)
target_link_libraries (h5repacktest PRIVATE ${HDF5_TOOLS_LIB_TARGET} ${HDF5_TEST_LIB_TARGET})
target_link_libraries (h5repacktest PRIVATE ${HDF5_TOOLS_LIB_TARGET} ${HDF5_TEST_LIB_TARGET} h5repackgentest)
Copy link
Collaborator

@jhendersonHDF jhendersonHDF Jun 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Since the tool gentest object libraries are currently only built when HDF5_BUILD_GENERATORS is ON, this linking step fails when it's OFF. I'm guessing the reason it passes in most of the workflows is because they use the cacheinit file which sets that ON by default. I would generally like to avoid the requirement of HDF5_BUILD_GENERATORS=ON in cases where we don't need it, but this PR seems like it will need that, even for regular non-VOL testing. Maybe we need to decide if the HDF5_BUILD_GENERATORS option is still useful or if we should just always build the generators? Or maybe we should always build the generator libraries but only build the generator binaries when HDF5_BUILD_GENERATORS=ON.

else ()
TARGET_C_PROPERTIES (h5repacktest SHARED)
target_link_libraries (h5repacktest PRIVATE ${HDF5_TOOLS_LIBSH_TARGET} ${HDF5_TEST_LIBSH_TARGET})
target_link_libraries (h5repacktest PRIVATE ${HDF5_TOOLS_LIBSH_TARGET} ${HDF5_TEST_LIBSH_TARGET} h5repackgentest)
endif ()
set_target_properties (h5repacktest PROPERTIES FOLDER tools)

#-----------------------------------------------------------------------------
# If plugin library tests can be tested
#-----------------------------------------------------------------------------
Expand Down
2 changes: 1 addition & 1 deletion tools/test/h5repack/CMakeVFDTests.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,4 +108,4 @@ endmacro ()
# Run test with different Virtual File Driver
foreach (vfd ${VFD_LIST})
ADD_VFD_TEST (${vfd} 0)
endforeach ()
endforeach ()
Loading
Loading