From 22def01adf117d38dbe20c8f719fbd2f50fe5393 Mon Sep 17 00:00:00 2001 From: alalazo Date: Mon, 19 Jun 2017 14:16:18 +0200 Subject: [PATCH] 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 | 4 +- lib/spack/spack/directives.py | 25 ++- lib/spack/spack/mixins.py | 163 ++++++++++++++---- lib/spack/spack/package.py | 6 +- .../repos/builtin/packages/hdf5/package.py | 12 +- .../repos/builtin/packages/mpich/package.py | 13 +- .../builtin/packages/mvapich2/package.py | 13 +- .../repos/builtin/packages/openmpi/package.py | 45 ++--- var/spack/repos/builtin/packages/r/package.py | 8 +- 9 files changed, 174 insertions(+), 115 deletions(-) diff --git a/lib/spack/spack/__init__.py b/lib/spack/spack/__init__.py index 83355ad28c..ee310c2c3a 100644 --- a/lib/spack/spack/__init__.py +++ b/lib/spack/spack/__init__.py @@ -214,8 +214,8 @@ 'IntelPackage', ] -import spack.mixins as mixins -__all__ += ['mixins'] +from spack.mixins import filter_compiler_wrappers +__all__ += ['filter_compiler_wrappers'] from spack.version import Version, ver __all__ += ['Version', 'ver'] diff --git a/lib/spack/spack/directives.py b/lib/spack/spack/directives.py index df19e85d54..d26910f1d9 100644 --- a/lib/spack/spack/directives.py +++ b/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) - DirectiveMetaMixin._directives_to_be_executed = [] + DirectiveMeta._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') diff --git a/lib/spack/spack/mixins.py b/lib/spack/spack/mixins.py index 0a354362ce..d757a963b3 100644 --- a/lib/spack/spack/mixins.py +++ b/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): - """This mixin class registers a callback that filters a list of files - after installation and substitutes hardcoded paths pointing to the Spack - compiler wrappers with the corresponding 'real' compilers. +__all__ = [ + 'filter_compiler_wrappers' +] + + +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) - to_be_filtered_for_wrappers = [] + _methods_to_be_added = {} + _add_method_before = collections.defaultdict(list) + _add_method_after = collections.defaultdict(list) - #: phase after which the callback is invoked (default 'install') - filter_phase = 'install' + @staticmethod + 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 - # the instance), so that we can modify it and add the mixin method - phase = getattr(type(self), attr_name) + Args: + fn: function taking a single argument (self) + 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 - # 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) + def __init__(cls, name, bases, attr_dict): - phase.run_after.append( - FilterCompilerWrappers.filter_compilers + # Add the methods to the class being created + 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): - """Substitutes any path referring to a Spack compiler wrapper - with the path of the underlying compiler that has been used. + attr_name = attr_fmt.format(phase) - 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. - """ + # Here we want to get the attribute directly from the class (not + # from the instance), so that we can modify it and add the mixin + # 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( - *self.to_be_filtered_for_wrappers - ) + x = llnl.util.filesystem.FileFilter(*abs_files) - x.filter(os.environ['CC'], self.compiler.cc, **kwargs) - x.filter(os.environ['CXX'], self.compiler.cxx, **kwargs) - x.filter(os.environ['F77'], self.compiler.f77, **kwargs) - x.filter(os.environ['FC'], self.compiler.fc, **kwargs) + x.filter(os.environ['CC'], self.compiler.cc, **kwargs) + x.filter(os.environ['CXX'], self.compiler.cxx, **kwargs) + x.filter(os.environ['F77'], self.compiler.f77, **kwargs) + x.filter(os.environ['FC'], self.compiler.fc, **kwargs) - # Remove this linking flag if present (it turns RPATH into RUNPATH) - x.filter('-Wl,--enable-new-dtags', '', **kwargs) + # Remove this linking flag if present (it turns RPATH into RUNPATH) + x.filter('-Wl,--enable-new-dtags', '', **kwargs) + + PackageMixinsMeta.register_method_after( + _filter_compiler_wrappers_impl, after + ) diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py index f0172565bb..82af2375af 100644 --- a/lib/spack/spack/package.py +++ b/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 diff --git a/var/spack/repos/builtin/packages/hdf5/package.py b/var/spack/repos/builtin/packages/hdf5/package.py index 5c164e049a..3c729412ab 100644 --- a/var/spack/repos/builtin/packages/hdf5/package.py +++ b/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'), - ] diff --git a/var/spack/repos/builtin/packages/mpich/package.py b/var/spack/repos/builtin/packages/mpich/package.py index b689fefbdb..69dd75e3e1 100644 --- a/var/spack/repos/builtin/packages/mpich/package.py +++ b/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') - ] diff --git a/var/spack/repos/builtin/packages/mvapich2/package.py b/var/spack/repos/builtin/packages/mvapich2/package.py index cc6568883f..1e3a14f2f3 100644 --- a/var/spack/repos/builtin/packages/mvapich2/package.py +++ b/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') - ] diff --git a/var/spack/repos/builtin/packages/openmpi/package.py b/var/spack/repos/builtin/packages/openmpi/package.py index 3c0db25ced..343b69ea7e 100644 --- a/var/spack/repos/builtin/packages/openmpi/package.py +++ b/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)] diff --git a/var/spack/repos/builtin/packages/r/package.py b/var/spack/repos/builtin/packages/r/package.py index 2f5fc1b3af..cb955579a3 100644 --- a/var/spack/repos/builtin/packages/r/package.py +++ b/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. # ========================================================================