From bb70e6ffcd6e6920b24677593a35987d1eb05ce1 Mon Sep 17 00:00:00 2001 From: Dom Heinzeller Date: Thu, 14 Apr 2022 11:10:30 -0600 Subject: netcdf-cxx4 Package: updates to build on Mac OS (#29246) Bug fixes for package netcdf-cxx4 so that it builds on macOS semi case-sensitive filesystems; this includes additional changes to build netcdf-cxx4 consistently with netcdf-fortran. * netcdf-fortran: remove unused config_flags * netcdf-fortran: avoid building without the optimization flags * netcdf-cxx4: do not enforce autoreconf. This was a rudiment from the times when the package was fetched with git, which broke timestamp order of the automatically generated Autoconf files. * netcdf-cxx4: inject PIC flags for C++ when '+pic' * netcdf-cxx4: inject C/CXXFLAGS via the wrapper * netcdf-cxx4: fix the underlinking problem for platforms other than darwin (add netcdf-c libs netcdf-cxx4 ldlibs flags) * netcdf-cxx4: remove redundant extension of CPPFLAGS * netcdf-cxx4: only need to use MPI compiler wrapper when building C (vs both C and C++) * netcdf-cxx4: remove variant 'static' This makes it consistent with other packages from the NetCDF constellation: always build the static libraries and additionally build the shared ones when '+shared'. * netcdf-cxx4: do not configure --with/--without-pic. This makes it consistent with other packages from the NetCDF constellation: build the shared libraries with the PIC flag and the static ones without it (the default for Autotools) when '~pic', and build the static libraries with PIC when '+pic' (to make them injectable into other shared libraries). * netcdf-cxx4: run the tests serially * netcdf-cxx4: build the plugins only when the tests are run Co-authored-by: Sergey Kosukhin --- .../repos/builtin/packages/netcdf-cxx4/package.py | 133 +++++++++++++++------ .../builtin/packages/netcdf-fortran/package.py | 16 +-- 2 files changed, 103 insertions(+), 46 deletions(-) (limited to 'var') diff --git a/var/spack/repos/builtin/packages/netcdf-cxx4/package.py b/var/spack/repos/builtin/packages/netcdf-cxx4/package.py index fc475be5ca..012ab9ace2 100644 --- a/var/spack/repos/builtin/packages/netcdf-cxx4/package.py +++ b/var/spack/repos/builtin/packages/netcdf-cxx4/package.py @@ -3,6 +3,8 @@ # # SPDX-License-Identifier: (Apache-2.0 OR MIT) +import os + from spack import * @@ -19,61 +21,122 @@ class NetcdfCxx4(AutotoolsPackage): version('4.3.1', sha256='6a1189a181eed043b5859e15d5c080c30d0e107406fbb212c8fb9814e90f3445') version('4.3.0', sha256='e34fbc6aba243ec82c23e9ee99db2430555ada849c54c1f3ab081b0ddd0f5f30') - # Usually the configure automatically inserts the pic flags, but we can - # force its usage with this variant. - variant('static', default=True, description='Enable building static libraries') variant('shared', default=True, description='Enable shared library') variant('pic', default=True, description='Produce position-independent code (for shared libs)') - variant('doxygen', default=True, description='Enable doxygen docs') + variant('doc', default=False, description='Enable doxygen docs') depends_on('netcdf-c') - depends_on('automake', type='build') - depends_on('autoconf', type='build') - depends_on('libtool', type='build') - depends_on('m4', type='build') - depends_on('doxygen', when='+doxygen', type='build') - - conflicts('~shared', when='~static') - - force_autoreconf = True + depends_on('doxygen', when='+doc', type='build') def flag_handler(self, name, flags): if name == 'cflags' and '+pic' in self.spec: flags.append(self.compiler.cc_pic_flag) - elif name == 'cppflags': - flags.append('-I' + self.spec['netcdf-c'].prefix.include) - - return (None, None, flags) + 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): - shared = True - return find_libraries( - 'libnetcdf_c++4', root=self.prefix, shared=shared, recursive=True - ) + libraries = ['libnetcdf_c++4'] - def configure_args(self): - config_args = [] + query_parameters = self.spec.last_query.extra_parameters - if '+static' in self.spec: - config_args.append('--enable-static') + if 'shared' in query_parameters: + shared = True + elif 'static' in query_parameters: + shared = False else: - config_args.append('--disable-static') + shared = '+shared' in self.spec - if '+shared' in self.spec: - config_args.append('--enable-shared') - else: - config_args.append('--disable-shared') + libs = find_libraries( + libraries, root=self.prefix, shared=shared, recursive=True + ) - if '+pic' in self.spec: - config_args.append('--with-pic') - else: - config_args.append('--without-pic') + if libs: + return libs + + msg = 'Unable to recursively locate {0} {1} libraries in {2}' + raise spack.error.NoLibrariesError( + 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 + # user. This, however, has a side effect: an extra file 'libh5bzip2.so' + # gets installed (note that the file has .so extension even on darwin). + # 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'\1noinst\2+\3', join_path(self.stage.source_path, + 'plugins', 'Makefile.in')) + + def configure_args(self): + config_args = self.enable_or_disable('shared') - if '+doxygen' in self.spec: + if '+doc' in self.spec: config_args.append('--enable-doxygen') else: config_args.append('--disable-doxygen') + if self.spec.satisfies('@4.3.1:'): + 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 in the filter test C code, which + # requires 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 . For example, 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) diff --git a/var/spack/repos/builtin/packages/netcdf-fortran/package.py b/var/spack/repos/builtin/packages/netcdf-fortran/package.py index d3843ccd0b..c79c8af46a 100644 --- a/var/spack/repos/builtin/packages/netcdf-fortran/package.py +++ b/var/spack/repos/builtin/packages/netcdf-fortran/package.py @@ -62,8 +62,6 @@ class NetcdfFortran(AutotoolsPackage): patch('no_parallel_build.patch', when='@4.5.2') def flag_handler(self, name, flags): - config_flags = None - if name == 'cflags': if '+pic' in self.spec: flags.append(self.compiler.cc_pic_flag) @@ -81,15 +79,11 @@ class NetcdfFortran(AutotoolsPackage): # The following flag forces the compiler to produce module # files with lowercase names. flags.append('-ef') - elif name == 'ldflags': - # We need to specify LDFLAGS to get correct dependency_libs - # in libnetcdff.la, so packages that use libtool for linking - # could correctly link to all the dependencies even when the - # building takes place outside of Spack environment, i.e. - # without Spack's compiler wrappers. - config_flags = [self.spec['netcdf-c'].libs.search_flags] - - return flags, None, config_flags + + # Note that cflags and fflags 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): -- cgit v1.2.3-70-g09d2