-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Build hdf5 using CMake instead of Autogen #248
Build hdf5 using CMake instead of Autogen #248
Conversation
This reverts commit dbf21a5.
@Algiane @tbeltzun I'm sorry I had to revert your PR #243: our policy is that As a result, anyone cloning both I've reopened the current PR (#248), I've reopened the issues that were closed with the previous PR, and I updated GEOS-DEV/GEOS#2721 with the new TPL tag so everything should be ready on your side (no additional work I hope). Thank you for your understanding! |
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 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 |
…UG-flag-in-Debug-build
…IBRARY_SUFFIX to find the extension of hdf5 shared library.
…hat Silo's autogen script searches for libhdf5 library while CMake's build of hdf5 creates the libhdf5_debug library in Debug mode.
…id a compilation error with release 4.11 of Silo (fixed by release 4.11.1).
In last commits, we:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
@Algiane you can approve it yourself! I cannot do it since I've reopened it, it's mine and github does not want me to approve my PR!
Co-authored-by: TotoGaz <[email protected]>
Co-authored-by: TotoGaz <[email protected]>
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 |
…n error of geos with Cuda on ubuntu.
Note that we could alternatively try to build silo with |
Hi @TotoGaz , I think that both PR are ready to be merged now :
Thank you by advance for your help to merge this work. |
…UG-flag-in-Debug-build
This PR avoids the spurious detection of the
NDEBUG
compilation flag byCMake
when importing thehdf5
package.As a side-effect, it will allow to test the
GEOS
asserts in github-ci configs running inDebug
mode.See:
GEOS
issue Failing ci tests in Debug mode GEOS#2631hdf5
issue Export of theNDEBUG
flag inHDF5_DEFINITIONS
CMake variable HDFGroup/hdf5#3526 (comment)I initially proposed this PR from a fork of the
TPL
s but the ci fail due to missing authorization to connect to DockerHub.Previous discussion is available in the initial PR.
Reverts #247 which was reverting #243
closes GEOS-DEV/GEOS#2631
Relates to GEOS-DEV/GEOS#2721