mixins: implemented declarative syntax

Implemented a declarative syntax for the additional behavior that can
get attached to classes. Implemented a function to filter compiler
wrappers that uses the mechanism above.

lib/spack/spack/__init__.py

@@ -214,8 +214,8 @@
     'IntelPackage',
 ]
 
-import spack.mixins as mixins
+from spack.mixins import filter_compiler_wrappers
-__all__ += ['mixins']
+__all__ += ['filter_compiler_wrappers']
 
 from spack.version import Version, ver
 __all__ += ['Version', 'ver']

lib/spack/spack/directives.py

@@ -74,7 +74,7 @@ class OpenMpi(Package):
 reserved_names = ['patches']
 
 
-class DirectiveMetaMixin(type):
+class DirectiveMeta(type):
     """Flushes the directives that were temporarily stored in the staging
     area into the package.
     """
@@ -107,12 +107,12 @@ def __new__(mcs, name, bases, attr_dict):
 
         # Move things to be executed from module scope (where they
         # are collected first) to class scope
-        if DirectiveMetaMixin._directives_to_be_executed:
+        if DirectiveMeta._directives_to_be_executed:
             attr_dict['_directives_to_be_executed'].extend(
-                DirectiveMetaMixin._directives_to_be_executed)
+                DirectiveMeta._directives_to_be_executed)
-            DirectiveMetaMixin._directives_to_be_executed = []
+            DirectiveMeta._directives_to_be_executed = []
 
