netcdf-cxx4: convert to CMake-based build (#42766)

The CMake-based build is anticipated to work in all cases where the
Autotools-based build did, and to address all prior issues with less
maintenance of the package. In detail:

* Fixes #42735 (CMake's find_package helps with linking to proper
  netcdf-c)
* Replaces older Autotools-based build
* All preexisting variants are handled
* Record hdf5 as an explicit dependency (was missing before)
* Add +tests option

Co-authored-by: Chrismarsh <Chrismarsh@users.noreply.github.com>

var/spack/repos/builtin/packages/netcdf-cxx4/package.py

@@ -3,12 +3,10 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
-import os
-
 from spack.package import *
 
 
-class NetcdfCxx4(AutotoolsPackage):
+class NetcdfCxx4(CMakePackage):
     """NetCDF (network Common Data Form) is a set of software libraries and
     machine-independent data formats that support the creation, access, and
     sharing of array-oriented scientific data. This is the C++ distribution."""
@@ -26,9 +24,17 @@ class NetcdfCxx4(AutotoolsPackage):
     variant("shared", default=True, description="Enable shared library")
     variant("pic", default=True, description="Produce position-independent code (for shared libs)")
     variant("doc", default=False, description="Enable doxygen docs")
+    variant("tests", default=False, description="Enable CTest-based tests, dashboards.")
 
-    depends_on("netcdf-c")
+    # If another cmake-built netcdf-c exists outside of spack  e.g., homebrew's libnetcdf,
+    # then cmake will choose that external netcdf-c.
+    # This approach ensures the config.cmake exists, and thus ensures the spack version is
+    #  found before the system's
+    depends_on("netcdf-c build_system=cmake")
+    depends_on("hdf5")
 
+    # if we link against an mpi-aware hdf5 then this needs to also be mpi aware for tests
+    depends_on("mpi", when="+tests ^hdf5+mpi")
     depends_on("doxygen", when="+doc", type="build")
 
     filter_compiler_wrappers("ncxx4-config", relative_root="bin")
@@ -38,32 +44,15 @@ def flag_handler(self, name, flags):
             flags.append(self.compiler.cc_pic_flag)
         if name == "cxxflags" and "+pic" in self.spec:
             flags.append(self.compiler.cxx_pic_flag)
-        elif name == "ldlibs":
-            # Address the underlinking problem reported in
-            # https://github.com/Unidata/netcdf-cxx4/issues/86, which also
-            # results into a linking error on macOS:
-            flags.append(self.spec["netcdf-c"].libs.link_flags)
 
-        # Note that cflags and cxxflags should be added by the compiler wrapper
-        # and not on the command line to avoid overriding the default
-        # compilation flags set by the configure script:
         return flags, None, None
 
     @property
     def libs(self):
         libraries = ["libnetcdf_c++4"]
-
+        shared = "+shared" in spec
-        query_parameters = self.spec.last_query.extra_parameters
-
-        if "shared" in query_parameters:
-            shared = True
-        elif "static" in query_parameters:
-            shared = False
-        else:
-            shared = "+shared" in self.spec
 
         libs = find_libraries(libraries, root=self.prefix, shared=shared, recursive=True)
-
         if libs:
             return libs
 
@@ -72,74 +61,31 @@ def libs(self):
             msg.format("shared" if shared else "static", self.spec.name, self.spec.prefix)
         )
 
-    @when("@4.3.1:+shared")
-    @on_package_attributes(run_tests=True)
     def patch(self):
-        # We enable the filter tests only when the tests are requested by the
+        # An incorrect value is queried post find_package(HDF5)
-        # user. This, however, has a side effect: an extra file 'libh5bzip2.so'
+        # This looks to be resolved in master, but not any of the tag releases
-        # gets installed (note that the file has .so extension even on darwin).
+        # https://github.com/Unidata/netcdf-cxx4/issues/88
-        # It's unclear whether that is intended but given the description of the
-        # configure option --disable-filter-testing (Do not run filter test and
-        # example; requires shared libraries and netCDF-4), we currently assume
-        # that the file is not really meant for the installation. To make all
-        # installations consistent and independent of whether the shared
-        # libraries or the tests are requested, we prevent installation of
-        # 'libh5bzip2.so':
         filter_file(
-            r"(^\s*)lib(_LTLIBRARIES\s*)(=\s*libh5bzip2\.la\s*$)",
+            r"HDF5_C_LIBRARY_hdf5",
-            r"\1noinst\2+\3",
+            "HDF5_C_LIBRARIES",
-            join_path(self.stage.source_path, "plugins", "Makefile.in"),
+            join_path(self.stage.source_path, "CMakeLists.txt"),
         )
 
-    def configure_args(self):
+        filter_file(
-        config_args = self.enable_or_disable("shared")
+            r"HDF5_C_LIBRARY_hdf5",
+            "HDF5_C_LIBRARIES",
+            join_path(self.stage.source_path, "cxx4", "CMakeLists.txt"),
+        )
 
-        if "+doc" in self.spec:
+    def cmake_args(self):
-            config_args.append("--enable-doxygen")
+        args = [
-        else:
+            self.define_from_variant("BUILD_SHARED_LIBS", "shared"),
-            config_args.append("--disable-doxygen")
+            self.define_from_variant("ENABLE_DOXYGEN", "doc"),
+            self.define_from_variant("NCXX_ENABLE_TESTS", "tests"),
+        ]
 
-        if self.spec.satisfies("@4.3.1:"):
+        return args
-            if self.run_tests and "+shared" in self.spec:
-                config_args.append("--enable-filter-testing")
-                if self.spec.satisfies("^hdf5+mpi"):
-                    # The package itself does not need the MPI libraries but
-                    # includes <hdf5.h> in the filter test C code, which
-                    # requires <mpi.h> when HDF5 is built with the MPI support.
-                    # Using the MPI wrapper introduces overlinking to MPI
-                    # libraries and we would prefer not to use it but it is the
-                    # only reliable way to provide the compiler with the correct
-                    # path to <mpi.h>. For example, <mpi.h> of a MacPorts-built
-                    # MPICH might reside in /opt/local/include/mpich-gcc10,
-                    # which Spack does not know about and cannot inject with its
-                    # compiler wrapper.
-                    config_args.append("CC={0}".format(self.spec["mpi"].mpicc))
-            else:
-                config_args.append("--disable-filter-testing")
-
-        return config_args
-
-    @run_after("configure")
-    def rename_version(self):
-        # See https://github.com/Unidata/netcdf-cxx4/issues/109
-        # The issue is fixed upstream:
-        #   https://github.com/Unidata/netcdf-cxx4/commit/e7cc5bab02cf089dc79616456a0a951fee979fe9
-        # We do not apply the upstream patch because we want to avoid running
-        # autoreconf and introduce additional dependencies. We do not generate a
-        # patch for the configure script because the patched string contains the
-        # version and we would need a patch file for each supported version of
-        # the library. We do not implement the patching with filter_file in the
-        # patch method because writing a robust regexp seems to be more
-        # difficult that simply renaming the file if exists. It also looks like
-        # we can simply remove the file since it is not used anywhere.
-        if not self.spec.satisfies("@:4.3.1 platform=darwin"):
-            return
-
-        with working_dir(self.build_directory):
-            fname = "VERSION"
-            if os.path.exists(fname):
-                os.rename(fname, "{0}.txt".format(fname))
 
     def check(self):
         with working_dir(self.build_directory):
-            make("check", parallel=False)
+            make("test", parallel=False)