Update FindMOAB to use MOAB config file #1526
Conversation
|
@gonuke, could you please take a look at this? This PR should be merged unless we encounter issues installing PyNE with the latest MOAB. |
There was a problem hiding this comment.
I have a few changes here about what's required. It's technically possible to build PyNE without MOAB and DAGMC and many folks may want to do that if they are only interested in the more fundamental capabilities. This is also reflected in our CI testing process.
| @@ -103,17 +103,7 @@ if(WITH_MOAB) | |||
| set(MOAB_ROOT "${DEPS_ROOT_DIR}") | |||
| endif() | |||
| MESSAGE("-- MOAB Root: ${MOAB_ROOT}") | |||
| find_package(MOAB) | |||
| message("-- MOAB Found: ${MOAB_FOUND}") | |||
There was a problem hiding this comment.
I think some kind of message about what is found in MOAB would be useful, unless the MOAB cmake file does that? Like the version string, perhaps?
There was a problem hiding this comment.
Ah, I see. For the current latest release, the MOAB config file does not come with version strings, since my PR was recently merged to the develop branch. It will then be available in the master branch. So, we need to wait for a new release of MOAB, like 5.6. For now, I think I can add some message strings to the FindMOAB file.
cmake/FindMOAB.cmake
Outdated
There was a problem hiding this comment.
Can we just delete this now and rely on MOAB to install the right info? It may require changing some variables later in the file to match the ones MOAB sets
There was a problem hiding this comment.
I think it is good practice to have a custom find method. This will let the user set a custom path for MOAB, for example, in the /opt or /home directory or anywhere else. Additionally, in the updated FindMOAB, I am using the MOAB config file to set all the necessary variables.
There was a problem hiding this comment.
Also added some custom messages for more information. Let me know if you are still interested to remove this file.
There was a problem hiding this comment.
I disagree with the idea of a custom FindMOAB as it frequently can become out of date with the software it is trying to find. Relying on the files installed by the package is the most robust and CMake's search rules (https://cmake.org/cmake/help/latest/command/find_package.html#config-mode-search-procedure) still allow a user to specify a custom location.
There was a problem hiding this comment.
I think we are also ready to remove FindDAGMC as well?
There was a problem hiding this comment.
I'm not sure about that one because I'm not sure if we are installing the appropriate file with DAGMC yet... that's a new issue for us to tackle in DAGMC
There was a problem hiding this comment.
I have dropped FindDAGMC file and it seems tests are passing.
There was a problem hiding this comment.
I think I'd like to test this with DAGMC installed in an unusual location and confirm that the build process still works/finds it. Perhaps it does because if we set DAGMC_ROOT in the build process, the build in CMake FindPackage macro will recognize that variable and use it as a hint?
There was a problem hiding this comment.
-- HDF5 Root: /root/opt/hdf5/hdf5-1_14_3
-- Found HDF5: /root/opt/hdf5/hdf5-1_14_3/lib/libhdf5.so;/root/opt/hdf5/hdf5-1_14_3/lib/libhdf5_hl.so;/root/opt/hdf5/hdf5-1_14_3/lib/libhdf5.so;/usr/lib/x86_64-linux-gnu/libz.so;/usr/lib/x86_64-linux-gnu/libdl.a;/usr/lib/x86_64-linux-gnu/libm.so (found version "1.14.3")
-- HDF5 Include directory: /root/opt/hdf5/hdf5-1_14_3/include
-- HDF5 Library directories: /root/opt/hdf5/hdf5-1_14_3/lib
-- HDF5 Libraries: /root/opt/hdf5/hdf5-1_14_3/lib/libhdf5.so;/usr/lib/x86_64-linux-gnu/libz.so;/usr/lib/x86_64-linux-gnu/libdl.a;/usr/lib/x86_64-linux-gnu/libm.so
-- HDF5 library version: 1.14.3
-- MOAB library Version:
-- MOAB Include Directory: /root/opt/moab/include;/usr/include/eigen3
-- MOAB Library Directories: /root/opt/moab/lib
-- MOAB Libraries: -L/root/opt/moab/lib -lMOAB /root/opt/hdf5/hdf5-1_14_3/lib/libhdf5_hl.so;/root/opt/hdf5/hdf5-1_14_3/lib/libhdf5.so;/usr/lib/x86_64-linux-gnu/libdl.a;/usr/lib/x86_64-linux-gnu/libz.so;/usr/lib/x86_64-linux-gnu/libm.so
-- Configuring DAGMC 3.2.3
-- DAGMC library version: 3.2.3
-- DAGMC Include directory: /root/opt/dagmc/include;/root/opt/moab/include;/usr/include/eigen3
-- DAGMC Library directories: /root/opt/dagmc/lib;/root/opt/moab/lib
-- DAGMC Libraries: MOAB;dagmcWe have already installed different libraries in a custom location: /root/*.
There was a problem hiding this comment.
This should fix a few problems and clean up our build maintenance burden - thanks @ahnaf-tahmid-chowdhury
Description
This PR updates the
FindMOAB.cmakefile to utilize the MOAB config file for obtaining theMOAB_INCLUDE_DIRSvariable. Previously, this variable was configured manually, causing issues with newer versions of MOAB.Changes Made
FindMOAB.cmaketo use MOAB config file forMOAB_INCLUDE_DIRS.MOAB_INCLUDE_DIRSto prevent conflicts with newer MOAB versions.