Update abinit package (#23670)

This PR updates the abinit package. The underlying build system has
several changes from previous versions, which are reflected in the
package recipe.

- added version 9.4.2
- removed commented out code
- add new libxml2 variant, with dependency and conflicts
- add dependency on atompaw
- depend on fftw-api when ~openmp
  This allows other fftw implementations to be used. This PR adds MKL.
- depend on netcdf explicitly
- remove hdf5 variant as hdf5 is required
- only use wannier90 if +mpi as the wannier90 spack package is MPI only
- allow newer versions of libxc for abinit 9
- split configure options for versions before and after abinit 9
- always use MPI compiler wrappers
- add patch to remove march settings for version 9
- Set conflict for fftw~openmp if abinit+openmp
  This allows the virtual fftw-api to be used for the dependency. If fftw
  is the fftw-api provider then bail if fftw~openmp is set when
  abinit+openmp is used.
- Set conflicts for +openmp and mkl
- Be explicit about +mkl for intel-parallel-studio
- Add TODO entry for switching conflicts/depends_on logic

2 files changed, 197 insertions, 76 deletions

diff --git a/var/spack/repos/builtin/packages/abinit/package.py b/var/spack/repos/builtin/packages/abinit/package.py
index 1cc464fb53..39be0efd34 100644
--- a/var/spack/repos/builtin/packages/abinit/package.py
+++ b/var/spack/repos/builtin/packages/abinit/package.py
@@ -30,6 +30,7 @@ class Abinit(AutotoolsPackage):
     homepage = 'http://www.abinit.org'
     url      = 'https://www.abinit.org/sites/default/files/packages/abinit-8.6.3.tar.gz'
 
+    version('9.4.2', sha256='d40886f5c8b138bb4aa1ca05da23388eb70a682790cfe5020ecce4db1b1a76bc')
     version('8.10.3', sha256='ed626424b4472b93256622fbb9c7645fa3ffb693d4b444b07d488771ea7eaa75')
     version('8.10.2', sha256='4ee2e0329497bf16a9b2719fe0536cc50c5d5a07c65e18edaf15ba02251cbb73')
     version('8.8.2', sha256='15216703bd56a799a249a112b336d07d733627d3756487a4b1cb48ebb625c3e7')
@@ -44,26 +45,14 @@ class Abinit(AutotoolsPackage):
             description='Enables OpenMP threads. Use threaded FFTW3')
     variant('scalapack', default=False,
             description='Enables scalapack support. Requires MPI')
-    # variant('elpa', default=False,
-    #         description='Uses elpa instead of scalapack. Requires MPI')
-
-    # TODO: To be tested.
-    # It was working before the last `git pull` but now all tests crash.
-    # For the time being, the default is netcdf3 and the internal fallbacks
-    # FIXME: rename (trio?) and use multivalued variants to cover
-    # --with-trio-flavor={netcdf, none}
-    # Note that Abinit@8: does not support etsf_io anymore because it is not
-    # compatible with HDF5 and MPI-IO
-    variant('hdf5', default=False,
-            description='Enables HDF5+Netcdf4 with MPI. WARNING: experimental')
 
     variant('wannier90', default=False,
             description='Enables the Wannier90 library')
+    variant('libxml2', default=False,
+            description='Enable libxml2 support, used by multibinit')
 
     # Add dependencies
-    # currently one cannot forward options to virtual packages, see #1712.
-    # depends_on('blas', when='~openmp')
-    # depends_on('blas+openmp', when='+openmp')
+    depends_on('atompaw')
     depends_on('blas')
     depends_on('lapack')
 
@@ -72,29 +61,53 @@ class Abinit(AutotoolsPackage):
 
     depends_on('scalapack', when='+scalapack+mpi')
 
-    # depends_on('elpa~openmp', when='+elpa+mpi~openmp')
-    # depends_on('elpa+openmp', when='+elpa+mpi+openmp')
+    depends_on('fftw-api')
 
-    depends_on('fftw precision=float,double')
-    depends_on('fftw~openmp', when='~openmp')
-    depends_on('fftw+openmp', when='+openmp')
+    depends_on('netcdf-fortran')
+    depends_on('netcdf-c+mpi', when='+mpi')
+    depends_on('netcdf-c~mpi', when='~mpi')
+    depends_on('hdf5+mpi', when='+mpi')
+    depends_on('hdf5~mpi', when='~mpi')
+    depends_on("wannier90+shared", when='+wannier90+mpi')
 
