From 8ed028e2d67f191a32751db8bd487c03f7687b50 Mon Sep 17 00:00:00 2001 From: alalazo Date: Fri, 8 Jul 2016 12:29:49 +0200 Subject: package : introduced InstallPhase, added decorators for prerequisites and sanity_checks of phases --- lib/spack/spack/package.py | 214 ++++++++++++++--------- var/spack/repos/builtin/packages/szip/package.py | 4 + 2 files changed, 133 insertions(+), 85 deletions(-) diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py index e159bc59c5..4194f30827 100644 --- a/lib/spack/spack/package.py +++ b/lib/spack/spack/package.py @@ -35,10 +35,13 @@ README. """ import os import re +import string import textwrap import time -import glob -import string +import inspect +import functools +from StringIO import StringIO +from urlparse import urlparse import llnl.util.tty as tty import spack @@ -52,26 +55,95 @@ import spack.mirror import spack.repository import spack.url import spack.util.web - -from urlparse import urlparse -from StringIO import StringIO from llnl.util.filesystem import * from llnl.util.lang import * from llnl.util.link_tree import LinkTree from llnl.util.tty.log import log_output +from spack import directory_layout from spack.stage import Stage, ResourceStage, StageComposite from spack.util.compression import allowed_archive from spack.util.environment import dump_environment -from spack.util.executable import ProcessError, Executable, which +from spack.util.executable import ProcessError, which from spack.version import * -from spack import directory_layout -from urlparse import urlparse """Allowed URL schemes for spack packages.""" _ALLOWED_URL_SCHEMES = ["http", "https", "ftp", "file", "git"] -class Package(object): +class InstallPhase(object): + """Manages a single phase of the installation + + This descriptor stores at creation time the name of the method it should search + for execution. The method is retrieved at get time, so that it can be overridden + by subclasses of whatever class declared the phases. + + It also provides hooks to execute prerequisite and sanity checks. + """ + def __init__(self, name): + self.name = name + self.preconditions = [] + self.sanity_checks = [] + + def __get__(self, instance, owner): + # The caller is a class that is trying to customize + # my behavior adding something + if instance is None: + return self + # If instance is there the caller wants to execute the + # install phase, thus return a properly set wrapper + phase = getattr(instance, self.name) + @functools.wraps(phase) + def phase_wrapper(spec, prefix): + # Execute phase pre-conditions, + # and give them the chance to fail + for check in self.preconditions: + check(instance) + # Do something sensible at some point + phase(spec, prefix) + # Execute phase sanity_checks, + # and give them the chance to fail + for check in self.sanity_checks: + check(instance) + + return phase_wrapper + + +class PackageMeta(type): + """Conveniently transforms attributes to permit extensible phases + + Iterates over the attribute 'phase' and creates / updates private + InstallPhase attributes in the class that is being initialized + """ + phase_fmt = '_InstallPhase_{0}' + + def __init__(cls, name, bases, attr_dict): + super(PackageMeta, cls).__init__(name, bases, attr_dict) + # Parse if phases is in attr dict, then set + # install phases wrappers + if 'phases' in attr_dict: + cls.phases = [PackageMeta.phase_fmt.format(name) for name in attr_dict['phases']] + for phase_name, callback_name in zip(cls.phases, attr_dict['phases']): + setattr(cls, phase_name, InstallPhase(callback_name)) + + def _transform_checks(check_name): + attr_name = PackageMeta.phase_fmt.format(check_name) + checks = getattr(cls, attr_name, None) + if checks: + for phase_name, funcs in checks.items(): + phase = getattr(cls, PackageMeta.phase_fmt.format(phase_name)) + getattr(phase, check_name).extend(funcs) + # TODO : this should delete the attribute, as it is just a placeholder + # TODO : to know what to do at class definition time. Clearing it is fine + # TODO : too, but it just leaves an empty dictionary in place + setattr(cls, attr_name, {}) + + # Preconditions + _transform_checks('preconditions') + # Sanity checks + _transform_checks('sanity_checks') + + +class PackageBase(object): """This is the superclass for all spack packages. ***The Package class*** @@ -304,6 +376,7 @@ class Package(object): clean() (some of them do this), and others to provide custom behavior. """ + __metaclass__ = PackageMeta # # These are default values for instance variables. # @@ -410,7 +483,6 @@ class Package(object): """Return the directory where the package.py file lives.""" return os.path.dirname(self.module.__file__) - @property def global_license_dir(self): """Returns the directory where global license files for all @@ -875,7 +947,6 @@ class Package(object): resource_stage_folder = '-'.join(pieces) return resource_stage_folder - _phases = ['install', 'create_spack_logs'] def do_install(self, keep_prefix=False, keep_stage=False, @@ -909,7 +980,7 @@ class Package(object): """ # FIXME : we need a better semantic if allowed_phases is None: - allowed_phases = self._phases + allowed_phases = self.phases if not self.spec.concrete: raise ValueError("Can only install concrete packages.") @@ -921,8 +992,8 @@ class Package(object): return # Ensure package is not already installed - # FIXME : This should be a pre-requisite to a phase - if 'install' in self._phases and spack.install_layout.check_installed(self.spec): + # FIXME : skip condition : if any is True skip the installation + if 'install' in self.phases and spack.install_layout.check_installed(self.spec): tty.msg("%s is already installed in %s" % (self.name, self.prefix)) rec = spack.installed_db.get_record(self.spec) if (not rec.explicit) and explicit: @@ -994,15 +1065,9 @@ class Package(object): True): dump_environment(env_path) try: - for phase in filter(lambda x: x in allowed_phases, self._phases): + for phase in filter(lambda x: x in allowed_phases, self.phases): # TODO : Log to screen the various phases - action = getattr(self, phase) - if getattr(action, 'preconditions', None): - action.preconditions() - action(self.spec, self.prefix) - if getattr(action, 'postconditions', None): - action.postconditions() - + getattr(self, phase)(self.spec, self.prefix) except AttributeError as e: # FIXME : improve error messages raise ProcessError(e.message, long_message='') @@ -1012,11 +1077,6 @@ class Package(object): e.build_log = log_path raise e - # Ensure that something was actually installed. - # FIXME : This should be executed after 'install' as a postcondition - # if 'install' in self._phases: - # self.sanity_check_prefix() - # Run post install hooks before build stage is removed. spack.hooks.post_install(self) @@ -1034,7 +1094,7 @@ class Package(object): # Create the install prefix and fork the build process. spack.install_layout.create_install_directory(self.spec) except directory_layout.InstallDirectoryAlreadyExistsError: - if 'install' in self._phases: + if 'install' in self.phases: # Abort install if install directory exists. # But do NOT remove it (you'd be overwriting someon else's stuff) tty.warn("Keeping existing install prefix in place.") @@ -1062,7 +1122,7 @@ class Package(object): # the database, so that we don't need to re-read from file. spack.installed_db.add(self.spec, self.prefix, explicit=explicit) - def create_spack_logs(self, spec, prefix): + def log(self, spec, prefix): # Copy provenance into the install directory on success log_install_path = spack.install_layout.build_log_path( self.spec) @@ -1241,13 +1301,6 @@ class Package(object): """ pass - def install(self, spec, prefix): - """ - Package implementations override this with their own configuration - """ - raise InstallError("Package %s provides no install method!" % - self.name) - def do_uninstall(self, force=False): if not self.installed: raise InstallError(str(self.spec) + " is not installed.") @@ -1446,6 +1499,33 @@ class Package(object): """ return " ".join("-Wl,-rpath,%s" % p for p in self.rpath) + @classmethod + def _register_checks(cls, check_type, *args): + def _register_sanity_checks(func): + attr_name = PackageMeta.phase_fmt.format(check_type) + sanity_checks = getattr(cls, attr_name, {}) + for item in args: + checks = sanity_checks.setdefault(item, []) + checks.append(func) + setattr(cls, attr_name, sanity_checks) + return func + return _register_sanity_checks + + @classmethod + def precondition(cls, *args): + return cls._register_checks('preconditions', *args) + + @classmethod + def sanity_check(cls, *args): + return cls._register_checks('sanity_checks', *args) + + +class Package(PackageBase): + phases = ['install', 'log'] + # This will be used as a registration decorator in user + # packages, if need be + PackageBase.sanity_check('install')(PackageBase.sanity_check_prefix) + def install_dependency_symlinks(pkg, spec, prefix): """Execute a dummy install and flatten dependencies""" @@ -1548,53 +1628,16 @@ def _hms(seconds): parts.append("%.2fs" % s) return ' '.join(parts) -#class StagedPackage(Package): -# """A Package subclass where the install() is split up into stages.""" -# _phases = ['configure'] -# def install_setup(self): -# """Creates an spack_setup.py script to configure the package later if we like.""" -# raise InstallError("Package %s provides no install_setup() method!" % self.name) -# -# def install_configure(self): -# """Runs the configure process.""" -# raise InstallError("Package %s provides no install_configure() method!" % self.name) -# -# def install_build(self): -# """Runs the build process.""" -# raise InstallError("Package %s provides no install_build() method!" % self.name) -# -# def install_install(self): -# """Runs the install process.""" -# raise InstallError("Package %s provides no install_install() method!" % self.name) -# -# def install(self, spec, prefix): -# if 'setup' in self._phases: -# self.install_setup() -# -# if 'configure' in self._phases: -# self.install_configure() -# -# if 'build' in self._phases: -# self.install_build() -# -# if 'install' in self._phases: -# self.install_install() -# else: -# # Create a dummy file so the build doesn't fail. -# # That way, the module file will also be created. -# with open(os.path.join(prefix, 'dummy'), 'w') as fout: -# pass - +# FIXME : remove this after checking that set_executable works the same way # stackoverflow.com/questions/12791997/how-do-you-do-a-simple-chmod-x-from-within-python -def make_executable(path): - mode = os.stat(path).st_mode - mode |= (mode & 0o444) >> 2 # copy R bits to X - os.chmod(path, mode) +#def make_executable(path): +# mode = os.stat(path).st_mode +# mode |= (mode & 0o444) >> 2 # copy R bits to X +# os.chmod(path, mode) - -class CMakePackage(Package): - _phases = ['configure', 'build', 'install', 'provenance'] +class CMakePackage(PackageBase): + phases = ['setup', 'configure', 'build', 'install', 'provenance'] def make_make(self): import multiprocessing @@ -1614,6 +1657,7 @@ class CMakePackage(Package): def configure_env(self): """Returns package-specific environment under which the configure command should be run.""" + # FIXME : Why not EnvironmentModules return dict() def spack_transitive_include_path(self): @@ -1622,7 +1666,7 @@ class CMakePackage(Package): for dep in os.environ['SPACK_DEPENDENCIES'].split(os.pathsep) ) - def setup(self): + def setup(self, spec, prefix): cmd = [str(which('cmake'))] + \ spack.build_environment.get_std_cmake_args(self) + \ ['-DCMAKE_INSTALL_PREFIX=%s' % os.environ['SPACK_PREFIX'], @@ -1674,10 +1718,10 @@ env = dict(os.environ) fout.write(' %s\n' % arg) fout.write('""") + sys.argv[1:]\n') fout.write('\nproc = subprocess.Popen(cmd, env=env)\nproc.wait()\n') - make_executable(setup_fname) + set_executable(setup_fname) - def configure(self): + def configure(self, spec, prefix): cmake = which('cmake') with working_dir(self.build_directory, create=True): os.environ.update(self.configure_env()) @@ -1685,12 +1729,12 @@ env = dict(os.environ) options = self.configure_args() + spack.build_environment.get_std_cmake_args(self) cmake(self.source_directory, *options) - def build(self): + def build(self, spec, prefix): make = self.make_make() with working_dir(self.build_directory, create=False): make() - def install(self): + def install(self, spec, prefix): make = self.make_make() with working_dir(self.build_directory, create=False): make('install') diff --git a/var/spack/repos/builtin/packages/szip/package.py b/var/spack/repos/builtin/packages/szip/package.py index fd3a2a209d..ec6d8239a9 100644 --- a/var/spack/repos/builtin/packages/szip/package.py +++ b/var/spack/repos/builtin/packages/szip/package.py @@ -34,6 +34,10 @@ class Szip(Package): version('2.1', '902f831bcefb69c6b635374424acbead') + @Package.sanity_check('install') + def always_raise(self): + raise RuntimeError('Precondition not respected') + def install(self, spec, prefix): configure('--prefix=%s' % prefix, '--enable-production', -- cgit v1.2.3-70-g09d2