Find NetCDF and HDF5 using only CMake config files

[ibaned]

Since recent versions of NetCDF and HDF5 both install good CMake config files and export proper CMake targets, it would be great (actually, it is necessary for me) if TriBITS could bring in these TPLs using only these CMake config files and targets. In particular, TriBITS should do the equivalent of the following:

find_package(hdf5 CONFIG REQUIRED)
find_package(netCDF CONFIG REQUIRED)
target_link_libraries(downstream PUBLIC netCDF::netcdf)

and only require hdf5_ROOT and netCDF_ROOT to be defined during CMake configuration. I'm currently doing a lot of work to avoid having SEACAS and Trilinos in my application build due to not being able to do this.

[bartlettroscoe]

@ibaned,

That is the idea. See Creating FindTPL<tplName>.cmake `using find_package() with IMPORTED targets.

What are the minimum versions of HDF5 and NetCDF needed to supply these package config files? I think you would have to feed those versions into the find_package(...) calls. And updated versions of HDF5 and NetCDF would have to be installed everywhere to make a change like this. (But it is possible to upgrade the TriBITS FindTPL<tplName>.cmake modules to work with both old and new versions of these TPLs to maintain backward compatibility.)

We will need to coordinate with @gsjaardema and this will need to get rolled out and we should likely complete the transition to CMake 3.22 as the minimum version to be safe.

I will mention the need to upgrade how we pull in external packages at my talk at the TUG next week. This type of activity will likely be the most time consuming in the transition to modern CMake for the entire ecosystem.

[ibaned]

It looks like at least HDF5 1.10.8 and netCDF 4.3.0 export a CMake config file at least... I'm not sure exactly what the compatibility range is.

However, I think we could do the following right now: change FindTPL<tplName>.cmake to attempt first to use CMake config files and only if that fails fall back to other methods. Does that sound reasonable? If it would be a change you would accept I could work on a PR.

[ibaned]

I tried the following changes:

https://github.com/sandialabs/seacas/pull/336/files

And just using tribits_extpkg_create_imported_all_libs_target_and_config_file and returning doesn't seem to work as I would expect:

Processing enabled external package/TPL: Netcdf (enabled by SEACASExodus, disable with -DTPL_ENABLE_Netcdf=OFF)
DAN: using find_package(netCDF CONFIG)
DAN: netCDF_FOUND=1
DAN: using tribits_extpkg_create_imported_all_libs_target_and_config_file
DAN: return
-- NOTE: The find module file for this failed TPL 'Netcdf' is:
     /Users/daibane/capp-lgr/source/seacas/cmake/tribits/common_tpls/FindTPLNetcdf.cmake
   which is pointed to in the file:
     /Users/daibane/capp-lgr/source/seacas/TPLsList.cmake

TIP: One way to get past the configure failure for the

[bartlettroscoe]

@ibaned,

I don't think you can use a return(). You will need more sophisticated logic, especially if you want to avoid breaking a bunch of existing configure scripts for Trilinos.

I think if you follow the second pattern shown here that includes calling tribits_tpl_allow_pre_find_package() towards the top, it should have the correct behavior. You might just try that in your local clone to make sure that works and then we can figure out how to incorporate that with the rest of the logic in that file. (It may be tricky but I have some ideas. For example, you could first try to call find_package(netCDF <version> CONFIG ) with sufficient version and if that does not find find netCDF, then move on with the rest of the logic. If you set <version> high enough, that should maintain backward compatibility for most existing configure scripts.)

If you are not sure what I mean, I can provide a branch that shows this.

[bartlettroscoe]

However, I think we could do the following right now: change FindTPL<tplName>.cmake to attempt first to use CMake config files and only if that fails fall back to other methods. Does that sound reasonable?

@ibaned, if you think about existing configure scripts, it is not that simple. Search for "legacy" under the section 9.4 How to add a new TriBITS external package/TPL for a discussion of the issues.

However, if you are willing to help everyone update their configure processes for Trilinos for breakages, you can do anything you want :-) But a smother transition to modern CMake seems like the right thing to do (unless it is too much of an impediment to forward progress).

[gsjaardema]

One issue we might have with netCDF is that we had been setting some values while we were finding netCDF that were used to help drive the dependencies and build options. For example, NetCDF_NEEDS_HDF5, NetCDF_NEEDS_PnetCDF, andNetCDF_PARALLEL`.

The same information should be available in the netCDF-supplied config files, but we may have to also change or adapt some downstream variable/dependency tests...

[gsjaardema]