-    depends_on('netcdf-fortran', when='+hdf5')
-    depends_on('hdf5+mpi', when='+mpi+hdf5')  # required for NetCDF-4 support
+    # constrain libxc version
+    depends_on('libxc')
+    depends_on('libxc@:2', when='@:8')
 
-    # pin libxc version
-    depends_on("libxc@2.2.2")
+    # libxml2
+    depends_on('libxml2', when='@9:')
 
     # Cannot ask for +scalapack if it does not depend on MPI
     conflicts('+scalapack', when='~mpi')
 
-    depends_on("wannier90+shared", when='+wannier90')
-
-    # Elpa is a substitute for scalapack and needs mpi
-    # conflicts('+elpa', when='~mpi')
-    # conflicts('+elpa', when='+scalapack')
-
-    patch('rm_march_settings.patch')
+    # Cannot ask for +wannier90 if it does not depend on MPI
+    conflicts('+wannier90', when='~mpi')
+
+    # libxml2 needs version 9 and above
+    conflicts('+libxml2', when='@:8')
+
+    conflicts('%gcc@7:', when='@:8.8')
+    conflicts('%gcc@9:', when='@:8.10')
+
+    # need openmp threading for abinit+openmp
+    # TODO: The logic here can be reversed with the new concretizer. Instead of
+    # using `conflicts`, `depends_on` could be used instead.
+    mkl_message = 'Need to set dependent variant to threads=openmp'
+    conflicts('+openmp',
+              when='^fftw~openmp',
+              msg='Need to request fftw +openmp')
+    conflicts('+openmp',
+              when='^intel-mkl threads=none',
+              msg=mkl_message)
+    conflicts('+openmp',
+              when='^intel-mkl threads=tbb',
+              msg=mkl_message)
+    conflicts('+openmp',
+              when='^intel-parallel-studio +mkl threads=none',
+              msg=mkl_message)
+
+    patch('rm_march_settings.patch', when='@:8')
+    patch('rm_march_settings_v9.patch', when='@9:')
 
     # Fix detection of Fujitsu compiler
     # Fix configure not to collect the option that causes an error
@@ -106,75 +119,138 @@ class Abinit(AutotoolsPackage):
         spec = self.spec
 
         options = []
+        options += self.with_or_without('libxml2')
+
         oapp = options.append
 
         if '+wannier90' in spec:
-            oapp('--with-wannier90-libs=-L{0}'
-                 .format(spec['wannier90'].prefix.lib + ' -lwannier -lm'))
-            oapp('--with-wannier90-incs=-I{0}'
-                 .format(spec['wannier90'].prefix.modules))
-            oapp('--with-wannier90-bins={0}'
-                 .format(spec['wannier90'].prefix.bin))
-            oapp('--enable-connectors')
-            oapp('--with-dft-flavor=wannier90')
+            if '@:8' in spec:
+                oapp('--with-wannier90-libs=-L{0}'
+                     .format(spec['wannier90'].prefix.lib + ' -lwannier -lm'))
+                oapp('--with-wannier90-incs=-I{0}'
+                     .format(spec['wannier90'].prefix.modules))
+                oapp('--with-wannier90-bins={0}'
+                     .format(spec['wannier90'].prefix.bin))
+                oapp('--enable-connectors')
+                oapp('--with-dft-flavor=atompaw+libxc+wannier90')
+            else:
+                options.extend([
+                    'WANNIER90_CPPFLAGS=-I{0}'.format(
+                        spec['wannier90'].prefix.modules),
+                    'WANNIER90_LIBS=-L{0} {1}'.format(
+                        spec['wannier90'].prefix.lib, '-lwannier'),
+                ])
+        else:
+            if '@:8' in spec:
+                oapp('--with-dft-flavor=atompaw+libxc')
+            else:
+                '--without-wannier90',
 
         if '+mpi' in spec:
+            oapp('CC={0}'.format(spec['mpi'].mpicc))
+            oapp('CXX={0}'.format(spec['mpi'].mpicxx))
+            oapp('FC={0}'.format(spec['mpi'].mpifc))
+
             # MPI version:
             # let the configure script auto-detect MPI support from mpi_prefix
-            oapp('--enable-mpi=yes')
-            if self.spec.satisfies('%fj'):
-                oapp('CC={0}'.format(spec['mpi'].mpicc))
-                oapp('CXX={0}'.format(spec['mpi'].mpicxx))
-                oapp('FC={0}'.format(spec['mpi'].mpifc))
+            if '@:8' in spec:
+                oapp('--enable-mpi=yes')
             else:
-                oapp('--with-mpi-prefix={0}'.format(spec['mpi'].prefix))
-                oapp('--enable-mpi-io=yes')
-                oapp('MPIFC={0}'.format(spec['mpi'].mpifc))
-            if '~wannier90' in spec:
-                oapp('--with-dft-flavor=atompaw+libxc')
+                oapp('--with-mpi')
+        else:
+            if '@:8' in spec:
+                oapp('--enable-mpi=no')
+            else:
+                oapp('--without-mpi')
 
         # Activate OpenMP in Abinit Fortran code.
         if '+openmp' in spec:
             oapp('--enable-openmp=yes')
+        else:
+            oapp('--enable-openmp=no')
 
         # BLAS/LAPACK/SCALAPACK-ELPA
         linalg = spec['lapack'].libs + spec['blas'].libs
+        if '^mkl' in spec:
+            linalg_flavor = 'mkl'
+        elif '@9:' in spec and '^openblas' in spec:
+            linalg_flavor = 'openblas'
+        else:
+            linalg_flavor = 'custom'
+
         if '+scalapack' in spec:
-            oapp('--with-linalg-flavor=custom+scalapack')
             linalg = spec['scalapack'].libs + linalg
+            if '@:8' in spec:
+                linalg_flavor = 'scalapack+{0}'.format(linalg_flavor)
 
-        # elif '+elpa' in spec:
+        if '@:8' in spec:
+            oapp('--with-linalg-libs={0}'.format(linalg.ld_flags))
         else:
-            oapp('--with-linalg-flavor=custom')
-
-        oapp('--with-linalg-libs={0}'.format(linalg.ld_flags))
-
-        # FFTW3: use sequential or threaded version if +openmp
-        fftflavor, fftlibs = 'fftw3', '-lfftw3 -lfftw3f'
-        if '+openmp' in spec:
-            fftflavor = 'fftw3-threads'
-            fftlibs = '-lfftw3_omp -lfftw3 -lfftw3f'
-
-        options.extend([
-            '--with-fft-flavor=%s' % fftflavor,
-            '--with-fft-incs=-I%s' % spec['fftw'].prefix.include,
-            '--with-fft-libs=-L%s %s' % (spec['fftw'].prefix.lib, fftlibs),
-        ])
+            oapp('LINALG_LIBS={0}'.format(linalg.ld_flags))
+
+        oapp('--with-linalg-flavor={0}'.format(linalg_flavor))
+
+        # FFTW3: use sequential from fftw3 or MKL
+        if '@:8' in spec:
+            if '+openmp' in spec:
+                fftflavor, fftlibs = 'fftw3-threads', '-lfftw3_omp -lfftw3 -lfftw3f'
+                options.extend([
+                    '--with-fft-incs={0}'.format(spec['fftw'].headers.cpp_flags),
+                    '--with-fft-libs=-L{0} {1}'.format(
+                        spec['fftw'].prefix.lib, fftlibs),
+                ])
+            else:
+                oapp('--with-fft-incs={0}'.format(spec['fftw-api'].headers.cpp_flags))
+                if '^mkl' in spec:
+                    fftflavor = 'dfti'
+                    oapp('--with-fft-libs={0}'.format(spec['fftw-api'].libs.ld_flags))
+                elif '^fftw' in spec:
+                    fftflavor, fftlibs = 'fftw3', '-lfftw3 -lfftw3f'
+                    oapp('--with-fft-libs=-L{0} {1}'.format(
+                        spec['fftw-api'].prefix.lib, fftlibs))
+        else:
+            if '+openmp' in spec:
+                fftflavor, fftlibs = 'fftw3-threads', '-lfftw3_omp -lfftw3 -lfftw3f'
+                options.extend([
+                    'FFTW3_CPPFLAGS={0}'.format(spec['fftw'].headers.cpp_flags),
+                    'FFTW3_LIBS=-L{0} {1}'.format(
+                        spec['fftw'].prefix.lib, fftlibs),
+                ])
+            else:
+                if '^mkl' in spec:
+                    fftflavor = 'dfti'
+                    options.extend([
+                        'FFT_CPPFLAGS={0}'.format(spec['fftw-api'].headers.cpp_flags),
+                        'FFT_LIBs={0}'.format(spec['fftw-api'].libs.ld_flags),
+                    ])
+                elif '^fftw' in spec:
+                    fftflavor, fftlibs = 'fftw3', '-lfftw3 -lfftw3f'
+                    options.extend([
+                        'FFTW3_CPPFLAGS={0}'.format(spec['fftw-api'].headers.cpp_flags),
+                        'FFTW3_LIBS=-L{0} {1}'.format(
+                            spec['fftw-api'].prefix.lib, fftlibs),
+                    ])
+
+        oapp('--with-fft-flavor={0}'.format(fftflavor))
 
         # LibXC library
         libxc = spec['libxc:fortran']
