Rework CMake code for v3.0.0.

[1uc]

This PR develops the next version of HighFive's CMake code. The ideas are:

• Avoid one flag for each per optional dependency (other than testing and examples).
• Use targets only.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f71c7c4) 85.75% compared to head (64e27c1) 84.55%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #897      +/-   ##
==========================================
- Coverage   85.75%   84.55%   -1.21%     
==========================================
  Files          89       86       -3     
  Lines        5695     5220     -475     
==========================================
- Hits         4884     4414     -470     
+ Misses        811      806       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[1uc]

Currently, there's two interesting things. First:

find_package(HighFive)
target_link_libraries(foo PUBLIC HighFive)

still works, if one doesn't use any optional dependencies. If one does, it'll requires changes to the source code, but not the build-system.

Second: we don't provide code to link to optional dependencies. The argument is that if a library has dependency on, say, Boost, then they already have requirement to do -I${BOOST_DIR}/include and -L${BOOST_DIR}/lib -lboost (or whatever the commands are) even if they don't use HighFive. Hence, we could leave it to them to set it up correctly. Not including the dependencies has advantages, because users are free to pick how it work. For example when they vendor their other optional dependencies find_dependency(Boost) might do the wrong thing.

The question is should we help them? One could by doing the following:

find_package(HighFive COMPONENTS boost)
target_link_libraries(foo PUBLIC HighFive::HighFive HighFive::boost)

The COMPONENTS boost triggers the find_dependency(boost) and HighFive::boost triggers adding -I and -L as needed for the boost dependencies.

[1uc]

AFAICT one one pitfall is that pHDF5 requires linking to MPI, i.e.

target_link_libraries(HighFive INTERFACE HDF5::HDF5)
if(HDF5_IS_PARALLEL)
  find_package(MPI)
  target_link_libraries(HighFive INTERFACE MPI::MPI_C MPI::MPI_CXX)
endif()

However, now the target HighFive uses the MPI target when on the build machine the HDF5 version happened to be pHDF5. This is why we're using $<BUILD_INTERFACE:MPI::MPI_C>.

[1uc]

Second: we don't provide code to link to optional dependencies.

I'm increasingly convinced that attempting to find and link optional dependencies will likely end up incorrect under too many circumstances. IIUC the Boost ecosystem seems to have not yet settled on a set of commands that works pretty much anywhere. OpenCV has targets, but doesn't document them AFAICT. Even if we did use its targets, they might change them in a manner that's not compatible with our CMake, which seems fair because the project is telling us to not use them. Moreover, we'd probably only include a very small subset of the library and users would almost certainly want more. Same argument for Boost.

Not using targets has issues as documented here:
https://cmake.org/cmake/help/latest/manual/cmake-packages.7.html#creating-relocatable-packages

[matz-e]

Shouldn't this get updated now?

[1uc]

That's been prepped in the other PR: #957.

[tdegeus]

It might not be crystal clear that this is an alternative for find_package

[1uc]

Thank you, we just spent the two immediately preceding subsections explaining the difference.

[tdegeus]

Thanks. It seems fair to simply strip third-party dependencies from the target and leave it up to the user. To stay uniform, the defines should be removed everywhere, incl. from H5Easy.

[1uc]

To stay uniform, the defines should be removed everywhere, incl. from H5Easy.

I've spotted this. Solving this nicely kinda requires stripping out all the logic in H5Easy and move everything XTensor and Eigen related into core. After that H5Easy would do very little just a light API change.

The problem is that XTensor doesn't have a compile time constant rank. Which is a non-issue when implemented Easy-style. When we want to move it into the inspector it starts messing with deduction of rank, which becomes an issue with empty arrays and also messes with the "broad-casting" feature. Additionally, the not related issue of string vs arrays of character which cause stripping of dimensions doesn't make that part of the code any easier to understand.

I've mildly come to peace with the idea that Easy is "easy" and opinionated. Therefore, it continues to guess the presence of libraries by checking the existence of dependency related macros.

OTOH, it would be nice to make Easy just a light API change.

[tdegeus]

To stay uniform, the defines should be removed everywhere, incl. from H5Easy.

I've spotted this. Solving this nicely kinda requires stripping out all the logic in H5Easy and move everything XTensor and Eigen related into core. After that H5Easy would do very little just a light API change.

The problem is that XTensor doesn't have a compile time constant rank. Which is a non-issue when implemented Easy-style. When we want to move it into the inspector it starts messing with deduction of rank, which becomes an issue with empty arrays and also messes with the "broad-casting" feature. Additionally, the not related issue of string vs arrays of character which cause stripping of dimensions doesn't make that part of the code any easier to understand.

I've mildly come to peace with the idea that Easy is "easy" and opinionated. Therefore, it continues to guess the presence of libraries by checking the existence of dependency related macros.

OTOH, it would be nice to make Easy just a light API change.

A simpler idea is to make the defines 'private' and have a uniform API:

# include <highfive/eigen.hpp>
# include <highfive/xtensor.hpp>
# include <highfive/H5easy.hpp>

One could then statically assert on features not (yet) in the core. This would simplify the API, I think.

I am not quite up to speed with the templated rank issue. I think it is a separate issue, though.
(That being said: highfive is header-only, it is fine to have templated features??)

[1uc]

Let's track the H5Easy related questions in a different issue/PR. They don't prevent us from merging this as it is; and sort H5Easy separately.