CMake build fails against netcdf-4.6.3

[erikjwaxx]

The CMakeLists.txt is setup to check the NetCDF library being built against for nc_def_var_filter and nc_inq_var_filter, and, when finding both of those, sets the compile-time option -DNC_LIB_VERSION=460:

if (has_nc_def_var_filter AND has_inq_var_filter)                                                                                                                                           
  add_definitions(-DNC_LIB_VERSION=460)                                                                                                                                                     
endif ()

This, in turn, causes the #if NC_LIB_VERSION >= 462 check at nco_netcdf.h:355 to fail and consequently, the #include <netcdf_mem.h> does not occur, which subsequently causes the stub struct NC_memio to be defined.

However, CMake also defines a HAVE_NETCDF_MEM_H, which causes the header to be pulled in at nco_netcdf.h:540, at which point compile fails due to a redefinition error of struct NC_memio.

I fixed this locally in the quickest way possible by removing the -DNC_LIB_VERSION flag generated by CMake. I'd be interested in submitting a pull request to fix this properly but I'd need some guidance.

[czender]

Thank you so much for figuring this out! This also helps explain why Appveyor fails to build NCO 4.8.0-alphaXX against netCDF 4.6.1, a problem that has been driving me crazy. That clause of CMakeLists.txt only works for netCDF 4.6.0. It needs to be changed to set NC_LIB_VERSION directly to the same value that nco.h sets it to (on line 339) which is based on netcdf_meta.h (when it exists) as

# define NC_LIB_VERSION ( NC_VERSION_MAJOR * 100 + NC_VERSION_MINOR * 10 + NC_VERSION_PATCH )
and set to 360 when none of those tokens exist. @pedro-vicente can you please help @erikjwaxx craft a PR for CMakeLists.txt that dynamically defines NC_LIB_VERSION as I just described?

[pedro-vicente]

ok

[pedro-vicente]

is this what you mean?
if file netcdf_meta.h exists , NCO defines NC_HAVE_META_H

find_path(path_netcdf_meta_h netcdf_meta.h PATHS ${NETCDF_INCLUDE})
if(path_netcdf_meta_h)
  message("-- Found netcdf_meta.h in " ${path_netcdf_meta_h})
  add_definitions(-DNC_HAVE_META_H)
else()
  message("-- netcdf_meta.h was not found; defining NC_LIB_VERSION=360")
  add_definitions(-DNC_LIB_VERSION=360)
endif()

[pedro-vicente]

my latest commit seems to have fixed this, AppVeyor was a success build

[czender]

Re-opening issue to improve the Appveyor build. As the Windows output shows, NC_HAVE_META_H is now redundantly defined in every file, e.g.,:

nco_att_utl.c
C:\Miniconda36-x64\envs\TEST\Library\include\netcdf.h(1993): warning C4005: 'NC_HAVE_META_H': macro redefinition
C:\Miniconda36-x64\envs\TEST\Library\include\netcdf.h(1993): note: command-line arguments:  see previous definition of 'NC_HAVE_META_H'

NCO does not need to define 'NC_HAVE_META_H' because the header file netcdf.h does that. Now that CMakeLists.txt correctly includes netcdf_meta.h, we can remove the kludgy NCO workarounds that tried to address the confusion caused by the old CMakeLists.txt. I am trying to detangle these issues, and we can close this once Appveyor and Travis both show good and clean builds.

[pedro-vicente]

NCO does not need to define 'NC_HAVE_META_H' because the header file netcdf.h

not in the netcdf version I have locally, 4.6.0; so this

add_definitions(-DNC_HAVE_META_H)

must be in CMakeLists.txt for this case; so I added it for now

[czender]

So this clumsy introduction of tokens by Unidata in the 4.6.x series creates a messy Appveyor log for all non-4.6.0 builds, which we wish to avoid. Please change CMakeLists.txt to examine netcdf_meta.h (when it is found), and subsequently define NC_HAVE_META_H only when NC_LIB_VERSION is 460 or, equivalently, only when netcdf_meta.h is found and netcdf.h does not define NC_HAVE_META_H.