Us nc-config as intended for static NetCDF #60
Conversation
Modules/FindNetCDF.cmake
Outdated
| @@ -183,7 +194,7 @@ else() | |||
| endif() | |||
|
|
|||
| if(NetCDF_PARALLEL) | |||
| find_package(MPI) | |||
| find_package(MPI REQUIRED) | |||
There was a problem hiding this comment.
We had an issue with requiring MPI here, so we made it optional.
There was a problem hiding this comment.
What was the problem? Seems weird to make it optional when you are using a parallel build of netcdf.
There was a problem hiding this comment.
Alright ... I'll change it back. Thanks for the explanation.
| function(netcdf_config2 exec flag1 flag2 output_var) | ||
| set(${output_var} False PARENT_SCOPE) | ||
| if( exec ) | ||
| execute_process( COMMAND ${exec} ${flag1} ${flag2} RESULT_VARIABLE _ret OUTPUT_VARIABLE _val) | ||
| if( _ret EQUAL 0 ) | ||
| string( STRIP ${_val} _val ) | ||
| set( ${output_var} ${_val} PARENT_SCOPE ) | ||
| endif() | ||
| endif() | ||
| endfunction() |
There was a problem hiding this comment.
Can netcdf_config be replaced by this? If not, a more descriptive name would be useful.
There was a problem hiding this comment.
No, doesn't work:
CMake Error at CMakeModules/Modules/FindNetCDF.cmake:186 (netcdf_config):
netcdf_config Function invoked with incorrect arguments for function named:
netcdf_config
Call Stack (most recent call first):
CMakeLists.txt:111 (find_package)
So I'll add a one-line documentation above each of the two functions. There is a generic option to have optional arguments, but the syntax looks cryptic and it would require if statements inside the functions.
…r netcdf_config* routines
|
This is a really good change! |
The way we have used
FindNetCDF.cmakefor static libraries is not how it is intended, I think.nc-configaccepts a number of flags, for example--libsto obtain the NetCDF library for linking. In addition to--libs, one can pass--static, which, according to thenc-configdocumentation and the code innc-configitself, adds the additional static libraries that are required for static linking (calledlibsprivateinnc-config).The current
FindNetCDF.cmakedoes not use--staticanywhere in the macro. Instead, it relies on the static flags being part of the default--libsresult. This is incorrect. Consider a case where both the dynamic and the static library are being built, and the application decides which one to use. In this case,--libsshould only return the libraries required for dynamic linking, and--libs --staticthe ones for static linking. This is the case on gaea with the new spack-stack, and I've tested it successfully there with the ufs-weather-model (which looks for the static libraries).Note 1. The
--staticline does not contain the paths to the static libraries. Is this because of how we call./configure? It's not a problem as long as we load the modules for the dependencies (zlib,hdf5,parallel-netcdf, ...) which we do anyway as part of our codes, and which get loaded automatically when using the spack-stack modules.Note 2. I've added a few formatting updates that will allow us to make the
FindNetCDF.cmakeidentical betweenCMakeModules(EMC) andjedi-cmake(JCSDA). In the long run - outside of the scope of this PR - we should consider consolidating the cmake modules into one repository that gets built as part of spack-stack. (Currently,jedi-cmakeis, but it contains more stuff and I think we should reorganize this).