Add capability to handle external netcdf library

[dylanjm]

This was previously attempted at the end of last year in #2388. This PR takes those commits with an additional commit modifying netcdf.m4. I've been able to successfully build and pass all tests from make check. Tagging @dschwen to make any additional comments.

[roystgnr]

Let me redo that re-bootstrap; we try not to change the autotools version used until we're ready to change the one we keep around in contrib, to keep diff sizes as small as is practical.

[jwpeterson]

I checked the build failure in the complex numbers enabled case and it's due to an unused variable warning which presumably pops up because we hit a code path we don't normally hit because NETCDF is not found during configure.

checking ../contrib/netcdf/v4/include/netcdf.h usability... no
checking ../contrib/netcdf/v4/include/netcdf.h presence... no
checking for ../contrib/netcdf/v4/include/netcdf.h... no
NETCDF header files not found!
defining -DNOT_NETCDF4 for our Exodus build

Not sure what's special about the complex numbers build that would cause this.

[jwpeterson]

Hmm... I just checked one of the other targets, and NETCDF is not found there either, consequently neither is Exodus

netcdf........................... : no
exodus........................... : no

so something's not quite right, but I don't have time to look into it carefully at the moment.

[jwpeterson]

@dylanjm I sent you an invite to join a group which gets their PRs tested automatically.

[dylanjm]

@jwpeterson Hmm. I'll do some further investigation as to what's happening. Thanks.

[jwpeterson]

OK, sounds good. There are some instructions on the wiki about how to set up the libmesh version of autotools which should fix the issue that @roystgnr mentioned.

[dschwen]

checking netcdf.h usability... yes
checking netcdf.h presence... no
configure: WARNING: netcdf.h: accepted by the compiler, rejected by the preprocessor!
configure: WARNING: netcdf.h: proceeding with the compiler's result
checking for netcdf.h... yes
configure: error: *** Linking a test program against the NETCDF libraries failed
ERROR: exiting with code 1

This looks grim.

[dschwen]

It works locally on mac and Linux. Whiskey, Tango, Foxtrot.

[jwpeterson]

It works locally on mac and Linux.

Presumably because you have an external NetCDF locally? The CIVET build boxes don't have that right?

[dylanjm]

@jwpeterson actually just replicating the configure command given by CIVET. It should default to libmesh provided netcdf.

EDIT: ahh didn't catch -lnetcdf in the linking flags.

[dschwen]

@jwpeterson yeah, that's it. The linking test should fail unless an external netcdf (binary dist) is used. I.e. it needs to get skipped for the bundled netcdf.

[dylanjm]

@dschwen Looks like this passes all tests. Have been able to build with an external netcdf on my local machine.

[roystgnr]

The diffs look good to me. Let me give this a shot with a few different configurations and see if I can turn up any problems.

[roystgnr]

Hmm... looks like something still isn't quite right. When I don't use any special configure options, but I configure on an Xubuntu 20.04 system with libnetcdf-dev installed, my contrib/Makefile says:

NETCDF_INCLUDE = -I$(top_srcdir)/contrib/netcdf/v4/include
NETCDF_LIBRARY = -L/usr/lib

Which strikes me as a bit scary - when /usr/include/netcdf.h exists, and when we're linking to NetCDF in /usr/lib, surely we shouldn't be using our contrib NetCDF at all, to avoid any possibility of version conflicts?

[dylanjm]

@roystgnr This is what confused me about the original netcdf.m4 where we don't specify any libs to link when using contrib/netcdf/4. Would it be better to only specify NETCDF_LIBRARY if we use special configure options?

[roystgnr]

This is what confused me about the original netcdf.m4 where we don't specify any libs to link when using contrib/netcdf/4

Yeah, we had that hardcoded in Makefile.am, whenever LIBMESH_ENABLE_NETCDF and LIBMESH_ENABLE_NETCDF_V4 are defined. ... which hasn't yet been changed, and is another problem, no? That means we're probably still trying to link our own contrib netcdf, too, even when we're supposed to be using an external library? That would be a huge bug but one which wouldn't crop up until someone uses a system with an actual ABI conflict between installed and contrib versions.

Would it be better to only specify NETCDF_LIBRARY if we use special configure options?

Flame war: BEGIN!

Actually, I forget where everyone else in the libMesh group falls on this question; it's been a while since I was in a proper argument about it and that might have been regarding a different project. My personal opinion is that we should autodetect as much as we can, and that we should prefer autodetectable third party libraries over our own contrib copies of them. But there's an argument to be made that, because so much stuff is often in /usr/lib/ which can conflict with users' desired/newer private builds and modules of the same software, the safest thing to do is avoid adding /usr/ to your compiler paths until/unless specifically requested to.

But even with that in mind, as long as we can use special configure options to disable any autodetection if desired, then doing autodetection by default is fine.

[dylanjm]

Yeah, we had that hardcoded in Makefile.am, whenever LIBMESH_ENABLE_NETCDF and LIBMESH_ENABLE_NETCDF_V4 are defined. ... which hasn't yet been changed, and is another problem, no?

Exactly what I was thinking last night. It also appears to be hardcoded to try and build contrib/netcdf/v4 if the netcdfversion is just 4. There is a hacky workaround in my additions but might be better to handle it more gracefully.

[jwpeterson]

This PR has been marked "do not merge" since we are no longer accepting PRs into the master branch. All new PRs should be made on the devel branch instead. Once this PR's target branch has been updated to devel, the "do not merge" label will be removed.