Cmake improvements

[planetmarshall]

Many thanks for creating this library - I'm incorporating into a project and have made what I hope are a few improvements mostly on the CMake side. I've also added a Github Workflow but I realise you may have your own CI system. Submitting these changes in case you'd like to incorporate these changes upstream.

Summary of changes -

• Add Github Workflow, compiles and tests builds for 36 configurations over Linux, Windows and Macos
• Support building as both static and shared library (added H5CPP_BUILD_SHARED for this. the default is OFF)
• Extend MPI support to OSX
• Add the Google Tests to CTest configuration (CMake 3.18+ now required)
• Add conanfile.py to retrieve dependencies from CCI based on configuration options
• Removed BoostLibraryConfig.cmake - this interferes with some settings that should be supplied by the client and doesn't appear to be required
• Removed FindGTestFix.cmake - this didn't appear to be required
• Prefix CMake options with H5CPP_* for clarity
• Various CMake modernization fixes such as use of targets

[eugenwintersberger]

This is a quite an ambitious PR :). Each of these commits is interesting by itself. Since I am still working on the Catch2 stuff trying to fix some build issues with CMake on Ubuntu 21.04 I hope you give me some time to get this stuff merged. My actual problem is that this PR might causes a lot of conflicts with my own branch I am working on. I will have a look on this during the weekend.

[eugenwintersberger]

Ok. I am so far done with my huge branch (issue_445) which leaves me with the problem which to merge first. I guess what I am going to do is to merge directly from your branch planetmarshall:cmake-improvements and then merge all this together back to master.

@jkotan can you please have a look on this PR if there is anything which is a total no-go for DESY?

[planetmarshall]

Marking as draft while I rebase on the Catch2 changes.

[eugenwintersberger]

Btw. one problem I stumbled upon when I tried to naively merge your branch: where do you get the hdf5 CMake targets from? I obviously do not get them from the FindHDF5 module shipped with my Linux distribution.

[planetmarshall]

Btw. one problem I stumbled upon when I tried to naively merge your branch: where do you get the hdf5 CMake targets from? I obviously do not get them from the FindHDF5 module shipped with my Linux distribution.

As documented here - https://cmake.org/cmake/help/latest/module/FindHDF5.html

They are mimicked by the Conan package in CCI (which I also updated :)

[eugenwintersberger]

Btw. one problem I stumbled upon when I tried to naively merge your branch: where do you get the hdf5 CMake targets from? I obviously do not get them from the FindHDF5 module shipped with my Linux distribution.

As documented here - https://cmake.org/cmake/help/latest/module/FindHDF5.html

They are mimicked by the Conan package in CCI (which I also updated :)

As much as I would love to use this. It seems this is a feature of the CMake >= 3.19 release and would therefore be a problem for us. The best I currently have at hand at my Ubuntu 21.04 installation is 3.18. Since we also have to support Debian package builds we should remain moderate with the expected cmake version :).

[planetmarshall]

OK, that's all my workflow configurations running now. @eugenwintersberger I'll address your comments as soon as I can.

[planetmarshall]

Marked as draft again. I want to add some workflow configurations to build under older CMake and without using conan.

[planetmarshall]

Added an additional 4 linux configurations to the workflow to build under Ubuntu 18.04 using the default tools:

• GCC 7.5
• CMake 3.10
• Boost 1.77.0 (built from source)
• HDF5 1.12.1 (built from source)

[eugenwintersberger]

Since we have finally merged the Catch2 branch I will have a look on this PR.

[jkotan]

@planetmarshall for the PR. @eugenwintersberger, are you looking on the PR? I understand that it would be much easier to do it with a few smaller PRs.

Bellow I add a few of my comments:

• Add Github Workflow, compiles and tests builds for 36 configurations over Linux, Windows and Macos - it would be nice to add possibility for tests to users/developers without an ESS account. However, according to https://github.com/pricing there is only 2,000 automation minutes/month Free for public repositories (~ 1h/day) so with 36 configurations it will easily exeed the limit. Also the current workspace belongs to ESS so it would be good @zjttoefs @SkyToGround or @amues could make a statement about it
• Support building as both static and shared library (added H5CPP_BUILD_SHARED for this. the default is OFF) - sure (at desy we use only dynamic libs)
• Extend MPI support to OSX - sure.
• Add the Google Tests to CTest configuration (CMake 3.18+ now required) - gtests not needed anymore
• Add conanfile.py to retrieve dependencies from CCI based on configuration options - sure if needed.
• Removed BoostLibraryConfig.cmake - this interferes with some settings that should be supplied by the client and doesn't appear to be required - I don't know if it is good to pass responsibility to our clients
• Removed FindGTestFix.cmake - this didn't appear to be required - gtests not needed anymore
• Prefix CMake options with H5CPP* for clarity_ - nice idea
• Various CMake modernization fixes such as use of targets - sure, unless you are not limited only to the newest cmake version.

[planetmarshall]

@planetmarshall for the PR. @eugenwintersberger, are you looking on the PR? I understand that it would be much easier to do it with a few smaller PRs.

Bellow I add a few of my comments:

• Add Github Workflow, compiles and tests builds for 36 configurations over Linux, Windows and Macos - it would be nice to add possibility for tests to users/developers without an ESS account. However, according to https://github.com/pricing there is only 2,000 automation minutes/month Free for public repositories (~ 1h/day) so with 36 configurations it will easily exeed the limit. Also the current workspace belongs to ESS so it would be good @zjttoefs @SkyToGround or @amues could make a statement about it

The 2000 minutes/month limit only applies to private repositories with a free account. Actions run for a public repository don't count towards this.

[planetmarshall]

• Removed BoostLibraryConfig.cmake - this interferes with some settings that should be supplied by the client and doesn't appear to be required - I don't know if it is good to pass responsibility to our clients

The problem is that by including this file h5cpp enforces some boost settings that should really be specified by the application that consumes h5cpp - eg enforcing use of dynamic libraries.

As an application developer, it should be my choice whether or not to build using shared libraries - this decision shouldn't be enforced by the libraries I link to, unless there is some specific technological reason for doing so.

[jkotan]

Test fails because I need to rebase/merge the branch

Tests are not working. The PR needs to be rebases

[jkotan]

After merging to master looks good to me.

[jkotan]

The ESS tests are available at https://jenkins.esss.dk/dm/blue/organizations/jenkins/ess-dmsc%2Fh5cpp/detail/pr_501c/4/pipeline

[jkotan]

I've just noticed that GitHub windows tests run only for windows-build (static, stdfs). It would be good to run them also for windows-build (shared, *)

Github Windows tests need to be fixed.

[jkotan]

The windows shared tests are not executed properly because when catch2 tries to executed compiled test files
to get a list of all tests with

 COMMAND ${TEST_EXECUTOR} "${TEST_EXECUTABLE}" ${spec} --list-test-names-only

it returns: Exit code 0xc0000135.
I don't know why is so. Probably some DLL library is not linked.

[planetmarshall]

it returns: Exit code 0xc0000135. I don't know why is so. Probably some DLL library is not linked.

Yes I've seen this kind of thing before. Leave it with me.

[jkotan]

Thanks @planetmarshall for fixing shared windows tests. It would be good to add -u (--update) to the conan install command to avoid the linux-build errors.

[jkotan]

@planetmarshall Thanks for the PR. It looks good to me.

[jkotan]

Tests on ESS nodes can be seen at https://jenkins.esss.dk/dm/blue/organizations/jenkins/ess-dmsc%2Fh5cpp/detail/pr_501f/1/pipeline/