-        options.extend([
-            'with_libxc_incs={0}'.format(libxc.headers.cpp_flags),
-            'with_libxc_libs={0}'.format(libxc.libs.ld_flags + ' -lm')
-        ])
+        if '@:8' in spec:
+            options.extend([
+                '--with-libxc-incs={0}'.format(libxc.headers.cpp_flags),
+                '--with-libxc-libs={0}'.format(libxc.libs.ld_flags + ' -lm')
+            ])
+        else:
+            oapp('--with-libxc={0}'.format(libxc.prefix))
 
         # Netcdf4/HDF5
-        if '+hdf5' in spec:
+        hdf5 = spec['hdf5:hl']
+        netcdfc = spec['netcdf-c']
+        netcdff = spec['netcdf-fortran:shared']
+        if '@:8' in spec:
             oapp('--with-trio-flavor=netcdf')
             # Since version 8, Abinit started to use netcdf4 + hdf5 and we have
             # to link with the high level HDF5 library
-            hdf5 = spec['hdf5:hl']
-            netcdff = spec['netcdf-fortran:shared']
             options.extend([
                 '--with-netcdf-incs={0}'.format(netcdff.headers.cpp_flags),
                 '--with-netcdf-libs={0}'.format(
@@ -182,9 +258,10 @@ class Abinit(AutotoolsPackage):
                 ),
             ])
         else:
-            # In Spack we do our best to avoid building any internally provided
-            # dependencies, such as netcdf3 in this case.
-            oapp('--with-trio-flavor=none')
+            options.extend([
+                '--with-netcdf={0}'.format(netcdfc.prefix),
+                '--with-netcdf-fortran={0}'.format(netcdff.prefix),
+            ])
 
         if self.spec.satisfies('%fj'):
             oapp('FCFLAGS_MODDIR=-M{0}'.format(join_path(
diff --git a/var/spack/repos/builtin/packages/abinit/rm_march_settings_v9.patch b/var/spack/repos/builtin/packages/abinit/rm_march_settings_v9.patch
new file mode 100644
index 0000000000..c876e7c769
--- /dev/null
+++ b/var/spack/repos/builtin/packages/abinit/rm_march_settings_v9.patch
@@ -0,0 +1,44 @@
+--- a/configure	2021-04-30 04:25:20.000000000 -0500
++++ b/configure	2021-05-14 17:34:11.666320745 -0500
+@@ -16794,11 +16794,11 @@
+           ;;
+         standard)
+           abi_optim_flavor_opt="standard"
+-          CFLAGS_OPTIM="-O2 -mtune=native -march=native"
++          CFLAGS_OPTIM="-O2"
+           ;;
+         aggressive)
+           abi_optim_flavor_opt="aggressive"
+-          CFLAGS_OPTIM="-O3 -mtune=native -march=native"
++          CFLAGS_OPTIM="-O3"
+           ;;
+       esac   # [case: abi_optim_flavor, indent: 2, item: True]
+       ;;
+@@ -16947,11 +16947,11 @@
+           ;;
+         standard)
+           abi_optim_flavor_opt="standard"
+-          CXXFLAGS_OPTIM="-O2 -mtune=native -march=native"
++          CXXFLAGS_OPTIM="-O2"
+           ;;
+         aggressive)
+           abi_optim_flavor_opt="aggressive"
+-          CXXFLAGS_OPTIM="-O3 -mtune=native -march=native"
++          CXXFLAGS_OPTIM="-O3"
+           ;;
+       esac   # [case: abi_optim_flavor, indent: 2, item: True]
+       ;;
+@@ -17100,11 +17100,11 @@
+           ;;
+         standard)
+           abi_optim_flavor_opt="standard"
+-          FCFLAGS_OPTIM="-O2 -mtune=native -march=native"
++          FCFLAGS_OPTIM="-O2"
+           ;;
+         aggressive)
+           abi_optim_flavor_opt="aggressive"
+-          FCFLAGS_OPTIM="-O3 -mtune=native -march=native -funroll-loops -faggressive-function-elimination"
++          FCFLAGS_OPTIM="-O3 -funroll-loops -faggressive-function-elimination"
+           ;;
+       esac   # [case: abi_optim_flavor, indent: 2, item: True]
+       ;;