It looks like they may be netCDF_HAS_PARALLEL, netCDF_HAS_PNETCDF and netCDF_HAS_HDF5

[ibaned]

Okay, I've gotten to a point that actually works for one of my applications:

https://github.com/sandialabs/seacas/blob/88aada319687049fb25e2b2b5cd44a9031876e1d/cmake/tribits/common_tpls/FindTPLNetcdf.cmake

I've had to write my own NetcdfConfig.cmake file because apparently the official netcdf project did not quite do their CMake correctly and does not add find_dependency(hdf5) into their netCDFConfig.cmake file, so this approach is to do it for them in NetcdfConfig.cmake.

I also had to set TPL_Netcdf_NOT_FOUND to FALSE, which the documentation seems to suggest should not be necessary but definitely seems to be.

I added an extra layer of backward compatibility in the form of a cache boolean Netcdf_ALLOW_MODERN which is off by default and disables all this code when off.

Please let me know if this is starting to look like something that could eventually go into TriBITS and what next steps you'd like me to take.

[gsjaardema]

@ibaned The netCDF developers would probably be open to fixing their CMake. I can open a PR if you show me the change; or you could...

[bartlettroscoe]

@ibaned, the file:

https://github.com/sandialabs/seacas/blob/88aada319687049fb25e2b2b5cd44a9031876e1d/cmake/tribits/common_tpls/FindTPLNetcdf.cmake

looks like it should maintain backward compatibility and if the cache var Netcdf_ALLOW_MODERN is set to TRUE should do what you want. We can test his out by first putting this on a Trilinos branch, merging that to the atdm-nightly branch, and see what happens in ATDM nightly testing. (If backward compatibility is maintained, we should see no new errors.)

What about the updated file NetcdfConfig.cmake mentioned above? Could it go under:

• https://github.com/TriBITSPub/TriBITS/tree/master/tribits/common_tpls/find_modules

along side the other CMake Find<extPkgName>.cmake modules?

[ibaned]

Oh sorry, I didn't describe that clearly. NetcdfConfig.cmake is generated by FindTPLNetcdf.cmake, in a manner similar to how tribits_extpkg_create_imported_all_libs_target_and_config_file would generate that config file.

I'm glad we have a plan to get this accepted! Should I create a branch of Trilinos and apply my patch to FindTPLNetcdf.cmake there? What should the starting point be for that branch, atdm-nightly?

[bartlettroscoe]

@ibaned

Oh sorry, I didn't describe that clearly. NetcdfConfig.cmake is generated by FindTPLNetcdf.cmake.

Got it.

I've had to write my own NetcdfConfig.cmake file because apparently the official netcdf project did not quite do their CMake correctly and does not add find_dependency(hdf5) into their netCDFConfig.cmake file, so this approach is to do it for them in NetcdfConfig.cmake.

Okay, this is where TriBITS may come to the rescue. All you need to do is to is update the file FindTPLHDF5.cmake along the lines here, add the file FindTPLNetcdfDependencies.cmake to declare the dependency from Netcdf to HDF5 (see here) and then enable the TPLs HDF5 and Netcdf and then TriBITS should take care of the rest of the details of calling things in the right order. Then you just link against Netcdf::all_libs and that should be it.

A lot of the old Find<extPkgName>.cmake modules don't correctly find their upstream dependencies as well.

This is an example of why we need a systematic way to glue together these external CMake packages to create a coherent system. (I think the TriBITS TPL system is pretty close to be that system.)

I'm glad we have a plan to get this accepted! Should I create a branch of Trilinos and apply my patch to FindTPLNetcdf.cmake there?

Yes

What should the starting point be for that branch, atdm-nightly?

No, branch off of the Trilinos develop branch and then merge it into the atdm-nightly-manual-updates branch as per the workflow described at the top of:

• https://cee-gitlab.sandia.gov/atdm/update-trilinos-atdm-nightly-branches/-/blob/master/update-atdm-develop-nightly-branches.sh

The branch atdm-nightly is just a throwaway integration test branch. You never branch off of that. (For the motivation for this workflow, see Restoring productivity through the advanced usage of Git.)

[ibaned]

Thanks @bartlettroscoe

I just realized I did this with a locally modified version of NetCDF, so let's pause this for a bit while I try to redo it with the mainline GitHub version of NetCDF.

[ibaned]

Okay, after a bit more work I did confirm that this works with the mainline NetCDF and fixed another issue that is unique to my application (#534) .

I've created the Trilinos branch tribits-netcdf-mpi that can be viewed via this draft PR: https://github.com/trilinos/Trilinos/pull/11175