1501 Add CMake option to allow building without HDF5#1502
1501 Add CMake option to allow building without HDF5#1502
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1502 +/- ##
=======================================
Coverage 97.40% 97.40%
=======================================
Files 189 189
Lines 16600 16600
=======================================
Hits 16170 16170
Misses 430 430 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
reneSchm
left a comment
reneSchm
left a comment
There was a problem hiding this comment.
I am honestly surprised that we are using the HAS_HDF5 macro that consistently, I expected more changes in the cpp directory.
Do we need an extra build option, or should we just group hdf into the "optional-deps" category? I think the latter would suffice, as we only need to check that nothing uses hdf without the corresponding macro.
| - `MEMILIO_BUILD_SIMULATIONS`: build the simulation applications in the simulations directory, ON or OFF, default ON. | ||
| - `MEMILIO_BUILD_SBML_MODELS`: build the SBML importer and imported models, i.e. everythin in the folder `sbml_model_generation`, ON or OFF, default ON. | ||
| - `MEMILIO_USE_BUNDLED_SPDLOG/_BOOST/_EIGEN/_JSONCPP`: use the corresponding dependency bundled with this project, ON or OFF, default ON. | ||
| - `MEMILIO_USE_HDF5`: build MEmilio with HDF5 IO support, ON or OFF, default ON. If OFF, HDF5 is not searched for and features like `save_result`/`read_result` are disabled. |
There was a problem hiding this comment.
From our current naming scheme I think this belongs to the "ENABLE" block below, i.e. MEMILIO_ENABLE_HDF5.
| fi | ||
| mkdir -p build && cd build | ||
| cmake -DCMAKE_C_COMPILER_LAUNCHER=ccache -DCMAKE_CXX_COMPILER_LAUNCHER=ccache -DCMAKE_BUILD_TYPE=${{ inputs.config }} -DMEMILIO_ENABLE_IPOPT=ON -DMEMILIO_TEST_COVERAGE=${{ inputs.coverage }} -DMEMILIO_SANITIZE_ADDRESS=${{ inputs.sanitizers }} -DMEMILIO_SANITIZE_UNDEFINED=${{ inputs.sanitizers }} -DMEMILIO_USE_BUNDLED_JSONCPP=${{ inputs.optional-dependencies }} -DMEMILIO_ENABLE_OPENMP=${{ inputs.openmp }} .. | ||
| cmake -DCMAKE_C_COMPILER_LAUNCHER=ccache -DCMAKE_CXX_COMPILER_LAUNCHER=ccache -DCMAKE_BUILD_TYPE=${{ inputs.config }} -DMEMILIO_ENABLE_IPOPT=ON -DMEMILIO_TEST_COVERAGE=${{ inputs.coverage }} -DMEMILIO_SANITIZE_ADDRESS=${{ inputs.sanitizers }} -DMEMILIO_SANITIZE_UNDEFINED=${{ inputs.sanitizers }} -DMEMILIO_USE_BUNDLED_JSONCPP=${{ inputs.optional-dependencies }} -DMEMILIO_ENABLE_OPENMP=${{ inputs.openmp }} -DMEMILIO_USE_HDF5=${{ inputs.hdf5 }} .. |
There was a problem hiding this comment.
If HDF5 should be an optional dependency, this should just use "inputs.optional-dependencies".
|
|
||
| # ## HDF5 | ||
| find_package(HDF5 COMPONENTS C) | ||
| option(MEMILIO_USE_HDF5 "Build memilio with HDF5." ON) |
There was a problem hiding this comment.
Please put the option next to the others in cpp/CMakeLists.txt.
2 participants
Changes and Information
Please briefly list the changes (main added features, changed items, or corrected bugs) made:
MEMILIO_USE_HDF5option to build without hdf5If need be, add additional information and what the reviewer should look out for in particular:
Merge Request - Guideline Checklist
Please check our git workflow. Use the draft feature if the Pull Request is not yet ready to review.
Checks by code author
Checks by code reviewer(s)