Updated CMakeLists to work with GEOS-ESMA/ESMA_cmake#57

[LiamBindle]

This PR accompanies GEOS-ESM/ESMA_cmake#57

[tclune]

@LiamBindle
MPI is not really part of Baselibs, though baselibs must of course do a find_package() for it.

Why won't this bit work as-is for you?

[LiamBindle]

This diff here is actually a bit misleading. There are two things going on

1. A line was added linking the base target, Baselibs, into GMAO_mpeu
2. The lines linking/including MPI were removed because they are redundant (link libraries and include directories are propoagated transitively through the base target Baselibs).

[tclune]

@LiamBindle I don't think that answers my concern. MPEU's only dependence is on MPI. It does not, nor should it depend on baselibs. Yes, baselibs does provide MPI but the general philosophy I've been following is to keep explicit (non transitive) dependencies when a package directly uses the dependency. Consider the case where component A depends directly on B and C, but B also depends directly on C. We could just say that A depends on B, but if the implementation of B changes such that it no longer depends on C, then A is now broken for no good reason.

And I'm still confused as to why you were even looking inside MPEU. The MPI dependency should not have triggered any errors in your configuration - at least I don't immediately see how.

[LiamBindle]

@tclune

I'm linking Baselibs into MPEU because MPEU is a leaf in MAPL's dependency-tree and Baselibs is an interface target that provides (a) the global GEOS compiler options set in esma.cmake, (2) the global pre-processor definitions defined in FindBaselibs.cmake, and (3) Baselibs. Essentially, I'm using MPEU (and GMAO_pFIO and MAPL_cfio) as a means of distributing global build properties to the rest of the MAPL build tree.

Sorry, it wasn't obvious what all the Baselibs interface target has actually been providing—I could fix that by separating the build properties out into their own interface target (and maybe getting rid of the Baselibs target all together).

---

The basic problem I was trying to solve was how do I make sure all GEOS-ESM libraries (now and in the future)

1. are compiled with the expected compiler options (set globally here)
2. are compiled with the appropriate pre-processor definitions (some of which are set globally here)
3. paths to external dependencies are resolved correctly (include directories, link libraries, compiler options, etc.)

and that GEOS-ESM's build properties don't conflict/have unintended consequences for GEOS-Chem's build—for example, something like -DHAS_NETCDF3 (from GEOS-ESM) turning on an unexpected block of code in GEOS-Chem (more subtle unintended consequences are possible with compiler options).

I'll skip over 3 because it's relatively easy to ensure with find_package() calls and the standard variables/targets defined by those calls. And points 1 and 2 are essentially the same problem, and I've described the two general approaches I can think of below.

The first—and current—approach is to set compiler options and definitions globally for the entire directory-tree that calls include(esma). This provides comprehensive control but means sub-projects within that tree are affected too. For example, with GCHPctm's current directory structure, all of GEOS-Chem would be compiled with definitions like -DHAS_NETCDF3 and GEOS's compiler options.

Another—my proposed—approach is to control GEOS-ESM libraries' compiler options and definitions through interface target(s) that they "link". This approach also provides comprehensive control of GEOS-ESM targets, but it doesn't affect non-GEOS-ESM targets. I opted for a single interface target to control these build properties, but multiple interface targets could be used to provide groups of build properties (e.g. for vectorized code). For GEOS-ESM libraries to get their compiler options and definitions they have to either (a) all explicitly "link" the interface target(s) with the INTERFACE keyword, or (b) the interface target(s) have to be "linked" with the PUBLIC keyword at strategic spots so that their properties are transitively propagated upwards. I opted for (b) because to minimize the diff (the strategic spots being GMAO_mpeu, GMAO_pFIO, and MAPL_cfio), and I lumped this single interface target into the Baselibs target.

Lumping the interface target with GEOS-ESM-wide build properties into Baselibs probably wasn't a good idea though, because it isn't obvious Baselibs also provides compiler options and preprocessor definitions.

So in summary, Baselibs is "linked" into GMAO_mpeu because that's how GMAO_mpeu and all its dependents get their compiler options and definitions. Baselibs also resolves the MPI dependency but unnecessarily provides all the rest of Baselibs (but they are propagated upwards resolving dependents' other dependencies). This is also why GMAO_pFIO and MAPL_cfio "link" Baselibs with the PUBLIC keyword.

Below is a graphviz image of GCHPctm's dependency-tree. The edges between libraries linked with generator expressions don't show up properly, but you can still see the general structure.

---

What are your thoughts on controlling build properties with interface target(s) rather than the global function calls? It sounds like linking the build-properties-interface-target(s) with the INTERFACE keyword rather than the PUBLIC keyword would be more inline with the current philosophy—is that right? In that case, every target would explicitly link its dependencies and the build-property-interface-target with the INTERFACE keyword.

[tclune]

OK - I need to look into this. MAPL should not be depending on mpeu. Nor should mpeu depend on MAPL. They have some overlapping roles, but mpeu is definitely the deprecated older layer. Some of the assimilation folk have some pet features in there that have not (yet) been replicated in MAPL, or we'd have scrapped it already.

I completely agree that we should not be using directory level compiler options wherever possible. Partly this is because I was unaware of target-specific approaches when I started CMake (possibly they did not exist yet then?), but also because it was not always easy to see all of the intended effects as due to the complex use in the original GNU make scripts.

But ... even then, a directory wide compiler option in our package should never pollute your components that use our components. So the whole discussion seems to be a bit beside the point.

I don't fully understand INTERFACE targets. For compiler options, I've generally found that they should almost always be PRIVATE to the target for which they apply.

[tclune]

Happy to continue the discussion, but this particular change is unnecessary, and seems implies incorrect dependencies among components. GMAO_mpeu has no dependency on baselibs, nor is there any dependency between MAPL and mpeu.

[LiamBindle]

@tclune Yep, that's totally fair. Let's continue discussing the ESMA_cmake changes, but I think it makes sense to close this one.