-        return super(DirectiveMetaMixin, mcs).__new__(
+        return super(DirectiveMeta, mcs).__new__(
             mcs, name, bases, attr_dict)
 
     def __init__(cls, name, bases, attr_dict):
@@ -127,7 +127,7 @@ def __init__(cls, name, bases, attr_dict):
 
             # Ensure the presence of the dictionaries associated
             # with the directives
-            for d in DirectiveMetaMixin._directive_names:
+            for d in DirectiveMeta._directive_names:
                 setattr(cls, d, {})
 
             # Lazily execute directives
@@ -136,9 +136,9 @@ def __init__(cls, name, bases, attr_dict):
 
             # Ignore any directives executed *within* top-level
             # directives by clearing out the queue they're appended to
-            DirectiveMetaMixin._directives_to_be_executed = []
+            DirectiveMeta._directives_to_be_executed = []
 
-        super(DirectiveMetaMixin, cls).__init__(name, bases, attr_dict)
+        super(DirectiveMeta, cls).__init__(name, bases, attr_dict)
 
     @staticmethod
     def directive(dicts=None):
@@ -188,7 +188,7 @@ class Foo(Package):
             message = "dicts arg must be list, tuple, or string. Found {0}"
             raise TypeError(message.format(type(dicts)))
         # Add the dictionary names if not already there
-        DirectiveMetaMixin._directive_names |= set(dicts)
+        DirectiveMeta._directive_names |= set(dicts)
 
         # This decorator just returns the directive functions
         def _decorator(decorated_function):
@@ -202,7 +202,7 @@ def _wrapper(*args, **kwargs):
                 # This allows nested directive calls in packages.  The
                 # caller can return the directive if it should be queued.
                 def remove_directives(arg):
-                    directives = DirectiveMetaMixin._directives_to_be_executed
+                    directives = DirectiveMeta._directives_to_be_executed
                     if isinstance(arg, (list, tuple)):
                         # Descend into args that are lists or tuples
                         for a in arg:
@@ -228,18 +228,17 @@ def remove_directives(arg):
                 if not isinstance(values, collections.Sequence):
                     values = (values, )
 
-                DirectiveMetaMixin._directives_to_be_executed.extend(values)
+                DirectiveMeta._directives_to_be_executed.extend(values)
 
                 # wrapped function returns same result as original so
                 # that we can nest directives
                 return result
-
             return _wrapper
 
         return _decorator
 
 
-directive = DirectiveMetaMixin.directive
+directive = DirectiveMeta.directive
 
 
 @directive('versions')

lib/spack/spack/mixins.py

@@ -22,63 +22,150 @@
 # License along with this program; if not, write to the Free Software
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 ##############################################################################
+"""This module contains additional behavior that can be attached to any given
+package.
+"""
+import collections
 import os
 
 import llnl.util.filesystem
+import llnl.util.tty as tty
 
 
-class FilterCompilerWrappers(object):
+__all__ = [
-    """This mixin class registers a callback that filters a list of files
+    'filter_compiler_wrappers'
-    after installation and substitutes hardcoded paths pointing to the Spack
+]
-    compiler wrappers with the corresponding 'real' compilers.
+
+
+class PackageMixinsMeta(type):
+    """This metaclass serves the purpose of implementing a declarative syntax
+    for package mixins.
+
+    Mixins are implemented below in the form of a function. Each one of them
+    needs to register a callable that takes a single argument to be run
+    before or after a certain phase. This callable is basically a method that
+    gets implicitly attached to the package class by calling the mixin.
     """
 
-    #: compiler wrappers to be filtered (needs to be overridden)
+    _methods_to_be_added = {}
-    to_be_filtered_for_wrappers = []
+    _add_method_before = collections.defaultdict(list)
+    _add_method_after = collections.defaultdict(list)
 
-    #: phase after which the callback is invoked (default 'install')
+    @staticmethod
-    filter_phase = 'install'
+    def register_method_before(fn, phase):
+        """Registers a method to be run before a certain phase.
 
-    def __init__(self):
+        Args:
+            fn: function taking a single argument (self)
+            phase (str): phase before which fn must run
+        """
+        PackageMixinsMeta._methods_to_be_added[fn.__name__] = fn
+        PackageMixinsMeta._add_method_before[phase].append(fn)
 
-        attr_name = '_InstallPhase_{0}'.format(self.filter_phase)
+    @staticmethod
+    def register_method_after(fn, phase):
+        """Registers a method to be run after a certain phase.
 
-        # Here we want to get the attribute directly from the class (not from
+        Args:
-        # the instance), so that we can modify it and add the mixin method
+            fn: function taking a single argument (self)
-        phase = getattr(type(self), attr_name)
+            phase (str): phase after which fn must run
+        """
+        PackageMixinsMeta._methods_to_be_added[fn.__name__] = fn
+        PackageMixinsMeta._add_method_after[phase].append(fn)
 
-        # Due to MRO, we may have taken a method from a parent class
+    def __init__(cls, name, bases, attr_dict):
-        # and modifying it may influence other packages in unwanted manners.
-        # Solve the problem by copying the phase into the most derived class.
-        setattr(type(self), attr_name, phase.copy())
-        phase = getattr(type(self), attr_name)
 
-        phase.run_after.append(
+        # Add the methods to the class being created
-            FilterCompilerWrappers.filter_compilers
+        if PackageMixinsMeta._methods_to_be_added:
+            attr_dict.update(PackageMixinsMeta._methods_to_be_added)
+            PackageMixinsMeta._methods_to_be_added.clear()
+
+        attr_fmt = '_InstallPhase_{0}'
+
+        # Copy the phases that needs it to the most derived classes
+        # in order not to interfere with other packages in the hierarchy
+        phases_to_be_copied = list(
+            PackageMixinsMeta._add_method_before.keys()
+        )
+        phases_to_be_copied += list(
+            PackageMixinsMeta._add_method_after.keys()
         )
 
-        super(FilterCompilerWrappers, self).__init__()
+        for phase in phases_to_be_copied:
 
-    def filter_compilers(self):
+            attr_name = attr_fmt.format(phase)
-        """Substitutes any path referring to a Spack compiler wrapper
-        with the path of the underlying compiler that has been used.
 
-        If this isn't done, the files will have CC, CXX, F77, and FC set
+            # Here we want to get the attribute directly from the class (not
-        to Spack's generic cc, c++, f77, and f90.  We want them to
+            # from the instance), so that we can modify it and add the mixin
-        be bound to whatever compiler they were built with.
+            # method to the pipeline.
-        """
+            phase = getattr(cls, attr_name)
+
+            # Due to MRO, we may have taken a method from a parent class
+            # and modifying it may influence other packages in unwanted
+            # manners. Solve the problem by copying the phase into the most
+            # derived class.
+            setattr(cls, attr_name, phase.copy())
+
+        # Insert the methods in the appropriate position
+        # in the installation pipeline.
+
+        for phase in PackageMixinsMeta._add_method_before:
+
+            attr_name = attr_fmt.format(phase)
+            phase_obj = getattr(cls, attr_name)
+            fn_list = PackageMixinsMeta._add_method_after[phase]
+
+            for f in fn_list:
+                phase_obj.run_before.append(f)
+
+        for phase in PackageMixinsMeta._add_method_after:
+
+            attr_name = attr_fmt.format(phase)
+            phase_obj = getattr(cls, attr_name)
+            fn_list = PackageMixinsMeta._add_method_after[phase]
+
+            for f in fn_list:
+                phase_obj.run_after.append(f)
+
+        super(PackageMixinsMeta, cls).__init__(name, bases, attr_dict)
+
+
+def filter_compiler_wrappers(*files, **kwargs):
+    """Substitutes any path referring to a Spack compiler wrapper with the
+    path of the underlying compiler that has been used.
+
+    If this isn't done, the files will have CC, CXX, F77, and FC set to
+    Spack's generic cc, c++, f77, and f90. We want them to be bound to
+    whatever compiler they were built with.
+
+    Args:
+        *files: files to be filtered
+        **kwargs: at present supports the keyword 'after' to specify after
+            which phase the files should be filtered (defaults to 'install').
+    """
+    after = kwargs.get('after', 'install')
+
+    def _filter_compiler_wrappers_impl(self):
+
+        tty.debug('Filtering compiler wrappers: {0}'.format(files))
+
+        # Compute the absolute path of the files to be filtered and
+        # remove links from the list.
+        abs_files = llnl.util.filesystem.find(self.prefix, files)
+        abs_files = [x for x in abs_files if not os.path.islink(x)]
 
         kwargs = {'ignore_absent': True, 'backup': False, 'string': True}
 
-        if self.to_be_filtered_for_wrappers:
+        x = llnl.util.filesystem.FileFilter(*abs_files)
-            x = llnl.util.filesystem.FileFilter(
-                *self.to_be_filtered_for_wrappers
-            )
 
-            x.filter(os.environ['CC'], self.compiler.cc, **kwargs)
+        x.filter(os.environ['CC'], self.compiler.cc, **kwargs)
-            x.filter(os.environ['CXX'], self.compiler.cxx, **kwargs)
+        x.filter(os.environ['CXX'], self.compiler.cxx, **kwargs)
-            x.filter(os.environ['F77'], self.compiler.f77, **kwargs)
+        x.filter(os.environ['F77'], self.compiler.f77, **kwargs)
-            x.filter(os.environ['FC'], self.compiler.fc, **kwargs)
+        x.filter(os.environ['FC'], self.compiler.fc, **kwargs)
 
-            # Remove this linking flag if present (it turns RPATH into RUNPATH)
+        # Remove this linking flag if present (it turns RPATH into RUNPATH)
-            x.filter('-Wl,--enable-new-dtags', '', **kwargs)
+        x.filter('-Wl,--enable-new-dtags', '', **kwargs)
+
+    PackageMixinsMeta.register_method_after(
+        _filter_compiler_wrappers_impl, after
+    )

lib/spack/spack/package.py

@@ -56,6 +56,7 @@
 import spack.fetch_strategy as fs
 import spack.hooks
 import spack.mirror
+import spack.mixins
 import spack.repository
 import spack.url
 import spack.util.web
@@ -141,7 +142,10 @@ def copy(self):
             return other
 
 
-class PackageMeta(spack.directives.DirectiveMetaMixin):
+class PackageMeta(
+    spack.directives.DirectiveMeta,
+    spack.mixins.PackageMixinsMeta
+):
     """Conveniently transforms attributes to permit extensible phases
 
     Iterates over the attribute 'phases' and creates / updates private

var/spack/repos/builtin/packages/hdf5/package.py

@@ -28,7 +28,7 @@
 from spack import *
 
 
-class Hdf5(AutotoolsPackage, mixins.FilterCompilerWrappers):
+class Hdf5(AutotoolsPackage):
     """HDF5 is a data model, library, and file format for storing and managing
     data. It supports an unlimited variety of datatypes, and is designed for
     flexible and efficient I/O and for high volume and complex data.
@@ -97,6 +97,8 @@ class Hdf5(AutotoolsPackage, mixins.FilterCompilerWrappers):
     patch('h5f90global-mult-obj-same-equivalence-same-common-block.patch',
           when='@1.10.1%intel@18')
 
+    filter_compiler_wrappers('h5cc', 'h5c++', 'h5fc')
+
     def url_for_version(self, version):
         url = "https://support.hdfgroup.org/ftp/HDF5/releases/hdf5-{0}/hdf5-{1}/src/hdf5-{1}.tar.gz"
         return url.format(version.up_to(2), version)
@@ -295,11 +297,3 @@ def check_install(self):
                 print('-' * 80)
                 raise RuntimeError("HDF5 install check failed")
         shutil.rmtree(checkdir)
-
-    @property
-    def to_be_filtered_for_wrappers(self):
-        return [
-            join_path(self.prefix.bin, 'h5c++'),
-            join_path(self.prefix.bin, 'h5cc'),
-            join_path(self.prefix.bin, 'h5fc'),
-        ]

var/spack/repos/builtin/packages/mpich/package.py

@@ -26,7 +26,7 @@
 import os
 
 
-class Mpich(AutotoolsPackage, mixins.FilterCompilerWrappers):
+class Mpich(AutotoolsPackage):
     """MPICH is a high performance and widely portable implementation of
     the Message Passing Interface (MPI) standard."""
 
@@ -71,6 +71,8 @@ class Mpich(AutotoolsPackage, mixins.FilterCompilerWrappers):
     provides('mpi@:3.0', when='@3:')
     provides('mpi@:1.3', when='@1:')
 
+    filter_compiler_wrappers('mpicc', 'mpicxx', 'mpif77', 'mpif90')
+
     # fix MPI_Barrier segmentation fault
     # see https://lists.mpich.org/pipermail/discuss/2016-May/004764.html
     # and https://lists.mpich.org/pipermail/discuss/2016-June/004768.html
@@ -169,12 +171,3 @@ def configure_args(self):
         config_args.append(device_config)
 
         return config_args
-
-    @property
-    def to_be_filtered_for_wrappers(self):
-        return [
-            join_path(self.prefix.bin, 'mpicc'),
-            join_path(self.prefix.bin, 'mpicxx'),
-            join_path(self.prefix.bin, 'mpif77'),
-            join_path(self.prefix.bin, 'mpif90')
-        ]

var/spack/repos/builtin/packages/mvapich2/package.py

@@ -35,7 +35,7 @@ def _process_manager_validator(values):
         )
 
 
-class Mvapich2(AutotoolsPackage, mixins.FilterCompilerWrappers):
+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"
@@ -107,6 +107,8 @@ class Mvapich2(AutotoolsPackage, mixins.FilterCompilerWrappers):
     depends_on('libpciaccess', when=(sys.platform != 'darwin'))
     depends_on('cuda', when='+cuda')
 
+    filter_compiler_wrappers('mpicc', 'mpicxx', 'mpif77', 'mpif90')
+
     def url_for_version(self, version):
         base_url = "http://mvapich.cse.ohio-state.edu/download"
         if version < Version('2.0'):
@@ -231,12 +233,3 @@ def configure_args(self):
         args.extend(self.process_manager_options)
         args.extend(self.network_options)
         return args
-
-    @property
-    def to_be_filtered_for_wrappers(self):
-        return [
-            join_path(self.prefix.bin, 'mpicc'),
-            join_path(self.prefix.bin, 'mpicxx'),
-            join_path(self.prefix.bin, 'mpif77'),
-            join_path(self.prefix.bin, 'mpif90')
-        ]

var/spack/repos/builtin/packages/openmpi/package.py

@@ -64,7 +64,7 @@ def _mxm_dir():
         return None
 
 
-class Openmpi(AutotoolsPackage, mixins.FilterCompilerWrappers):
+class Openmpi(AutotoolsPackage):
     """The Open MPI Project is an open source Message Passing Interface
        implementation that is developed and maintained by a consortium
        of academic, research, and industry partners. Open MPI is
@@ -214,6 +214,23 @@ class Openmpi(AutotoolsPackage, mixins.FilterCompilerWrappers):
     conflicts('fabrics=pmi', when='@:1.5.4')  # PMI support was added in 1.5.5
     conflicts('fabrics=mxm', when='@:1.5.3')  # MXM support was added in 1.5.4
 
+    filter_compiler_wrappers(
+        'mpicc-vt-wrapper-data.txt',
+        'mpicc-wrapper-data.txt',
+        'ortecc-wrapper-data.txt',
+        'shmemcc-wrapper-data.txt',
+        'mpic++-vt-wrapper-data.txt',
+        'mpic++-wrapper-data.txt',
+        'ortec++-wrapper-data.txt',
+        'mpifort-vt-wrapper-data.txt',
+        'mpifort-wrapper-data.txt',
+        'shmemfort-wrapper-data.txt',
+        'mpif90-vt-wrapper-data.txt',
+        'mpif90-wrapper-data.txt',
+        'mpif77-vt-wrapper-data.txt',
+        'mpif77-wrapper-data.txt'
+    )
+
     def url_for_version(self, version):
         url = "http://www.open-mpi.org/software/ompi/v{0}/downloads/openmpi-{1}.tar.bz2"
         return url.format(version.up_to(2), version)
@@ -374,29 +391,3 @@ def configure_args(self):
             config_args.append('--without-ucx')
 
         return config_args
-
-    @property
-    def to_be_filtered_for_wrappers(self):
-
-        basepath = join_path(self.prefix, 'share', 'openmpi')
-
-        wrappers = [
-            'mpicc-vt-wrapper-data.txt',
-            'mpicc-wrapper-data.txt',
-            'ortecc-wrapper-data.txt',
-            'shmemcc-wrapper-data.txt',
-            'mpic++-vt-wrapper-data.txt',
-            'mpic++-wrapper-data.txt',
-            'ortec++-wrapper-data.txt',
-            'mpifort-vt-wrapper-data.txt',
-            'mpifort-wrapper-data.txt',
-            'shmemfort-wrapper-data.txt',
-            'mpif90-vt-wrapper-data.txt',
-            'mpif90-wrapper-data.txt',
-            'mpif77-vt-wrapper-data.txt',
-            'mpif77-wrapper-data.txt'
-        ]
-
-        abs_wrappers = [join_path(basepath, x) for x in wrappers]
-
-        return [x for x in abs_wrappers if not os.path.islink(x)]

var/spack/repos/builtin/packages/r/package.py

@@ -26,7 +26,7 @@
 from spack import *
 
 
-class R(AutotoolsPackage, mixins.FilterCompilerWrappers):
+class R(AutotoolsPackage):
     """R is 'GNU S', a freely available language and environment for
     statistical computing and graphics which provides a wide variety of
     statistical and graphical techniques: linear and nonlinear modelling,
@@ -88,6 +88,8 @@ class R(AutotoolsPackage, mixins.FilterCompilerWrappers):
 
     patch('zlib.patch', when='@:3.3.2')
 
+    filter_compiler_wrappers('Makeconf')
+
     @property
     def etcdir(self):
         return join_path(prefix, 'rlib', 'R', 'etc')
@@ -129,10 +131,6 @@ def copy_makeconf(self):
         dst_makeconf = join_path(self.etcdir, 'Makeconf.spack')
         shutil.copy(src_makeconf, dst_makeconf)
 
-    @property
-    def to_be_filtered_for_wrappers(self):
-        return [join_path(self.etcdir, 'Makeconf')]
-
     # ========================================================================
     # Set up environment to make install easy for R extensions.
     # ========================================================================