Use the checkout_externals utility to checkout physics submodules with cmake

[jim-p-w]

core_atmosphere requires specific versions of the physics code from https://github.com/NCAR/MMM-physics.git and https://github.com/NOAA-GSL/UGWP.git

The checkout_externals utility uses the versions of those repositories specified in src/core_atmosphere/Externals.cfg.
The gnu make build system uses checkout_externals; this change modifies the cmake build files to use the same mechanism as the make build system.

This PR fixes issue 1420

This was tested by building MPAS-Model code with cmake. It was also tested by building mpas-bundle.

This change was copied from wrf-model where it has been in use for about a year.

[mgduda]

@jim-p-w Could you base your branch on the v8.3.0 tag?

[jim-p-w]

@jim-p-w Could you base your branch on the v8.3.0 tag?

Will do.

[guoqing-noaa]

FYI, we are addressing the same issue in different routes:
ufs-community#233 tries to use git submodules universally.
ufs-community#231 tries to apply a bandage to fix the cmake compiling failure.

I would advocate we adopt the git submodules universally.

[guoqing-noaa]

It will be great to add a logic if physics_mmm and UGWP exist, skip running checkout_externals. Thanks!

[mgduda]

Although physics_mmm and UGWP may already exist, they may be out of sync with the tag specified in the Externals.cfg file.

[guoqing-noaa]

I think an "existence" check is consistent with all other "git lone" in this CMakeLists.txt.
Folks may want to make modifications inside physics_mmm or UGWP and don't want them to get overwritten by checkout_externals. This check does not break anything if one do a clean clone of MPAS-Model

[islas]

checkout_externals will not overwrite dirty checkouts.

I think keeping the logic as one-to-one with the make build where possible helps reduce issues such as this one where extra or divergent code may lead to unexpected difference in behavior.

That said, it would be nice to wholly use submodules (in my opinion).

[guoqing-noaa]

@islas Thanks for the discussion!

Will we get any issue if we add an existence check?
If physics_mmm or UGWP is out of sync, it is mostly due to some corner cases and in those situations, developers/users are expected to handle it manually.

It is nice that if we may use git submodules for all subcomponents.
GSL is transitioning to do that.
Adding an existence check here will help a little bit since we use git clone --recursive to get all codes and don't want check_externals to do duplicate or unexpected work in that situation. Thanks!

[jim-p-w]

Because an existence check of directories is not sufficient to know the correct code has been checked out my inclination is to always run checkout_externals. For example, if the main branch is checked out, as currently implemented the correct tag will get checked out.

Also, in the event a different tag is needed, editing src/core_atmosphere/Externals.cfg and rerunning cmake will update things properly without requiring manual deletion of directories.

[guoqing-noaa]

@jim-p-w Thanks for the discussion! FYI, I would like to provide a few alternate scenarios:

1. One makes modifications under physics_mmm or UGWP, commits them to personal fork/branch. So these directories are clean (not dirty). If one wants to rebuild the MPAS-Model, checkout_externals will "overwrites" (to be accurate, switches the top reference) all changes. This is NOT what we expect.
2. If one add anything under physics_mmm or UGWP, even touching a file, these directories will be dirty and checkout_externals will no longer take effect (per previous @islas's comment), right?
3. I think we can only be able to make sure checking out specified frozen checkouts from a clean clone. For other situations, if one want to update hash/tag or get back the default frozen checkout, one can run checkout_externals or git checkout manually at the command line instead of our forcing it in the CMake runtime.
4. This PR changes the current behavior:

if(NOT EXISTS ${ATMOSPHERE_CORE_PHYSICS_MMM_DIR})
......
if(NOT EXISTS ${ATMOSPHERE_CORE_PHYSICS_NOAA_DIR})
......

and it may break downstream applications.

[mgduda]

@guoqing-noaa I think we can all agree that the existing CMakeLists.txt file is not correct. If I've understood the changes proposed in ufs-community#231 , it seems that there are a couple of potential drawbacks:

1. If we create a fresh clone of MPAS-Model, compile, change one of the tags in the Externals.cfg file, and re-compile, the build will not use the updated physics repository tags (because, e.g., the physics_mmm directory already exists). This is also a problem if we locally merge a branch that switches external physics repository tags and makes corresponding MPAS-Model code changes to work with the new external tag: compiling will make use of the modified MPAS-Model code without the corresponding updates to the external physics.
2. The behavior of the CMake build is inconsistent with that of the GNU Make build.

While it requires a change in expectations (i.e., the external physics used by the build will always be those specified in the Externals.cfg file), it looks to me like the changes proposed in this PR should address both of the above drawbacks.

I'm not sure if we'll reach an agreement in this PR discussion thread. If you intend to update the ufs-community fork in the near future with changes to the MPAS-Dev/MPAS-Model develop branch, perhaps that can be followed by a PR to the ufs-community fork to enforce whatever behavior you'd prefer, and subsequent merges of MPAS-Dev/MPAS-Model develop would hopefully leave those changes in place.

[guoqing-noaa]

@mgduda Thanks for the discussions! We have different use situations and preferences. That's fine and expected. We can make a little bit tweaks in the ufs-community fork after this PR is merged. Thanks!

[jim-p-w]

I'm making this a draft until the target branch gets updated to be v8.3.0

[mgduda]

I'm making this a draft until the target branch gets updated to be v8.3.0

What I'm suggesting is that your PR branch should begin at the v8.3.0 tag but still target the develop branch, adding a single commit to address the issue with fetching the HEAD of the MMM-physics main branch.

[mgduda]

@jim-p-w I think it would be safe to squash the changes from 8ff5e97 onto the first commit in this PR branch.