Improve HDF5_BUILD_PARALLEL_TOOLS docs and MFU not-found message#6428
Conversation
…message The option description for HDF5_BUILD_PARALLEL_TOOLS was too terse to be useful — it did not mention the required MFU, CIRCLE, or DTCMP external libraries, nor that HDF5_ENABLE_PARALLEL must also be ON. Expand it with the dependency list and a link to the mpiFileUtils project. The failure block in FindMFU.cmake had a logic inversion (VERBOSE message gated on NOT QUIET; FATAL_ERROR gated on QUIET+REQUIRED) and referenced a non-existent MFU_INSTALL variable. Replace it with a single STATUS message that names all three required libraries, gives their GitHub URLs, and explains that MFU_ROOT is an environment variable covering all three.
brtnfld
requested review from
bmribler,
derobins,
fortnern,
jhendersonHDF,
lrknox,
qkoziol and
vchoi-hdfgroup
as code owners
| option (HDF5_BUILD_DOC "Build documentation" OFF) | ||
|
|
||
| option (HDF5_BUILD_PARALLEL_TOOLS "Build Parallel HDF5 Tools" OFF) | ||
| option (HDF5_BUILD_PARALLEL_TOOLS "Build h5dwalk parallel tool. Requires HDF5_ENABLE_PARALLEL=ON and MFU/CIRCLE/DTCMP (mpiFileUtils)." OFF) |
There was a problem hiding this comment.
For the sake of line width and if there may be other parallel-enabled tools in the future, I think it makes sense to keep this description something short like "Build parallel-specific HDF5 tools". Arguably, ph5diff and part of h5perf should have been moved under this option when it was created.
There was a problem hiding this comment.
what about "Build MPI-enabled HDF5 tools"
There was a problem hiding this comment.
Seems reasonable to me. May be more misleading as that would include ph5diff and h5perf as well, but I think that's maybe a separate discussion.
There was a problem hiding this comment.
changed, it seems that this option was not fully thought through regarding its final intent.
- CMakeBuildOptions.cmake: rephrase HDF5_BUILD_PARALLEL_TOOLS description to "Build MPI-enabled HDF5 tools" (shorter, forward-compatible) - config/cmake/FindMFU.cmake: restore FATAL_ERROR for missing MFU so configure fails when required libraries are absent
|
docs/AutotoolsToCMakeOptions.md line 62 also has the description "Build Parallel HDF5 Tools", and similarly docs/INSTALL_CMake_options.md line 198 has " enables building of parallel HDF5 tools". Should those also be updated to "Build MPI-enabled HDF5 tools"? |
Yes, those should probably be updated as well |
…F5 tools" Consistent wording across AutotoolsToCMakeOptions.md and INSTALL_CMake_options.md.
|
done |
github-project-automation
Bot
moved this from To be triaged
to Done
in HDF5 - TRIAGE & TRACK
3 participants
CMakeBuildOptions.cmake: expand theHDF5_BUILD_PARALLEL_TOOLSoption description to name the tool it builds (h5dwalk), list all three required external libraries (MFU, CIRCLE, DTCMP), link to the mpiFileUtils project, and note thatHDF5_ENABLE_PARALLEL=ONis also required.config/cmake/FindMFU.cmake: replace the dead/logic-inverted error block (wrongVERBOSE/FATAL_ERRORbranching, reference to non-existentMFU_INSTALLvariable) with aSTATUSmessage that fires beforefind_package_handle_standard_argsterminates configure. The message names all three required libraries, provides their GitHub URLs, and explains thatMFU_ROOTis the environment variable to set for a standard mpiFileUtils install.Fixes [Cmake Building Question] Parallel tools or Parallel ? #2265