search

GEOS-DEV / thirdPartyLibs

Repository to build the GEOSX third party libraries
3 stars 12 forks source link

Build hdf5 using CMake instead of Autogen. #239

Closed Algiane closed 1 year ago

Algiane commented 1 year ago

It avoids the spurious detection of the NDEBUG compilation flag by CMake when importing the hdf5 package.

As a side-effect, it will allow to test the GEOS asserts in github-ci configs running in Debug mode.

See:

Closes #211, closes https://github.com/GEOS-DEV/GEOS/issues/2631, closes https://github.com/HDFGroup/hdf5/issues/3526.

untereiner commented 1 year ago

Should also close #211

Algiane commented 1 year ago

Hi @untereiner ,

Thanks for your feedbacks: I have tried to preserve exactly the options that where used to build hdf5 using autogen. As it was always built in production mode, I have chosen to not transfer the CMAKE_BUILD_TYPE info to hdf5 and to always build it in Release cmake build type.

In practice I don't really have an opinion on what is better: do we want to build all TPLs in Debug mode sometimes or only some of them?

tbeltzun commented 1 year ago

@TotoGaz is the dockerhub login failure induced by this PR being initiated from a fork ?

In that case won't this prevent opensource contributions for external developers ?

tbeltzun commented 1 year ago

In practice I don't really have an opinion on what is better: do we want to build all TPLs in Debug mode sometimes or only some of them?

I think it would be better to propagate CMAKE_BUILD_TYPE explicitly (locally and on P3 I build the TPLs in both Debug and RelWithDebInfo, never with Release :)).

Algiane commented 1 year ago

Ok, So I will transfer the global CMAKE_BUILD_TYPE toward hdf5.

Just for your info: now that I have commit rights in the repo, I redid this PR from a branch rather than a fork (it seems that the ci doesn't work if launched by a fork due to missing identifiers for connecting to DockerHub).

New PR is available here and point to this conversation.