CMake: Some minor cleanups #5948
Conversation
Remove OS concept from cmake macros. We can maybe remove it entirely from config_machines but that will be trickier. [BFB]
| cmake_minimum_required(VERSION 3.9) | ||
| cmake_policy(SET CMP0057 NEW) | ||
| set(CMAKE_CXX_STANDARD 17) | ||
|
|
||
| # Store caseroot in the cache, so that, if cmake re-runs, |
There was a problem hiding this comment.
CASEROOT is already always passed to cmake via -DCASEROOT=xxx so is already guaranteed to be in the cache. @bartgol , you had a commit that added this so maybe you can remember something I don't.
There was a problem hiding this comment.
I don't really recall. Maybe I wanted to have a doc string in CMakeCache.txt (which you don't get with -Dblah only)? Long stretch though...
There was a problem hiding this comment.
But while we're on it, I think it's good practice to add
set (XYZ val CACHE TYPE doc)
for the CACHE vars that e3sm expects to receive. It is a way for our cmake files to self-document themselves. Otherwise one has to know how CIME will invoke cmake to figure out what comes from where...
There was a problem hiding this comment.
@bartgol , thanks. I will add that to the list of objectives for the full refactor.
| @@ -127,16 +119,14 @@ list(APPEND CMAKE_MODULE_PATH ${CIMEROOT}/CIME/non_py/src/CMake) | |||
|
|
|||
| set(CMAKE_VERBOSE_MAKEFILE TRUE) | |||
|
|
|||
| if(USE_CUDA) | |||
There was a problem hiding this comment.
This was already being done like 20 lines above.
| # Scream manages its own flags | ||
| build_eamxx() | ||
|
|
||
| # We do want CMAKE_BUILD_TYPE to be set, but we do NOT want CMake to | ||
| # decide what optimization flags to append, based on build type, | ||
| # for components who rely on CIME for build flags, so make all the following empty. | ||
| # JGF: I think we can remove this once proper CMake names are being used in the |
There was a problem hiding this comment.
Agreed. I think the mach files should set these variables to some reasonable values.
|
@mahf708 , the sharedlibs builds still potentially use |
CMake: Some minor cleanups Remove OS concept from cmake macros. We can maybe remove it entirely from config_machines but that will be trickier. Remove some unneeded or repeated things from CMakeLists.txt. Remove FMS dependency support. [BFB] * jgfouca/cmake_minor_cleanups: Remove FMS support CMake: Some minor cleanups
|
@jgfouca I had an issue with you removing the linux-generic cmake files, but not removing the linux-generic "machine" entry in config_machines.xml. I don't think it is a healthy practice to keep inconsistent or half-working stuff around, that's why I suggested removing the linux-generic MACH entries as well, unless there is a need. <machine MACH="linux-generic">
<DESC>Linux workstation or laptop</DESC>
<NODENAME_REGEX>none</NODENAME_REGEX>
<OS>LINUX</OS>
<COMPILERS>gnu</COMPILERS>
<MPILIBS>openmpi,mpich</MPILIBS>
<CIME_OUTPUT_ROOT>$ENV{HOME}/projects/acme/scratch</CIME_OUTPUT_ROOT>
<DIN_LOC_ROOT>$ENV{HOME}/projects/acme/cesm-inputdata</DIN_LOC_ROOT>
I don't really know what the OS stuff is all about, because I was looking at MACH entries strictly speaking. Maybe they're being used interchangeably in confusing ways? |
Remove OS concept from cmake macros. We can maybe remove it entirely from config_machines but that will be trickier.
Remove some unneeded or repeated things from CMakeLists.txt.
Remove FMS dependency support.
[BFB]