Add THREADS check to configuration file #4746
Conversation
byrnHDF
added
Merge - To 1.14
Priority - 0. Blocker ⛔
byrnHDF
requested review from
lrknox,
derobins,
fortnern,
jhendersonHDF,
qkoziol,
vchoi-hdfgroup,
bmribler,
glennsong09,
mattjala and
brtnfld
as code owners
|
What's this change supposed to do? |
|
If the library is built with threads by any of the three options then when using the library, find the threads package. |
HDF5Examples/CMakeLists.txt
Outdated
| set (H5_HAVE_PTHREAD_H 1) | ||
| else () | ||
| message (FATAL_ERROR " **** thread support requires C11 threads, Win32 threads or Pthreads **** ") | ||
| #----------------------------------------------------------------------------- |
There was a problem hiding this comment.
This piece seems wrong, as it's eliminating all the macros that were previously set that show what kind of threads are found.
There was a problem hiding this comment.
No that section is not needed for Examples. These changes only affect the Examples when using the library binary.
There was a problem hiding this comment.
OK, I see.
Then this entire section of the CMake file should be removed, since the examples don't use threading.
There was a problem hiding this comment.
Until we add examples that test such feature.
There was a problem hiding this comment.
If the library requires the THREADS library it still needs to be checked.
There was a problem hiding this comment.
Why? Applications that link to a threadsafe version of HDF5 don't need to make this check in their CMake builds, correct?
config/cmake/hdf5-config.cmake.in
Outdated
| @@ -92,7 +92,7 @@ if (${HDF5_PACKAGE_NAME}_ENABLE_PARALLEL) | |||
| find_package(MPI QUIET REQUIRED) | |||
| endif () | |||
|
|
|||
| if (${HDF5_PACKAGE_NAME}_ENABLE_THREADSAFE OR ${HDF5_PACKAGE_NAME}_ENABLE_SUBFILING_VFD) | |||
| if (${HDF5_PACKAGE_NAME}_ENABLE_THREADSAFE OR ${HDF5_PACKAGE_NAME}_ENABLE_THREADS OR ${HDF5_PACKAGE_NAME}_ENABLE_SUBFILING_VFD) | |||
There was a problem hiding this comment.
The "${HDF5_PACKAGE_NAME}_ENABLE_THREADSAFE" and "${HDF5_PACKAGE_NAME}_ENABLE_SUBFILING_VFD)" components are no longer correct and should be removed.
(Leaving just ${HDF5_PACKAGE_NAME}_ENABLE_THREADS)
There was a problem hiding this comment.
Why are they no longer correct, if they are set in the library? This file is the status of the options used to build the library.
There was a problem hiding this comment.
Do you mean that since they require threads that is why we only need to check threads?
There was a problem hiding this comment.
This check is only about threads, not threadsafety or subfiling.
HDF5Examples/CMakeLists.txt
Outdated
| set (H5_HAVE_PTHREAD_H 1) | ||
| else () | ||
| message (FATAL_ERROR " **** thread support requires C11 threads, Win32 threads or Pthreads **** ") | ||
| #----------------------------------------------------------------------------- |
There was a problem hiding this comment.
OK, I see.
Then this entire section of the CMake file should be removed, since the examples don't use threading.
|
Why are there a ton of other changes in this commit? |
1 similar comment
|
Why are there a ton of other changes in this commit? |
It was requested that the option names for examples be standardized. |
* Cleanup threads package checks * Check first if package was found * Remove unnecessary dependent checks * Remove Unused options and fix names of option prefix
* Remove dummy comments that repeat function names. (#4775) * Fix grammar in H5Odtype.c comment block (#4777) * Add subfiling checks to the gcc action (#4776) * Replace non-VOL calls with VOL calls - part 3 (#4771) This PR switches H5I_object() to H5VL_vol_object() in H5O and H5T APIs. H5M is the last one and left out of this PR because it needs more work in documentation and there is no test for the API functions. Fixes GH-4730 * Add subfiling to CI more places where we test parallel (#4778) * CMake: gcc,, AOCC * Autotools: AOCC (gcc was added in a previous commit) NVHPC generates a lot of tools errors for some reason * Convert Collective Calls html file to doxygen (#4779) * Fix grammar in H5Fint.c comment block (#4782) * Improve the consistency of configure help messages (#4783) Fix grammar in configure message * Fixes Fortran parallel build race condition for tests (#4789) * Update URL documentation links to support site (#4781) * Fix grammar and simplify comment in H5Fint.c (#4790) * Fix char-subscripts warnings in H5private.h (#4793) * Bump the github-actions group with 3 updates (#4798) Bumps the github-actions group with 3 updates: [actions/checkout](https://github.com/actions/checkout), [DoozyX/clang-format-lint-action](https://github.com/doozyx/clang-format-lint-action) and [github/codeql-action](https://github.com/github/codeql-action). Updates `actions/checkout` from 4.1.1 to 4.1.7 - [Release notes](https://github.com/actions/checkout/releases) - [Commits](actions/checkout@v4.1.1...v4.1.7) Updates `DoozyX/clang-format-lint-action` from 0.17 to 0.18 - [Release notes](https://github.com/doozyx/clang-format-lint-action/releases) - [Commits](DoozyX/clang-format-lint-action@v0.17...v0.18) Updates `github/codeql-action` from 3.25.15 to 3.26.6 - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@afb54ba...4dd1613) --- updated-dependencies: - dependency-name: actions/checkout dependency-type: direct:production update-type: version-update:semver-patch dependency-group: github-actions - dependency-name: DoozyX/clang-format-lint-action dependency-type: direct:production update-type: version-update:semver-minor dependency-group: github-actions - dependency-name: github/codeql-action dependency-type: direct:production update-type: version-update:semver-minor dependency-group: github-actions ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Fix typo argueably in H5Cprivate.h (#4795) * Fix typos in H5Cpkg.h (#4796) * Add bounds checking to avoid Out-of-bounds Write for gif2h5 (#4786) * Replace non-VOL calls with VOL calls - part 5 (#4788) This PR switches H5I_object() and H5I_object_verify() to H5VL_vol_object() and H5VL_vol_object_verify(), respectively, in the H5M APIs and H5Gdeprec (was left out by mistake). This completes the fixes of issue GH-4730. * Correct the URL paths (#4802) * Fix a few issues with error reporting during sec2 reads/writes (#4794) * Update windows and apple signing process (#4806) * Use latest clang format action (#4807) * Correct path to document (#4808) * Detect invalid ID to H5Gmove2 (#4765) User's application segfaulted because the returned value H5I_BADID wasn't detected when H5I_get_type() was called. This PR adds checks for invalid file/group identifiers passed into H5Gmove2. This defect occurs in many other places, hence, issue GH-4764. Fixes #4737 * Fix use of public API calls (#4809) Switch public API calls to private ones. Root cause of #4672, which it fixes. Also minor code cleanups * Remove call to H5E_clear_stack() (#4810) Remove call to H5E_clear_stack() in H5G_node_debug() Add misc. minor cleanups * Enable win Intel signing (#4812) * Remove unneeded file name part (#4814) * Update NVHPC optimization settings (#4815) * Use -gopt in Autotools/CMake instead of -g * Autotools uses -O3 for release, -O1 for debug * Remove CMake optimization flag removal hack * Add mirror VFD to serial -Werror CI action (#4753) * Add mirror VFD to serial -Werror CI action * NUL terminate mirror_vfd.c strings * Add THREADS check to configuration file (#4746) * Cleanup threads package checks * Check first if package was found * Remove unnecessary dependent checks * Remove Unused options and fix names of option prefix * Rework RFC url in aliases (#4813) * Update config/cmake/hdf5-config.cmake.in _ENABLE_THREADS doesn't belong in hdf5_1_14
No description provided.