Unidata / netcdf-cxx4

Official GitHub repository for netCDF-C++ libraries and utilities.
Other
126 stars 49 forks source link

Use of CMAKE_SOURCE_DIR prevents building within another project #101

Closed brhillman closed 3 years ago

brhillman commented 3 years ago

We would like to use netcdf-cxx4 within a model that needs to run on a variety of large machines. In order to avoid maintaining modules that include netcdf-cxx4 on all of these machines, and because the library builds quickly, we would like to build netcdf-cxx4 as part of the model build. However, it appears that the use of CMAKE_SOURCE_DIR in the top-level CMakeLists.txt for netcdf-cxx4 to define the configure and tests paths prevents building the library via add_subdirectory in a higher-level CMakeLists.txt. Changing occurrences of CMAKE_SOURCE_DIR to PROJECT_SOURCE_DIR, however, fixes the problem, and seems to capture the intent of these lines in the CMakeLists.txt for netcdf-cxx4. That is, the following changes are sufficient:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 652826d..db43e8a 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -214,7 +214,7 @@ ENDMACRO()
 ###
 MACRO(add_sh_test prefix F)
   IF(HAVE_BASH)
-    ADD_TEST(${prefix}_${F} bash "-c" "export srcdir=${CMAKE_CURRENT_SOURCE_DIR};export TOPSRCDIR=${CMAKE_SOURCE_DIR};${CMAKE_CURRENT_BINARY_DIR}/${F}.sh")
+      ADD_TEST(${prefix}_${F} bash "-c" "export srcdir=${CMAKE_CURRENT_SOURCE_DIR};export TOPSRCDIR=${PROJECT_SOURCE_DIR};${CMAKE_CURRENT_BINARY_DIR}/${F}.sh")
   ENDIF()
 ENDMACRO()

@@ -619,10 +619,10 @@ INSTALL(PROGRAMS ${NCXX_BINARY_DIR}/ncxx4-config
 #####
 # Build test_common.sh
 #####
-SET(EXTRA_DIST ${EXTRA_DIST} ${CMAKE_SOURCE_DIR}/test_common.in)
-SET(TOPSRCDIR "${CMAKE_SOURCE_DIR}")
+SET(EXTRA_DIST ${EXTRA_DIST} ${PROJECT_SOURCE_DIR}/test_common.in)
+SET(TOPSRCDIR "${PROJECT_SOURCE_DIR}")
 SET(TOPBUILDDIR "${CMAKE_BINARY_DIR}")
-configure_file(${CMAKE_SOURCE_DIR}/test_common.in ${CMAKE_BINARY_DIR}/test_common.sh @ONLY NEWLINE_STYLE LF)
+configure_file(${PROJECT_SOURCE_DIR}/test_common.in ${CMAKE_BINARY_DIR}/test_common.sh @ONLY NEWLINE_STYLE LF)

 #####
 # Options
@@ -666,8 +666,8 @@ IF(NC_HAS_DEF_VAR_FILTER)
   # Build cxx4/findplugin.sh
   #####
   SET(ISCMAKE "1")
-  CONFIGURE_FILE(${CMAKE_SOURCE_DIR}/cxx4/findplugin.in ${CMAKE_BINARY_DIR}/cxx4/findplugin.sh @ONLY NEWLINE_STYLE LF)
-  CONFIGURE_FILE(${CMAKE_SOURCE_DIR}/examples/findplugin.in ${CMAKE_BINARY_DIR}/examples/findplugin.sh @ONLY NEWLINE_STYLE LF)
+  CONFIGURE_FILE(${PROJECT_SOURCE_DIR}/cxx4/findplugin.in ${CMAKE_BINARY_DIR}/cxx4/findplugin.sh @ONLY NEWLINE_STYLE LF)
+  CONFIGURE_FILE(${PROJECT_SOURCE_DIR}/examples/findplugin.in ${CMAKE_BINARY_DIR}/examples/findplugin.sh @ONLY NEWLINE_STYLE LF)

   ADD_SUBDIRECTORY(plugins)
 ENDIF(NC_HAS_DEF_VAR_FILTER)

This is with current master (c244f2de7818d9562eeadee548811872d4ac2975) and should apply to all environments.

ZedThree commented 3 years ago

Hi @brhillman, I have a PR open that should fix this -- please can you check if #100 works for you? It also exports the targets and makes sure a namespaced-alias is also created, which should allow your users to either build netcdf-cxx4 as part of your model build, or use an existing build. We use it like:

option(BOUT_DOWNLOAD_NETCDF_CXX4 "Download and build netCDF-cxx4" OFF)
if (BOUT_DOWNLOAD_NETCDF_CXX4)
  include(FetchContent)
  FetchContent_Declare(
    netcdf-cxx4
    GIT_REPOSITORY https://github.com/ZedThree/netcdf-cxx4
    GIT_TAG        "ad3e50953190615cb69dcc8a4652f9a88a8499cf"
    )
  # Don't build the netcdf tests, they have lots of warnings
  set(NCXX_ENABLE_TESTS OFF CACHE BOOL "" FORCE)
  # Use our own FindnetCDF module which uses nc-config
  find_package(netCDF REQUIRED)
  FetchContent_MakeAvailable(netcdf-cxx4)
else()
  find_package(netCDFCxx REQUIRED)
endif()

target_link_libraries(bout++ PUBLIC netCDF::netcdf-cxx4)
brhillman commented 3 years ago

@ZedThree yup that fixes it. Thank you! And thank you for the example usage! I'll close this issue.