GEOS-ESM / MAPL

MAPL is a foundation layer of the GEOS architecture, whose original purpose is to supplement the Earth System Modeling Framework (ESMF)
https://geos-esm.github.io/MAPL/
Apache License 2.0
27 stars 18 forks source link

MAPL CMake build broken with GNU compilers with no build type specified #545

Closed mathomp4 closed 3 years ago

mathomp4 commented 4 years ago

This was a fun one found by @WilliamJamieson. To wit, if you grab MAPL and do:

cmake .. -DBASEDIR=$BASEDIR/Darwin

you can't build with GNU. Why? Well, we have bits of MAPL that depend on -ffree-line-length-none.

The reason? Well the "default" build type for CMake is RelWithDebInfo...and we don't supply flags for RelWithDebInfo in our ESMA_cmake.

I never saw this because I use aliases that always do -DCMAKE_BUILD_TYPE=Release or -DCMAKE_BUILD_TYPE=Debug.

We don't see this in GEOSgcm (etc.) because we have:

# Set the default build type to release
if (NOT CMAKE_BUILD_TYPE)
  message (STATUS "Setting build type to 'Release' as none was specified.")
  set (CMAKE_BUILD_TYPE Release CACHE STRING "Choose the type of build." FORCE)
  # Set the possible values of build type for cmake-gui
  set_property (CACHE CMAKE_BUILD_TYPE PROPERTY STRINGS
    "Debug" "Release" "MinSizeRel" "RelWithDebInfo")
endif ()

in our top-level CMakeLists.txt file.

I suppose this question is now for @tclune: What should we do? Do we want to have a chunk like this in our MAPL CMakeLists.txt? Or should I extend all our ESMA_cmake bits to include RelWithDebInfo? Both?

mathomp4 commented 4 years ago

Note, our Release is essentially RelWithDebInfo as we add -g to all our Release builds. (I think in stock CMake fashion Release is -O3 -DNDEBUG and RelWithDebInfo is -O2 -g??)

tclune commented 4 years ago

For each compiler there should be a set of mandatory flags that must be present to build regardless of build type. Probably worth looking around for cmake conventions, but seems like something like this should work:

set (CMAKE_Fortran_FLAGS_${CMAKE_BUILD_TYPE} "${mandatory_flags} ${CMAKE_Fortran_FLAGS_${CMAKE_BUILD_TYPE}")

This would anticipate any additional build types arising in the future.

My deeper philosophic angst is whether any flags should be driven from the fixture or if each component should set its own flags. (Ran into an aspect of this with GCHP recently.)

tclune commented 4 years ago

This is an interesting page: https://subscription.packtpub.com/book/application_development/9781788470711/1/ch01lvl1sec24/controlling-compiler-flags

Esp the towards the end regarding generator expressions:

target_compile_option(compute-areas
  PRIVATE
    ${CXX_FLAGS}
    "$<$<CONFIG:Debug>:${CXX_FLAGS_DEBUG}>"
    "$<$<CONFIG:Release>:${CXX_FLAGS_RELEASE}>"
  )

And this is probably closer to the way we ought to be setting flags throughout GEOS.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. If there are no updates within 7 days, it will be closed. You can add the "long term" tag to prevent the Stale bot from closing this issue.