Multi-valued variants (#2386)

Modifications:
- added support for multi-valued variants
- refactored code related to variants into variant.py
- added new generic features to AutotoolsPackage that leverage multi-valued variants
- modified openmpi to use new features
- added unit tests for the new semantics

6 files changed, 258 insertions, 251 deletions

diff --git a/var/spack/repos/builtin.mock/packages/a/package.py b/var/spack/repos/builtin.mock/packages/a/package.py
index 0d75ee1256..b697f4d2a9 100644
--- a/var/spack/repos/builtin.mock/packages/a/package.py
+++ b/var/spack/repos/builtin.mock/packages/a/package.py
@@ -25,7 +25,7 @@
 from spack import *
 
 
-class A(Package):
+class A(AutotoolsPackage):
     """Simple package with no dependencies"""
 
     homepage = "http://www.example.com"
@@ -33,5 +33,35 @@ class A(Package):
 
     version('1.0', '0123456789abcdef0123456789abcdef')
 
+    variant(
+        'foo',
+        values=('bar', 'baz', 'fee'),
+        default='bar',
+        description='',
+        multi=True
+    )
+
+    variant(
+        'foobar',
+        values=('bar', 'baz', 'fee'),
+        default='bar',
+        description='',
+        multi=False
+    )
+
+    def with_or_without_fee(self, activated):
+        if not activated:
+            return '--no-fee'
+        return '--fee-all-the-time'
+
+    def autoreconf(self, spec, prefix):
+        pass
+
+    def configure(self, spec, prefix):
+        pass
+
+    def build(self, spec, prefix):
+        pass
+
     def install(self, spec, prefix):
         pass
diff --git a/var/spack/repos/builtin.mock/packages/multivalue_variant/package.py b/var/spack/repos/builtin.mock/packages/multivalue_variant/package.py
new file mode 100644
index 0000000000..13c3b02e77
--- /dev/null
+++ b/var/spack/repos/builtin.mock/packages/multivalue_variant/package.py
@@ -0,0 +1,57 @@
+##############################################################################
+# Copyright (c) 2013-2016, Lawrence Livermore National Security, LLC.
+# Produced at the Lawrence Livermore National Laboratory.
+#
+# This file is part of Spack.
+# Created by Todd Gamblin, tgamblin@llnl.gov, All rights reserved.
+# LLNL-CODE-647188
+#
+# For details, see https://github.com/llnl/spack
+# Please also see the LICENSE file for our notice and the LGPL.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU Lesser General Public License (as
+# published by the Free Software Foundation) version 2.1, February 1999.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the IMPLIED WARRANTY OF
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the terms and
+# conditions of the GNU Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+##############################################################################
+from spack import *
+
+
+class MultivalueVariant(Package):
+    homepage = "http://www.llnl.gov"
+    url = "http://www.llnl.gov/mpileaks-1.0.tar.gz"
+
+    version(1.0, 'foobarbaz')
+    version(2.1, 'foobarbaz')
+    version(2.2, 'foobarbaz')
+    version(2.3, 'foobarbaz')
+
+    variant('debug', default=False, description='Debug variant')
+    variant(
+        'foo',
+        description='Multi-valued variant',
+        values=('bar', 'baz', 'barbaz'),
+        multi=True
+    )
+
+    variant(
+        'fee',
+        description='Single-valued variant',
+        default='bar',
+        values=('bar', 'baz', 'barbaz'),
+        multi=False
+    )
+
+    depends_on('mpi')
+    depends_on('callpath')
+
+    def install(self, spec, prefix):
+        pass
diff --git a/var/spack/repos/builtin/packages/cdo/package.py b/var/spack/repos/builtin/packages/cdo/package.py
index 90039d4479..3cee41b299 100644
--- a/var/spack/repos/builtin/packages/cdo/package.py
+++ b/var/spack/repos/builtin/packages/cdo/package.py
@@ -27,7 +27,8 @@ from spack import *
 
 class Cdo(Package):
     """CDO is a collection of command line Operators to manipulate and analyse
-    Climate and NWP model Data. """
+    Climate and NWP model Data.
+    """
 
     homepage = "https://code.zmaw.de/projects/cdo"
     url      = "https://code.zmaw.de/attachments/download/12760/cdo-1.7.2.tar.gz"
diff --git a/var/spack/repos/builtin/packages/mvapich2/package.py b/var/spack/repos/builtin/packages/mvapich2/package.py
index d952165ac1..3844f804be 100644
--- a/var/spack/repos/builtin/packages/mvapich2/package.py
+++ b/var/spack/repos/builtin/packages/mvapich2/package.py
@@ -22,11 +22,20 @@
 # License along with this program; if not, write to the Free Software
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 ##############################################################################
-from spack import *
 import sys
 
+from spack import *
+from spack.error import SpackError
+
+
+def _process_manager_validator(values):
+    if len(values) > 1 and 'slurm' in values:
+        raise SpackError(
+            'slurm cannot be activated along with other process managers'
+        )
 
-class Mvapich2(Package):
+
+class Mvapich2(AutotoolsPackage):
     """MVAPICH2 is an MPI implementation for Infiniband networks."""
     homepage = "http://mvapich.cse.ohio-state.edu/"
     url = "http://mvapich.cse.ohio-state.edu/download/mvapich/mv2/mvapich2-2.2.tar.gz"
@@ -55,62 +64,39 @@ class Mvapich2(Package):
     #   serialized  - User serializes calls to MPI (MPI_THREAD_SERIALIZED)
     #   multiple    - Fully multi-threaded (MPI_THREAD_MULTIPLE)
     #   runtime     - Alias to "multiple"
-    variant('threads', default='multiple',
-            description='Control the level of thread support')
+    variant(
+        'threads',
+        default='multiple',
+        values=('single', 'funneled', 'serialized', 'multiple'),
+        multi=False,
+        description='Control the level of thread support'
+    )
 
     # 32 is needed when job size exceeds 32768 cores
-    variant('ch3_rank_bits', default=32,
-            description='Number of bits allocated to the rank field (16 or 32)'
-            )
-
-    ##########
-    # TODO : Process managers should be grouped into the same variant,
-    # as soon as variant capabilities will be extended See
-    # https://groups.google.com/forum/#!topic/spack/F8-f8B4_0so
-    SLURM = 'slurm'
-    HYDRA = 'hydra'
-    GFORKER = 'gforker'
-    REMSHELL = 'remshell'
-    SLURM_INCOMPATIBLE_PMS = (HYDRA, GFORKER, REMSHELL)
-    variant(SLURM, default=False,
-            description='Set slurm as the only process manager')
-    variant(HYDRA, default=False,
-            description='Set hydra as one of the process managers')
-    variant(GFORKER, default=False,
-            description='Set gforker as one of the process managers')
-    variant(REMSHELL, default=False,
-            description='Set remshell as one of the process managers')
-    ##########
-
-    ##########
-    # TODO : Network types should be grouped into the same variant, as
-    # soon as variant capabilities will be extended
-    PSM = 'psm'
-    SOCK = 'sock'
-    NEMESISIBTCP = 'nemesisibtcp'
-    NEMESISIB = 'nemesisib'
-    NEMESIS = 'nemesis'
-    MRAIL = 'mrail'
-    SUPPORTED_NETWORKS = (PSM, SOCK, NEMESIS, NEMESISIB, NEMESISIBTCP)
-    variant(
-        PSM, default=False,
-        description='Configure for QLogic PSM-CH3')
-    variant(
-        SOCK, default=False,
-        description='Configure for TCP/IP-CH3')
-    variant(
-        NEMESISIBTCP, default=False,
-        description='Configure for both OFA-IB-Nemesis and TCP/IP-Nemesis')
     variant(
-        NEMESISIB, default=False,
-        description='Configure for OFA-IB-Nemesis')
+        'ch3_rank_bits',
+        default='32',
+        values=('16', '32'),
+        multi=False,
+        description='Number of bits allocated to the rank field (16 or 32)'
+    )
+
     variant(
-        NEMESIS, default=False,
-        description='Configure for TCP/IP-Nemesis')
+        'process_managers',
+        description='List of the process managers to activate',
+        values=('slurm', 'hydra', 'gforker', 'remshell'),
+        multi=True,
+        validator=_process_manager_validator
+    )
+
     variant(
-        MRAIL, default=False,
-        description='Configure for OFA-IB-CH3')
-    ##########
+        'fabrics',
+        description='The fabric enabled for this build',
+        default='psm',
+        values=(
+            'psm', 'sock', 'nemesisib', 'nemesis', 'mrail', 'nemesisibtcp'
+        )
+    )
 
     # FIXME : CUDA support is missing
     depends_on('bison')
@@ -123,110 +109,52 @@ class Mvapich2(Package):
         else:
             return "%s/mvapich/mv2/mvapich2-%s.tar.gz"  % (base_url, version)
 
-    @staticmethod
-    def enabled(x):
-        """Given a variant name returns the string that means the variant is
-        enabled
-
-        :param x: variant name
-        :return:
-        """
-        return '+' + x
-
-    def set_build_type(self, spec, configure_args):
-        """Appends to configure_args the flags that depends only on the build
-        type (i.e. release or debug)
-
-        :param spec: spec
-        :param configure_args: list of current configure arguments
-        """
-        if '+debug' in spec:
-            build_type_options = [
-                "--disable-fast",
-                "--enable-error-checking=runtime",
-                "--enable-error-messages=all",
-                # Permits debugging with TotalView
-                "--enable-g=dbg", "--enable-debuginfo"
-            ]
-        else:
-            build_type_options = ["--enable-fast=all"]
-
-        configure_args.extend(build_type_options)
+    @property
+    def process_manager_options(self):
+        spec = self.spec
 
-    def set_process_manager(self, spec, configure_args):
-        """Appends to configure_args the flags that will enable the
-        appropriate process managers
+        other_pms = []
+        for x in ('hydra', 'gforker', 'remshell'):
+            if 'process_managers={0}'.format(x) in spec:
+                other_pms.append(x)
+        opts = ['--with-pm=%s' % ':'.join(other_pms)]
 
-        :param spec: spec
-        :param configure_args: list of current configure arguments
-        """
-        # Check that slurm variant is not activated together with
-        # other pm variants
-        has_slurm_incompatible_variants = \
-            any(self.enabled(x) in spec
-                for x in Mvapich2.SLURM_INCOMPATIBLE_PMS)
-
-        if self.enabled(Mvapich2.SLURM) in spec and \
-           has_slurm_incompatible_variants:
-            raise RuntimeError(" %s : 'slurm' cannot be activated \
-            together with other process managers" % self.name)
-
-        process_manager_options = []
         # See: http://slurm.schedmd.com/mpi_guide.html#mvapich2
-        if self.enabled(Mvapich2.SLURM) in spec:
+        if 'process_managers=slurm' in spec:
             if self.version > Version('2.0'):
-                process_manager_options = [
-                    "--with-pmi=pmi2",
-                    "--with-pm=slurm"
+                opts = [
+                    '--with-pmi=pmi2',
+                    '--with-pm=slurm'
                 ]
             else:
-                process_manager_options = [
-                    "--with-pmi=slurm",
-                    "--with-pm=no"
+                opts = [
+                    '--with-pmi=slurm',
+                    '--with-pm=no'
                 ]
 
-        elif has_slurm_incompatible_variants:
-            pms = []
-            # The variant name is equal to the process manager name in
-            # the configuration options
-            for x in Mvapich2.SLURM_INCOMPATIBLE_PMS:
-                if self.enabled(x) in spec:
-                    pms.append(x)
-            process_manager_options = [
-                "--with-pm=%s" % ':'.join(pms)
-            ]
-        configure_args.extend(process_manager_options)
-
-    def set_network_type(self, spec, configure_args):
-        # Check that at most one variant has been activated
-        count = 0
-        for x in Mvapich2.SUPPORTED_NETWORKS:
-            if self.enabled(x) in spec:
-                count += 1
-        if count > 1:
-            raise RuntimeError('network variants are mutually exclusive \
-            (only one can be selected at a time)')
-
-        network_options = []
+        return opts
+
+    @property
+    def network_options(self):
+        opts = []
         # From here on I can suppose that only one variant has been selected
-        if self.enabled(Mvapich2.PSM) in spec:
-            network_options = ["--with-device=ch3:psm"]
-        elif self.enabled(Mvapich2.SOCK) in spec:
-            network_options = ["--with-device=ch3:sock"]
-        elif self.enabled(Mvapich2.NEMESISIBTCP) in spec:
-            network_options = ["--with-device=ch3:nemesis:ib,tcp"]
-        elif self.enabled(Mvapich2.NEMESISIB) in spec:
-            network_options = ["--with-device=ch3:nemesis:ib"]
-        elif self.enabled(Mvapich2.NEMESIS) in spec:
-            network_options = ["--with-device=ch3:nemesis"]
-        elif self.enabled(Mvapich2.MRAIL) in spec:
-            network_options = ["--with-device=ch3:mrail", "--with-rdma=gen2"]
-
-        configure_args.extend(network_options)
+        if 'fabrics=psm' in self.spec:
+            opts = ["--with-device=ch3:psm"]
+        elif 'fabrics=sock' in self.spec:
+            opts = ["--with-device=ch3:sock"]
+        elif 'fabrics=nemesisibtcp' in self.spec:
+            opts = ["--with-device=ch3:nemesis:ib,tcp"]
+        elif 'fabrics=nemesisib' in self.spec:
+            opts = ["--with-device=ch3:nemesis:ib"]
+        elif 'fabrics=nemesis' in self.spec:
+            opts = ["--with-device=ch3:nemesis"]
+        elif 'fabrics=mrail' in self.spec:
+            opts = ["--with-device=ch3:mrail", "--with-rdma=gen2"]
+        return opts
 
     def setup_environment(self, spack_env, run_env):
-        if self.enabled(Mvapich2.SLURM) in self.spec and \
-           self.version > Version('2.0'):
+        spec = self.spec
+        if 'process_managers=slurm' in spec and spec.satisfies('@2.0:'):
             run_env.set('SLURM_MPI_TYPE', 'pmi2')
 
     def setup_dependent_environment(self, spack_env, run_env, dependent_spec):
@@ -251,48 +179,44 @@ class Mvapich2(Package):
             join_path(self.prefix.lib, 'libmpi.{0}'.format(dso_suffix))
         ]
 
-    def install(self, spec, prefix):
+    @run_before('configure')
+    def die_without_fortran(self):
         # Until we can pass variants such as +fortran through virtual
         # dependencies depends_on('mpi'), require Fortran compiler to
         # avoid delayed build errors in dependents.
         if (self.compiler.f77 is None) or (self.compiler.fc is None):
-            raise InstallError('Mvapich2 requires both C and Fortran ',
-                               'compilers!')
-
-        # we'll set different configure flags depending on our
-        # environment
-        configure_args = [
-            "--prefix=%s" % prefix,
-            "--enable-shared",
-            "--enable-romio",
-            "--disable-silent-rules",
+            raise InstallError(
+                'Mvapich2 requires both C and Fortran compilers!'
+            )
+
+    def configure_args(self):
+        args = [
+            '--enable-shared',
+            '--enable-romio',
+            '-disable-silent-rules',
+            '--enable-fortran=all',
             "--enable-threads={0}".format(spec.variants['threads'].value),
             "--with-ch3-rank-bits={0}".format(
                 spec.variants['ch3_rank_bits'].value),
         ]
 
-        if self.compiler.f77 and self.compiler.fc:
-            configure_args.append("--enable-fortran=all")
-        elif self.compiler.f77:
-            configure_args.append("--enable-fortran=f77")
-        elif self.compiler.fc:
-            configure_args.append("--enable-fortran=fc")
+        if '+debug' in self.spec:
+            args.extend([
+                '--disable-fast',
+                '--enable-error-checking=runtime',
+                '--enable-error-messages=all',
+                # Permits debugging with TotalView
+                '--enable-g=dbg',
+                '--enable-debuginfo'
+            ])
         else:
-            configure_args.append("--enable-fortran=none")
-
-        # Set the type of the build (debug, release)
-        self.set_build_type(spec, configure_args)
-        # Set the process manager
-        self.set_process_manager(spec, configure_args)
-        # Determine network type by variant
-        self.set_network_type(spec, configure_args)
-
-        configure(*configure_args)
-        make()
-        make("install")
+            args.append('--enable-fast=all')
 
-        self.filter_compilers()
+        args.extend(self.process_manager_options)
+        args.extend(self.network_options)
+        return args
 
+    @run_after('install')
     def filter_compilers(self):
         """Run after install to make the MPI compilers use the
            compilers that Spack built the package with.
@@ -302,7 +226,7 @@ class Mvapich2(Package):
            be bound to whatever compiler they were built with.
         """
         bin = self.prefix.bin
-        mpicc  = join_path(bin, 'mpicc')
+        mpicc = join_path(bin, 'mpicc')
         mpicxx = join_path(bin, 'mpicxx')
         mpif77 = join_path(bin, 'mpif77')
         mpif90 = join_path(bin, 'mpif90')
diff --git a/var/spack/repos/builtin/packages/netcdf/package.py b/var/spack/repos/builtin/packages/netcdf/package.py
index ca6a30e9a4..f3f0ed59a6 100644
--- a/var/spack/repos/builtin/packages/netcdf/package.py
+++ b/var/spack/repos/builtin/packages/netcdf/package.py
@@ -24,6 +24,16 @@
 ##############################################################################
 from spack import *
 
+import numbers
+
+
+def is_integral(x):
+    """Any integer value"""
+    try:
+        return isinstance(int(x), numbers.Integral) and not isinstance(x, bool)
+    except ValueError:
+        return False
+
 
 class Netcdf(AutotoolsPackage):
     """NetCDF is a set of software libraries and self-describing,
@@ -52,10 +62,18 @@ class Netcdf(AutotoolsPackage):
     # These variants control the number of dimensions (i.e. coordinates and
     # attributes) and variables (e.g. time, entity ID, number of coordinates)
     # that can be used in any particular NetCDF file.
-    variant('maxdims', default=1024,
-            description='Defines the maximum dimensions of NetCDF files.')
-    variant('maxvars', default=8192,
-            description='Defines the maximum variables of NetCDF files.')
+    variant(
+        'maxdims',
+        default=1024,
+        description='Defines the maximum dimensions of NetCDF files.',
+        values=is_integral
+    )
+    variant(
+        'maxvars',
+        default=8192,
+        description='Defines the maximum variables of NetCDF files.',
+        values=is_integral
+    )
 
     depends_on("m4", type='build')
     depends_on("hdf", when='+hdf4')
diff --git a/var/spack/repos/builtin/packages/openmpi/package.py b/var/spack/repos/builtin/packages/openmpi/package.py
index 63579efe0e..2761df543d 100644
--- a/var/spack/repos/builtin/packages/openmpi/package.py
+++ b/var/spack/repos/builtin/packages/openmpi/package.py
@@ -22,13 +22,16 @@
 # License along with this program; if not, write to the Free Software
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 ##############################################################################
-from spack import *
+
 import os
 
+from spack import *
+
 
 def _verbs_dir():
     """Try to find the directory where the OpenFabrics verbs package is
-    installed. Return None if not found."""
+    installed. Return None if not found.
+    """
     try:
         # Try to locate Verbs by looking for a utility in the path
         ibv_devices = which("ibv_devices")
@@ -43,7 +46,7 @@ def _verbs_dir():
         if path == "/":
             path = "/usr"
         return path
-    except:
+    except TypeError:
         return None
 
 
@@ -82,22 +85,20 @@ class Openmpi(AutotoolsPackage):
     patch('configure.patch', when="@1.10.0:1.10.1")
     patch('fix_multidef_pmi_class.patch', when="@2.0.0:2.0.1")
 
-    # Fabrics
-    variant('psm', default=False, description='Build support for the PSM library')
-    variant('psm2', default=False,
-            description='Build support for the Intel PSM2 library')
-    variant('pmi', default=False,
-            description='Build support for PMI-based launchers')
-    variant('verbs', default=_verbs_dir() is not None,
-            description='Build support for OpenFabrics verbs')
-    variant('mxm', default=False, description='Build Mellanox Messaging support')
-
-    # Schedulers
-    # TODO: support for alps and loadleveler is missing
-    variant('tm', default=False,
-            description='Build TM (Torque, PBSPro, and compatible) support')
-    variant('slurm', default=False,
-            description='Build SLURM scheduler component')
+    variant(
+        'fabrics',
+        default=None if _verbs_dir() is None else 'verbs',
+        description='List of fabrics that are enabled',
+        values=('psm', 'psm2', 'pmi', 'verbs', 'mxm'),
+        multi=True
+    )
+
+    variant(
+        'schedulers',
+        description='List of schedulers for which support is enabled',
+        values=('alps', 'lsf', 'tm', 'slurm', 'sge', 'loadleveler'),
+        multi=True
+    )
 
     # Additional support options
     variant('java', default=False, description='Build Java support')
@@ -114,7 +115,7 @@ class Openmpi(AutotoolsPackage):
     depends_on('hwloc')
     depends_on('hwloc +cuda', when='+cuda')
     depends_on('jdk', when='+java')
-    depends_on('sqlite', when='+sqlite3')
+    depends_on('sqlite', when='+sqlite3@:1.11')
 
     def url_for_version(self, version):
         return "http://www.open-mpi.org/software/ompi/v%s/downloads/openmpi-%s.tar.bz2" % (
@@ -153,15 +154,22 @@ class Openmpi(AutotoolsPackage):
             join_path(self.prefix.lib, 'libmpi.{0}'.format(dso_suffix))
         ]
 
-    @property
-    def verbs(self):
+    def with_or_without_verbs(self, activated):
         # Up through version 1.6, this option was previously named
         # --with-openib
-        if self.spec.satisfies('@:1.6'):
-            return 'openib'
+        opt = 'openib'
         # In version 1.7, it was renamed to be --with-verbs
-        elif self.spec.satisfies('@1.7:'):
-            return 'verbs'
+        if self.spec.satisfies('@1.7:'):
+            opt = 'verbs'
+        # If the option has not been activated return
+        # --without-openib or --without-verbs
+        if not activated:
+            return '--without-{0}'.format(opt)
+        line = '--with-{0}'.format(opt)
+        path = _verbs_dir()
+        if (path is not None) and (path not in ('/usr', '/usr/local')):
+            line += '={0}'.format(path)
+        return line
 
     @run_before('autoreconf')
     def die_without_fortran(self):
@@ -175,48 +183,17 @@ class Openmpi(AutotoolsPackage):
 
     def configure_args(self):
         spec = self.spec
-
         config_args = [
             '--enable-shared',
-            '--enable-static',
-            '--enable-mpi-cxx',
-            # Schedulers
-            '--with-tm' if '+tm' in spec else '--without-tm',
-            '--with-slurm' if '+slurm' in spec else '--without-slurm',
-            # Fabrics
-            '--with-psm' if '+psm' in spec else '--without-psm',
+            '--enable-static'
         ]
+        if self.spec.satisfies('@2.0:'):
+            # for Open-MPI 2.0:, C++ bindings are disabled by default.
+            config_args.extend(['--enable-mpi-cxx'])
 
-        # Intel PSM2 support
-        if spec.satisfies('@1.10:'):
-            if '+psm2' in spec:
-                config_args.append('--with-psm2')
-            else:
-                config_args.append('--without-psm2')
-
-        # PMI support
-        if spec.satisfies('@1.5.5:'):
-            if '+pmi' in spec:
-                config_args.append('--with-pmi')
-            else:
-                config_args.append('--without-pmi')
-
-        # Mellanox Messaging support
-        if spec.satisfies('@1.5.4:'):
-            if '+mxm' in spec:
-                config_args.append('--with-mxm')
-            else:
-                config_args.append('--without-mxm')
-
-        # OpenFabrics verbs support
-        if '+verbs' in spec:
-            path = _verbs_dir()
-            if path is not None and path not in ('/usr', '/usr/local'):
-                config_args.append('--with-{0}={1}'.format(self.verbs, path))
-            else:
-                config_args.append('--with-{0}'.format(self.verbs))
-        else:
-            config_args.append('--without-{0}'.format(self.verbs))
+        # Fabrics and schedulers
+        config_args.extend(self.with_or_without('fabrics'))
+        config_args.extend(self.with_or_without('schedulers'))
 
         # Hwloc support
         if spec.satisfies('@1.5.2:'):
@@ -270,11 +247,11 @@ class Openmpi(AutotoolsPackage):
     @run_after('install')
     def filter_compilers(self):
         """Run after install to make the MPI compilers use the
-           compilers that Spack built the package with.
+        compilers that Spack built the package with.
 
-           If this isn't done, they'll have CC, CXX and FC set
-           to Spack's generic cc, c++ and f90.  We want them to
-           be bound to whatever compiler they were built with.
+        If this isn't done, they'll have CC, CXX and FC set
+        to Spack's generic cc, c++ and f90.  We want them to
+        be bound to whatever compiler they were built with.
         """
         kwargs = {'ignore_absent': True, 'backup': False, 'string': False}
         wrapper_basepath = join_path(self.prefix, 'share', 'openmpi')