search

NOAA-EMC / NCEPLIBS

Top level repo containing submodules for NCEPLIBS and associated dependencies for superproject builds
Other
42 stars 18 forks source link

Logical flaw in combination with NCEPLIBS-external #35

Closed climbfuji closed 4 years ago

climbfuji commented 4 years ago

There is a flaw in the logic for building NCEPLIBS and NCEPLIBS-external if not all packages are built by NCEPLIBS-external. For example, when -DBUILD_MPI=OFF -DBUILD_NETCDF is used for building NCEPLIBS-external, the user must set the environment variables MPI_ROOT and NETCDF prior to compling NCEPLIBS (regardless of whether the packages can be found because they are in the system library search path).

If the user does not set MPI_ROOT and NETCDF, the NCEPLIBS top-level CMakeLists.txt will set the environment variables MPI_ROOT and NETCDF to EXTERNAL_LIBS_DIR. This doesn't have any consequences, because the packages are found without the MPI_ROOT or NETCDF hint (otherwise the user would have set the environment variables beforehand), but it creates confusing warning messages.

The corresponding logic is in lines 53-86 of https://github.com/NOAA-EMC/NCEPLIBS/blob/release/public-v1/CMakeLists.txt.

climbfuji commented 4 years ago

@mark-a-potts @kgerheiser @DusanJovic-NOAA if you have any ideas that are better than my first one, namely to call the find_package command for each of those libraries beforehand and then test for XYZ_NOT_FOUND in addition to testing for the environment variable, please share your wisdom.

find_package(MPI)
...
if(DEFINED EXTERNAL_LIBS_DIR)
  if(NOT DEFINED ENV{MPI_ROOT} AND NOT MPI_C_FOUND)
...
kgerheiser commented 4 years ago

This was fixed by #38, right?

climbfuji commented 4 years ago

Yes, fixed in #38.