GEOS-DEV / thirdPartyLibs

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

Build hdf5 using CMake instead of Autogen #248

Closed TotoGaz closed 10 months ago

TotoGaz commented 11 months ago

This PR 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:

I initially proposed this PR from a fork of the TPLs but the ci fail due to missing authorization to connect to DockerHub. Previous discussion is available in the initial PR.

Reverts GEOS-DEV/thirdPartyLibs#247 which was reverting https://github.com/GEOS-DEV/thirdPartyLibs/pull/243

closes https://github.com/GEOS-DEV/GEOS/issues/2631 Relates to https://github.com/GEOS-DEV/GEOS/pull/2721/

TotoGaz commented 11 months ago

@Algiane @tbeltzun I'm sorry I had to revert your PR https://github.com/GEOS-DEV/thirdPartyLibs/pull/243: our policy is that geos always points to the HEAD of the TPL, which was not the case since https://github.com/GEOS-DEV/GEOS/pull/2721/ is not merged. PR https://github.com/GEOS-DEV/thirdPartyLibs/pull/243 should not have been merged.

As a result, anyone cloning both geos and the TPLs would not be able to build. Furthermore, it was preventing any other upgrade of the TPL.

I've reopened the current PR (#248), I've reopened the issues that were closed with the previous PR, and I updated https://github.com/GEOS-DEV/GEOS/pull/2721 with the new TPL tag so everything should be ready on your side (no additional work I hope). Thank you for your understanding!

Algiane commented 11 months ago

Hello @TotoGaz ,

Thanks for the revert and the feedback: I also realized that with this PR the compilation of Silo fails when hdf5 is built in Debug mode, at least on OSX (the hdf5 library is named libhdf5_debug and Silo doesn't find it).

I will try to fix this as soon as possible and to flag the PR as ready when it will be done (I need to free up some time to get back on Geos ;-) ).

Best

Algiane commented 11 months ago

In last commits, we:

Algiane commented 11 months ago

Hi @TotoGaz ,

Thanks, I will approve the current PR asap!

From the TPL side I think that everything is OK but there are still some issues when building Geos in Debug with Cuda on Ubuntu (we cannot remove the "ready to be merged" flag as Cuda jobs are not launched without this flag).

Best

tbeltzun commented 11 months ago

Note that we could alternatively try to build silo with --enabled-shared instead of adding a -fPIC flag: this is reserved for a following PR.

Algiane commented 11 months ago

Hi @TotoGaz ,

I think that both PR are ready to be merged now :