Use cmake netCDF with target_* for many options

[K20shores]

Ignore the name of the branch

This branch

• normalizes more cmake commands to use lowercase
• moved the netcdf target to the top level file
• changed several add_compile_definitions to target_compile_definitions on the netcdf cmake target. Some were not set on netCDF
  • HAVE_CONFIG_H used by the include files, libdap2, libap4, libdispatch, libncxml, libsrc, ncump, ncxarr, several plugins
  • DLL_NETCDF, unclear on what the ramifications of setting this only on netcdf are
  • DLL_EXPORT, unclear on what the ramifications of setting this only on netcdf are
• removed netCDF_LIB_CORENAME from the top level cmake file; it wasn't used anywhere
• bumped cmake version to 3.13 which allows you to add things to targets in directories where the target wasn't defined
• instead of global include_directories, set target_include_directories with PRIVATE scope since it seems unlikely more than the netcdf target would need these

This partially addresses some stuff in #2845. In particular, setting the definitions on the netcdf target and not globally

@ZedThree @WardF @LecrisUT if

[DennisHeimbigner]

Kyle, could you begin making a checklist of conventions you are using to build cmake files.
Things like lower-case-commands, and such.

[K20shores]

@DennisHeimbigner yes! I'll add it to #2845

[LecrisUT]

90% of the issues are writing autotools in CMake, but they could be addressed later, just highlighted some as I read the PR. There are a few choices of public that should be discussed

[LecrisUT]

This should not be needed

[K20shores]

I think it is, otherwise msvc will place the output in separate Debug or Release directories

[K20shores]

@K20shores @WardF This looks pretty good to me. I think you could reasonably do a second pass to add some interface libraries to replace things like ALL_TLL and setting common options, but maybe in another PR?

Yes, I would say that's for another PR. I've made a note in #2845

As mentioned above, AppVeyor is failing as we now require CMake >= 3.20 to ensure we get the HDF5::HDF5 target. I think it would be possible to support earlier versions with a little bit of logic though

@WardF this seems to be the biggest sticking point. Bumping to this version of cmake has a huge benefit in terms of simplifying finding and using HDF5, but does seem to drop support for older versions on windows.

[WardF]

I'm seeing other issues, so far simply syntax related, on Windows. I'll make these changes and push them into the branch.

[WardF]

@DennisHeimbigner quick question, can you remind me why we explicitly disable filters/plugins in MINGW/MSYS environments?

[WardF]

Update: shared build on Windows works, seeing failures with static build on Windows, will address that.

[WardF]

Taking a last pass at this and seeing what the potential impact on other PR's from @ZedThree might be (amongst others); as it stands, this looks ok, despite the expected failure on appveyor. After merging this, I'll need to add the GitHub action-based tests for Windows+Visual Studio, but that will be a priority. We are focused on internship interviews tomorrow but I should be able to address the other tasks early next week.

[ZedThree]

@WardF I know there's a PR moving the Windows CI to Github Actions, but might be worth just bumping the Appveyor image for this PR?

modified   appveyor.yml
@@ -1,4 +1,4 @@
-image: Visual Studio 2017
+image: Visual Studio 2019
 
 environment:
   matrix:

[WardF]

That is easy enough to do, but my vague recollection was also there was a cmake version issue. But this will be a super easy change to make. And for what it's worth, the current failure in AppVeyor is not a roadblock to merging this, it's simply the next issue to deal with. And before I merge this PR, I want to get a better idea of the potential impact to the other PR's you've submitted; it will either go very smoothly, or be a bit of a mess; I just want to be prepared to address the latter immediately instead of getting bogged down :). Speaking of 'bogged down', the latter part of this week was a bit of an administrative time sink, but resolving this PR and the subsequent knock-on effects is top of the list for Monday.

[ZedThree]

That is easy enough to do, but my vague recollection was also there was a cmake version issue.

Yep, exactly, it's just the easiest way to bump the cmake version on Appveyor is to bump the base image version :)

The remaining test failures look like the usual flaky server issues.